<!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>[189517] trunk/Source</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/189517">189517</a></dd>
<dt>Author</dt> <dd>mark.lam@apple.com</dd>
<dt>Date</dt> <dd>2015-09-08 17:19:15 -0700 (Tue, 08 Sep 2015)</dd>
</dl>
<h3>Log Message</h3>
<pre>GC stack scan should include ABI red zone.
https://bugs.webkit.org/show_bug.cgi?id=148976
Reviewed by Geoffrey Garen and Benjamin Poulain.
Source/JavaScriptCore:
The x86_64 ABI section 3.2.2[1] and ARM64 ABI[2] both state that there is a
128 byte red zone below the stack pointer (reserved by the OS), and that
"functions may use this area for temporary data that is not needed across
function calls".
Hence, it is possible for a thread to store JSCell pointers in the red zone
area, and the conservative GC thread scanner needs to scan that area as well.
Note: the red zone should not be scanned for the GC thread itself (in
gatherFromCurrentThread()). This because we're guaranteed that there will
be GC frames below the lowest (top of stack) frame that we need to scan.
Hence, we are guaranteed that there are no red zone areas there containing
JSObject pointers of relevance.
No test added for this issue because the issue relies on:
1. the compiler tool chain generating code that stores local variables
containing the sole reference to a JS object (that needs to be kept
alive) in the stack red zone, and
2. GC has to run on another thread while that red zone containing the
JS object reference is in use.
These conditions require a race that cannot be reliably reproduced.
[1]: http://people.freebsd.org/~obrien/amd64-elf-abi.pdf
[2]: https://developer.apple.com/library/ios/documentation/Xcode/Conceptual/iPhoneOSABIReference/Articles/ARM64FunctionCallingConventions.html#//apple_ref/doc/uid/TP40013702-SW7
* heap/MachineStackMarker.cpp:
(JSC::MachineThreads::Thread::Thread):
(JSC::MachineThreads::Thread::createForCurrentThread):
(JSC::MachineThreads::Thread::freeRegisters):
(JSC::osRedZoneAdjustment):
(JSC::MachineThreads::Thread::captureStack):
Source/WTF:
* wtf/StackBounds.h:
(WTF::StackBounds::origin):
(WTF::StackBounds::end):
(WTF::StackBounds::size):</pre>
<h3>Modified Paths</h3>
<ul>
<li><a href="#trunkSourceJavaScriptCoreChangeLog">trunk/Source/JavaScriptCore/ChangeLog</a></li>
<li><a href="#trunkSourceJavaScriptCoreheapMachineStackMarkercpp">trunk/Source/JavaScriptCore/heap/MachineStackMarker.cpp</a></li>
<li><a href="#trunkSourceWTFChangeLog">trunk/Source/WTF/ChangeLog</a></li>
<li><a href="#trunkSourceWTFwtfStackBoundsh">trunk/Source/WTF/wtf/StackBounds.h</a></li>
</ul>
</div>
<div id="patch">
<h3>Diff</h3>
<a id="trunkSourceJavaScriptCoreChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/ChangeLog (189516 => 189517)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/ChangeLog 2015-09-09 00:07:51 UTC (rev 189516)
+++ trunk/Source/JavaScriptCore/ChangeLog 2015-09-09 00:19:15 UTC (rev 189517)
</span><span class="lines">@@ -1,3 +1,43 @@
</span><ins>+2015-09-08 Mark Lam <mark.lam@apple.com>
+
+ GC stack scan should include ABI red zone.
+ https://bugs.webkit.org/show_bug.cgi?id=148976
+
+ Reviewed by Geoffrey Garen and Benjamin Poulain.
+
+ The x86_64 ABI section 3.2.2[1] and ARM64 ABI[2] both state that there is a
+ 128 byte red zone below the stack pointer (reserved by the OS), and that
+ "functions may use this area for temporary data that is not needed across
+ function calls".
+
+ Hence, it is possible for a thread to store JSCell pointers in the red zone
+ area, and the conservative GC thread scanner needs to scan that area as well.
+
+ Note: the red zone should not be scanned for the GC thread itself (in
+ gatherFromCurrentThread()). This because we're guaranteed that there will
+ be GC frames below the lowest (top of stack) frame that we need to scan.
+ Hence, we are guaranteed that there are no red zone areas there containing
+ JSObject pointers of relevance.
+
+ No test added for this issue because the issue relies on:
+ 1. the compiler tool chain generating code that stores local variables
+ containing the sole reference to a JS object (that needs to be kept
+ alive) in the stack red zone, and
+ 2. GC has to run on another thread while that red zone containing the
+ JS object reference is in use.
+
+ These conditions require a race that cannot be reliably reproduced.
+
+ [1]: http://people.freebsd.org/~obrien/amd64-elf-abi.pdf
+ [2]: https://developer.apple.com/library/ios/documentation/Xcode/Conceptual/iPhoneOSABIReference/Articles/ARM64FunctionCallingConventions.html#//apple_ref/doc/uid/TP40013702-SW7
+
+ * heap/MachineStackMarker.cpp:
+ (JSC::MachineThreads::Thread::Thread):
+ (JSC::MachineThreads::Thread::createForCurrentThread):
+ (JSC::MachineThreads::Thread::freeRegisters):
+ (JSC::osRedZoneAdjustment):
+ (JSC::MachineThreads::Thread::captureStack):
+
</ins><span class="cx"> 2015-09-08 Geoffrey Garen <ggaren@apple.com>
</span><span class="cx">
</span><span class="cx"> GC should be able to discover new strong CodeBlock references during marking
</span></span></pre></div>
<a id="trunkSourceJavaScriptCoreheapMachineStackMarkercpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/heap/MachineStackMarker.cpp (189516 => 189517)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/heap/MachineStackMarker.cpp 2015-09-09 00:07:51 UTC (rev 189516)
+++ trunk/Source/JavaScriptCore/heap/MachineStackMarker.cpp 2015-09-09 00:19:15 UTC (rev 189517)
</span><span class="lines">@@ -160,9 +160,10 @@
</span><span class="cx"> class MachineThreads::Thread {
</span><span class="cx"> WTF_MAKE_FAST_ALLOCATED;
</span><span class="cx">
</span><del>- Thread(const PlatformThread& platThread, void* base)
</del><ins>+ Thread(const PlatformThread& platThread, void* base, void* end)
</ins><span class="cx"> : platformThread(platThread)
</span><span class="cx"> , stackBase(base)
</span><ins>+ , stackEnd(end)
</ins><span class="cx"> {
</span><span class="cx"> #if USE(PTHREADS) && !OS(WINDOWS) && !OS(DARWIN) && defined(SA_RESTART)
</span><span class="cx"> // if we have SA_RESTART, enable SIGUSR2 debugging mechanism
</span><span class="lines">@@ -195,7 +196,8 @@
</span><span class="cx">
</span><span class="cx"> static Thread* createForCurrentThread()
</span><span class="cx"> {
</span><del>- return new Thread(getCurrentPlatformThread(), wtfThreadData().stack().origin());
</del><ins>+ auto stackBounds = wtfThreadData().stack();
+ return new Thread(getCurrentPlatformThread(), stackBounds.origin(), stackBounds.end());
</ins><span class="cx"> }
</span><span class="cx">
</span><span class="cx"> struct Registers {
</span><span class="lines">@@ -241,6 +243,7 @@
</span><span class="cx"> Thread* next;
</span><span class="cx"> PlatformThread platformThread;
</span><span class="cx"> void* stackBase;
</span><ins>+ void* stackEnd;
</ins><span class="cx"> #if OS(WINDOWS)
</span><span class="cx"> HANDLE platformThreadHandle;
</span><span class="cx"> #endif
</span><span class="lines">@@ -510,14 +513,35 @@
</span><span class="cx"> #endif
</span><span class="cx"> }
</span><span class="cx">
</span><ins>+static inline int osRedZoneAdjustment()
+{
+ int redZoneAdjustment = 0;
+#if !OS(WINDOWS)
+#if CPU(X86_64)
+ // See http://people.freebsd.org/~obrien/amd64-elf-abi.pdf Section 3.2.2.
+ redZoneAdjustment = -128;
+#elif CPU(ARM64)
+ // See https://developer.apple.com/library/ios/documentation/Xcode/Conceptual/iPhoneOSABIReference/Articles/ARM64FunctionCallingConventions.html#//apple_ref/doc/uid/TP40013702-SW7
+ redZoneAdjustment = -128;
+#endif
+#endif // OS(DARWIN)
+ return redZoneAdjustment;
+}
+
</ins><span class="cx"> std::pair<void*, size_t> MachineThreads::Thread::captureStack(void* stackTop)
</span><span class="cx"> {
</span><del>- void* begin = stackBase;
- void* end = reinterpret_cast<void*>(
- WTF::roundUpToMultipleOf<sizeof(void*)>(reinterpret_cast<uintptr_t>(stackTop)));
- if (begin > end)
- std::swap(begin, end);
- return std::make_pair(begin, static_cast<char*>(end) - static_cast<char*>(begin));
</del><ins>+ char* begin = reinterpret_cast_ptr<char*>(stackBase);
+ char* end = reinterpret_cast_ptr<char*>(WTF::roundUpToMultipleOf<sizeof(void*)>(reinterpret_cast<uintptr_t>(stackTop)));
+ ASSERT(begin >= end);
+
+ char* endWithRedZone = end + osRedZoneAdjustment();
+ ASSERT(WTF::roundUpToMultipleOf<sizeof(void*)>(reinterpret_cast<uintptr_t>(endWithRedZone)) == reinterpret_cast<uintptr_t>(endWithRedZone));
+
+ if (endWithRedZone < stackEnd)
+ endWithRedZone = reinterpret_cast_ptr<char*>(stackEnd);
+
+ std::swap(begin, endWithRedZone);
+ return std::make_pair(begin, endWithRedZone - begin);
</ins><span class="cx"> }
</span><span class="cx">
</span><span class="cx"> SUPPRESS_ASAN
</span></span></pre></div>
<a id="trunkSourceWTFChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/Source/WTF/ChangeLog (189516 => 189517)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WTF/ChangeLog 2015-09-09 00:07:51 UTC (rev 189516)
+++ trunk/Source/WTF/ChangeLog 2015-09-09 00:19:15 UTC (rev 189517)
</span><span class="lines">@@ -1,3 +1,15 @@
</span><ins>+2015-09-08 Mark Lam <mark.lam@apple.com>
+
+ GC stack scan should include ABI red zone.
+ https://bugs.webkit.org/show_bug.cgi?id=148976
+
+ Reviewed by Geoffrey Garen and Benjamin Poulain.
+
+ * wtf/StackBounds.h:
+ (WTF::StackBounds::origin):
+ (WTF::StackBounds::end):
+ (WTF::StackBounds::size):
+
</ins><span class="cx"> 2015-09-04 Csaba Osztrogonác <ossy@webkit.org>
</span><span class="cx">
</span><span class="cx"> Enable reference qualified functions for GCC
</span></span></pre></div>
<a id="trunkSourceWTFwtfStackBoundsh"></a>
<div class="modfile"><h4>Modified: trunk/Source/WTF/wtf/StackBounds.h (189516 => 189517)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WTF/wtf/StackBounds.h 2015-09-09 00:07:51 UTC (rev 189516)
+++ trunk/Source/WTF/wtf/StackBounds.h 2015-09-09 00:19:15 UTC (rev 189517)
</span><span class="lines">@@ -54,6 +54,12 @@
</span><span class="cx"> return m_origin;
</span><span class="cx"> }
</span><span class="cx">
</span><ins>+ void* end() const
+ {
+ ASSERT(m_bound);
+ return m_bound;
+ }
+
</ins><span class="cx"> size_t size() const
</span><span class="cx"> {
</span><span class="cx"> if (isGrowingDownward())
</span></span></pre>
</div>
</div>
</body>
</html>