<!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>[192352] 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/192352">192352</a></dd>
<dt>Author</dt> <dd>mark.lam@apple.com</dd>
<dt>Date</dt> <dd>2015-11-11 22:59:13 -0800 (Wed, 11 Nov 2015)</dd>
</dl>
<h3>Log Message</h3>
<pre>Crash in sorting-benchmark.js on ARM64 with full op_sub FTL coverage.
https://bugs.webkit.org/show_bug.cgi?id=150936
Reviewed by Michael Saboff.
The OSR entry thunk does not currently restore the callee saved registers that the baseline
JIT saves but the DFG does not. As a result, sorting-benchmark.js was crashing with the
following scenario:
1. There exists 2 functions: benchmark() and bottom_up_merge_sort().
Let's call them A() and B() respectively for brevity.
2. A() is FTL compiled.
3. B() is FTL compiled.
4. FTL A() calls FTL B(). FTL B() trashes register x26.
5. FTL B() goes through an OSR exit, and deopts to baseline. The OSR exit off-ramp
puts x26's original value in the baseline B()'s stash location for x26 in its stack
frame. It expects baseline B() to restore x26 when it returns.
6. Baseline B() gets DFG optimized, and we OSR enter into DFG B().
The OSR entry thunk does nothing to restore x26's original value. Hence, x26 contains
whatever value FTL B() left in it.
7. DFG B() returns to FTL A().
x26 is not one of the callee saved registers used by DFG B(). Hence, it does nothing
to restore it.
8. FTL A() tries to use x26 and crashes, because x26 contains FTL B()'s value for it, and
not FTL A()'s.
This patch fixes this issue by having the OSR entry thunk restore all the callee saved
registers before running the DFG code.
No new test needed because this issue will be covered by sorting-benchmark.js as soon as
https://bugs.webkit.org/show_bug.cgi?id=150712 lands.
* dfg/DFGThunks.cpp:
(JSC::DFG::osrEntryThunkGenerator):</pre>
<h3>Modified Paths</h3>
<ul>
<li><a href="#trunkSourceJavaScriptCoreChangeLog">trunk/Source/JavaScriptCore/ChangeLog</a></li>
<li><a href="#trunkSourceJavaScriptCoredfgDFGThunkscpp">trunk/Source/JavaScriptCore/dfg/DFGThunks.cpp</a></li>
</ul>
</div>
<div id="patch">
<h3>Diff</h3>
<a id="trunkSourceJavaScriptCoreChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/ChangeLog (192351 => 192352)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/ChangeLog 2015-11-12 06:33:53 UTC (rev 192351)
+++ trunk/Source/JavaScriptCore/ChangeLog 2015-11-12 06:59:13 UTC (rev 192352)
</span><span class="lines">@@ -1,3 +1,40 @@
</span><ins>+2015-11-11 Mark Lam <mark.lam@apple.com>
+
+ Crash in sorting-benchmark.js on ARM64 with full op_sub FTL coverage.
+ https://bugs.webkit.org/show_bug.cgi?id=150936
+
+ Reviewed by Michael Saboff.
+
+ The OSR entry thunk does not currently restore the callee saved registers that the baseline
+ JIT saves but the DFG does not. As a result, sorting-benchmark.js was crashing with the
+ following scenario:
+
+ 1. There exists 2 functions: benchmark() and bottom_up_merge_sort().
+ Let's call them A() and B() respectively for brevity.
+ 2. A() is FTL compiled.
+ 3. B() is FTL compiled.
+ 4. FTL A() calls FTL B(). FTL B() trashes register x26.
+ 5. FTL B() goes through an OSR exit, and deopts to baseline. The OSR exit off-ramp
+ puts x26's original value in the baseline B()'s stash location for x26 in its stack
+ frame. It expects baseline B() to restore x26 when it returns.
+ 6. Baseline B() gets DFG optimized, and we OSR enter into DFG B().
+ The OSR entry thunk does nothing to restore x26's original value. Hence, x26 contains
+ whatever value FTL B() left in it.
+ 7. DFG B() returns to FTL A().
+ x26 is not one of the callee saved registers used by DFG B(). Hence, it does nothing
+ to restore it.
+ 8. FTL A() tries to use x26 and crashes, because x26 contains FTL B()'s value for it, and
+ not FTL A()'s.
+
+ This patch fixes this issue by having the OSR entry thunk restore all the callee saved
+ registers before running the DFG code.
+
+ No new test needed because this issue will be covered by sorting-benchmark.js as soon as
+ https://bugs.webkit.org/show_bug.cgi?id=150712 lands.
+
+ * dfg/DFGThunks.cpp:
+ (JSC::DFG::osrEntryThunkGenerator):
+
</ins><span class="cx"> 2015-11-11 Benjamin Poulain <bpoulain@apple.com>
</span><span class="cx">
</span><span class="cx"> [JSC] Add a comment explaining the opcode suffixes on x86
</span></span></pre></div>
<a id="trunkSourceJavaScriptCoredfgDFGThunkscpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/dfg/DFGThunks.cpp (192351 => 192352)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/dfg/DFGThunks.cpp 2015-11-12 06:33:53 UTC (rev 192351)
+++ trunk/Source/JavaScriptCore/dfg/DFGThunks.cpp 2015-11-12 06:59:13 UTC (rev 192352)
</span><span class="lines">@@ -97,8 +97,8 @@
</span><span class="cx">
</span><span class="cx"> MacroAssemblerCodeRef osrEntryThunkGenerator(VM* vm)
</span><span class="cx"> {
</span><del>- MacroAssembler jit;
-
</del><ins>+ AssemblyHelpers jit(vm, nullptr);
+
</ins><span class="cx"> // We get passed the address of a scratch buffer. The first 8-byte slot of the buffer
</span><span class="cx"> // is the frame size. The second 8-byte slot is the pointer to where we are supposed to
</span><span class="cx"> // jump. The remaining bytes are the new call frame header followed by the locals.
</span><span class="lines">@@ -128,7 +128,12 @@
</span><span class="cx"> jit.loadPtr(MacroAssembler::Address(GPRInfo::regT0, offsetOfTargetPC), GPRInfo::regT1);
</span><span class="cx"> MacroAssembler::Jump ok = jit.branchPtr(MacroAssembler::Above, GPRInfo::regT1, MacroAssembler::TrustedImmPtr(bitwise_cast<void*>(static_cast<intptr_t>(1000))));
</span><span class="cx"> jit.abortWithReason(DFGUnreasonableOSREntryJumpDestination);
</span><ins>+
</ins><span class="cx"> ok.link(&jit);
</span><ins>+ RegisterSet usedRegisters(RegisterSet::stubUnavailableRegisters(), GPRInfo::regT1);
+ jit.restoreCalleeSavesFromVMCalleeSavesBuffer(usedRegisters);
+ jit.emitMaterializeTagCheckRegisters();
+
</ins><span class="cx"> jit.jump(GPRInfo::regT1);
</span><span class="cx">
</span><span class="cx"> LinkBuffer patchBuffer(*vm, jit, GLOBAL_THUNK_ID);
</span></span></pre>
</div>
</div>
</body>
</html>