[webkit-changes] [178266] trunk/Source/JavaScriptCore

Mon, 12 Jan 2015 08:29:49 -0800

Title: [178266] trunk/Source/_javascript_Core
Revision
178266
Author
msab...@apple.com
Date
2015-01-12 08:29:22 -0800 (Mon, 12 Jan 2015)

Log 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:

Modified Paths

Diff

Modified: trunk/Source/_javascript_Core/ChangeLog (178265 => 178266)


--- trunk/Source/_javascript_Core/ChangeLog	2015-01-12 16:22:50 UTC (rev 178265)
+++ trunk/Source/_javascript_Core/ChangeLog	2015-01-12 16:29:22 UTC (rev 178266)
@@ -1,3 +1,26 @@
+2015-01-12  Michael Saboff  <msab...@apple.com>
+
+        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  <eric.carl...@apple.com>
 
         Fix typo in testate.c error messages

Modified: trunk/Source/_javascript_Core/heap/Heap.cpp (178265 => 178266)


--- trunk/Source/_javascript_Core/heap/Heap.cpp	2015-01-12 16:22:50 UTC (rev 178265)
+++ trunk/Source/_javascript_Core/heap/Heap.cpp	2015-01-12 16:29:22 UTC (rev 178266)
@@ -504,9 +504,8 @@
 
     // 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);
 
@@ -566,11 +565,11 @@
         m_storageSpace.doneCopying();
 }
 
-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);
 }
 
 void Heap::gatherJSStackRoots(ConservativeRoots& roots)

Modified: trunk/Source/_javascript_Core/heap/Heap.h (178265 => 178266)


--- trunk/Source/_javascript_Core/heap/Heap.h	2015-01-12 16:22:50 UTC (rev 178265)
+++ trunk/Source/_javascript_Core/heap/Heap.h	2015-01-12 16:29:22 UTC (rev 178266)
@@ -275,7 +275,7 @@
     void stopAllocation();
 
     void markRoots(double gcStartTime);
-    void gatherStackRoots(ConservativeRoots&, void** dummy);
+    void gatherStackRoots(ConservativeRoots&);
     void gatherJSStackRoots(ConservativeRoots&);
     void gatherScratchBufferRoots(ConservativeRoots&);
     void clearLivenessData();

Modified: trunk/Source/_javascript_Core/heap/MachineStackMarker.cpp (178265 => 178266)


--- trunk/Source/_javascript_Core/heap/MachineStackMarker.cpp	2015-01-12 16:22:50 UTC (rev 178265)
+++ trunk/Source/_javascript_Core/heap/MachineStackMarker.cpp	2015-01-12 16:29:22 UTC (rev 178266)
@@ -221,10 +221,11 @@
 #define REGISTER_BUFFER_ALIGNMENT
 #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)
 #pragma warning(disable: 4611)
@@ -238,7 +239,9 @@
     void* registersEnd = reinterpret_cast<void*>(roundUpToMultipleOf<sizeof(void*)>(reinterpret_cast<uintptr_t>(&registers + 1)));
     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);
 }
@@ -445,9 +448,9 @@
     freePlatformThreadRegisters(regs);
 }
 
-void MachineThreads::gatherConservativeRoots(ConservativeRoots& conservativeRoots, JITStubRoutineSet& jitStubRoutines, CodeBlockSet& codeBlocks, void* stackCurrent)
+void MachineThreads::gatherConservativeRoots(ConservativeRoots& conservativeRoots, JITStubRoutineSet& jitStubRoutines, CodeBlockSet& codeBlocks)
 {
-    gatherFromCurrentThread(conservativeRoots, jitStubRoutines, codeBlocks, stackCurrent);
+    gatherFromCurrentThread(conservativeRoots, jitStubRoutines, codeBlocks);
 
     if (m_threadSpecific) {
         PlatformThread currentPlatformThread = getCurrentPlatformThread();

Modified: trunk/Source/_javascript_Core/heap/MachineStackMarker.h (178265 => 178266)


--- trunk/Source/_javascript_Core/heap/MachineStackMarker.h	2015-01-12 16:22:50 UTC (rev 178265)
+++ trunk/Source/_javascript_Core/heap/MachineStackMarker.h	2015-01-12 16:29:22 UTC (rev 178266)
@@ -39,13 +39,13 @@
         MachineThreads(Heap*);
         ~MachineThreads();
 
-        void gatherConservativeRoots(ConservativeRoots&, JITStubRoutineSet&, CodeBlockSet&, void* stackCurrent);
+        void gatherConservativeRoots(ConservativeRoots&, JITStubRoutineSet&, CodeBlockSet&);
 
         JS_EXPORT_PRIVATE void makeUsableFromMultipleThreads();
         JS_EXPORT_PRIVATE void addCurrentThread(); // Only needs to be called by clients that can use the same heap from multiple threads.
 
     private:
-        void gatherFromCurrentThread(ConservativeRoots&, JITStubRoutineSet&, CodeBlockSet&, void* stackCurrent);
+        void gatherFromCurrentThread(ConservativeRoots&, JITStubRoutineSet&, CodeBlockSet&);
 
         class Thread;
 

_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to