Prepare for white allocations during gc (1/9): Write barrier for Lookups

Lookups can (and do) reference HeapItems. This was safe in a
non-incremental gc world, as Lookups are always reachable via their
containing CompilationUnits, which are part of the root set. However,
when using an incremental gc, an already marked Lookup might reference a
new heap item, which then becomes otherwise unreachable.
This is alleviated  by the fact that Lookups generally either refer to
something already existing or a freshly allocated string. The latter
however is only safe when we can rely on black allocations during gc,
and the former is somewhat reckless.

Remedy this by employing the WriteBarrier for Lookups. We wrap all
HeapItems in a helper class, which -while trivial itself- can't be used
for direct assignments. Intead, it employs a set method which ensures
that the WriteBarrier is used.

Task-number: QTBUG-121910
Change-Id: I6a0ede66ad044076d2e87f134bc95686cb586aee
Reviewed-by: Olivier De Cannière <olivier.decanniere@qt.io>
Reviewed-by: Sami Shalayel <sami.shalayel@qt.io>
Reviewed-by: Ulf Hermann <ulf.hermann@qt.io>

6 files changed, 47 insertions, 37 deletions

diff --git a/src/qml/jsruntime/qv4lookup.cpp b/src/qml/jsruntime/qv4lookup.cpp
index 402cf70408..654275a709 100644
--- a/src/qml/jsruntime/qv4lookup.cpp
+++ b/src/qml/jsruntime/qv4lookup.cpp
@@ -54,12 +54,12 @@ ReturnedValue Lookup::resolvePrimitiveGetter(ExecutionEngine *engine, const Valu
         return engine->throwTypeError(message);
     }
     case Value::Boolean_Type:
-        primitiveLookup.proto = engine->booleanPrototype()->d();
+        primitiveLookup.proto.set(engine, engine->booleanPrototype()->d());
         break;
     case Value::Managed_Type: {
         // ### Should move this over to the Object path, as strings also have an internalClass
         Q_ASSERT(object.isStringOrSymbol());
-        primitiveLookup.proto = static_cast<const Managed &>(object).internalClass()->prototype;
+        primitiveLookup.proto.set(engine, static_cast<const Managed &>(object).internalClass()->prototype);
         Q_ASSERT(primitiveLookup.proto);
         Scope scope(engine);
         ScopedString name(scope, engine->currentStackFrame->v4Function->compilationUnit->runtimeStrings[nameIndex]);
@@ -72,7 +72,7 @@ ReturnedValue Lookup::resolvePrimitiveGetter(ExecutionEngine *engine, const Valu
     }
     case Value::Integer_Type:
     default: // Number
-        primitiveLookup.proto = engine->numberPrototype()->d();
+        primitiveLookup.proto.set(engine, engine->numberPrototype()->d());
     }
 
     PropertyKey name = engine->identifierTable->asPropertyKey(engine->currentStackFrame->v4Function->compilationUnit->runtimeStrings[nameIndex]);
@@ -122,9 +122,10 @@ static inline void setupObjectLookupTwoClasses(Lookup *l, const Lookup &first, c
     const uint offset1 = first.objectLookup.offset;
     Heap::InternalClass *ic2 = second.objectLookup.ic;
     const uint offset2 = second.objectLookup.offset;
+    auto engine = ic1->engine;
 
-    l->objectLookupTwoClasses.ic = ic1;
-    l->objectLookupTwoClasses.ic2 = ic2;
+    l->objectLookupTwoClasses.ic.set(engine, ic1);
+    l->objectLookupTwoClasses.ic2.set(engine, ic2);
     l->objectLookupTwoClasses.offset = offset1;
     l->objectLookupTwoClasses.offset2 = offset2;
 }
@@ -565,8 +566,9 @@ bool Lookup::setterTwoClasses(Lookup *l, ExecutionEngine *engine, Value &object,
         }
 
         if (l->setter == Lookup::setter0MemberData || l->setter == Lookup::setter0Inline) {
-            l->objectLookupTwoClasses.ic = ic;
-            l->objectLookupTwoClasses.ic2 = ic;
+            auto engine = ic->engine;
+            l->objectLookupTwoClasses.ic.set(engine, ic);
+            l->objectLookupTwoClasses.ic2.set(engine, ic);
             l->objectLookupTwoClasses.offset = index;
             l->objectLookupTwoClasses.offset2 = index;
             l->setter = setter0setter0;
diff --git a/src/qml/jsruntime/qv4lookup_p.h b/src/qml/jsruntime/qv4lookup_p.h
index e4bb08d2f6..258184cd37 100644
--- a/src/qml/jsruntime/qv4lookup_p.h
+++ b/src/qml/jsruntime/qv4lookup_p.h
@@ -19,6 +19,7 @@
 #include "qv4internalclass_p.h"
 #include "qv4qmlcontext_p.h"
 #include <private/qqmltypewrapper_p.h>
+#include <private/qv4mm_p.h>
 
 QT_BEGIN_NAMESPACE
 
@@ -28,6 +29,9 @@ namespace Heap {
     struct QObjectMethod;
 }
 
+template <typename T, int PhantomTag>
+using HeapObjectWrapper = WriteBarrier::HeapObjectWrapper<T, PhantomTag>;
+
 // Note: We cannot hide the copy ctor and assignment operator of this class because it needs to
 //       be trivially copyable. But you should never ever copy it. There are refcounted members
 //       in there.
@@ -48,7 +52,7 @@ struct Q_QML_EXPORT Lookup {
             quintptr unused2;
         } markDef;
         struct {
-            Heap::InternalClass *ic;
+            HeapObjectWrapper<Heap::InternalClass, 0> ic;
             quintptr unused;
             uint index;
             uint offset;
@@ -59,8 +63,8 @@ struct Q_QML_EXPORT Lookup {
             const Value *data;
         } protoLookup;
         struct {
-            Heap::InternalClass *ic;
-            Heap::InternalClass *ic2;
+            HeapObjectWrapper<Heap::InternalClass, 1> ic;
+            HeapObjectWrapper<Heap::InternalClass, 2> ic2;
             uint offset;
             uint offset2;
         } objectLookupTwoClasses;
@@ -73,12 +77,12 @@ struct Q_QML_EXPORT Lookup {
         struct {
             // Make sure the next two values are in sync with protoLookup
             quintptr protoId;
-            Heap::Object *proto;
+            HeapObjectWrapper<Heap::Object, 3> proto;
             const Value *data;
             quintptr type;
         } primitiveLookup;
         struct {
-            Heap::InternalClass *newClass;
+            HeapObjectWrapper<Heap::InternalClass, 4> newClass;
             quintptr protoId;
             uint offset;
             uint unused;
@@ -90,14 +94,14 @@ struct Q_QML_EXPORT Lookup {
             uint unused;
         } indexedLookup;
         struct {
-            Heap::InternalClass *ic;
-            Heap::InternalClass *qmlTypeIc; // only used when lookup goes through QQmlTypeWrapper
+            HeapObjectWrapper<Heap::InternalClass, 5> ic;
+            HeapObjectWrapper<Heap::InternalClass, 6> qmlTypeIc; // only used when lookup goes through QQmlTypeWrapper
             const QQmlPropertyCache *propertyCache;
             const QQmlPropertyData *propertyData;
         } qobjectLookup;
         struct {
-            Heap::InternalClass *ic;
-            Heap::QObjectMethod *method;
+            HeapObjectWrapper<Heap::InternalClass, 7> ic;
+            HeapObjectWrapper<Heap::QObjectMethod, 8> method;
             const QQmlPropertyCache *propertyCache;
             const QQmlPropertyData *propertyData;
         } qobjectMethodLookup;
@@ -108,7 +112,7 @@ struct Q_QML_EXPORT Lookup {
             int notifyIndex;
         } qobjectFallbackLookup;
         struct {
-            Heap::InternalClass *ic;
+            HeapObjectWrapper<Heap::InternalClass, 9> ic;
             quintptr metaObject; // a (const QMetaObject* & 1) or nullptr
             const QtPrivate::QMetaTypeInterface *metaType; // cannot use QMetaType; class must be trivial
             quint16 coreIndex;
@@ -121,7 +125,7 @@ struct Q_QML_EXPORT Lookup {
             int scriptIndex;
         } qmlContextScriptLookup;
         struct {
-            Heap::Base *singletonObject;
+            HeapObjectWrapper<Heap::Base, 10> singletonObject;
             quintptr unused2;
             QV4::ReturnedValue singletonValue;
         } qmlContextSingletonLookup;
@@ -138,18 +142,18 @@ struct Q_QML_EXPORT Lookup {
             ReturnedValue (*getterTrampoline)(Lookup *l, ExecutionEngine *engine);
         } qmlContextGlobalLookup;
         struct {
-            Heap::Base *qmlTypeWrapper;
+            HeapObjectWrapper<Heap::Base, 11> qmlTypeWrapper;
             quintptr unused2;
         } qmlTypeLookup;
         struct {
-            Heap::InternalClass *ic;
+            HeapObjectWrapper<Heap::InternalClass, 12> ic;
             quintptr unused;
             ReturnedValue encodedEnumValue;
             const QtPrivate::QMetaTypeInterface *metaType;
         } qmlEnumValueLookup;
         struct {
-            Heap::InternalClass *ic;
-            Heap::Object *qmlScopedEnumWrapper;
+            HeapObjectWrapper<Heap::InternalClass, 13> ic;
+            HeapObjectWrapper<Heap::Object, 14> qmlScopedEnumWrapper;
         } qmlScopedEnumWrapperLookup;
     };
 
@@ -251,7 +255,7 @@ inline void setupQObjectLookup(
         const Object *self)
 {
     setupQObjectLookup(lookup, ddata, propertyData);
-    lookup->qobjectLookup.ic = self->internalClass();
+    lookup->qobjectLookup.ic.set(self->engine(), self->internalClass());
 }
 
 
@@ -260,17 +264,20 @@ inline void setupQObjectLookup(
         const Object *self, const Object *qmlType)
 {
     setupQObjectLookup(lookup, ddata, propertyData, self);
-    lookup->qobjectLookup.qmlTypeIc = qmlType->internalClass();
+    lookup->qobjectLookup.qmlTypeIc.set(self->engine(), qmlType->internalClass());
 }
 
+// template parameter is an ugly trick to avoid pulling in the QObjectMethod header here
+template<typename QObjectMethod = Heap::QObjectMethod>
 inline void setupQObjectMethodLookup(
         Lookup *lookup, const QQmlData *ddata, const QQmlPropertyData *propertyData,
-        const Object *self, Heap::QObjectMethod *method)
+        const Object *self, QObjectMethod *method)
 {
     lookup->releasePropertyCache();
     Q_ASSERT(!ddata->propertyCache.isNull());
-    lookup->qobjectMethodLookup.method = method;
-    lookup->qobjectMethodLookup.ic = self->internalClass();
+    auto engine = self->engine();
+    lookup->qobjectMethodLookup.method.set(engine, method);
+    lookup->qobjectMethodLookup.ic.set(engine, self->internalClass());
     lookup->qobjectMethodLookup.propertyCache = ddata->propertyCache.data();
     lookup->qobjectMethodLookup.propertyCache->addref();
     lookup->qobjectMethodLookup.propertyData = propertyData;
diff --git a/src/qml/jsruntime/qv4object.cpp b/src/qml/jsruntime/qv4object.cpp
index 49d351da67..1ad5e063e8 100644
--- a/src/qml/jsruntime/qv4object.cpp
+++ b/src/qml/jsruntime/qv4object.cpp
@@ -768,7 +768,7 @@ ReturnedValue Object::virtualResolveLookupGetter(const Object *object, Execution
         } else {
             lookup->getter = Lookup::getterAccessor;
         }
-        lookup->objectLookup.ic = obj->internalClass;
+        lookup->objectLookup.ic.set(engine, obj->internalClass.get());
         lookup->objectLookup.offset = index.index;
         return lookup->getter(lookup, engine, *object);
     }
@@ -795,7 +795,7 @@ bool Object::virtualResolveLookupSetter(Object *object, ExecutionEngine *engine,
             lookup->setter = Lookup::arrayLengthSetter;
             return lookup->setter(lookup, engine, *object, value);
         } else if (idx.attrs.isData() && idx.attrs.isWritable()) {
-            lookup->objectLookup.ic = object->internalClass();
+            lookup->objectLookup.ic.set(engine, object->internalClass());
             lookup->objectLookup.index = idx.index;
             const auto nInline = object->d()->vtable()->nInlineProperties;
             if (idx.index < nInline) {
@@ -829,7 +829,7 @@ bool Object::virtualResolveLookupSetter(Object *object, ExecutionEngine *engine,
         lookup->setter = Lookup::setterFallback;
         return false;
     }
-    lookup->insertionLookup.newClass = object->internalClass();
+    lookup->insertionLookup.newClass.set(engine, object->internalClass());
     lookup->insertionLookup.offset = idx.index;
     lookup->setter = Lookup::setterInsert;
     return true;
diff --git a/src/qml/jsruntime/qv4qmlcontext.cpp b/src/qml/jsruntime/qv4qmlcontext.cpp
index 708339013f..53444cddb7 100644
--- a/src/qml/jsruntime/qv4qmlcontext.cpp
+++ b/src/qml/jsruntime/qv4qmlcontext.cpp
@@ -223,10 +223,10 @@ ReturnedValue QQmlContextWrapper::getPropertyAndBase(const QQmlContextWrapper *r
                         QQmlEnginePrivate *e = QQmlEnginePrivate::get(v4->qmlEngine());
                         if (r.type.isQObjectSingleton() || r.type.isCompositeSingleton()) {
                             e->singletonInstance<QObject*>(r.type);
-                            lookup->qmlContextSingletonLookup.singletonObject =
+                            lookup->qmlContextSingletonLookup.singletonObject.set(v4,
                                         Value::fromReturnedValue(
                                             QQmlTypeWrapper::create(v4, nullptr, r.type)
-                                        ).heapObject();
+                                        ).heapObject());
                         } else {
                             QJSValue singleton = e->singletonInstance<QJSValue>(r.type);
 
@@ -235,7 +235,7 @@ ReturnedValue QQmlContextWrapper::getPropertyAndBase(const QQmlContextWrapper *r
                             Q_ASSERT(!QJSValuePrivate::asQString(&singleton));
 
                             if (QV4::Value *val = QJSValuePrivate::takeManagedValue(&singleton)) {
-                                lookup->qmlContextSingletonLookup.singletonObject = val->heapObject();
+                                lookup->qmlContextSingletonLookup.singletonObject.set(v4, val->heapObject());
                             } else {
                                 lookup->qmlContextSingletonLookup.singletonValue = QJSValuePrivate::asReturnedValue(&singleton);
                                 isValueSingleton = true;
@@ -251,7 +251,7 @@ ReturnedValue QQmlContextWrapper::getPropertyAndBase(const QQmlContextWrapper *r
                 result = QQmlTypeWrapper::create(v4, scopeObject, context->imports(), r.importNamespace);
             }
             if (lookup) {
-                lookup->qmlTypeLookup.qmlTypeWrapper = result->heapObject();
+                lookup->qmlTypeLookup.qmlTypeWrapper.set(v4, result->heapObject());
                 lookup->qmlContextPropertyGetter = QQmlContextWrapper::lookupType;
             }
             return result->asReturnedValue();
@@ -815,7 +815,7 @@ ReturnedValue QQmlContextWrapper::lookupType(Lookup *l, ExecutionEngine *engine,
 
     Heap::Base *heapObject = l->qmlTypeLookup.qmlTypeWrapper;
     if (static_cast<Heap::QQmlTypeWrapper *>(heapObject)->object != scopeObject) {
-        l->qmlTypeLookup.qmlTypeWrapper = nullptr;
+        l->qmlTypeLookup.qmlTypeWrapper.clear();
         l->qmlContextPropertyGetter = QQmlContextWrapper::resolveQmlContextPropertyLookupGetter;
         return QQmlContextWrapper::resolveQmlContextPropertyLookupGetter(l, engine, base);
     }
diff --git a/src/qml/jsruntime/qv4qobjectwrapper.cpp b/src/qml/jsruntime/qv4qobjectwrapper.cpp
index c37031068c..354b3ad874 100644
--- a/src/qml/jsruntime/qv4qobjectwrapper.cpp
+++ b/src/qml/jsruntime/qv4qobjectwrapper.cpp
@@ -1115,7 +1115,8 @@ ReturnedValue QObjectWrapper::virtualResolveLookupGetter(const Object *object, E
             && !property->isVarProperty()
             && !property->isVMEFunction() // Handled by QObjectLookup
             && !property->isSignalHandler()) { // TODO: Optimize SignalHandler, too
-        setupQObjectMethodLookup(lookup, ddata, property, This, nullptr);
+        QV4::Heap::QObjectMethod *method = nullptr;
+        setupQObjectMethodLookup(lookup, ddata, property, This, method);
         lookup->getter = Lookup::getterQObjectMethod;
         return lookup->getter(lookup, engine, *object);
     }
diff --git a/src/qml/jsruntime/qv4qobjectwrapper_p.h b/src/qml/jsruntime/qv4qobjectwrapper_p.h
index 2cd2faedf8..17c2acb459 100644
--- a/src/qml/jsruntime/qv4qobjectwrapper_p.h
+++ b/src/qml/jsruntime/qv4qobjectwrapper_p.h
@@ -341,7 +341,7 @@ inline ReturnedValue QObjectWrapper::lookupMethodGetterImpl(
     if (!v->as<QObjectMethod>())
         return revertLookup();
 
-    lookup->qobjectMethodLookup.method = static_cast<Heap::QObjectMethod *>(v->heapObject());
+    lookup->qobjectMethodLookup.method.set(engine, static_cast<Heap::QObjectMethod *>(v->heapObject()));
     return v->asReturnedValue();
 }