<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.1//EN"
"http://www.w3.org/TR/xhtml11/DTD/xhtml11.dtd">
<html xmlns="http://www.w3.org/1999/xhtml">
<head><meta http-equiv="content-type" content="text/html; charset=utf-8" />
<title>[178364] trunk/Source/JavaScriptCore</title>
</head>
<body>
<style type="text/css"><!--
#msg dl.meta { border: 1px #006 solid; background: #369; padding: 6px; color: #fff; }
#msg dl.meta dt { float: left; width: 6em; font-weight: bold; }
#msg dt:after { content:':';}
#msg dl, #msg dt, #msg ul, #msg li, #header, #footer, #logmsg { font-family: verdana,arial,helvetica,sans-serif; font-size: 10pt; }
#msg dl a { font-weight: bold}
#msg dl a:link { color:#fc3; }
#msg dl a:active { color:#ff0; }
#msg dl a:visited { color:#cc6; }
h3 { font-family: verdana,arial,helvetica,sans-serif; font-size: 10pt; font-weight: bold; }
#msg pre { overflow: auto; background: #ffc; border: 1px #fa0 solid; padding: 6px; }
#logmsg { background: #ffc; border: 1px #fa0 solid; padding: 1em 1em 0 1em; }
#logmsg p, #logmsg pre, #logmsg blockquote { margin: 0 0 1em 0; }
#logmsg p, #logmsg li, #logmsg dt, #logmsg dd { line-height: 14pt; }
#logmsg h1, #logmsg h2, #logmsg h3, #logmsg h4, #logmsg h5, #logmsg h6 { margin: .5em 0; }
#logmsg h1:first-child, #logmsg h2:first-child, #logmsg h3:first-child, #logmsg h4:first-child, #logmsg h5:first-child, #logmsg h6:first-child { margin-top: 0; }
#logmsg ul, #logmsg ol { padding: 0; list-style-position: inside; margin: 0 0 0 1em; }
#logmsg ul { text-indent: -1em; padding-left: 1em; }#logmsg ol { text-indent: -1.5em; padding-left: 1.5em; }
#logmsg > ul, #logmsg > ol { margin: 0 0 1em 0; }
#logmsg pre { background: #eee; padding: 1em; }
#logmsg blockquote { border: 1px solid #fa0; border-left-width: 10px; padding: 1em 1em 0 1em; background: white;}
#logmsg dl { margin: 0; }
#logmsg dt { font-weight: bold; }
#logmsg dd { margin: 0; padding: 0 0 0.5em 0; }
#logmsg dd:before { content:'\00bb';}
#logmsg table { border-spacing: 0px; border-collapse: collapse; border-top: 4px solid #fa0; border-bottom: 1px solid #fa0; background: #fff; }
#logmsg table th { text-align: left; font-weight: normal; padding: 0.2em 0.5em; border-top: 1px dotted #fa0; }
#logmsg table td { text-align: right; border-top: 1px dotted #fa0; padding: 0.2em 0.5em; }
#logmsg table thead th { text-align: center; border-bottom: 1px solid #fa0; }
#logmsg table th.Corner { text-align: left; }
#logmsg hr { border: none 0; border-top: 2px dashed #fa0; height: 1px; }
#header, #footer { color: #fff; background: #636; border: 1px #300 solid; padding: 6px; }
#patch { width: 100%; }
#patch h4 {font-family: verdana,arial,helvetica,sans-serif;font-size:10pt;padding:8px;background:#369;color:#fff;margin:0;}
#patch .propset h4, #patch .binary h4 {margin:0;}
#patch pre {padding:0;line-height:1.2em;margin:0;}
#patch .diff {width:100%;background:#eee;padding: 0 0 10px 0;overflow:auto;}
#patch .propset .diff, #patch .binary .diff {padding:10px 0;}
#patch span {display:block;padding:0 10px;}
#patch .modfile, #patch .addfile, #patch .delfile, #patch .propset, #patch .binary, #patch .copfile {border:1px solid #ccc;margin:10px 0;}
#patch ins {background:#dfd;text-decoration:none;display:block;padding:0 10px;}
#patch del {background:#fdd;text-decoration:none;display:block;padding:0 10px;}
#patch .lines, .info {color:#888;background:#fff;}
--></style>
<div id="msg">
<dl class="meta">
<dt>Revision</dt> <dd><a href="http://trac.webkit.org/projects/webkit/changeset/178364">178364</a></dd>
<dt>Author</dt> <dd>msaboff@apple.com</dd>
<dt>Date</dt> <dd>2015-01-13 09:46:40 -0800 (Tue, 13 Jan 2015)</dd>
</dl>
<h3>Log Message</h3>
<pre>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:</pre>
<h3>Modified Paths</h3>
<ul>
<li><a href="#trunkSourceJavaScriptCoreChangeLog">trunk/Source/JavaScriptCore/ChangeLog</a></li>
<li><a href="#trunkSourceJavaScriptCoreheapHeapcpp">trunk/Source/JavaScriptCore/heap/Heap.cpp</a></li>
<li><a href="#trunkSourceJavaScriptCoreheapHeaph">trunk/Source/JavaScriptCore/heap/Heap.h</a></li>
<li><a href="#trunkSourceJavaScriptCoreheapMachineStackMarkercpp">trunk/Source/JavaScriptCore/heap/MachineStackMarker.cpp</a></li>
<li><a href="#trunkSourceJavaScriptCoreheapMachineStackMarkerh">trunk/Source/JavaScriptCore/heap/MachineStackMarker.h</a></li>
</ul>
</div>
<div id="patch">
<h3>Diff</h3>
<a id="trunkSourceJavaScriptCoreChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/ChangeLog (178363 => 178364)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/ChangeLog 2015-01-13 16:59:49 UTC (rev 178363)
+++ trunk/Source/JavaScriptCore/ChangeLog 2015-01-13 17:46:40 UTC (rev 178364)
</span><span class="lines">@@ -1,3 +1,38 @@
</span><ins>+2015-01-12 Michael Saboff <msaboff@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:
+
</ins><span class="cx"> 2015-01-12 Benjamin Poulain <benjamin@webkit.org>
</span><span class="cx">
</span><span class="cx"> Add basic pattern matching support to the url filters
</span></span></pre></div>
<a id="trunkSourceJavaScriptCoreheapHeapcpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/heap/Heap.cpp (178363 => 178364)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/heap/Heap.cpp 2015-01-13 16:59:49 UTC (rev 178363)
+++ trunk/Source/JavaScriptCore/heap/Heap.cpp 2015-01-13 17:46:40 UTC (rev 178364)
</span><span class="lines">@@ -505,8 +505,9 @@
</span><span class="cx"> // We gather conservative roots before clearing mark bits because conservative
</span><span class="cx"> // gathering uses the mark bits to determine whether a reference is valid.
</span><span class="cx"> void* dummy;
</span><ins>+ ALLOCATE_AND_GET_REGISTER_STATE(registers);
</ins><span class="cx"> ConservativeRoots conservativeRoots(&m_objectSpace.blocks(), &m_storageSpace);
</span><del>- gatherStackRoots(conservativeRoots, &dummy);
</del><ins>+ gatherStackRoots(conservativeRoots, &dummy, registers);
</ins><span class="cx"> gatherJSStackRoots(conservativeRoots);
</span><span class="cx"> gatherScratchBufferRoots(conservativeRoots);
</span><span class="cx">
</span><span class="lines">@@ -566,11 +567,11 @@
</span><span class="cx"> m_storageSpace.doneCopying();
</span><span class="cx"> }
</span><span class="cx">
</span><del>-void Heap::gatherStackRoots(ConservativeRoots& roots, void** dummy)
</del><ins>+void Heap::gatherStackRoots(ConservativeRoots& roots, void** dummy, MachineThreads::RegisterState& registers)
</ins><span class="cx"> {
</span><span class="cx"> GCPHASE(GatherStackRoots);
</span><span class="cx"> m_jitStubRoutines.clearMarks();
</span><del>- m_machineThreads.gatherConservativeRoots(roots, m_jitStubRoutines, m_codeBlocks, dummy);
</del><ins>+ m_machineThreads.gatherConservativeRoots(roots, m_jitStubRoutines, m_codeBlocks, dummy, registers);
</ins><span class="cx"> }
</span><span class="cx">
</span><span class="cx"> void Heap::gatherJSStackRoots(ConservativeRoots& roots)
</span></span></pre></div>
<a id="trunkSourceJavaScriptCoreheapHeaph"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/heap/Heap.h (178363 => 178364)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/heap/Heap.h 2015-01-13 16:59:49 UTC (rev 178363)
+++ trunk/Source/JavaScriptCore/heap/Heap.h 2015-01-13 17:46:40 UTC (rev 178364)
</span><span class="lines">@@ -275,7 +275,7 @@
</span><span class="cx"> void stopAllocation();
</span><span class="cx">
</span><span class="cx"> void markRoots(double gcStartTime);
</span><del>- void gatherStackRoots(ConservativeRoots&, void** dummy);
</del><ins>+ void gatherStackRoots(ConservativeRoots&, void** dummy, MachineThreads::RegisterState& registers);
</ins><span class="cx"> void gatherJSStackRoots(ConservativeRoots&);
</span><span class="cx"> void gatherScratchBufferRoots(ConservativeRoots&);
</span><span class="cx"> void clearLivenessData();
</span></span></pre></div>
<a id="trunkSourceJavaScriptCoreheapMachineStackMarkercpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/heap/MachineStackMarker.cpp (178363 => 178364)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/heap/MachineStackMarker.cpp 2015-01-13 16:59:49 UTC (rev 178363)
+++ trunk/Source/JavaScriptCore/heap/MachineStackMarker.cpp 2015-01-13 17:46:40 UTC (rev 178364)
</span><span class="lines">@@ -215,25 +215,8 @@
</span><span class="cx"> }
</span><span class="cx"> }
</span><span class="cx">
</span><del>-#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)
</del><ins>+void MachineThreads::gatherFromCurrentThread(ConservativeRoots& conservativeRoots, JITStubRoutineSet& jitStubRoutines, CodeBlockSet& codeBlocks, void* stackCurrent, RegisterState& registers)
</ins><span class="cx"> {
</span><del>- // 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
-
</del><span class="cx"> void* registersBegin = &registers;
</span><span class="cx"> void* registersEnd = reinterpret_cast<void*>(roundUpToMultipleOf<sizeof(void*)>(reinterpret_cast<uintptr_t>(&registers + 1)));
</span><span class="cx"> conservativeRoots.add(registersBegin, registersEnd, jitStubRoutines, codeBlocks);
</span><span class="lines">@@ -445,9 +428,9 @@
</span><span class="cx"> freePlatformThreadRegisters(regs);
</span><span class="cx"> }
</span><span class="cx">
</span><del>-void MachineThreads::gatherConservativeRoots(ConservativeRoots& conservativeRoots, JITStubRoutineSet& jitStubRoutines, CodeBlockSet& codeBlocks, void* stackCurrent)
</del><ins>+void MachineThreads::gatherConservativeRoots(ConservativeRoots& conservativeRoots, JITStubRoutineSet& jitStubRoutines, CodeBlockSet& codeBlocks, void* stackCurrent, RegisterState& registers)
</ins><span class="cx"> {
</span><del>- gatherFromCurrentThread(conservativeRoots, jitStubRoutines, codeBlocks, stackCurrent);
</del><ins>+ gatherFromCurrentThread(conservativeRoots, jitStubRoutines, codeBlocks, stackCurrent, registers);
</ins><span class="cx">
</span><span class="cx"> if (m_threadSpecific) {
</span><span class="cx"> PlatformThread currentPlatformThread = getCurrentPlatformThread();
</span></span></pre></div>
<a id="trunkSourceJavaScriptCoreheapMachineStackMarkerh"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/heap/MachineStackMarker.h (178363 => 178364)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/heap/MachineStackMarker.h 2015-01-13 16:59:49 UTC (rev 178363)
+++ trunk/Source/JavaScriptCore/heap/MachineStackMarker.h 2015-01-13 17:46:40 UTC (rev 178364)
</span><span class="lines">@@ -22,6 +22,7 @@
</span><span class="cx"> #ifndef MachineThreads_h
</span><span class="cx"> #define MachineThreads_h
</span><span class="cx">
</span><ins>+#include <setjmp.h>
</ins><span class="cx"> #include <wtf/Noncopyable.h>
</span><span class="cx"> #include <wtf/ThreadSpecific.h>
</span><span class="cx"> #include <wtf/ThreadingPrimitives.h>
</span><span class="lines">@@ -36,16 +37,18 @@
</span><span class="cx"> class MachineThreads {
</span><span class="cx"> WTF_MAKE_NONCOPYABLE(MachineThreads);
</span><span class="cx"> public:
</span><ins>+ typedef jmp_buf RegisterState;
+
</ins><span class="cx"> MachineThreads(Heap*);
</span><span class="cx"> ~MachineThreads();
</span><span class="cx">
</span><del>- void gatherConservativeRoots(ConservativeRoots&, JITStubRoutineSet&, CodeBlockSet&, void* stackCurrent);
</del><ins>+ void gatherConservativeRoots(ConservativeRoots&, JITStubRoutineSet&, CodeBlockSet&, void* stackCurrent, RegisterState& registers);
</ins><span class="cx">
</span><span class="cx"> JS_EXPORT_PRIVATE void makeUsableFromMultipleThreads();
</span><span class="cx"> JS_EXPORT_PRIVATE void addCurrentThread(); // Only needs to be called by clients that can use the same heap from multiple threads.
</span><span class="cx">
</span><span class="cx"> private:
</span><del>- void gatherFromCurrentThread(ConservativeRoots&, JITStubRoutineSet&, CodeBlockSet&, void* stackCurrent);
</del><ins>+ void gatherFromCurrentThread(ConservativeRoots&, JITStubRoutineSet&, CodeBlockSet&, void* stackCurrent, RegisterState& registers);
</ins><span class="cx">
</span><span class="cx"> class Thread;
</span><span class="cx">
</span><span class="lines">@@ -64,4 +67,24 @@
</span><span class="cx">
</span><span class="cx"> } // namespace JSC
</span><span class="cx">
</span><ins>+#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
+
</ins><span class="cx"> #endif // MachineThreads_h
</span></span></pre>
</div>
</div>
</body>
</html>