Make GC checks more aggressive in release builds

https://bugs.webkit.org/show_bug.cgi?id=66001

Reviewed by Gavin Barraclough.

../../../../Volumes/Data/git/WebKit/OpenSource/Source/JavaScriptCore:

* heap/HandleHeap.cpp:
(JSC::HandleHeap::visitStrongHandles):
(JSC::HandleHeap::visitWeakHandles):
(JSC::HandleHeap::finalizeWeakHandles):
(JSC::HandleHeap::writeBarrier):
(JSC::HandleHeap::isLiveNode):
(JSC::HandleHeap::isValidWeakNode):
   Increase handle heap validation logic, and make some of
   the crashes trigger in release builds as well as debug.
* heap/HandleHeap.h:
(JSC::HandleHeap::allocate):
(JSC::HandleHeap::makeWeak):
   Ditto
* runtime/JSGlobalData.cpp:
(WTF::Recompiler::operator()):
* runtime/JSGlobalObject.cpp:
(JSC::JSGlobalObject::visitChildren):
   Fix GC bugs found while testing this patch

../../../../Volumes/Data/git/WebKit/OpenSource/Source/WebCore:

Fix GC bugs found while testing increased validation logic

* bindings/js/JSDOMWindowShell.cpp:
(WebCore::JSDOMWindowShell::JSDOMWindowShell):
* bindings/js/JSDOMWindowShell.h:
* bindings/js/ScriptController.cpp:
(WebCore::ScriptController::createWindowShell):
* bridge/objc/ObjCRuntimeObject.h:
(JSC::Bindings::ObjCRuntimeObject::create):
* bridge/objc/ObjCRuntimeObject.mm:
(JSC::Bindings::ObjCRuntimeObject::ObjCRuntimeObject):
* bridge/objc/objc_instance.mm:

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@92788 268f45cc-cd09-0410-ab3c-d52691b4dbfc

diff --git a/Source/JavaScriptCore/ChangeLog b/Source/JavaScriptCore/ChangeLog
index c6ffaf5..8c1e11a 100644 (file)
--- a/Source/JavaScriptCore/ChangeLog
+++ b/Source/JavaScriptCore/ChangeLog
@@ -1,5 +1,31 @@
 2011-08-10  Oliver Hunt  <oliver@apple.com>
 
+        Make GC checks more aggressive in release builds
+        https://bugs.webkit.org/show_bug.cgi?id=66001
+
+        Reviewed by Gavin Barraclough.
+
+        * heap/HandleHeap.cpp:
+        (JSC::HandleHeap::visitStrongHandles):
+        (JSC::HandleHeap::visitWeakHandles):
+        (JSC::HandleHeap::finalizeWeakHandles):
+        (JSC::HandleHeap::writeBarrier):
+        (JSC::HandleHeap::isLiveNode):
+        (JSC::HandleHeap::isValidWeakNode):
+           Increase handle heap validation logic, and make some of
+           the crashes trigger in release builds as well as debug.
+        * heap/HandleHeap.h:
+        (JSC::HandleHeap::allocate):
+        (JSC::HandleHeap::makeWeak):
+           Ditto
+        * runtime/JSGlobalData.cpp:
+        (WTF::Recompiler::operator()):
+        * runtime/JSGlobalObject.cpp:
+        (JSC::JSGlobalObject::visitChildren):
+           Fix GC bugs found while testing this patch
+
+2011-08-10  Oliver Hunt  <oliver@apple.com>
+
         JSEvaluteScript does not return the correct object when given JSONP data
         https://bugs.webkit.org/show_bug.cgi?id=66003
 

diff --git a/Source/JavaScriptCore/heap/HandleHeap.cpp b/Source/JavaScriptCore/heap/HandleHeap.cpp
index e13b654..f2648bf 100644 (file)
--- a/Source/JavaScriptCore/heap/HandleHeap.cpp
+++ b/Source/JavaScriptCore/heap/HandleHeap.cpp
@@ -64,8 +64,13 @@ void HandleHeap::grow()
 void HandleHeap::visitStrongHandles(HeapRootVisitor& heapRootVisitor)
 {
     Node* end = m_strongList.end();
-    for (Node* node = m_strongList.begin(); node != end; node = node->next())
+    for (Node* node = m_strongList.begin(); node != end; node = node->next()) {
+#if ENABLE(GC_VALIDATION)
+        if (!isLiveNode(node))
+            CRASH();
+#endif
         heapRootVisitor.visit(node->slot());
+    }
 }
 
 void HandleHeap::visitWeakHandles(HeapRootVisitor& heapRootVisitor)
@@ -74,7 +79,10 @@ void HandleHeap::visitWeakHandles(HeapRootVisitor& heapRootVisitor)
 
     Node* end = m_weakList.end();
     for (Node* node = m_weakList.begin(); node != end; node = node->next()) {
-        ASSERT(isValidWeakNode(node));
+#if ENABLE(GC_VALIDATION)
+        if (!isValidWeakNode(node))
+            CRASH();
+#endif
         JSCell* cell = node->slot()->asCell();
         if (Heap::isMarked(cell))
             continue;
@@ -95,8 +103,11 @@ void HandleHeap::finalizeWeakHandles()
     Node* end = m_weakList.end();
     for (Node* node = m_weakList.begin(); node != end; node = m_nextToFinalize) {
         m_nextToFinalize = node->next();
+#if ENABLE(GC_VALIDATION)
+        if (!isValidWeakNode(node))
+            CRASH();
+#endif
 
-        ASSERT(isValidWeakNode(node));
         JSCell* cell = node->slot()->asCell();
         if (Heap::isMarked(cell))
             continue;
@@ -106,7 +117,10 @@ void HandleHeap::finalizeWeakHandles()
             if (m_nextToFinalize != node->next()) // Owner deallocated node.
                 continue;
         }
-
+#if ENABLE(GC_VALIDATION)
+        if (!isLiveNode(node))
+            CRASH();
+#endif
         *node->slot() = JSValue();
         SentinelLinkedList<Node>::remove(node);
         m_immediateList.push(node);
@@ -117,12 +131,19 @@ void HandleHeap::finalizeWeakHandles()
 
 void HandleHeap::writeBarrier(HandleSlot slot, const JSValue& value)
 {
-    ASSERT(!m_nextToFinalize); // Forbid assignment to handles during the finalization phase, since it would violate many GC invariants.
+    // Forbid assignment to handles during the finalization phase, since it would violate many GC invariants.
+    // File a bug with stack trace if you hit this.
+    if (m_nextToFinalize)
+        CRASH();
 
     if (!value == !*slot && slot->isCell() == value.isCell())
         return;
 
     Node* node = toNode(slot);
+#if ENABLE(GC_VALIDATION)
+    if (!isLiveNode(node))
+        CRASH();
+#endif
     SentinelLinkedList<Node>::remove(node);
     if (!value || !value.isCell()) {
         m_immediateList.push(node);
@@ -131,10 +152,18 @@ void HandleHeap::writeBarrier(HandleSlot slot, const JSValue& value)
 
     if (node->isWeak()) {
         m_weakList.push(node);
+#if ENABLE(GC_VALIDATION)
+        if (!isLiveNode(node))
+            CRASH();
+#endif
         return;
     }
 
     m_strongList.push(node);
+#if ENABLE(GC_VALIDATION)
+    if (!isLiveNode(node))
+        CRASH();
+#endif
 }
 
 unsigned HandleHeap::protectedGlobalObjectCount()
@@ -149,9 +178,21 @@ unsigned HandleHeap::protectedGlobalObjectCount()
     return count;
 }
 
-#if !ASSERT_DISABLED
+#if ENABLE(GC_VALIDATION) || !ASSERT_DISABLED
+bool HandleHeap::isLiveNode(Node* node)
+{
+    if (node->prev()->next() != node)
+        return false;
+    if (node->next()->prev() != node)
+        return false;
+        
+    return true;
+}
+
 bool HandleHeap::isValidWeakNode(Node* node)
 {
+    if (!isLiveNode(node))
+        return false;
     if (!node->isWeak())
         return false;
 

diff --git a/Source/JavaScriptCore/heap/HandleHeap.h b/Source/JavaScriptCore/heap/HandleHeap.h
index 7946a05..13eb9eb 100644 (file)
--- a/Source/JavaScriptCore/heap/HandleHeap.h
+++ b/Source/JavaScriptCore/heap/HandleHeap.h
@@ -113,8 +113,9 @@ private:
 
     void grow();
     
-#if !ASSERT_DISABLED
+#if ENABLE(GC_VALIDATION) || !ASSERT_DISABLED
     bool isValidWeakNode(Node*);
+    bool isLiveNode(Node*);
 #endif
 
     JSGlobalData* m_globalData;
@@ -149,6 +150,10 @@ inline HandleHeap::Node* HandleHeap::toNode(HandleSlot handle)
 
 inline HandleSlot HandleHeap::allocate()
 {
+    // Forbid assignment to handles during the finalization phase, since it would violate many GC invariants.
+    // File a bug with stack trace if you hit this.
+    if (m_nextToFinalize)
+        CRASH();
     if (m_freeList.isEmpty())
         grow();
 
@@ -181,6 +186,10 @@ inline HandleSlot HandleHeap::copyWeak(HandleSlot other)
 
 inline void HandleHeap::makeWeak(HandleSlot handle, WeakHandleOwner* weakOwner, void* context)
 {
+    // Forbid assignment to handles during the finalization phase, since it would violate many GC invariants.
+    // File a bug with stack trace if you hit this.
+    if (m_nextToFinalize)
+        CRASH();
     Node* node = toNode(handle);
     node->makeWeak(weakOwner, context);
 

diff --git a/Source/JavaScriptCore/runtime/JSGlobalData.cpp b/Source/JavaScriptCore/runtime/JSGlobalData.cpp
index b0e19d1..d647195 100644 (file)
--- a/Source/JavaScriptCore/runtime/JSGlobalData.cpp
+++ b/Source/JavaScriptCore/runtime/JSGlobalData.cpp
@@ -84,7 +84,7 @@ inline void Recompiler::operator()(JSCell* cell)
     if (!cell->inherits(&JSFunction::s_info))
         return;
     JSFunction* function = asFunction(cell);
-    if (function->executable()->isHostFunction())
+    if (!function->executable() || function->executable()->isHostFunction())
         return;
     function->jsExecutable()->discardCode();
 }

diff --git a/Source/JavaScriptCore/runtime/JSGlobalObject.cpp b/Source/JavaScriptCore/runtime/JSGlobalObject.cpp
index 201e831..2cb4333 100644 (file)
--- a/Source/JavaScriptCore/runtime/JSGlobalObject.cpp
+++ b/Source/JavaScriptCore/runtime/JSGlobalObject.cpp
@@ -349,6 +349,7 @@ void JSGlobalObject::visitChildren(SlotVisitor& visitor)
     visitIfNeeded(visitor, &m_nullPrototypeObjectStructure);
     visitIfNeeded(visitor, &m_errorStructure);
     visitIfNeeded(visitor, &m_functionStructure);
+    visitIfNeeded(visitor, &m_namedFunctionStructure);
     visitIfNeeded(visitor, &m_numberObjectStructure);
     visitIfNeeded(visitor, &m_regExpMatchesArrayStructure);
     visitIfNeeded(visitor, &m_regExpStructure);

diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
index 86f40a3..7a2d7e8 100644 (file)
--- a/Source/WebCore/ChangeLog
+++ b/Source/WebCore/ChangeLog
@@ -1,3 +1,23 @@
+2011-08-10  Oliver Hunt  <oliver@apple.com>
+
+        Make GC checks more aggressive in release builds
+        https://bugs.webkit.org/show_bug.cgi?id=66001
+
+        Reviewed by Gavin Barraclough.
+
+        Fix GC bugs found while testing increased validation logic
+
+        * bindings/js/JSDOMWindowShell.cpp:
+        (WebCore::JSDOMWindowShell::JSDOMWindowShell):
+        * bindings/js/JSDOMWindowShell.h:
+        * bindings/js/ScriptController.cpp:
+        (WebCore::ScriptController::createWindowShell):
+        * bridge/objc/ObjCRuntimeObject.h:
+        (JSC::Bindings::ObjCRuntimeObject::create):
+        * bridge/objc/ObjCRuntimeObject.mm:
+        (JSC::Bindings::ObjCRuntimeObject::ObjCRuntimeObject):
+        * bridge/objc/objc_instance.mm:
+
 2011-08-10  Ben Wells  <benwells@chromium.org>
 
         [skia] Move calls to makeGrContextCurrent into clipPathAntiAliased from callers

diff --git a/Source/WebCore/bindings/js/JSDOMWindowShell.cpp b/Source/WebCore/bindings/js/JSDOMWindowShell.cpp
index 180bb21..cc00b6e 100644 (file)
--- a/Source/WebCore/bindings/js/JSDOMWindowShell.cpp
+++ b/Source/WebCore/bindings/js/JSDOMWindowShell.cpp
@@ -43,8 +43,8 @@ ASSERT_CLASS_FITS_IN_CELL(JSDOMWindowShell);
 
 const ClassInfo JSDOMWindowShell::s_info = { "JSDOMWindowShell", &Base::s_info, 0, 0 };
 
-JSDOMWindowShell::JSDOMWindowShell(PassRefPtr<DOMWindow> window, DOMWrapperWorld* world)
-    : Base(*world->globalData(), JSDOMWindowShell::createStructure(*world->globalData(), jsNull()))
+JSDOMWindowShell::JSDOMWindowShell(PassRefPtr<DOMWindow> window, Structure* structure, DOMWrapperWorld* world)
+    : Base(*world->globalData(), structure)
     , m_world(world)
 {
     ASSERT(inherits(&s_info));

diff --git a/Source/WebCore/bindings/js/JSDOMWindowShell.h b/Source/WebCore/bindings/js/JSDOMWindowShell.h
index d609e33..ebf6deb 100644 (file)
--- a/Source/WebCore/bindings/js/JSDOMWindowShell.h
+++ b/Source/WebCore/bindings/js/JSDOMWindowShell.h
@@ -39,7 +39,7 @@ namespace WebCore {
     class JSDOMWindowShell : public JSC::JSNonFinalObject {
         typedef JSC::JSNonFinalObject Base;
     public:
-        JSDOMWindowShell(PassRefPtr<DOMWindow>, DOMWrapperWorld* world);
+        JSDOMWindowShell(PassRefPtr<DOMWindow>, JSC::Structure*, DOMWrapperWorld*);
         virtual ~JSDOMWindowShell();
 
         JSDOMWindow* window() const { return m_window.get(); }

diff --git a/Source/WebCore/bindings/js/ScriptController.cpp b/Source/WebCore/bindings/js/ScriptController.cpp
index 18a5072..42faade 100644 (file)
--- a/Source/WebCore/bindings/js/ScriptController.cpp
+++ b/Source/WebCore/bindings/js/ScriptController.cpp
@@ -107,7 +107,8 @@ void ScriptController::destroyWindowShell(DOMWrapperWorld* world)
 JSDOMWindowShell* ScriptController::createWindowShell(DOMWrapperWorld* world)
 {
     ASSERT(!m_windowShells.contains(world));
-    Strong<JSDOMWindowShell> windowShell(*world->globalData(), new JSDOMWindowShell(m_frame->domWindow(), world));
+    Structure* structure = JSDOMWindowShell::createStructure(*world->globalData(), jsNull());
+    Strong<JSDOMWindowShell> windowShell(*world->globalData(), new JSDOMWindowShell(m_frame->domWindow(), structure, world));
     Strong<JSDOMWindowShell> windowShell2(windowShell);
     m_windowShells.add(world, windowShell);
     world->didCreateWindowShell(this);

diff --git a/Source/WebCore/bridge/objc/ObjCRuntimeObject.h b/Source/WebCore/bridge/objc/ObjCRuntimeObject.h
index 69fac38..64ccff6 100644 (file)
--- a/Source/WebCore/bridge/objc/ObjCRuntimeObject.h
+++ b/Source/WebCore/bridge/objc/ObjCRuntimeObject.h
@@ -39,7 +39,8 @@ public:
 
     static ObjCRuntimeObject* create(ExecState* exec, JSGlobalObject* globalObject, PassRefPtr<ObjcInstance> inst)
     {
-        return new (allocateCell<ObjCRuntimeObject>(*exec->heap())) ObjCRuntimeObject(exec, globalObject, inst);
+        Structure* structure = WebCore::deprecatedGetDOMStructure<ObjCRuntimeObject>(exec);
+        return new (allocateCell<ObjCRuntimeObject>(*exec->heap())) ObjCRuntimeObject(exec, globalObject, inst, structure);
     }
 
     virtual ~ObjCRuntimeObject();
@@ -54,7 +55,7 @@ public:
     }
 
 private:
-    ObjCRuntimeObject(ExecState*, JSGlobalObject*, PassRefPtr<ObjcInstance>);
+    ObjCRuntimeObject(ExecState*, JSGlobalObject*, PassRefPtr<ObjcInstance>, Structure*);
 };
 
 }

diff --git a/Source/WebCore/bridge/objc/ObjCRuntimeObject.mm b/Source/WebCore/bridge/objc/ObjCRuntimeObject.mm
index d9d3767..5848fe3 100644 (file)
--- a/Source/WebCore/bridge/objc/ObjCRuntimeObject.mm
+++ b/Source/WebCore/bridge/objc/ObjCRuntimeObject.mm
@@ -35,10 +35,10 @@ namespace Bindings {
 
 const ClassInfo ObjCRuntimeObject::s_info = { "ObjCRuntimeObject", &RuntimeObject::s_info, 0, 0 };
 
-ObjCRuntimeObject::ObjCRuntimeObject(ExecState* exec, JSGlobalObject* globalObject, PassRefPtr<ObjcInstance> instance)
+ObjCRuntimeObject::ObjCRuntimeObject(ExecState* exec, JSGlobalObject* globalObject, PassRefPtr<ObjcInstance> instance, Structure* structure)
     // FIXME: deprecatedGetDOMStructure uses the prototype off of the wrong global object
     // We need to pass in the right global object for "i".
-    : RuntimeObject(exec, globalObject, WebCore::deprecatedGetDOMStructure<ObjCRuntimeObject>(exec), instance)
+    : RuntimeObject(exec, globalObject, structure, instance)
 {
     ASSERT(inherits(&s_info));
 }

diff --git a/Source/WebCore/bridge/objc/objc_instance.mm b/Source/WebCore/bridge/objc/objc_instance.mm
index b6c4d8a..041f923 100644 (file)
--- a/Source/WebCore/bridge/objc/objc_instance.mm
+++ b/Source/WebCore/bridge/objc/objc_instance.mm
@@ -27,6 +27,7 @@
 #import "objc_instance.h"
 
 #import "runtime_method.h"
+#import <runtime/ObjectPrototype.h>
 #import "JSDOMBinding.h"
 #import "ObjCRuntimeObject.h"
 #import "WebScriptObject.h"