Changeset 47022 in webkit for trunk/JavaScriptCore/runtime/Collector.cpp

Timestamp:

Aug 10, 2009, 9:35:02 PM (16 years ago)

Author:

[email protected]

Message:

Stack overflow crash in JavaScript garbage collector mark pass
​https://bugs.webkit.org/show_bug.cgi?id=12216

Reviewed by Gavin Barraclough and Sam Weinig

Make the GC mark phase iterative by using an explicit mark stack.
To do this marking any single object is performed in multiple stages

• The object is appended to the MarkStack, this sets the marked bit for the object using the new markDirect() function, and then returns
• When the MarkStack is drain()ed the object is popped off the stack and markChildren(MarkStack&) is called on the object to collect all of its children. drain() then repeats until the stack is empty.

Additionally I renamed a number of methods from 'mark' to 'markAggregate'
in order to make it more clear that marking of those object was not
going to result in an actual recursive mark.

File:

• trunk/JavaScriptCore/runtime/Collector.cpp (modified) (15 diffs)

trunk/JavaScriptCore/runtime/Collector.cpp

@@ -1 +1 @@
 /*
- *  Copyright (C) 2003, 2004, 2005, 2006, 2007, 2008 Apple Inc. All rights reserved.
+ *  Copyright (C) 2003, 2004, 2005, 2006, 2007, 2008, 2009 Apple Inc. All rights reserved.
  *  Copyright (C) 2007 Eric Seidel <[email protected]>
  *
@@ -31 +31 @@
 #include "JSString.h"
 #include "JSValue.h"
+#include "MarkStack.h"
 #include "Nodes.h"
 #include "Tracing.h"
@@ -643 +644 @@
 #define IS_HALF_CELL_ALIGNED(p) (((intptr_t)(p) & (CELL_MASK >> 1)) == 0)
 
-void Heap::markConservatively(void* start, void* end)
+void Heap::markConservatively(MarkStack& markStack, void* start, void* end)
 {
     if (start > end) {
@@ -684 +685 @@
                 if ((primaryBlocks[block] == blockAddr) & (offset <= lastCellOffset)) {
                     if (reinterpret_cast<CollectorCell*>(xAsBits)->u.freeCell.zeroIfFree != 0) {
-                        JSCell* imp = reinterpret_cast<JSCell*>(xAsBits);
-                        if (!imp->marked())
-                            imp->mark();
+                        markStack.append(reinterpret_cast<JSCell*>(xAsBits));
+                        markStack.drain();
                     }
                     break;
@@ -697 +697 @@
 }
 
-void NEVER_INLINE Heap::markCurrentThreadConservativelyInternal()
+void NEVER_INLINE Heap::markCurrentThreadConservativelyInternal(MarkStack& markStack)
 {
     void* dummy;
     void* stackPointer = &dummy;
     void* stackBase = currentThreadStackBase();
-    markConservatively(stackPointer, stackBase);
-}
-
-void Heap::markCurrentThreadConservatively()
+    markConservatively(markStack, stackPointer, stackBase);
+}
+
+void Heap::markCurrentThreadConservatively(MarkStack& markStack)
 {
     // setjmp forces volatile registers onto the stack
@@ -718 +718 @@
 #endif
 
-    markCurrentThreadConservativelyInternal();
+    markCurrentThreadConservativelyInternal(markStack);
 }
 
@@ -850 +850 @@
 }
 
-void Heap::markOtherThreadConservatively(Thread* thread)
+void Heap::markOtherThreadConservatively(MarkStack& markStack, Thread* thread)
 {
     suspendThread(thread->platformThread);
@@ -858 +858 @@
 
     // mark the thread's registers
-    markConservatively(static_cast<void*>(&regs), static_cast<void*>(reinterpret_cast<char*>(&regs) + regSize));
+    markConservatively(markStack, static_cast<void*>(&regs), static_cast<void*>(reinterpret_cast<char*>(&regs) + regSize));
 
     void* stackPointer = otherThreadStackPointer(regs);
-    markConservatively(stackPointer, thread->stackBase);
+    markConservatively(markStack, stackPointer, thread->stackBase);
 
     resumeThread(thread->platformThread);
@@ -868 +868 @@
 #endif
 
-void Heap::markStackObjectsConservatively()
-{
-    markCurrentThreadConservatively();
+void Heap::markStackObjectsConservatively(MarkStack& markStack)
+{
+    markCurrentThreadConservatively(markStack);
 
 #if ENABLE(JSC_MULTIPLE_THREADS)
@@ -880 +880 @@
 #ifndef NDEBUG
         // Forbid malloc during the mark phase. Marking a thread suspends it, so 
-        // a malloc inside mark() would risk a deadlock with a thread that had been 
+        // a malloc inside markChildren() would risk a deadlock with a thread that had been 
         // suspended while holding the malloc lock.
         fastMallocForbid();
@@ -888 +888 @@
         for (Thread* thread = m_registeredThreads; thread; thread = thread->next) {
             if (!pthread_equal(thread->posixThread, pthread_self()))
-                markOtherThreadConservatively(thread);
+                markOtherThreadConservatively(markStack, thread);
         }
 #ifndef NDEBUG
@@ -948 +948 @@
 }
 
-void Heap::markProtectedObjects()
+void Heap::markProtectedObjects(MarkStack& markStack)
 {
     if (m_protectedValuesMutex)
@@ -956 +956 @@
     for (ProtectCountSet::iterator it = m_protectedValues.begin(); it != end; ++it) {
         JSCell* val = it->first;
-        if (!val->marked())
-            val->mark();
+        if (!val->marked()) {
+            markStack.append(val);
+            markStack.drain();
+        }
     }
 
@@ -1062 +1064 @@
     return numLiveObjects;
 }
-    
+
 bool Heap::collect()
 {
@@ -1081 +1083 @@
 
     // MARK: first mark all referenced objects recursively starting out from the set of root objects
-
-    markStackObjectsConservatively();
-    markProtectedObjects();
+    MarkStack& markStack = m_globalData->markStack;
+    markStackObjectsConservatively(markStack);
+    markProtectedObjects(markStack);
     if (m_markListSet && m_markListSet->size())
-        MarkedArgumentBuffer::markLists(*m_markListSet);
+        MarkedArgumentBuffer::markLists(markStack, *m_markListSet);
     if (m_globalData->exception && !m_globalData->exception.marked())
-        m_globalData->exception.mark();
-    m_globalData->interpreter->registerFile().markCallFrames(this);
+        markStack.append(m_globalData->exception);
+    m_globalData->interpreter->registerFile().markCallFrames(markStack, this);
     m_globalData->smallStrings.mark();
     if (m_globalData->scopeNodeBeingReparsed)
-        m_globalData->scopeNodeBeingReparsed->mark();
+        m_globalData->scopeNodeBeingReparsed->markAggregate(markStack);
     if (m_globalData->firstStringifierToMark)
-        JSONObject::markStringifiers(m_globalData->firstStringifierToMark);
-
+        JSONObject::markStringifiers(markStack, m_globalData->firstStringifierToMark);
+
+    markStack.drain();
+    markStack.compact();
     JAVASCRIPTCORE_GC_MARKED();