Turn on exact garbage collection by default

Keep conservative GC as a fallback for testing
Enable all tests again that were skipped due to
GC issues.

Change-Id: I8e0fa728207bdd39a96d0acf95e27841157d8402
Reviewed-by: Simon Hausmann <simon.hausmann@digia.com>

4 files changed, 30 insertions, 94 deletions

diff --git a/src/qml/jsruntime/qv4mm.cpp b/src/qml/jsruntime/qv4mm.cpp
index 8050b2782e..6fe49cdf7f 100644
--- a/src/qml/jsruntime/qv4mm.cpp
+++ b/src/qml/jsruntime/qv4mm.cpp
@@ -171,7 +171,7 @@ struct MemoryManager::Data
         memset(allocCount, 0, sizeof(allocCount));
         scribble = !qgetenv("QV4_MM_SCRIBBLE").isEmpty();
         aggressiveGC = !qgetenv("QV4_MM_AGGRESSIVE_GC").isEmpty();
-        exactGC = !qgetenv("QV4_MM_EXACT_GC").isEmpty();
+        exactGC = qgetenv("QV4_MM_CONSERVATIVE_GC").isEmpty();
     }
 
     ~Data()
@@ -332,37 +332,38 @@ void MemoryManager::mark()
         persistent = persistent->next;
     }
 
-    // push all caller saved registers to the stack, so we can find the objects living in these registers
+    collectFromJSStack();
+
+    if (!m_d->exactGC) {
+        // push all caller saved registers to the stack, so we can find the objects living in these registers
 #if COMPILER(MSVC)
 #  if CPU(X86_64)
-    HANDLE thread = GetCurrentThread();
-    WOW64_CONTEXT ctxt;
-    /*bool success =*/ Wow64GetThreadContext(thread, &ctxt);
+        HANDLE thread = GetCurrentThread();
+        WOW64_CONTEXT ctxt;
+        /*bool success =*/ Wow64GetThreadContext(thread, &ctxt);
 #  elif CPU(X86)
-    HANDLE thread = GetCurrentThread();
-    CONTEXT ctxt;
-    /*bool success =*/ GetThreadContext(thread, &ctxt);
+        HANDLE thread = GetCurrentThread();
+        CONTEXT ctxt;
+        /*bool success =*/ GetThreadContext(thread, &ctxt);
 #  endif // CPU
 #elif COMPILER(CLANG) || COMPILER(GCC)
 #  if CPU(X86_64)
-    quintptr regs[5];
-    asm(
-        "mov %%rbp, %0\n"
-        "mov %%r12, %1\n"
-        "mov %%r13, %2\n"
-        "mov %%r14, %3\n"
-        "mov %%r15, %4\n"
-        : "=m" (regs[0]), "=m" (regs[1]), "=m" (regs[2]), "=m" (regs[3]), "=m" (regs[4])
-        :
-        :
-    );
+        quintptr regs[5];
+        asm(
+            "mov %%rbp, %0\n"
+            "mov %%r12, %1\n"
+            "mov %%r13, %2\n"
+            "mov %%r14, %3\n"
+            "mov %%r15, %4\n"
+            : "=m" (regs[0]), "=m" (regs[1]), "=m" (regs[2]), "=m" (regs[3]), "=m" (regs[4])
+            :
+            :
+        );
 #  endif // CPU
 #endif // COMPILER
 
-    collectFromJSStack();
-
-    if (!m_d->exactGC)
         collectFromStack();
+    }
 
     // Preserve QObject ownership rules within JavaScript: A parent with c++ ownership
     // keeps all of its children alive in JavaScript.
diff --git a/tests/auto/qml/qjsengine/tst_qjsengine.cpp b/tests/auto/qml/qjsengine/tst_qjsengine.cpp
index 7dc530b278..39086d75ac 100644
--- a/tests/auto/qml/qjsengine/tst_qjsengine.cpp
+++ b/tests/auto/qml/qjsengine/tst_qjsengine.cpp
@@ -64,21 +64,6 @@
 Q_DECLARE_METATYPE(QList<int>)
 Q_DECLARE_METATYPE(QObjectList)
 
-// The JavaScriptCore GC marks the C stack. To try to ensure that there is
-// no JSObject* left in stack memory by the compiler, we call this function
-// to zap some bytes of memory before calling collectGarbage().
-static void NO_INLINE zapSomeStack()
-{
-    char *buf = (char*)alloca(4096);
-    memset(buf, 0, 4096);
-}
-
-static void collectGarbage_helper(QJSEngine &eng)
-{
-    zapSomeStack();
-    eng.collectGarbage();
-}
-
 class tst_QJSEngine : public QObject
 {
     Q_OBJECT
@@ -452,7 +437,6 @@ void tst_QJSEngine::newQObject()
 
 void tst_QJSEngine::newQObject_ownership()
 {
-    QSKIP("unreliable test due to our conservative GC");
     QJSEngine eng;
     {
         QPointer<QObject> ptr = new QObject();
@@ -460,7 +444,7 @@ void tst_QJSEngine::newQObject_ownership()
         {
             QJSValue v = eng.newQObject(ptr);
         }
-        collectGarbage_helper(eng);
+        eng.collectGarbage();
         if (ptr)
             QGuiApplication::sendPostedEvents(ptr, QEvent::DeferredDelete);
         QVERIFY(ptr == 0);
@@ -472,7 +456,7 @@ void tst_QJSEngine::newQObject_ownership()
             QJSValue v = eng.newQObject(ptr);
         }
         QObject *before = ptr;
-        collectGarbage_helper(eng);
+        eng.collectGarbage();
         QVERIFY(ptr == before);
         delete ptr;
     }
@@ -490,7 +474,7 @@ void tst_QJSEngine::newQObject_ownership()
         {
             QJSValue v = eng.newQObject(ptr);
         }
-        collectGarbage_helper(eng);
+        eng.collectGarbage();
         // no parent, so it should be like ScriptOwnership
         if (ptr)
             QGuiApplication::sendPostedEvents(ptr, QEvent::DeferredDelete);
@@ -503,7 +487,7 @@ void tst_QJSEngine::newQObject_ownership()
         {
             QJSValue v = eng.newQObject(child);
         }
-        collectGarbage_helper(eng);
+        eng.collectGarbage();
         // has parent, so it should be like QtOwnership
         QVERIFY(child != 0);
         delete parent;
@@ -1241,7 +1225,6 @@ void tst_QJSEngine::castWithMultipleInheritance()
 
 void tst_QJSEngine::collectGarbage()
 {
-    QSKIP("This test is not reliable due to our conservative GC");
     QJSEngine eng;
     eng.evaluate("a = new Object(); a = new Object(); a = new Object()");
     QJSValue a = eng.newObject();
@@ -1250,7 +1233,7 @@ void tst_QJSEngine::collectGarbage()
     QPointer<QObject> ptr = new QObject();
     QVERIFY(ptr != 0);
     (void)eng.newQObject(ptr);
-    collectGarbage_helper(eng);
+    eng.collectGarbage();
     if (ptr)
         QGuiApplication::sendPostedEvents(ptr, QEvent::DeferredDelete);
     QVERIFY(ptr == 0);
diff --git a/tests/auto/qml/qqmlcomponent/tst_qqmlcomponent.cpp b/tests/auto/qml/qqmlcomponent/tst_qqmlcomponent.cpp
index 408baeffee..dbf28a5471 100644
--- a/tests/auto/qml/qqmlcomponent/tst_qqmlcomponent.cpp
+++ b/tests/auto/qml/qqmlcomponent/tst_qqmlcomponent.cpp
@@ -96,18 +96,8 @@ public slots:
     }
 };
 
-// The JavaScriptCore GC marks the C stack. To try to ensure that there is
-// no JSObject* left in stack memory by the compiler, we call this function
-// to zap some bytes of memory before calling collectGarbage().
-static void zapSomeStack()
-{
-    char *buf = (char*)alloca(4096);
-    memset(buf, 0, 4096);
-}
-
 static void gc(QQmlEngine &engine)
 {
-    zapSomeStack();
     engine.collectGarbage();
     QCoreApplication::sendPostedEvents(0, QEvent::DeferredDelete);
     QCoreApplication::processEvents();
diff --git a/tests/auto/qml/qqmlecmascript/tst_qqmlecmascript.cpp b/tests/auto/qml/qqmlecmascript/tst_qqmlecmascript.cpp
index b89d3da924..47b7a6640c 100644
--- a/tests/auto/qml/qqmlecmascript/tst_qqmlecmascript.cpp
+++ b/tests/auto/qml/qqmlecmascript/tst_qqmlecmascript.cpp
@@ -325,24 +325,9 @@ private:
     }
 };
 
-// The JavaScriptCore GC marks the C stack. To try to ensure that there is
-// no JSObject* left in stack memory by the compiler, we call this function
-// to zap some bytes of memory before calling collectGarbage().
-static void NO_INLINE zapSomeStack()
-{
-    char *buf = (char*)alloca(4096);
-    memset(buf, 0, 4096);
-}
-
-static void gcWithoutDeferredObjectDeletion(QQmlEngine &engine)
-{
-    zapSomeStack();
-    engine.collectGarbage();
-}
-
 static void gc(QQmlEngine &engine)
 {
-    gcWithoutDeferredObjectDeletion(engine);
+    engine.collectGarbage();
     QCoreApplication::sendPostedEvents(0, QEvent::DeferredDelete);
     QCoreApplication::processEvents();
 }
@@ -4830,7 +4815,7 @@ void tst_qqmlecmascript::propertyVarOwnership()
     QObject *object = component.create();
     QVERIFY(object != 0);
     QMetaObject::invokeMethod(object, "createComponent");
-    gcWithoutDeferredObjectDeletion(engine);
+    engine.collectGarbage();
     QMetaObject::invokeMethod(object, "runTest");
     QCOMPARE(object->property("test").toBool(), true);
     delete object;
@@ -4997,18 +4982,8 @@ void tst_qqmlecmascript::propertyVarCircular2()
     delete object;
 }
 
-#if defined(Q_CC_GNU)
-#if (__GNUC__ * 100 + __GNUC_MINOR__) >= 404
-#define pop_gcc_flags
-#pragma GCC push_options
-#pragma GCC optimize ("O0")
-#endif
-#endif
-
 void tst_qqmlecmascript::propertyVarInheritance()
 {
-    QSKIP("This test does not work reliably with our conservative GC.");
-
     // enforce behaviour regarding element inheritance - ensure handle disposal.
     // The particular component under test here has a chain of references.
     QQmlComponent component(&engine, testFileUrl("propertyVar.inherit.qml"));
@@ -5045,13 +5020,8 @@ void tst_qqmlecmascript::propertyVarInheritance()
     // ensure that there are only weak handles to the underlying varProperties array remaining.
     gc(engine);
     // an equivalent for pragma GCC optimize is still work-in-progress for CLang, so this test will fail.
-#if defined(Q_CC_MSVC)
-    QSKIP("This test does not work reliably with MSVC.");
-#endif
-#if !defined(Q_CC_CLANG)
     QVERIFY(icoCanaryHandle.isUndefined());
     QVERIFY(ccoCanaryHandle.isUndefined());
-#endif
     delete object;
     // since there are no parent vmemo's to keep implicit references alive, and the only handles
     // to what remains are weak, all varProperties arrays must have been collected.
@@ -5059,8 +5029,6 @@ void tst_qqmlecmascript::propertyVarInheritance()
 
 void tst_qqmlecmascript::propertyVarInheritance2()
 {
-    QSKIP("This test does not work reliably with our conservative GC.");
-
     // The particular component under test here does NOT have a chain of references; the
     // only link between rootObject and childObject is that rootObject is the parent of childObject.
     QQmlComponent component(&engine, testFileUrl("propertyVar.circular.2.qml"));
@@ -5087,16 +5055,10 @@ void tst_qqmlecmascript::propertyVarInheritance2()
     QMetaObject::invokeMethod(object, "deassignCircular");
     gc(engine);
     // an equivalent for pragma GCC optimize is still work-in-progress for CLang, so this test will fail.
-#if !defined(Q_CC_CLANG)
     QVERIFY(childObjectVarArrayValueHandle.isUndefined()); // should have been collected now.
-#endif
     delete object;
 }
 
-#if defined(pop_gcc_flags)
-#pragma GCC pop_options
-#endif
-
 
 // Ensure that QObject type conversion works on binding assignment
 void tst_qqmlecmascript::elementAssign()