Don't crash when marking arguments object of native context

JSC assumes that the callee is always valid, since JSC::Arguments
is used for JS frames, which must have a callee.
But we use JSC::Arguments for arguments object of
pushContext()-created contexts, and then there is no callee.
But the callee member can't be null, so now we put a fake callee
there and make sure it doesn't bleed up to the public API.

Alternative solution: Add "if (d->callee)" to
JSC::Arguments::markChildren(), then no other changes would be
needed. But we don't want to patch JSC any more.

Non-solution: Subclass JSC::Arguments and reimplement
markChildren() to temporarily set a dummy callee during marking.
Can't be done, as JSC::Arguments::d is private (again, we don't
want to patch JSC).

Task-number: QTBUG-17788
Reviewed-by: Olivier Goffart

3 files changed, 27 insertions, 2 deletions

diff --git a/src/script/api/qscriptcontext.cpp b/src/script/api/qscriptcontext.cpp
index 2468a46..5454df5 100644
--- a/src/script/api/qscriptcontext.cpp
+++ b/src/script/api/qscriptcontext.cpp
@@ -268,8 +268,14 @@ QScriptValue QScriptContext::argument(int index) const
 QScriptValue QScriptContext::callee() const
 {
     const JSC::CallFrame *frame = QScriptEnginePrivate::frameForContext(this);
-    QScript::APIShim shim(QScript::scriptEngineFromExec(frame));
-    return QScript::scriptEngineFromExec(frame)->scriptValueFromJSCValue(frame->callee());
+    QScriptEnginePrivate *eng = QScript::scriptEngineFromExec(frame);
+    QScript::APIShim shim(eng);
+    if (frame->callee() == eng->originalGlobalObject()) {
+        // This is a pushContext()-created context; the callee is a lie.
+        Q_ASSERT(QScriptEnginePrivate::contextFlags(const_cast<JSC::CallFrame*>(frame)) & QScriptEnginePrivate::NativeContext);
+        return QScriptValue();
+    }
+    return eng->scriptValueFromJSCValue(frame->callee());
 }
 
 /*!
diff --git a/src/script/api/qscriptengine.cpp b/src/script/api/qscriptengine.cpp
index d3e5f2f..9e880b6 100644
--- a/src/script/api/qscriptengine.cpp
+++ b/src/script/api/qscriptengine.cpp
@@ -2727,6 +2727,14 @@ JSC::CallFrame *QScriptEnginePrivate::pushContext(JSC::CallFrame *exec, JSC::JSV
                                                   bool clearScopeChain)
 {
     JSC::JSValue thisObject = _thisObject;
+    if (!callee) {
+        // callee can't be zero, as this can cause JSC to crash during GC
+        // marking phase if the context's Arguments object has been created.
+        // Fake it by using the global object. Note that this is also handled
+        // in QScriptContext::callee(), as that function should still return
+        // an invalid value.
+        callee = originalGlobalObject();
+    }
     if (calledAsConstructor) {
         //JSC doesn't create default created object for native functions. so we do it
         JSC::JSValue prototype = callee->get(exec, exec->propertyNames().prototype);
diff --git a/tests/auto/qscriptengine/tst_qscriptengine.cpp b/tests/auto/qscriptengine/tst_qscriptengine.cpp
index 8de6fbc..6c89bcb 100644
--- a/tests/auto/qscriptengine/tst_qscriptengine.cpp
+++ b/tests/auto/qscriptengine/tst_qscriptengine.cpp
@@ -169,6 +169,7 @@ private slots:
     void nativeFunctionScopes();
     void evaluateProgram();
     void collectGarbageAfterConnect();
+    void collectGarbageAfterNativeArguments();
     void promoteThisObjectToQObjectInConstructor();
 
     void qRegExpInport_data();
@@ -5040,6 +5041,16 @@ void tst_QScriptEngine::collectGarbageAfterConnect()
     QVERIFY(widget == 0);
 }
 
+void tst_QScriptEngine::collectGarbageAfterNativeArguments()
+{
+    // QTBUG-17788
+    QScriptEngine eng;
+    QScriptContext *ctx = eng.pushContext();
+    QScriptValue arguments = ctx->argumentsObject();
+    // Shouldn't crash when marking the arguments object.
+    collectGarbage_helper(eng);
+}
+
 static QScriptValue constructQObjectFromThisObject(QScriptContext *ctx, QScriptEngine *eng)
 {
     Q_ASSERT(ctx->isCalledAsConstructor());