Fix marking of prototype objects in internal class pool

As per reported bug, we have to protect ourselves against potential loops
and can mark the internal classes much simpler by just walking through
the memory pool they were allocated in.

Task-number: QTBUG-38299
Change-Id: I3ae96e8082e76d06f4321c5aa6d2e9645d2830a0
Reviewed-by: Lars Knoll <lars.knoll@digia.com>

5 files changed, 58 insertions, 17 deletions

diff --git a/src/qml/jsruntime/qv4engine.cpp b/src/qml/jsruntime/qv4engine.cpp
index b95197e16b..8916cc597e 100644
--- a/src/qml/jsruntime/qv4engine.cpp
+++ b/src/qml/jsruntime/qv4engine.cpp
@@ -215,7 +215,9 @@ ExecutionEngine::ExecutionEngine(EvalISelFactory *factory)
 
     identifierTable = new IdentifierTable(this);
 
-    emptyClass =  new (classPool.allocate(sizeof(InternalClass))) InternalClass(this);
+    classPool = new InternalClassPool;
+
+    emptyClass =  new (classPool) InternalClass(this);
     executionContextClass = InternalClass::create(this, ExecutionContext::staticVTable(), 0);
     constructClass = InternalClass::create(this, Object::staticVTable(), 0);
     stringClass = InternalClass::create(this, String::staticVTable(), 0);
@@ -429,6 +431,7 @@ ExecutionEngine::~ExecutionEngine()
 
     delete m_qmlExtensions;
     emptyClass->destroy();
+    delete classPool;
     delete bumperPointerAllocator;
     delete regExpCache;
     delete regExpAllocator;
@@ -464,7 +467,7 @@ void ExecutionEngine::initRootContext()
 
 InternalClass *ExecutionEngine::newClass(const InternalClass &other)
 {
-    return new (classPool.allocate(sizeof(InternalClass))) InternalClass(other);
+    return new (classPool) InternalClass(other);
 }
 
 ExecutionContext *ExecutionEngine::pushGlobalContext()
@@ -897,7 +900,7 @@ void ExecutionEngine::markObjects()
     if (m_qmlExtensions)
         m_qmlExtensions->markObjects(this);
 
-    emptyClass->markObjects();
+    classPool->markObjects(this);
 
     for (QSet<CompiledData::CompilationUnit*>::ConstIterator it = compilationUnits.constBegin(), end = compilationUnits.constEnd();
          it != end; ++it)
diff --git a/src/qml/jsruntime/qv4engine_p.h b/src/qml/jsruntime/qv4engine_p.h
index b93af514b1..d678d6595e 100644
--- a/src/qml/jsruntime/qv4engine_p.h
+++ b/src/qml/jsruntime/qv4engine_p.h
@@ -109,6 +109,7 @@ struct SequencePrototype;
 struct EvalFunction;
 struct IdentifierTable;
 struct InternalClass;
+struct InternalClassPool;
 class MultiplyWrappedQObjectMap;
 class RegExp;
 class RegExpCache;
@@ -197,7 +198,7 @@ public:
     Value uRIErrorCtor;
     Value sequencePrototype;
 
-    QQmlJS::MemoryPool classPool;
+    InternalClassPool *classPool;
     InternalClass *emptyClass;
     InternalClass *executionContextClass;
     InternalClass *constructClass;
diff --git a/src/qml/jsruntime/qv4internalclass.cpp b/src/qml/jsruntime/qv4internalclass.cpp
index aacc5bf517..3dc20b8e76 100644
--- a/src/qml/jsruntime/qv4internalclass.cpp
+++ b/src/qml/jsruntime/qv4internalclass.cpp
@@ -129,7 +129,7 @@ uint PropertyHash::lookup(const Identifier *identifier) const
 InternalClass::InternalClass(ExecutionEngine *engine)
     : engine(engine)
     , prototype(0)
-    , vtable(&Managed::static_vtbl)
+    , vtable(&QV4::Managed::static_vtbl)
     , m_sealed(0)
     , m_frozen(0)
     , size(0)
@@ -138,7 +138,8 @@ InternalClass::InternalClass(ExecutionEngine *engine)
 
 
 InternalClass::InternalClass(const QV4::InternalClass &other)
-    : engine(other.engine)
+    : QQmlJS::Managed()
+    , engine(other.engine)
     , prototype(other.prototype)
     , vtable(other.vtable)
     , propertyTable(other.propertyTable)
@@ -455,17 +456,24 @@ void InternalClass::destroy()
     transitions.clear();
 }
 
-void InternalClass::markObjects()
+struct InternalClassPoolVisitor
 {
-    // all prototype changes are done on the empty class
-    Q_ASSERT(!prototype || this != engine->emptyClass);
-
-    if (prototype)
-        prototype->mark(engine);
+    ExecutionEngine *engine;
+    void operator()(InternalClass *klass)
+    {
+        // all prototype changes are done on the empty class
+        Q_ASSERT(!klass->prototype || klass != engine->emptyClass);
+
+        if (klass->prototype)
+            klass->prototype->mark(engine);
+    }
+};
 
-    for (QHash<Transition, InternalClass *>::ConstIterator it = transitions.begin(), end = transitions.end();
-         it != end; ++it)
-        it.value()->markObjects();
+void InternalClassPool::markObjects(ExecutionEngine *engine)
+{
+    InternalClassPoolVisitor v;
+    v.engine = engine;
+    visitManagedPool<InternalClass, InternalClassPoolVisitor>(v);
 }
 
 QT_END_NAMESPACE
diff --git a/src/qml/jsruntime/qv4internalclass_p.h b/src/qml/jsruntime/qv4internalclass_p.h
index 91c6e264db..bd1828a146 100644
--- a/src/qml/jsruntime/qv4internalclass_p.h
+++ b/src/qml/jsruntime/qv4internalclass_p.h
@@ -44,6 +44,7 @@
 #include <QHash>
 #include <QVector>
 #include "qv4global_p.h"
+#include <private/qqmljsmemorypool_p.h>
 
 QT_BEGIN_NAMESPACE
 
@@ -213,7 +214,7 @@ struct InternalClassTransition
 };
 uint qHash(const QV4::InternalClassTransition &t, uint = 0);
 
-struct InternalClass {
+struct InternalClass : public QQmlJS::Managed {
     ExecutionEngine *engine;
     Object *prototype;
     const ManagedVTable *vtable;
@@ -247,7 +248,6 @@ struct InternalClass {
     InternalClass *frozen();
 
     void destroy();
-    void markObjects();
 
 private:
     InternalClass *addMemberImpl(String *string, PropertyAttributes data, uint *index);
@@ -256,6 +256,11 @@ private:
     InternalClass(const InternalClass &other);
 };
 
+struct InternalClassPool : public QQmlJS::MemoryPool
+{
+    void markObjects(ExecutionEngine *engine);
+};
+
 }
 
 QT_END_NAMESPACE
diff --git a/src/qml/parser/qqmljsmemorypool_p.h b/src/qml/parser/qqmljsmemorypool_p.h
index 29103930ad..1809f500e3 100644
--- a/src/qml/parser/qqmljsmemorypool_p.h
+++ b/src/qml/parser/qqmljsmemorypool_p.h
@@ -65,6 +65,8 @@ QT_QML_BEGIN_NAMESPACE
 
 namespace QQmlJS {
 
+class Managed;
+
 class QML_PARSER_EXPORT MemoryPool : public QSharedData
 {
     MemoryPool(const MemoryPool &other);
@@ -110,6 +112,28 @@ public:
 
     template <typename _Tp> _Tp *New() { return new (this->allocate(sizeof(_Tp))) _Tp(); }
 
+    template <typename PoolContentType, typename Visitor>
+    void visitManagedPool(Visitor &visitor)
+    {
+        for (int i = 0; i <= _blockCount; ++i) {
+            char *p = _blocks[i];
+            char *end = p + BLOCK_SIZE;
+            if (i == _blockCount) {
+                Q_ASSERT(_ptr <= end);
+                end = _ptr;
+            }
+
+            Q_ASSERT(p <= end);
+
+            const qptrdiff increment = (sizeof(PoolContentType) + 7) & ~7;
+
+            while (p + increment <= end) {
+                visitor(reinterpret_cast<PoolContentType*>(p));
+                p += increment;
+            }
+        }
+    }
+
 private:
     void *allocate_helper(size_t size)
     {