Fix native stack traversal

* For the traversal range, don't take the top of the stack but one
value pointer below, as the top is actually the end of the stack
and we can't read beyond it. For the bottom go one pointer beyond
a locally declared (and thus aligned) value. This ensure sane and
aligned boundaries for the traversal.

* For quick elimination of pointer values on the stack that do
not actually point into one of our managed objects, implement Lars'
idea: Take the heap chunk beginning and end pointers and do a lower
bound search. An even index indicates that the pointer is before the
start of a chunk, thus out of range. An odd index indicates that it
is before a chunk end and therefore in range.

* For obscure reasons we also seem to sometimes hit "dangling" pointers
into otherwise already dealloc'ed objects (as debug output in dealloc()
indicates), so protect ourselves against that.

Change-Id: Ic3337932777871bec370a3441581801273d53bd4
Reviewed-by: Lars Knoll <lars.knoll@digia.com>

diff --git a/qv4mm.cpp b/qv4mm.cpp
index a5215cd..aa3844b 100644 (file)
--- a/qv4mm.cpp
+++ b/qv4mm.cpp
@@ -40,6 +40,7 @@
 
 #include <iostream>
 #include <cstdlib>
+#include <alloca.h>
 
 using namespace QQmlJS::VM;
 
@@ -420,13 +421,38 @@ MemoryManagerWithNativeStack::~MemoryManagerWithNativeStack()
 
 void MemoryManagerWithNativeStack::collectRootsOnStack(QVector<VM::Object *> &roots) const
 {
+    if (!m_d->heapChunks.count())
+        return;
+    Value valueOnStack = Value::undefinedValue();
     StackBounds bounds = StackBounds::currentThreadStackBounds();
-    Value* top = reinterpret_cast<Value*>(bounds.origin());
-    Value* current = reinterpret_cast<Value*>(bounds.current());
-    qDebug("Collecting on stack. top %p current %p\n", top, current);
-    for (; current < top; ++current) {
-        if (current->asObject())
-            qDebug("found object %p on stack", (void*)current);
-        add(roots, *current);
+    Value* top = reinterpret_cast<Value*>(bounds.origin()) - 1;
+    Value* current = (&valueOnStack) + 1;
+
+    char** heapChunkBoundaries = (char**)alloca(m_d->heapChunks.count() * 2 * sizeof(char*));
+    char** heapChunkBoundariesEnd = heapChunkBoundaries + 2 * m_d->heapChunks.count();
+    int i = 0;
+    for (QLinkedList<QPair<char *, std::size_t> >::Iterator it = m_d->heapChunks.begin(), end =
+         m_d->heapChunks.end(); it != end; ++it) {
+        heapChunkBoundaries[i++] = it->first;
+        heapChunkBoundaries[i++] = it->first + it->second;
+    }
+    qSort(heapChunkBoundaries, heapChunkBoundariesEnd);
+
+    int blah = 0;
+    for (; current < top; ++current, ++blah) {
+        Object* possibleObject = current->asObject();
+        if (!possibleObject)
+            continue;
+        char* genericPtr = reinterpret_cast<char*>(possibleObject);
+        if (genericPtr < *heapChunkBoundaries || genericPtr >= *(heapChunkBoundariesEnd - 1))
+            continue;
+        int index = qLowerBound(heapChunkBoundaries, heapChunkBoundariesEnd, genericPtr) - heapChunkBoundaries;
+        // An odd index means the pointer is _before_ the end of a heap chunk and therefore valid.
+        if (index & 1) {
+            // It appears to happen that the stack can still contain a pointer to an already
+            // dealloc'ed. Skip those.
+            if (toObject(possibleObject)->info.inUse)
+                roots.append(possibleObject);
+        }
     }
 }

diff --git a/qv4mm.h b/qv4mm.h
index 530eadb..a4ea893 100644 (file)
--- a/qv4mm.h
+++ b/qv4mm.h
@@ -161,7 +161,7 @@ private:
     std::size_t sweep(std::size_t &largestFreedBlock);
     std::size_t sweep(char *chunkStart, std::size_t chunkSize, std::size_t &largestFreedBlock);
 
-private:
+protected:
     QScopedPointer<Data> m_d;
 };