Encapsulate and protect all accesses to the vtable of Heap objects

This is required, so we can safely access the vtable even while
we're marking objects during GC.

Change-Id: I34f56b61b4bca0d0742faf607eb5ab8b2c30685e
Reviewed-by: Simon Hausmann <simon.hausmann@theqtcompany.com>

diff --git a/src/qml/jsruntime/qv4argumentsobject.cpp b/src/qml/jsruntime/qv4argumentsobject.cpp
index 55753ae..39dce8e 100644 (file)
--- a/src/qml/jsruntime/qv4argumentsobject.cpp
+++ b/src/qml/jsruntime/qv4argumentsobject.cpp
@@ -45,7 +45,7 @@ Heap::ArgumentsObject::ArgumentsObject(QV4::CallContext *context)
     , context(context->d())
     , fullyCreated(false)
 {
-    Q_ASSERT(vtable == QV4::ArgumentsObject::staticVTable());
+    Q_ASSERT(vtable() == QV4::ArgumentsObject::staticVTable());
 
     ExecutionEngine *v4 = context->d()->engine;
     Scope scope(v4);

diff --git a/src/qml/jsruntime/qv4argumentsobject_p.h b/src/qml/jsruntime/qv4argumentsobject_p.h
index 1965942..aab5e2c 100644 (file)
--- a/src/qml/jsruntime/qv4argumentsobject_p.h
+++ b/src/qml/jsruntime/qv4argumentsobject_p.h
@@ -106,7 +106,7 @@ struct ArgumentsObject: Object {
     Heap::MemberData *mappedArguments() { return d()->mappedArguments; }
 
     static bool isNonStrictArgumentsObject(Managed *m) {
-        return m->d()->vtable->type == Type_ArgumentsObject &&
+        return m->d()->vtable()->type == Type_ArgumentsObject &&
                 !static_cast<ArgumentsObject *>(m)->context()->strictMode;
     }
 

diff --git a/src/qml/jsruntime/qv4arraydata_p.h b/src/qml/jsruntime/qv4arraydata_p.h
index 8be15fc..729e657 100644 (file)
--- a/src/qml/jsruntime/qv4arraydata_p.h
+++ b/src/qml/jsruntime/qv4arraydata_p.h
@@ -96,7 +96,7 @@ struct ArrayData : public Base {
 
     bool isSparse() const { return type == Sparse; }
 
-    const ArrayVTable *vtable() const { return reinterpret_cast<const ArrayVTable *>(Base::vtable); }
+    const ArrayVTable *vtable() const { return reinterpret_cast<const ArrayVTable *>(Base::vtable()); }
 
     inline ReturnedValue get(uint i) const {
         return vtable()->get(this, i);

diff --git a/src/qml/jsruntime/qv4dateobject_p.h b/src/qml/jsruntime/qv4dateobject_p.h
index 4bf0fd5..7a6413e 100644 (file)
--- a/src/qml/jsruntime/qv4dateobject_p.h
+++ b/src/qml/jsruntime/qv4dateobject_p.h
@@ -81,7 +81,7 @@ struct DateObject: Object {
 
 template<>
 inline const DateObject *Value::as() const {
-    return isManaged() && m() && m()->vtable->type == Managed::Type_DateObject ? static_cast<const DateObject *>(this) : 0;
+    return isManaged() && m() && m()->vtable()->type == Managed::Type_DateObject ? static_cast<const DateObject *>(this) : 0;
 }
 
 struct DateCtor: FunctionObject

diff --git a/src/qml/jsruntime/qv4engine.cpp b/src/qml/jsruntime/qv4engine.cpp
index 4f22f90..320ed59 100644 (file)
--- a/src/qml/jsruntime/qv4engine.cpp
+++ b/src/qml/jsruntime/qv4engine.cpp
@@ -303,7 +303,7 @@ ExecutionEngine::ExecutionEngine(EvalISelFactory *factory)
     strictArgumentsObjectClass = strictArgumentsObjectClass->addMember(id_caller(), Attr_Accessor|Attr_NotConfigurable|Attr_NotEnumerable);
 
     *static_cast<Value *>(globalObject) = newObject();
-    Q_ASSERT(globalObject->d()->vtable);
+    Q_ASSERT(globalObject->d()->vtable());
     initRootContext();
 
     jsObjects[StringProto] = memoryManager->alloc<StringPrototype>(emptyClass, objectPrototype());
@@ -403,7 +403,7 @@ ExecutionEngine::ExecutionEngine(EvalISelFactory *factory)
     //
     rootContext()->d()->global = globalObject->d();
     rootContext()->d()->callData->thisObject = globalObject;
-    Q_ASSERT(globalObject->d()->vtable);
+    Q_ASSERT(globalObject->d()->vtable());
 
     globalObject->defineDefaultProperty(QStringLiteral("Object"), *objectCtor());
     globalObject->defineDefaultProperty(QStringLiteral("String"), *stringCtor());
@@ -919,7 +919,7 @@ void ExecutionEngine::markObjects()
         Q_ASSERT(c->inUse());
         if (!c->isMarked()) {
             c->setMarkBit();
-            c->gcGetVtable()->markObjects(c, this);
+            c->vtable()->markObjects(c, this);
         }
         c = c->parent;
     }
@@ -1553,7 +1553,7 @@ QV4::ReturnedValue ExecutionEngine::metaTypeToJS(int type, const void *data)
 
 void ExecutionEngine::assertObjectBelongsToEngine(const Heap::Base &baseObject)
 {
-    Q_ASSERT(!baseObject.vtable->isObject || static_cast<const Heap::Object&>(baseObject).internalClass->engine == this);
+    Q_ASSERT(!baseObject.vtable()->isObject || static_cast<const Heap::Object&>(baseObject).internalClass->engine == this);
     Q_UNUSED(baseObject);
 }
 

diff --git a/src/qml/jsruntime/qv4errorobject_p.h b/src/qml/jsruntime/qv4errorobject_p.h
index 2595801..e0fbcb4 100644 (file)
--- a/src/qml/jsruntime/qv4errorobject_p.h
+++ b/src/qml/jsruntime/qv4errorobject_p.h
@@ -142,7 +142,7 @@ struct ErrorObject: Object {
 
 template<>
 inline const ErrorObject *Value::as() const {
-    return isManaged() && m() && m()->vtable->isErrorObject ? reinterpret_cast<const ErrorObject *>(this) : 0;
+    return isManaged() && m() && m()->vtable()->isErrorObject ? reinterpret_cast<const ErrorObject *>(this) : 0;
 }
 
 struct EvalErrorObject: ErrorObject {

diff --git a/src/qml/jsruntime/qv4functionobject.cpp b/src/qml/jsruntime/qv4functionobject.cpp
index d8cdda4..da01592 100644 (file)
--- a/src/qml/jsruntime/qv4functionobject.cpp
+++ b/src/qml/jsruntime/qv4functionobject.cpp
@@ -208,12 +208,12 @@ Heap::FunctionObject *FunctionObject::createScriptFunction(ExecutionContext *sco
 
 bool FunctionObject::isBinding() const
 {
-    return d()->vtable == QQmlBindingFunction::staticVTable();
+    return d()->vtable() == QQmlBindingFunction::staticVTable();
 }
 
 bool FunctionObject::isBoundFunction() const
 {
-    return d()->vtable == BoundFunction::staticVTable();
+    return d()->vtable() == BoundFunction::staticVTable();
 }
 
 QQmlSourceLocation FunctionObject::sourceLocation() const
@@ -513,7 +513,10 @@ ReturnedValue SimpleScriptFunction::construct(const Managed *that, CallData *cal
     ExecutionContextSaver ctxSaver(scope, v4->currentContext());
 
     CallContext::Data ctx(v4);
-    ctx.vtable = CallContext::staticVTable();
+#ifndef QT_NO_DEBUG
+    ctx.mm_data = 0; // make sure we don't run into the assertion in setVTable when allocating a context on the stack
+#endif
+    ctx.setVtable(CallContext::staticVTable());
     ctx.strictMode = f->strictMode();
     ctx.callData = callData;
     ctx.function = f->d();
@@ -548,7 +551,10 @@ ReturnedValue SimpleScriptFunction::call(const Managed *that, CallData *callData
     ExecutionContextSaver ctxSaver(scope, v4->currentContext());
 
     CallContext::Data ctx(v4);
-    ctx.vtable = CallContext::staticVTable();
+#ifndef QT_NO_DEBUG
+    ctx.mm_data = 0; // make sure we don't run into the assertion in setVTable when allocating a context on the stack
+#endif
+    ctx.setVtable(CallContext::staticVTable());
     ctx.strictMode = f->strictMode();
     ctx.callData = callData;
     ctx.function = f->d();
@@ -604,7 +610,10 @@ ReturnedValue BuiltinFunction::call(const Managed *that, CallData *callData)
     ExecutionContextSaver ctxSaver(scope, v4->currentContext());
 
     CallContext::Data ctx(v4);
-    ctx.vtable = CallContext::staticVTable();
+#ifndef QT_NO_DEBUG
+    ctx.mm_data = 0; // make sure we don't run into the assertion in setVTable when allocating a context on the stack
+#endif
+    ctx.setVtable(CallContext::staticVTable());
     ctx.strictMode = f->scope()->strictMode; // ### needed? scope or parent context?
     ctx.callData = callData;
     Q_ASSERT(v4->currentContext() == &ctx);
@@ -625,7 +634,10 @@ ReturnedValue IndexedBuiltinFunction::call(const Managed *that, CallData *callDa
     ExecutionContextSaver ctxSaver(scope, v4->currentContext());
 
     CallContext::Data ctx(v4);
-    ctx.vtable = CallContext::staticVTable();
+#ifndef QT_NO_DEBUG
+    ctx.mm_data = 0; // make sure we don't run into the assertion in setVTable when allocating a context on the stack
+#endif
+    ctx.setVtable(CallContext::staticVTable());
     ctx.strictMode = f->scope()->strictMode; // ### needed? scope or parent context?
     ctx.callData = callData;
     Q_ASSERT(v4->currentContext() == &ctx);

diff --git a/src/qml/jsruntime/qv4functionobject_p.h b/src/qml/jsruntime/qv4functionobject_p.h
index 2ce3068..2c6a195 100644 (file)
--- a/src/qml/jsruntime/qv4functionobject_p.h
+++ b/src/qml/jsruntime/qv4functionobject_p.h
@@ -149,7 +149,7 @@ struct Q_QML_EXPORT FunctionObject: Object {
 
 template<>
 inline const FunctionObject *Value::as() const {
-    return isManaged() && m() && m()->vtable->isFunctionObject ? reinterpret_cast<const FunctionObject *>(this) : 0;
+    return isManaged() && m() && m()->vtable()->isFunctionObject ? reinterpret_cast<const FunctionObject *>(this) : 0;
 }
 
 

diff --git a/src/qml/jsruntime/qv4identifiertable_p.h b/src/qml/jsruntime/qv4identifiertable_p.h
index ff37422..58f808b 100644 (file)
--- a/src/qml/jsruntime/qv4identifiertable_p.h
+++ b/src/qml/jsruntime/qv4identifiertable_p.h
@@ -80,8 +80,8 @@ public:
             if (!entry || entry->isMarked())
                 continue;
             entry->setMarkBit();
-            Q_ASSERT(entry->gcGetVtable()->markObjects);
-            entry->gcGetVtable()->markObjects(entry, e);
+            Q_ASSERT(entry->vtable()->markObjects);
+            entry->vtable()->markObjects(entry, e);
         }
     }
 };

diff --git a/src/qml/jsruntime/qv4managed.cpp b/src/qml/jsruntime/qv4managed.cpp
index 5461900..bb7ee43 100644 (file)
--- a/src/qml/jsruntime/qv4managed.cpp
+++ b/src/qml/jsruntime/qv4managed.cpp
@@ -59,7 +59,7 @@ const VTable Managed::static_vtbl =
 QString Managed::className() const
 {
     const char *s = 0;
-    switch (Type(d()->vtable->type)) {
+    switch (Type(d()->vtable()->type)) {
     case Type_Invalid:
     case Type_String:
         return QString();

diff --git a/src/qml/jsruntime/qv4managed_p.h b/src/qml/jsruntime/qv4managed_p.h
index e0ad0d8..9b7df9e 100644 (file)
--- a/src/qml/jsruntime/qv4managed_p.h
+++ b/src/qml/jsruntime/qv4managed_p.h
@@ -148,15 +148,15 @@ public:
     };
     Q_MANAGED_TYPE(Invalid)
 
-    bool isListType() const { return d()->vtable->type == Type_QmlSequence; }
+    bool isListType() const { return d()->vtable()->type == Type_QmlSequence; }
 
-    bool isArrayObject() const { return d()->vtable->type == Type_ArrayObject; }
-    bool isStringObject() const { return d()->vtable->type == Type_StringObject; }
+    bool isArrayObject() const { return d()->vtable()->type == Type_ArrayObject; }
+    bool isStringObject() const { return d()->vtable()->type == Type_StringObject; }
 
     QString className() const;
 
     bool isEqualTo(const Managed *other) const
-    { return d()->vtable->isEqualTo(const_cast<Managed *>(this), const_cast<Managed *>(other)); }
+    { return d()->vtable()->isEqualTo(const_cast<Managed *>(this), const_cast<Managed *>(other)); }
 
     static bool isEqualTo(Managed *m, Managed *other);
 
@@ -180,7 +180,7 @@ inline const Managed *Value::as() const {
 
 template<>
 inline const Object *Value::as() const {
-    return isManaged() && m() && m()->vtable->isObject ? objectValue() : 0;
+    return isManaged() && m() && m()->vtable()->isObject ? objectValue() : 0;
 }
 
 }

diff --git a/src/qml/jsruntime/qv4object.cpp b/src/qml/jsruntime/qv4object.cpp
index e5608a2..b3c0863 100644 (file)
--- a/src/qml/jsruntime/qv4object.cpp
+++ b/src/qml/jsruntime/qv4object.cpp
@@ -290,7 +290,7 @@ Property *Object::__getPropertyDescriptor__(uint index, PropertyAttributes *attr
             *attrs = o->arrayData->attributes(index);
             return p;
         }
-        if (o->vtable->type == Type_StringObject) {
+        if (o->vtable()->type == Type_StringObject) {
             if (index < static_cast<const Heap::StringObject *>(o)->length()) {
                 // this is an evil hack, but it works, as the method is only ever called from putIndexed,
                 // where we don't use the returned pointer there for non writable attributes

diff --git a/src/qml/jsruntime/qv4object_p.h b/src/qml/jsruntime/qv4object_p.h
index 24efaff..f129312 100644 (file)
--- a/src/qml/jsruntime/qv4object_p.h
+++ b/src/qml/jsruntime/qv4object_p.h
@@ -140,7 +140,7 @@ struct Q_QML_EXPORT Object: Managed {
     const Property *propertyAt(uint index) const { return d()->propertyAt(index); }
     Property *propertyAt(uint index) { return d()->propertyAt(index); }
 
-    const ObjectVTable *vtable() const { return reinterpret_cast<const ObjectVTable *>(d()->vtable); }
+    const ObjectVTable *vtable() const { return reinterpret_cast<const ObjectVTable *>(d()->vtable()); }
     Heap::Object *prototype() const { return d()->prototype; }
     bool setPrototype(Object *proto);
 
@@ -462,7 +462,7 @@ inline void Object::arraySet(uint index, const Value &value)
 
 template<>
 inline const ArrayObject *Value::as() const {
-    return isManaged() && m() && m()->vtable->type == Managed::Type_ArrayObject ? static_cast<const ArrayObject *>(this) : 0;
+    return isManaged() && m() && m()->vtable()->type == Managed::Type_ArrayObject ? static_cast<const ArrayObject *>(this) : 0;
 }
 
 #ifndef V4_BOOTSTRAP

diff --git a/src/qml/jsruntime/qv4persistent.cpp b/src/qml/jsruntime/qv4persistent.cpp
index 7eca12f..4ec7103 100644 (file)
--- a/src/qml/jsruntime/qv4persistent.cpp
+++ b/src/qml/jsruntime/qv4persistent.cpp
@@ -178,8 +178,8 @@ static void drainMarkStack(QV4::ExecutionEngine *engine, Value *markBase)
 {
     while (engine->jsStackTop > markBase) {
         Heap::Base *h = engine->popForGC();
-        Q_ASSERT (h->gcGetVtable()->markObjects);
-        h->gcGetVtable()->markObjects(h, engine);
+        Q_ASSERT (h->vtable()->markObjects);
+        h->vtable()->markObjects(h, engine);
     }
 }
 

diff --git a/src/qml/jsruntime/qv4string.cpp b/src/qml/jsruntime/qv4string.cpp
index cf01312..2ab608e 100644 (file)
--- a/src/qml/jsruntime/qv4string.cpp
+++ b/src/qml/jsruntime/qv4string.cpp
@@ -110,7 +110,7 @@ bool String::isEqualTo(Managed *t, Managed *o)
     if (t == o)
         return true;
 
-    if (!o->d()->vtable->isString)
+    if (!o->d()->vtable()->isString)
         return false;
 
     return static_cast<String *>(t)->isEqualTo(static_cast<String *>(o));

diff --git a/src/qml/jsruntime/qv4string_p.h b/src/qml/jsruntime/qv4string_p.h
index cf796a0..568c112 100644 (file)
--- a/src/qml/jsruntime/qv4string_p.h
+++ b/src/qml/jsruntime/qv4string_p.h
@@ -186,7 +186,7 @@ public:
 
 template<>
 inline const String *Value::as() const {
-    return isManaged() && m() && m()->vtable->isString ? static_cast<const String *>(this) : 0;
+    return isManaged() && m() && m()->vtable()->isString ? static_cast<const String *>(this) : 0;
 }
 
 #ifndef V4_BOOTSTRAP

diff --git a/src/qml/jsruntime/qv4stringobject.cpp b/src/qml/jsruntime/qv4stringobject.cpp
index a6b59b0..9fbafa7 100644 (file)
--- a/src/qml/jsruntime/qv4stringobject.cpp
+++ b/src/qml/jsruntime/qv4stringobject.cpp
@@ -70,7 +70,7 @@ DEFINE_OBJECT_VTABLE(StringObject);
 Heap::StringObject::StringObject(InternalClass *ic, QV4::Object *prototype)
     : Heap::Object(ic, prototype)
 {
-    Q_ASSERT(vtable == QV4::StringObject::staticVTable());
+    Q_ASSERT(vtable() == QV4::StringObject::staticVTable());
     string = ic->engine->newString();
 
     Scope scope(ic->engine);

diff --git a/src/qml/jsruntime/qv4value_p.h b/src/qml/jsruntime/qv4value_p.h
index 718bdbb..3aa1a6d 100644 (file)
--- a/src/qml/jsruntime/qv4value_p.h
+++ b/src/qml/jsruntime/qv4value_p.h
@@ -329,11 +329,11 @@ struct Q_QML_PRIVATE_EXPORT Value
         if (!m() || !isManaged())
             return 0;
 
-        Q_ASSERT(m()->vtable);
+        Q_ASSERT(m()->vtable());
 #if !defined(QT_NO_QOBJECT_CHECK)
         static_cast<const T *>(this)->qt_check_for_QMANAGED_macro(static_cast<const T *>(this));
 #endif
-        const VTable *vt = m()->vtable;
+        const VTable *vt = m()->vtable();
         while (vt) {
             if (vt == T::staticVTable())
                 return static_cast<const T *>(this);
@@ -396,13 +396,13 @@ inline bool Value::isString() const
 {
     if (!isManaged())
         return false;
-    return m() && m()->vtable->isString;
+    return m() && m()->vtable()->isString;
 }
 inline bool Value::isObject() const
 {
     if (!isManaged())
         return false;
-    return m() && m()->vtable->isObject;
+    return m() && m()->vtable()->isObject;
 }
 
 inline bool Value::isPrimitive() const

diff --git a/src/qml/memory/qv4heap_p.h b/src/qml/memory/qv4heap_p.h
index 606fd47..a93e419 100644 (file)
--- a/src/qml/memory/qv4heap_p.h
+++ b/src/qml/memory/qv4heap_p.h
@@ -60,10 +60,7 @@ struct VTable
 namespace Heap {
 
 struct Q_QML_EXPORT Base {
-    union {
-        const VTable *vtable;
-        quintptr mm_data;
-    };
+    quintptr mm_data; // vtable and markbit
 
     inline ReturnedValue asReturnedValue() const;
     inline void mark(QV4::ExecutionEngine *engine);
@@ -74,7 +71,11 @@ struct Q_QML_EXPORT Base {
         PointerMask = ~0x3
     };
 
-    VTable *gcGetVtable() const {
+    void setVtable(const VTable *v) {
+        Q_ASSERT(!(mm_data & MarkBit));
+        mm_data = reinterpret_cast<quintptr>(v);
+    }
+    VTable *vtable() const {
         return reinterpret_cast<VTable *>(mm_data & PointerMask);
     }
     inline bool isMarked() const {

diff --git a/src/qml/memory/qv4mm.cpp b/src/qml/memory/qv4mm.cpp
index 7dbf12f..7e75570 100644 (file)
--- a/src/qml/memory/qv4mm.cpp
+++ b/src/qml/memory/qv4mm.cpp
@@ -183,8 +183,8 @@ bool sweepChunk(MemoryManager::Data::ChunkHeader *header, uint *itemsInUse, Exec
 #ifdef V4_USE_VALGRIND
                 VALGRIND_ENABLE_ERROR_REPORTING;
 #endif
-                if (m->gcGetVtable()->destroy)
-                    m->gcGetVtable()->destroy(m);
+                if (m->vtable()->destroy)
+                    m->vtable()->destroy(m);
 
                 memset(m, 0, header->itemSize);
 #ifdef V4_USE_VALGRIND
@@ -324,8 +324,8 @@ static void drainMarkStack(QV4::ExecutionEngine *engine, Value *markBase)
 {
     while (engine->jsStackTop > markBase) {
         Heap::Base *h = engine->popForGC();
-        Q_ASSERT (h->gcGetVtable()->markObjects);
-        h->gcGetVtable()->markObjects(h, engine);
+        Q_ASSERT (h->vtable()->markObjects);
+        h->vtable()->markObjects(h, engine);
     }
 }
 
@@ -348,7 +348,7 @@ void MemoryManager::mark()
     for (PersistentValueStorage::Iterator it = m_weakValues->begin(); it != m_weakValues->end(); ++it) {
         if (!(*it).isManaged())
             continue;
-        if ((*it).managed()->d()->gcGetVtable() != QObjectWrapper::staticVTable())
+        if ((*it).managed()->d()->vtable() != QObjectWrapper::staticVTable())
             continue;
         QObjectWrapper *qobjectWrapper = static_cast<QObjectWrapper*>((*it).managed());
         if (!qobjectWrapper)
@@ -444,8 +444,8 @@ void MemoryManager::sweep(bool lastSweep)
             i = i->next;
             continue;
         }
-        if (m->gcGetVtable()->destroy)
-            m->gcGetVtable()->destroy(m);
+        if (m->vtable()->destroy)
+            m->vtable()->destroy(m);
 
         *last = i->next;
         free(Q_V4_PROFILE_DEALLOC(m_d->engine, i, i->size + sizeof(Data::LargeItem),

diff --git a/src/qml/memory/qv4mm_p.h b/src/qml/memory/qv4mm_p.h
index 422809b..a7b4e6e 100644 (file)
--- a/src/qml/memory/qv4mm_p.h
+++ b/src/qml/memory/qv4mm_p.h
@@ -87,7 +87,7 @@ public:
     {
         size = align(size);
         Heap::Base *o = allocData(size);
-        o->vtable = ManagedType::staticVTable();
+        o->setVtable(ManagedType::staticVTable());
         return static_cast<typename ManagedType::Data *>(o);
     }