<!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>[182421] releases/WebKitGTK/webkit-2.6/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/182421">182421</a></dd>
<dt>Author</dt> <dd>carlosgc@webkit.org</dd>
<dt>Date</dt> <dd>2015-04-06 08:23:54 -0700 (Mon, 06 Apr 2015)</dd>
</dl>
<h3>Log Message</h3>
<pre>Merge <a href="http://trac.webkit.org/projects/webkit/changeset/181835">r181835</a> - DFG OSR exit shouldn't assume that the frame count for exit is greater than the frame count in DFG
https://bugs.webkit.org/show_bug.cgi?id=142948
Reviewed by Sam Weinig.
It's necessary to ensure that the stack pointer accounts for the extent of our stack usage
since a signal may clobber the area below the stack pointer. When the DFG is executing,
the stack pointer accounts for the DFG's worst-case stack usage. When we OSR exit back to
baseline, we will use a different amount of stack. This is because baseline is a different
compiler. It will make different decisions. So it will use a different amount of stack.
This gets tricky when we are in the process of doing an OSR exit, because we are sort of
incrementally transforming the stack from how it looked in the DFG to how it will look in
baseline. The most conservative approach would be to set the stack pointer to the max of
DFG and baseline.
When this code was written, a reckless assumption was made: that the stack usage in
baseline is always at least as large as the stack usage in DFG. Based on this incorrect
assumption, the code first adjusts the stack pointer to account for the baseline stack
usage. This sort of usually works, because usually baseline does happen to use more stack.
But that's not an invariant. Nobody guarantees this. We will never make any changes that
would make this be guaranteed, because that would be antithetical to how optimizing
compilers work. The DFG should be allowed to use however much stack it decides that it
should use in order to get good performance, and it shouldn't try to guarantee that it
always uses less stack than baseline.
As such, we must always assume that the frame size for DFG execution (i.e.
frameRegisterCount) and the frame size in baseline once we exit (i.e.
requiredRegisterCountForExit) are two independent quantities and they have no
relationship.
Fortunately, though, this code can be made correct by just moving the stack adjustment to
just before we do conversions. This is because we have since changed the OSR exit
algorithm to first lift up all state from the DFG state into a scratch buffer, and then to
drop it out of the scratch buffer and into the stack according to the baseline layout. The
point just before conversions is the point where we have finished reading the DFG frame
and will not read it anymore, and we haven't started writing the baseline frame. So, at
this point it is safe to set the stack pointer to account for the frame size at exit.
This is benign because baseline happens to create larger frames than DFG.
* dfg/DFGOSRExitCompiler32_64.cpp:
(JSC::DFG::OSRExitCompiler::compileExit):
* dfg/DFGOSRExitCompiler64.cpp:
(JSC::DFG::OSRExitCompiler::compileExit):
* dfg/DFGOSRExitCompilerCommon.cpp:
(JSC::DFG::adjustAndJumpToTarget):</pre>
<h3>Modified Paths</h3>
<ul>
<li><a href="#releasesWebKitGTKwebkit26SourceJavaScriptCoreChangeLog">releases/WebKitGTK/webkit-2.6/Source/JavaScriptCore/ChangeLog</a></li>
<li><a href="#releasesWebKitGTKwebkit26SourceJavaScriptCoredfgDFGOSRExitCompiler32_64cpp">releases/WebKitGTK/webkit-2.6/Source/JavaScriptCore/dfg/DFGOSRExitCompiler32_64.cpp</a></li>
<li><a href="#releasesWebKitGTKwebkit26SourceJavaScriptCoredfgDFGOSRExitCompiler64cpp">releases/WebKitGTK/webkit-2.6/Source/JavaScriptCore/dfg/DFGOSRExitCompiler64.cpp</a></li>
<li><a href="#releasesWebKitGTKwebkit26SourceJavaScriptCoredfgDFGOSRExitCompilerCommoncpp">releases/WebKitGTK/webkit-2.6/Source/JavaScriptCore/dfg/DFGOSRExitCompilerCommon.cpp</a></li>
</ul>
</div>
<div id="patch">
<h3>Diff</h3>
<a id="releasesWebKitGTKwebkit26SourceJavaScriptCoreChangeLog"></a>
<div class="modfile"><h4>Modified: releases/WebKitGTK/webkit-2.6/Source/JavaScriptCore/ChangeLog (182420 => 182421)</h4>
<pre class="diff"><span>
<span class="info">--- releases/WebKitGTK/webkit-2.6/Source/JavaScriptCore/ChangeLog 2015-04-06 15:19:54 UTC (rev 182420)
+++ releases/WebKitGTK/webkit-2.6/Source/JavaScriptCore/ChangeLog 2015-04-06 15:23:54 UTC (rev 182421)
</span><span class="lines">@@ -1,3 +1,53 @@
</span><ins>+2015-03-22 Filip Pizlo <fpizlo@apple.com>
+
+ DFG OSR exit shouldn't assume that the frame count for exit is greater than the frame count in DFG
+ https://bugs.webkit.org/show_bug.cgi?id=142948
+
+ Reviewed by Sam Weinig.
+
+ It's necessary to ensure that the stack pointer accounts for the extent of our stack usage
+ since a signal may clobber the area below the stack pointer. When the DFG is executing,
+ the stack pointer accounts for the DFG's worst-case stack usage. When we OSR exit back to
+ baseline, we will use a different amount of stack. This is because baseline is a different
+ compiler. It will make different decisions. So it will use a different amount of stack.
+
+ This gets tricky when we are in the process of doing an OSR exit, because we are sort of
+ incrementally transforming the stack from how it looked in the DFG to how it will look in
+ baseline. The most conservative approach would be to set the stack pointer to the max of
+ DFG and baseline.
+
+ When this code was written, a reckless assumption was made: that the stack usage in
+ baseline is always at least as large as the stack usage in DFG. Based on this incorrect
+ assumption, the code first adjusts the stack pointer to account for the baseline stack
+ usage. This sort of usually works, because usually baseline does happen to use more stack.
+ But that's not an invariant. Nobody guarantees this. We will never make any changes that
+ would make this be guaranteed, because that would be antithetical to how optimizing
+ compilers work. The DFG should be allowed to use however much stack it decides that it
+ should use in order to get good performance, and it shouldn't try to guarantee that it
+ always uses less stack than baseline.
+
+ As such, we must always assume that the frame size for DFG execution (i.e.
+ frameRegisterCount) and the frame size in baseline once we exit (i.e.
+ requiredRegisterCountForExit) are two independent quantities and they have no
+ relationship.
+
+ Fortunately, though, this code can be made correct by just moving the stack adjustment to
+ just before we do conversions. This is because we have since changed the OSR exit
+ algorithm to first lift up all state from the DFG state into a scratch buffer, and then to
+ drop it out of the scratch buffer and into the stack according to the baseline layout. The
+ point just before conversions is the point where we have finished reading the DFG frame
+ and will not read it anymore, and we haven't started writing the baseline frame. So, at
+ this point it is safe to set the stack pointer to account for the frame size at exit.
+
+ This is benign because baseline happens to create larger frames than DFG.
+
+ * dfg/DFGOSRExitCompiler32_64.cpp:
+ (JSC::DFG::OSRExitCompiler::compileExit):
+ * dfg/DFGOSRExitCompiler64.cpp:
+ (JSC::DFG::OSRExitCompiler::compileExit):
+ * dfg/DFGOSRExitCompilerCommon.cpp:
+ (JSC::DFG::adjustAndJumpToTarget):
+
</ins><span class="cx"> 2015-03-09 Mark Lam <mark.lam@apple.com>
</span><span class="cx">
</span><span class="cx"> 8-bit version of weakCompareAndSwap() can cause an infinite loop.
</span></span></pre></div>
<a id="releasesWebKitGTKwebkit26SourceJavaScriptCoredfgDFGOSRExitCompiler32_64cpp"></a>
<div class="modfile"><h4>Modified: releases/WebKitGTK/webkit-2.6/Source/JavaScriptCore/dfg/DFGOSRExitCompiler32_64.cpp (182420 => 182421)</h4>
<pre class="diff"><span>
<span class="info">--- releases/WebKitGTK/webkit-2.6/Source/JavaScriptCore/dfg/DFGOSRExitCompiler32_64.cpp 2015-04-06 15:19:54 UTC (rev 182420)
+++ releases/WebKitGTK/webkit-2.6/Source/JavaScriptCore/dfg/DFGOSRExitCompiler32_64.cpp 2015-04-06 15:23:54 UTC (rev 182421)
</span><span class="lines">@@ -48,12 +48,6 @@
</span><span class="cx"> m_jit.debugCall(debugOperationPrintSpeculationFailure, debugInfo);
</span><span class="cx"> }
</span><span class="cx">
</span><del>- // Need to ensure that the stack pointer accounts for the worst-case stack usage at exit.
- m_jit.addPtr(
- CCallHelpers::TrustedImm32(
- -m_jit.codeBlock()->jitCode()->dfgCommon()->requiredRegisterCountForExit * sizeof(Register)),
- CCallHelpers::framePointerRegister, CCallHelpers::stackPointerRegister);
-
</del><span class="cx"> // 2) Perform speculation recovery. This only comes into play when an operation
</span><span class="cx"> // starts mutating state before verifying the speculation it has already made.
</span><span class="cx">
</span><span class="lines">@@ -259,6 +253,14 @@
</span><span class="cx"> }
</span><span class="cx"> }
</span><span class="cx">
</span><ins>+ // Need to ensure that the stack pointer accounts for the worst-case stack usage at exit. This
+ // could toast some stack that the DFG used. We need to do it before storing to stack offsets
+ // used by baseline.
+ m_jit.addPtr(
+ CCallHelpers::TrustedImm32(
+ -m_jit.codeBlock()->jitCode()->dfgCommon()->requiredRegisterCountForExit * sizeof(Register)),
+ CCallHelpers::framePointerRegister, CCallHelpers::stackPointerRegister);
+
</ins><span class="cx"> // 7) Do all data format conversions and store the results into the stack.
</span><span class="cx">
</span><span class="cx"> bool haveArguments = false;
</span></span></pre></div>
<a id="releasesWebKitGTKwebkit26SourceJavaScriptCoredfgDFGOSRExitCompiler64cpp"></a>
<div class="modfile"><h4>Modified: releases/WebKitGTK/webkit-2.6/Source/JavaScriptCore/dfg/DFGOSRExitCompiler64.cpp (182420 => 182421)</h4>
<pre class="diff"><span>
<span class="info">--- releases/WebKitGTK/webkit-2.6/Source/JavaScriptCore/dfg/DFGOSRExitCompiler64.cpp 2015-04-06 15:19:54 UTC (rev 182420)
+++ releases/WebKitGTK/webkit-2.6/Source/JavaScriptCore/dfg/DFGOSRExitCompiler64.cpp 2015-04-06 15:23:54 UTC (rev 182421)
</span><span class="lines">@@ -1,5 +1,5 @@
</span><span class="cx"> /*
</span><del>- * Copyright (C) 2011, 2013, 2014 Apple Inc. All rights reserved.
</del><ins>+ * Copyright (C) 2011, 2013-2015 Apple Inc. All rights reserved.
</ins><span class="cx"> *
</span><span class="cx"> * Redistribution and use in source and binary forms, with or without
</span><span class="cx"> * modification, are permitted provided that the following conditions
</span><span class="lines">@@ -52,12 +52,6 @@
</span><span class="cx"> m_jit.debugCall(debugOperationPrintSpeculationFailure, debugInfo);
</span><span class="cx"> }
</span><span class="cx">
</span><del>- // Need to ensure that the stack pointer accounts for the worst-case stack usage at exit.
- m_jit.addPtr(
- CCallHelpers::TrustedImm32(
- -m_jit.codeBlock()->jitCode()->dfgCommon()->requiredRegisterCountForExit * sizeof(Register)),
- CCallHelpers::framePointerRegister, CCallHelpers::stackPointerRegister);
-
</del><span class="cx"> // 2) Perform speculation recovery. This only comes into play when an operation
</span><span class="cx"> // starts mutating state before verifying the speculation it has already made.
</span><span class="cx">
</span><span class="lines">@@ -251,6 +245,14 @@
</span><span class="cx"> }
</span><span class="cx"> }
</span><span class="cx">
</span><ins>+ // Need to ensure that the stack pointer accounts for the worst-case stack usage at exit. This
+ // could toast some stack that the DFG used. We need to do it before storing to stack offsets
+ // used by baseline.
+ m_jit.addPtr(
+ CCallHelpers::TrustedImm32(
+ -m_jit.codeBlock()->jitCode()->dfgCommon()->requiredRegisterCountForExit * sizeof(Register)),
+ CCallHelpers::framePointerRegister, CCallHelpers::stackPointerRegister);
+
</ins><span class="cx"> // 7) Do all data format conversions and store the results into the stack.
</span><span class="cx">
</span><span class="cx"> bool haveArguments = false;
</span></span></pre></div>
<a id="releasesWebKitGTKwebkit26SourceJavaScriptCoredfgDFGOSRExitCompilerCommoncpp"></a>
<div class="modfile"><h4>Modified: releases/WebKitGTK/webkit-2.6/Source/JavaScriptCore/dfg/DFGOSRExitCompilerCommon.cpp (182420 => 182421)</h4>
<pre class="diff"><span>
<span class="info">--- releases/WebKitGTK/webkit-2.6/Source/JavaScriptCore/dfg/DFGOSRExitCompilerCommon.cpp 2015-04-06 15:19:54 UTC (rev 182420)
+++ releases/WebKitGTK/webkit-2.6/Source/JavaScriptCore/dfg/DFGOSRExitCompilerCommon.cpp 2015-04-06 15:23:54 UTC (rev 182421)
</span><span class="lines">@@ -270,7 +270,6 @@
</span><span class="cx"> void adjustAndJumpToTarget(CCallHelpers& jit, const OSRExitBase& exit)
</span><span class="cx"> {
</span><span class="cx"> #if ENABLE(GGC)
</span><del>- // 11) Write barrier the owner executables because we're jumping into a different block.
</del><span class="cx"> jit.move(AssemblyHelpers::TrustedImmPtr(jit.codeBlock()->ownerExecutable()), GPRInfo::nonArgGPR0);
</span><span class="cx"> osrWriteBarrier(jit, GPRInfo::nonArgGPR0, GPRInfo::nonArgGPR1);
</span><span class="cx"> InlineCallFrameSet* inlineCallFrames = jit.codeBlock()->jitCode()->dfgCommon()->inlineCallFrames.get();
</span></span></pre>
</div>
</div>
</body>
</html>