From d4166fa6ce24b55b483f29e8ef447c0f63f0a30f Mon Sep 17 00:00:00 2001 From: Kent Hansen Date: Fri, 25 Feb 2011 13:23:07 +0100 Subject: 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 --- src/script/api/qscriptcontext.cpp | 10 ++++++++-- src/script/api/qscriptengine.cpp | 8 ++++++++ tests/auto/qscriptengine/tst_qscriptengine.cpp | 11 +++++++++++ 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(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()); -- cgit v0.12