Changeset 178266 in webkit

Timestamp:

Jan 12, 2015, 8:29:22 AM (11 years ago)

Author:

[email protected]

Message:

Local JSArray* "keys" in objectConstructorKeys() is not marked during garbage collection
​https://bugs.webkit.org/show_bug.cgi?id=140348

Reviewed by Mark Lam.

Move the address of the local variable that is used to demarcate the top of the stack for
conservative roots down to MachineThreads::gatherFromCurrentThread() since it also gets
the register values using setjmp(). That way we don't lose any callee save register
contents between Heap::markRoots(), where it was set, and gatherFromCurrentThread().
If we lose any JSObject* that are only in callee save registers, they will be GC'ed
erroneously.

• heap/Heap.cpp:

(JSC::Heap::markRoots):
(JSC::Heap::gatherStackRoots):

• heap/Heap.h:
• heap/MachineStackMarker.cpp:

(JSC::MachineThreads::gatherFromCurrentThread):
(JSC::MachineThreads::gatherConservativeRoots):

• heap/MachineStackMarker.h:

Location:

trunk/Source/JavaScriptCore

Files:

• ChangeLog (modified) (1 diff)
• heap/Heap.cpp (modified) (2 diffs)
• heap/Heap.h (modified) (1 diff)
• heap/MachineStackMarker.cpp (modified) (3 diffs)
• heap/MachineStackMarker.h (modified) (2 diffs)

trunk/Source/JavaScriptCore/ChangeLog

@@ +1 @@
+2015-01-12  Michael Saboff  <[email protected]>
+
+        Local JSArray* "keys" in objectConstructorKeys() is not marked during garbage collection
+        https://bugs.webkit.org/show_bug.cgi?id=140348
+
+        Reviewed by Mark Lam.
+
+        Move the address of the local variable that is used to demarcate the top of the stack for 
+        conservative roots down to MachineThreads::gatherFromCurrentThread() since it also gets
+        the register values using setjmp().  That way we don't lose any callee save register
+        contents between Heap::markRoots(), where it was set, and gatherFromCurrentThread().
+        If we lose any JSObject* that are only in callee save registers, they will be GC'ed
+        erroneously.
+
+        * heap/Heap.cpp:
+        (JSC::Heap::markRoots):
+        (JSC::Heap::gatherStackRoots):
+        * heap/Heap.h:
+        * heap/MachineStackMarker.cpp:
+        (JSC::MachineThreads::gatherFromCurrentThread):
+        (JSC::MachineThreads::gatherConservativeRoots):
+        * heap/MachineStackMarker.h:
+
 2015-01-11  Eric Carlson  <[email protected]>
 

trunk/Source/JavaScriptCore/heap/Heap.cpp

@@ -505 +505 @@
     // We gather conservative roots before clearing mark bits because conservative
     // gathering uses the mark bits to determine whether a reference is valid.
-    void* dummy;
     ConservativeRoots conservativeRoots(&m_objectSpace.blocks(), &m_storageSpace);
-    gatherStackRoots(conservativeRoots, &dummy);
+    gatherStackRoots(conservativeRoots);
     gatherJSStackRoots(conservativeRoots);
     gatherScratchBufferRoots(conservativeRoots);
@@ -567 +566 @@
 }
 
-void Heap::gatherStackRoots(ConservativeRoots& roots, void** dummy)
+void Heap::gatherStackRoots(ConservativeRoots& roots)
 {
     GCPHASE(GatherStackRoots);
     m_jitStubRoutines.clearMarks();
-    m_machineThreads.gatherConservativeRoots(roots, m_jitStubRoutines, m_codeBlocks, dummy);
+    m_machineThreads.gatherConservativeRoots(roots, m_jitStubRoutines, m_codeBlocks);
 }
 

trunk/Source/JavaScriptCore/heap/Heap.h

@@ -276 +276 @@
 
     void markRoots(double gcStartTime);
-    void gatherStackRoots(ConservativeRoots&, void** dummy);
+    void gatherStackRoots(ConservativeRoots&);
     void gatherJSStackRoots(ConservativeRoots&);
     void gatherScratchBufferRoots(ConservativeRoots&);

trunk/Source/JavaScriptCore/heap/MachineStackMarker.cpp

@@ -222 +222 @@
 #endif
 
-void MachineThreads::gatherFromCurrentThread(ConservativeRoots& conservativeRoots, JITStubRoutineSet& jitStubRoutines, CodeBlockSet& codeBlocks, void* stackCurrent)
+void MachineThreads::gatherFromCurrentThread(ConservativeRoots& conservativeRoots, JITStubRoutineSet& jitStubRoutines, CodeBlockSet& codeBlocks)
 {
     // setjmp forces volatile registers onto the stack
     jmp_buf registers REGISTER_BUFFER_ALIGNMENT;
+
 #if COMPILER(MSVC)
 #pragma warning(push)
@@ -239 +240 @@
     conservativeRoots.add(registersBegin, registersEnd, jitStubRoutines, codeBlocks);
 
-    void* stackBegin = stackCurrent;
+    // We need to mark the stack top in this function so that callee saves are either already on the stack,
+    // or will be saved in registers.
+    void* stackBegin = &registers;
     void* stackEnd = wtfThreadData().stack().origin();
     conservativeRoots.add(stackBegin, stackEnd, jitStubRoutines, codeBlocks);
@@ -446 +449 @@
 }
 
-void MachineThreads::gatherConservativeRoots(ConservativeRoots& conservativeRoots, JITStubRoutineSet& jitStubRoutines, CodeBlockSet& codeBlocks, void* stackCurrent)
-{
-    gatherFromCurrentThread(conservativeRoots, jitStubRoutines, codeBlocks, stackCurrent);
+void MachineThreads::gatherConservativeRoots(ConservativeRoots& conservativeRoots, JITStubRoutineSet& jitStubRoutines, CodeBlockSet& codeBlocks)
+{
+    gatherFromCurrentThread(conservativeRoots, jitStubRoutines, codeBlocks);
 
     if (m_threadSpecific) {

trunk/Source/JavaScriptCore/heap/MachineStackMarker.h

@@ -40 +40 @@
         ~MachineThreads();
 
-        void gatherConservativeRoots(ConservativeRoots&, JITStubRoutineSet&, CodeBlockSet&, void* stackCurrent);
+        void gatherConservativeRoots(ConservativeRoots&, JITStubRoutineSet&, CodeBlockSet&);
 
         JS_EXPORT_PRIVATE void makeUsableFromMultipleThreads();
@@ -46 +46 @@
 
     private:
-        void gatherFromCurrentThread(ConservativeRoots&, JITStubRoutineSet&, CodeBlockSet&, void* stackCurrent);
+        void gatherFromCurrentThread(ConservativeRoots&, JITStubRoutineSet&, CodeBlockSet&);
 
         class Thread;