Fix assert in stub-cache.cc when loading qml-mode functions

Commit c511fa8a6a631e45ee4075453bcb2eeb7f01ba63 introduced a
regression that could cause an assert in V8 to be triggered:

Fatal error in ../3rdparty/v8/src/stub-cache.cc, line 1171
CHECK(!CallIC::Contextual::decode( Code::ExtractExtraICStateFromFlags(flags))) failed

The assert would be triggered when 1) the "QML global object"
(implicit receiver in qml-mode evaluation) defined more than 12
properties, and 2) a function property of the QML global object was
looked up at least twice from JavaScript. In particular, the V8 IC
would assert that there should be an explicit receiver when
advancing the function lookup from pre-monomorphic state to
monomorphic state.

(The magic number 12 (number of properties needed to trigger the bug)
is the kMaxFastProperties constant. This is the threshold where V8
switches to using a dictionary for the object storage, instead of
using "fast properties" (array).)

The IC correctly determines that the property holder object (the QML
global object) is not the JS Global Object, and hence compiles it as
a "normal" call, in which it asserts that "there must be an explicit
receiver here". But in QML mode, the receiver can be implicit when
the holder is the QML global object.

Propagate this information to the stub cache to avoid the assert
triggering. The generated JIT code still works as expected when the
receiver is implicit (i.e., the receiver is always loaded).
None of the other property/function-load IC handlers have a similar
assert.

It might be worth investigating whether "QML global object" property
loading should have dedicated cache stubs; e.g. a
ComputeCallQmlGlobal() akin to ComputeCallGlobal(). This could
potentially simplify/speed up the generated code, since we can take
advantage of the read-only nature of the QML global object type right
off the bat. But that's a bigger change that would require all the
JIT compilers and the GC to be adapted.

Task-number: QTBUG-24871
Change-Id: I55e499d9c61ff264e3a96e5628e2f30292ee565d
Reviewed-by: Kent Hansen <kent.hansen@nokia.com>

diff --git a/src/3rdparty/v8/src/ic.cc b/src/3rdparty/v8/src/ic.cc
index fbe77b0..3b6ec67 100644 (file)
--- a/src/3rdparty/v8/src/ic.cc
+++ b/src/3rdparty/v8/src/ic.cc
@@ -622,7 +622,7 @@ Handle<Code> CallICBase::ComputeMonomorphicStub(LookupResult* lookup,
         // applicable.
         if (!holder.is_identical_to(receiver)) return Handle<Code>::null();
         return isolate()->stub_cache()->ComputeCallNormal(
-            argc, kind_, extra_state);
+            argc, kind_, extra_state, IsQmlGlobal(holder));
       }
       break;
     }

diff --git a/src/3rdparty/v8/src/stub-cache.cc b/src/3rdparty/v8/src/stub-cache.cc
index 8b6e28f..9e7fe16 100644 (file)
--- a/src/3rdparty/v8/src/stub-cache.cc
+++ b/src/3rdparty/v8/src/stub-cache.cc
@@ -757,7 +757,8 @@ Handle<Code> StubCache::ComputeCallPreMonomorphic(
 
 Handle<Code> StubCache::ComputeCallNormal(int argc,
                                           Code::Kind kind,
-                                          Code::ExtraICState extra_state) {
+                                          Code::ExtraICState extra_state,
+                                          bool has_qml_global_receiver) {
   Code::Flags flags =
       Code::ComputeFlags(kind, MONOMORPHIC, extra_state, NORMAL, argc);
   Handle<NumberDictionary> cache = isolate_->factory()->non_monomorphic_cache();
@@ -765,7 +766,7 @@ Handle<Code> StubCache::ComputeCallNormal(int argc,
   if (entry != -1) return Handle<Code>(Code::cast(cache->ValueAt(entry)));
 
   StubCompiler compiler(isolate_);
-  Handle<Code> code = compiler.CompileCallNormal(flags);
+  Handle<Code> code = compiler.CompileCallNormal(flags, has_qml_global_receiver);
   FillCache(isolate_, code);
   return code;
 }
@@ -1162,13 +1163,15 @@ Handle<Code> StubCompiler::CompileCallPreMonomorphic(Code::Flags flags) {
 }
 
 
-Handle<Code> StubCompiler::CompileCallNormal(Code::Flags flags) {
+Handle<Code> StubCompiler::CompileCallNormal(Code::Flags flags, bool has_qml_global_receiver) {
   int argc = Code::ExtractArgumentsCountFromFlags(flags);
   Code::Kind kind = Code::ExtractKindFromFlags(flags);
   if (kind == Code::CALL_IC) {
-    // Call normal is always with a explict receiver.
+    // Call normal is always with a explict receiver,
+    // or with an implicit qml global receiver.
     ASSERT(!CallIC::Contextual::decode(
-        Code::ExtractExtraICStateFromFlags(flags)));
+        Code::ExtractExtraICStateFromFlags(flags)) ||
+        has_qml_global_receiver);
     CallIC::GenerateNormal(masm(), argc);
   } else {
     KeyedCallIC::GenerateNormal(masm(), argc);

diff --git a/src/3rdparty/v8/src/stub-cache.h b/src/3rdparty/v8/src/stub-cache.h
index cc42e05..725c70c 100644 (file)
--- a/src/3rdparty/v8/src/stub-cache.h
+++ b/src/3rdparty/v8/src/stub-cache.h
@@ -218,7 +218,8 @@ class StubCache {
 
   Handle<Code> ComputeCallNormal(int argc,
                                  Code::Kind kind,
-                                 Code::ExtraICState state);
+                                 Code::ExtraICState state,
+                                 bool has_qml_global_receiver);
 
   Handle<Code> ComputeCallArguments(int argc, Code::Kind kind);
 
@@ -383,7 +384,7 @@ class StubCompiler BASE_EMBEDDED {
   // is extracted from the code flags.
   Handle<Code> CompileCallInitialize(Code::Flags flags);
   Handle<Code> CompileCallPreMonomorphic(Code::Flags flags);
-  Handle<Code> CompileCallNormal(Code::Flags flags);
+  Handle<Code> CompileCallNormal(Code::Flags flags, bool has_qml_global_receiver);
   Handle<Code> CompileCallMegamorphic(Code::Flags flags);
   Handle<Code> CompileCallArguments(Code::Flags flags);
   Handle<Code> CompileCallMiss(Code::Flags flags);

diff --git a/tests/auto/v8/tst_v8.cpp b/tests/auto/v8/tst_v8.cpp
index a131441..005951e 100644 (file)
--- a/tests/auto/v8/tst_v8.cpp
+++ b/tests/auto/v8/tst_v8.cpp
@@ -62,6 +62,7 @@ private slots:
     void getcallingqmlglobal();
     void typeOf();
     void referenceerror();
+    void qtbug_24871();
 };
 
 void tst_v8::eval()
@@ -104,6 +105,11 @@ void tst_v8::referenceerror()
     QVERIFY(v8test_referenceerror());
 }
 
+void tst_v8::qtbug_24871()
+{
+    QVERIFY(v8test_qtbug_24871());
+}
+
 int main(int argc, char *argv[])
 {
     V8::SetFlagsFromCommandLine(&argc, argv, true);

diff --git a/tests/auto/v8/v8test.cpp b/tests/auto/v8/v8test.cpp
index f0d3aa8..3ea313f 100644 (file)
--- a/tests/auto/v8/v8test.cpp
+++ b/tests/auto/v8/v8test.cpp
@@ -493,5 +493,41 @@ cleanup:
     ENDTEST();
 }
 
+bool v8test_qtbug_24871()
+{
+    BEGINTEST();
+
+    HandleScope handle_scope;
+    Persistent<Context> context = Context::New();
+    Context::Scope context_scope(context);
+
+    Local<Object> qmlglobal = Object::New();
+
+    Local<Script> script = Script::Compile(String::New(
+                                               // Create a bunch of properties to exceed kMaxFastProperties
+                                               "var a1, a2, a3, a4, a5, a6, a7, a8;\n"
+                                               "var b1, b2, b3, b4, b5, b6, b7, b8;\n"
+                                               "var c1, c2, c3, c4, c5, c6, c7, c8;\n"
+                                               "var d1, d2, d3, d4, d5, d6, d7, d8;\n"
+                                               "function index(a) { return a + 1; }\n"
+                                               "function init() {\n"
+                                               "  for (var i = 0; i < 300; ++i)\n"
+                                               "    index(i);\n"
+                                               "}\n"
+                                               "init();"), NULL, NULL,
+                                           Handle<String>(), Script::QmlMode);
+
+    TryCatch tc;
+    Local<Value> result = script->Run(qmlglobal);
+
+    VERIFY(!tc.HasCaught());
+    VERIFY(result->IsUndefined());
+
+cleanup:
+    context.Dispose();
+
+    ENDTEST();
+}
+
 #undef VARNAME
 #undef VARVALUE

diff --git a/tests/auto/v8/v8test.h b/tests/auto/v8/v8test.h
index 38dfa62..d497384 100644 (file)
--- a/tests/auto/v8/v8test.h
+++ b/tests/auto/v8/v8test.h
@@ -56,6 +56,7 @@ bool v8test_globalcall();
 bool v8test_getcallingqmlglobal();
 bool v8test_typeof();
 bool v8test_referenceerror();
+bool v8test_qtbug_24871();
 
 #endif // V8TEST_H