Don't crash when creating backtrace for built-in JS function (2nd try)

Commit 147df10403ba280b3f04c1e3d6c4b1cf386abe5d did not quite
fix the issue; other places need the same checks.

When the JIT is enabled, frames for built-in JS host calls
(such as Array.prototype.forEach) are not fully initialized.
In particular, the CodeBlock register of such frames is not
set (see comment in JITCall.cpp).

We need to check if the codeBlock is actually valid before we
start using it.

This fixes the crash(es) but not the problem of actually getting
the arguments for such frames through the API. There's also a
related problem when a QtScript function (newFunction()) is called
as a callback of a built-in JS host function (QTBUG-17287).
These problems will go away once JavaScriptCore is updated to a
more recent version (4.8 at the earliest), since the
native-vs-script frame handling has been unified.

Task-number: QTBUG-17137
Reviewed-by: Olivier Goffart
(cherry picked from commit 640436345645b6cf6ff3334399f33c9d1c089492)

6 files changed, 76 insertions, 5 deletions

diff --git a/src/script/api/qscriptcontext.cpp b/src/script/api/qscriptcontext.cpp
index 59ea52d..2468a46 100644
--- a/src/script/api/qscriptcontext.cpp
+++ b/src/script/api/qscriptcontext.cpp
@@ -299,6 +299,12 @@ QScriptValue QScriptContext::argumentsObject() const
 
     //for a js function
     if (frame->codeBlock() && frame->callee()) {
+        if (!QScriptEnginePrivate::hasValidCodeBlockRegister(frame)) {
+            // We have a built-in JS host call.
+            // codeBlock is needed by retrieveArguments(), but since it
+            // contains junk, we would crash. Return an invalid value for now.
+            return QScriptValue();
+        }
         JSC::JSValue result = frame->interpreter()->retrieveArguments(frame, JSC::asFunction(frame->callee()));
         return QScript::scriptEngineFromExec(frame)->scriptValueFromJSCValue(result);
     }
@@ -309,7 +315,8 @@ QScriptValue QScriptContext::argumentsObject() const
     }
 
     //for a native function
-    if (!frame->optionalCalleeArguments()) {
+    if (!frame->optionalCalleeArguments()
+        && QScriptEnginePrivate::hasValidCodeBlockRegister(frame)) { // Make sure we don't go here for host JSFunctions
         Q_ASSERT(frame->argumentCount() > 0); //we need at least 'this' otherwise we'll crash later
         JSC::Arguments* arguments = new (&frame->globalData())JSC::Arguments(frame, JSC::Arguments::NoParameters);
         frame->setCalleeArguments(arguments);
diff --git a/src/script/api/qscriptcontextinfo.cpp b/src/script/api/qscriptcontextinfo.cpp
index d39abe6..0f9de1d 100644
--- a/src/script/api/qscriptcontextinfo.cpp
+++ b/src/script/api/qscriptcontextinfo.cpp
@@ -157,7 +157,7 @@ QScriptContextInfoPrivate::QScriptContextInfoPrivate(const QScriptContext *conte
 
             JSC::Instruction *returnPC = rewindContext->returnPC();
             JSC::CodeBlock *codeBlock = frame->codeBlock();
-            if (returnPC && codeBlock) {
+            if (returnPC && codeBlock && QScriptEnginePrivate::hasValidCodeBlockRegister(frame)) {
 #if ENABLE(JIT)
                 unsigned bytecodeOffset = codeBlock->getBytecodeIndex(frame, JSC::ReturnAddressPtr(returnPC));
 #else
@@ -171,7 +171,7 @@ QScriptContextInfoPrivate::QScriptContextInfoPrivate(const QScriptContext *conte
 
     // Get the filename and the scriptId:
     JSC::CodeBlock *codeBlock = frame->codeBlock();
-    if (codeBlock) {
+    if (codeBlock && QScriptEnginePrivate::hasValidCodeBlockRegister(frame)) {
            JSC::SourceProvider *source = codeBlock->source();
            scriptId = source->asID();
            fileName = source->url();
diff --git a/src/script/api/qscriptengine.cpp b/src/script/api/qscriptengine.cpp
index 54039c0..478fdaa 100644
--- a/src/script/api/qscriptengine.cpp
+++ b/src/script/api/qscriptengine.cpp
@@ -858,7 +858,8 @@ JSC::JSValue JSC_HOST_CALL functionQsTr(JSC::ExecState *exec, JSC::JSObject*, JS
     {
         JSC::ExecState *frame = exec->callerFrame()->removeHostCallFrameFlag();
         while (frame) {
-            if (frame->codeBlock() && frame->codeBlock()->source()
+            if (frame->codeBlock() && QScriptEnginePrivate::hasValidCodeBlockRegister(frame)
+                && frame->codeBlock()->source()
                 && !frame->codeBlock()->source()->url().isEmpty()) {
                 context = engine->translationContextFromUrl(frame->codeBlock()->source()->url());
                 break;
diff --git a/src/script/api/qscriptengine_p.h b/src/script/api/qscriptengine_p.h
index f8144e9..94d195e 100644
--- a/src/script/api/qscriptengine_p.h
+++ b/src/script/api/qscriptengine_p.h
@@ -56,6 +56,7 @@
 #include "Debugger.h"
 #include "ErrorInstance.h"
 #include "JSArray.h"
+#include "Executable.h"
 #include "Lexer.h"
 #include "RefPtr.h"
 #include "RegExpConstructor.h"
@@ -231,6 +232,8 @@ public:
     static inline JSC::ExecState *frameForContext(QScriptContext *context);
     static inline const JSC::ExecState *frameForContext(const QScriptContext *context);
 
+    static inline bool hasValidCodeBlockRegister(JSC::ExecState *frame);
+
     JSC::JSGlobalObject *originalGlobalObject() const;
     JSC::JSObject *getOriginalGlobalObjectProxy();
     JSC::JSObject *customGlobalObject() const;
@@ -862,6 +865,21 @@ inline const JSC::ExecState *QScriptEnginePrivate::frameForContext(const QScript
     return reinterpret_cast<const JSC::ExecState*>(context);
 }
 
+inline bool QScriptEnginePrivate::hasValidCodeBlockRegister(JSC::ExecState *frame)
+{
+#if ENABLE(JIT)
+    // Frames created by the VM don't have their CodeBlock register
+    // initialized. We can detect such frames by checking if the
+    // callee is a host JSFunction.
+    JSC::JSObject *callee = frame->callee();
+    return !(callee && callee->inherits(&JSC::JSFunction::info)
+             && JSC::asFunction(callee)->isHostFunction());
+#else
+    Q_UNUSED(frame);
+    return true;
+#endif
+}
+
 inline JSC::ExecState *QScriptEnginePrivate::globalExec() const
 {
     return originalGlobalObject()->globalExec();
diff --git a/tests/auto/qscriptcontext/tst_qscriptcontext.cpp b/tests/auto/qscriptcontext/tst_qscriptcontext.cpp
index dd21555..7915eb0 100644
--- a/tests/auto/qscriptcontext/tst_qscriptcontext.cpp
+++ b/tests/auto/qscriptcontext/tst_qscriptcontext.cpp
@@ -871,7 +871,36 @@ void tst_QScriptContext::backtrace_data()
         QStringList expected;
         expected << "<native>() at -1"
                  << "<anonymous>(0, 0, 0) at testfile:3"
-                 << "forEach(0) at -1"
+                 << QString::fromLatin1("forEach(%0) at -1")
+                    // Because the JIT doesn't store the arguments in the frame
+                    // for built-in functions, arguments are not available.
+                    // Will work when the copy of JavaScriptCore is updated
+                    // (QTBUG-16568).
+                    .arg(qt_script_isJITEnabled()
+                         ? ""
+                         : "function () {\n        result = bt();\n}")
+                 << "<global>() at testfile:4";
+        QTest::newRow("js callback from built-in") << source << expected;
+    }
+
+    {
+        QString source = QString::fromLatin1(
+            "[10,20].forEach(\n"
+            "    function() {\n"
+            "        result = bt();\n"
+            "}); result");
+
+        QStringList expected;
+        expected << "<native>() at -1"
+                 << "<anonymous>(20, 1, 10,20) at testfile:3"
+                 << QString::fromLatin1("forEach(%0) at -1")
+                    // Because the JIT doesn't store the arguments in the frame
+                    // for built-in functions, arguments are not available.
+                    // Will work when the copy of JavaScriptCore is updated
+                    // (QTBUG-16568).
+                    .arg(qt_script_isJITEnabled()
+                         ? ""
+                         : "function () {\n        result = bt();\n}")
                  << "<global>() at testfile:4";
         QTest::newRow("js callback from built-in") << source << expected;
     }
diff --git a/tests/auto/qscriptengine/tst_qscriptengine.cpp b/tests/auto/qscriptengine/tst_qscriptengine.cpp
index c3a0ba1..8de6fbc 100644
--- a/tests/auto/qscriptengine/tst_qscriptengine.cpp
+++ b/tests/auto/qscriptengine/tst_qscriptengine.cpp
@@ -164,6 +164,7 @@ private slots:
     void translationContext_data();
     void translationContext();
     void translateScriptIdBased();
+    void translateFromBuiltinCallback();
     void functionScopes();
     void nativeFunctionScopes();
     void evaluateProgram();
@@ -4725,6 +4726,21 @@ void tst_QScriptEngine::translateScriptIdBased()
              QString::fromLatin1("qtn_foo_bar")); // Doesn't have plural
 }
 
+void tst_QScriptEngine::translateFromBuiltinCallback()
+{
+    QScriptEngine eng;
+    eng.installTranslatorFunctions();
+
+    // Callback has no translation context.
+    eng.evaluate("function foo() { qsTr('foo'); }");
+
+    // Stack at translation time will be:
+    // qsTr, foo, forEach, global
+    // qsTr() needs to walk to the outer-most (global) frame before it finds
+    // a translation context, and this should not crash.
+    eng.evaluate("[10,20].forEach(foo)", "script.js");
+}
+
 void tst_QScriptEngine::functionScopes()
 {
     QScriptEngine eng;