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

Tue, 13 Jan 2015 09:46:54 -0800

Title: [178364] trunk/Source/_javascript_Core
Revision
178364
Author
msab...@apple.com
Date
2015-01-13 09:46:40 -0800 (Tue, 13 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.

We used to read registers in MachineThreads::gatherFromCurrentThread(), but that is too late
because those registers may have been spilled on the stack and replaced with other values by
the time we call down to gatherFromCurrentThread().

Now we get the register contents at the same place that we demarcate the current top of
stack using the address of a local variable, in Heap::markRoots().  The register contents
buffer is passed along with the demarcation pointer.  These need to be done at this level 
in the call tree and no lower, as markRoots() calls various functions that visit object
pointers that may be latter proven dead.  Any of those pointers that are left on the
stack or in registers could be incorrectly marked as live if we scan the stack contents
from a called function or one of its callees.  The stack demarcation pointer and register
saving need to be done in the same function so that we have a consistent stack, active
and spilled registers.

Because we don't want to make unnecessary calls to get the register contents, we use
a macro to allocated, and possibly align, the register structure and get the actual
register contents.


* 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 (178363 => 178364)


--- trunk/Source/_javascript_Core/ChangeLog	2015-01-13 16:59:49 UTC (rev 178363)
+++ trunk/Source/_javascript_Core/ChangeLog	2015-01-13 17:46:40 UTC (rev 178364)
@@ -1,3 +1,38 @@
+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.
+
+        We used to read registers in MachineThreads::gatherFromCurrentThread(), but that is too late
+        because those registers may have been spilled on the stack and replaced with other values by
+        the time we call down to gatherFromCurrentThread().
+
+        Now we get the register contents at the same place that we demarcate the current top of
+        stack using the address of a local variable, in Heap::markRoots().  The register contents
+        buffer is passed along with the demarcation pointer.  These need to be done at this level 
+        in the call tree and no lower, as markRoots() calls various functions that visit object
+        pointers that may be latter proven dead.  Any of those pointers that are left on the
+        stack or in registers could be incorrectly marked as live if we scan the stack contents
+        from a called function or one of its callees.  The stack demarcation pointer and register
+        saving need to be done in the same function so that we have a consistent stack, active
+        and spilled registers.
+
+        Because we don't want to make unnecessary calls to get the register contents, we use
+        a macro to allocated, and possibly align, the register structure and get the actual
+        register contents.
+
+
+        * 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-12  Benjamin Poulain  <benja...@webkit.org>
 
         Add basic pattern matching support to the url filters

Modified: trunk/Source/_javascript_Core/heap/Heap.cpp (178363 => 178364)


--- trunk/Source/_javascript_Core/heap/Heap.cpp	2015-01-13 16:59:49 UTC (rev 178363)
+++ trunk/Source/_javascript_Core/heap/Heap.cpp	2015-01-13 17:46:40 UTC (rev 178364)
@@ -505,8 +505,9 @@
     // We gather conservative roots before clearing mark bits because conservative
     // gathering uses the mark bits to determine whether a reference is valid.
     void* dummy;
+    ALLOCATE_AND_GET_REGISTER_STATE(registers);
     ConservativeRoots conservativeRoots(&m_objectSpace.blocks(), &m_storageSpace);
-    gatherStackRoots(conservativeRoots, &dummy);
+    gatherStackRoots(conservativeRoots, &dummy, registers);
     gatherJSStackRoots(conservativeRoots);
     gatherScratchBufferRoots(conservativeRoots);
 
@@ -566,11 +567,11 @@
         m_storageSpace.doneCopying();
 }
 
-void Heap::gatherStackRoots(ConservativeRoots& roots, void** dummy)
+void Heap::gatherStackRoots(ConservativeRoots& roots, void** dummy, MachineThreads::RegisterState& registers)
 {
     GCPHASE(GatherStackRoots);
     m_jitStubRoutines.clearMarks();
-    m_machineThreads.gatherConservativeRoots(roots, m_jitStubRoutines, m_codeBlocks, dummy);
+    m_machineThreads.gatherConservativeRoots(roots, m_jitStubRoutines, m_codeBlocks, dummy, registers);
 }
 
 void Heap::gatherJSStackRoots(ConservativeRoots& roots)

Modified: trunk/Source/_javascript_Core/heap/Heap.h (178363 => 178364)


--- trunk/Source/_javascript_Core/heap/Heap.h	2015-01-13 16:59:49 UTC (rev 178363)
+++ trunk/Source/_javascript_Core/heap/Heap.h	2015-01-13 17:46:40 UTC (rev 178364)
@@ -275,7 +275,7 @@
     void stopAllocation();
 
     void markRoots(double gcStartTime);
-    void gatherStackRoots(ConservativeRoots&, void** dummy);
+    void gatherStackRoots(ConservativeRoots&, void** dummy, MachineThreads::RegisterState& registers);
     void gatherJSStackRoots(ConservativeRoots&);
     void gatherScratchBufferRoots(ConservativeRoots&);
     void clearLivenessData();

Modified: trunk/Source/_javascript_Core/heap/MachineStackMarker.cpp (178363 => 178364)


--- trunk/Source/_javascript_Core/heap/MachineStackMarker.cpp	2015-01-13 16:59:49 UTC (rev 178363)
+++ trunk/Source/_javascript_Core/heap/MachineStackMarker.cpp	2015-01-13 17:46:40 UTC (rev 178364)
@@ -215,25 +215,8 @@
     }
 }
 
-#if COMPILER(GCC)
-#define REGISTER_BUFFER_ALIGNMENT __attribute__ ((aligned (sizeof(void*))))
-#else
-#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, void* stackCurrent, RegisterState& registers)
 {
-    // setjmp forces volatile registers onto the stack
-    jmp_buf registers REGISTER_BUFFER_ALIGNMENT;
-#if COMPILER(MSVC)
-#pragma warning(push)
-#pragma warning(disable: 4611)
-#endif
-    setjmp(registers);
-#if COMPILER(MSVC)
-#pragma warning(pop)
-#endif
-
     void* registersBegin = &registers;
     void* registersEnd = reinterpret_cast<void*>(roundUpToMultipleOf<sizeof(void*)>(reinterpret_cast<uintptr_t>(&registers + 1)));
     conservativeRoots.add(registersBegin, registersEnd, jitStubRoutines, codeBlocks);
@@ -445,9 +428,9 @@
     freePlatformThreadRegisters(regs);
 }
 
-void MachineThreads::gatherConservativeRoots(ConservativeRoots& conservativeRoots, JITStubRoutineSet& jitStubRoutines, CodeBlockSet& codeBlocks, void* stackCurrent)
+void MachineThreads::gatherConservativeRoots(ConservativeRoots& conservativeRoots, JITStubRoutineSet& jitStubRoutines, CodeBlockSet& codeBlocks, void* stackCurrent, RegisterState& registers)
 {
-    gatherFromCurrentThread(conservativeRoots, jitStubRoutines, codeBlocks, stackCurrent);
+    gatherFromCurrentThread(conservativeRoots, jitStubRoutines, codeBlocks, stackCurrent, registers);
 
     if (m_threadSpecific) {
         PlatformThread currentPlatformThread = getCurrentPlatformThread();

Modified: trunk/Source/_javascript_Core/heap/MachineStackMarker.h (178363 => 178364)


--- trunk/Source/_javascript_Core/heap/MachineStackMarker.h	2015-01-13 16:59:49 UTC (rev 178363)
+++ trunk/Source/_javascript_Core/heap/MachineStackMarker.h	2015-01-13 17:46:40 UTC (rev 178364)
@@ -22,6 +22,7 @@
 #ifndef MachineThreads_h
 #define MachineThreads_h
 
+#include <setjmp.h>
 #include <wtf/Noncopyable.h>
 #include <wtf/ThreadSpecific.h>
 #include <wtf/ThreadingPrimitives.h>
@@ -36,16 +37,18 @@
     class MachineThreads {
         WTF_MAKE_NONCOPYABLE(MachineThreads);
     public:
+        typedef jmp_buf RegisterState;
+
         MachineThreads(Heap*);
         ~MachineThreads();
 
-        void gatherConservativeRoots(ConservativeRoots&, JITStubRoutineSet&, CodeBlockSet&, void* stackCurrent);
+        void gatherConservativeRoots(ConservativeRoots&, JITStubRoutineSet&, CodeBlockSet&, void* stackCurrent, RegisterState& registers);
 
         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&, void* stackCurrent, RegisterState& registers);
 
         class Thread;
 
@@ -64,4 +67,24 @@
 
 } // namespace JSC
 
+#if COMPILER(GCC)
+#define REGISTER_BUFFER_ALIGNMENT __attribute__ ((aligned (sizeof(void*))))
+#else
+#define REGISTER_BUFFER_ALIGNMENT
+#endif
+
+// ALLOCATE_AND_GET_REGISTER_STATE() is a macro so that it is always "inlined" even in debug builds.
+#if COMPILER(MSVC)
+#pragma warning(push)
+#pragma warning(disable: 4611)
+#define ALLOCATE_AND_GET_REGISTER_STATE(registers) \
+    MachineThreads::RegisterState registers REGISTER_BUFFER_ALIGNMENT; \
+    setjmp(registers)
+#pragma warning(pop)
+#else
+#define ALLOCATE_AND_GET_REGISTER_STATE(registers) \
+    MachineThreads::RegisterState registers REGISTER_BUFFER_ALIGNMENT; \
+    setjmp(registers)
+#endif
+
 #endif // MachineThreads_h

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

Reply via email to