- 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>(®isters + 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 = ®isters;
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;