<!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>[199406] releases/WebKitGTK/webkit-2.12/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/199406">199406</a></dd>
<dt>Author</dt> <dd>carlosgc@webkit.org</dd>
<dt>Date</dt> <dd>2016-04-12 23:34:46 -0700 (Tue, 12 Apr 2016)</dd>
</dl>

<h3>Log Message</h3>
<pre>Merge <a href="http://trac.webkit.org/projects/webkit/changeset/198296">r198296</a> - ASSERTION FAILED: !edge-&gt;isPhantomAllocation() in regress/script-tests/sink-huge-activation.js.ftl-eager in debug mode
https://bugs.webkit.org/show_bug.cgi?id=153805

Reviewed by Mark Lam.

The object allocation sinking phase uses InferredValue::isStillValid() in the opposite
way from most clients: it will do an *extra* optimization if it returns false. The
phase will first compute sink candidates and then it will compute materialization
points. If something is a sink candidate then it is not a materialization point. A
NewFunction node may appear as not being a sink candidate during the first pass, so it's
not added to the set of things that will turn into PhantomNewFunction. But on the second
pass where we add materializations, we check isStillValid() again. Now this may become
false, so that second pass thinks that NewFunction is a sink candidate (even though it's
not in the sink candidates set) and so is not a materialization point.

This manifests as the NewFunction referring to a PhantomCreateActivation or whatever.

The solution is to have the phase cache results of calls to isStillValid(). It's OK if
we just remember the result of the first call and assume that it's not a sink candidate.
That's the worst that can happen.

No new tests since this is a super hard race and sink-huge-activation seemed to already
be catching it.

* dfg/DFGObjectAllocationSinkingPhase.cpp:</pre>

<h3>Modified Paths</h3>
<ul>
<li><a href="#releasesWebKitGTKwebkit212SourceJavaScriptCoreChangeLog">releases/WebKitGTK/webkit-2.12/Source/JavaScriptCore/ChangeLog</a></li>
<li><a href="#releasesWebKitGTKwebkit212SourceJavaScriptCoredfgDFGObjectAllocationSinkingPhasecpp">releases/WebKitGTK/webkit-2.12/Source/JavaScriptCore/dfg/DFGObjectAllocationSinkingPhase.cpp</a></li>
</ul>

</div>
<div id="patch">
<h3>Diff</h3>
<a id="releasesWebKitGTKwebkit212SourceJavaScriptCoreChangeLog"></a>
<div class="modfile"><h4>Modified: releases/WebKitGTK/webkit-2.12/Source/JavaScriptCore/ChangeLog (199405 => 199406)</h4>
<pre class="diff"><span>
<span class="info">--- releases/WebKitGTK/webkit-2.12/Source/JavaScriptCore/ChangeLog        2016-04-13 06:26:15 UTC (rev 199405)
+++ releases/WebKitGTK/webkit-2.12/Source/JavaScriptCore/ChangeLog        2016-04-13 06:34:46 UTC (rev 199406)
</span><span class="lines">@@ -1,3 +1,31 @@
</span><ins>+2016-03-15  Filip Pizlo  &lt;fpizlo@apple.com&gt;
+
+        ASSERTION FAILED: !edge-&gt;isPhantomAllocation() in regress/script-tests/sink-huge-activation.js.ftl-eager in debug mode
+        https://bugs.webkit.org/show_bug.cgi?id=153805
+
+        Reviewed by Mark Lam.
+
+        The object allocation sinking phase uses InferredValue::isStillValid() in the opposite
+        way from most clients: it will do an *extra* optimization if it returns false. The
+        phase will first compute sink candidates and then it will compute materialization
+        points. If something is a sink candidate then it is not a materialization point. A
+        NewFunction node may appear as not being a sink candidate during the first pass, so it's
+        not added to the set of things that will turn into PhantomNewFunction. But on the second
+        pass where we add materializations, we check isStillValid() again. Now this may become
+        false, so that second pass thinks that NewFunction is a sink candidate (even though it's
+        not in the sink candidates set) and so is not a materialization point.
+
+        This manifests as the NewFunction referring to a PhantomCreateActivation or whatever.
+
+        The solution is to have the phase cache results of calls to isStillValid(). It's OK if
+        we just remember the result of the first call and assume that it's not a sink candidate.
+        That's the worst that can happen.
+
+        No new tests since this is a super hard race and sink-huge-activation seemed to already
+        be catching it.
+
+        * dfg/DFGObjectAllocationSinkingPhase.cpp:
+
</ins><span class="cx"> 2016-03-14  Julien Brianceau  &lt;jbriance@cisco.com&gt;
</span><span class="cx"> 
</span><span class="cx">         [mips] Fix unaligned access in LLINT.
</span></span></pre></div>
<a id="releasesWebKitGTKwebkit212SourceJavaScriptCoredfgDFGObjectAllocationSinkingPhasecpp"></a>
<div class="modfile"><h4>Modified: releases/WebKitGTK/webkit-2.12/Source/JavaScriptCore/dfg/DFGObjectAllocationSinkingPhase.cpp (199405 => 199406)</h4>
<pre class="diff"><span>
<span class="info">--- releases/WebKitGTK/webkit-2.12/Source/JavaScriptCore/dfg/DFGObjectAllocationSinkingPhase.cpp        2016-04-13 06:26:15 UTC (rev 199405)
+++ releases/WebKitGTK/webkit-2.12/Source/JavaScriptCore/dfg/DFGObjectAllocationSinkingPhase.cpp        2016-04-13 06:34:46 UTC (rev 199406)
</span><span class="lines">@@ -838,7 +838,7 @@
</span><span class="cx">         case NewFunction:
</span><span class="cx">         case NewArrowFunction:
</span><span class="cx">         case NewGeneratorFunction: {
</span><del>-            if (node-&gt;castOperand&lt;FunctionExecutable*&gt;()-&gt;singletonFunction()-&gt;isStillValid()) {
</del><ins>+            if (isStillValid(node-&gt;castOperand&lt;FunctionExecutable*&gt;()-&gt;singletonFunction())) {
</ins><span class="cx">                 m_heap.escape(node-&gt;child1().node());
</span><span class="cx">                 break;
</span><span class="cx">             }
</span><span class="lines">@@ -855,7 +855,7 @@
</span><span class="cx">         }
</span><span class="cx"> 
</span><span class="cx">         case CreateActivation: {
</span><del>-            if (node-&gt;castOperand&lt;SymbolTable*&gt;()-&gt;singletonScope()-&gt;isStillValid()) {
</del><ins>+            if (isStillValid(node-&gt;castOperand&lt;SymbolTable*&gt;()-&gt;singletonScope())) {
</ins><span class="cx">                 m_heap.escape(node-&gt;child1().node());
</span><span class="cx">                 break;
</span><span class="cx">             }
</span><span class="lines">@@ -2173,6 +2173,15 @@
</span><span class="cx">         }
</span><span class="cx">     }
</span><span class="cx"> 
</span><ins>+    // This is a great way of asking value-&gt;isStillValid() without having to worry about getting
+    // different answers. It turns out that this analysis works OK regardless of what this
+    // returns but breaks badly if this changes its mind for any particular InferredValue. This
+    // method protects us from that.
+    bool isStillValid(InferredValue* value)
+    {
+        return m_validInferredValues.add(value, value-&gt;isStillValid()).iterator-&gt;value;
+    }
+
</ins><span class="cx">     SSACalculator m_pointerSSA;
</span><span class="cx">     SSACalculator m_allocationSSA;
</span><span class="cx">     HashSet&lt;Node*&gt; m_sinkCandidates;
</span><span class="lines">@@ -2183,6 +2192,8 @@
</span><span class="cx">     InsertionSet m_insertionSet;
</span><span class="cx">     CombinedLiveness m_combinedLiveness;
</span><span class="cx"> 
</span><ins>+    HashMap&lt;InferredValue*, bool&gt; m_validInferredValues;
+
</ins><span class="cx">     HashMap&lt;Node*, Node*&gt; m_materializationToEscapee;
</span><span class="cx">     HashMap&lt;Node*, Vector&lt;Node*&gt;&gt; m_materializationSiteToMaterializations;
</span><span class="cx">     HashMap&lt;Node*, Vector&lt;PromotedHeapLocation&gt;&gt; m_materializationSiteToRecoveries;
</span></span></pre>
</div>
</div>

</body>
</html>