<!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>[199783] 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/199783">199783</a></dd>
<dt>Author</dt> <dd>msaboff@apple.com</dd>
<dt>Date</dt> <dd>2016-04-20 13:31:21 -0700 (Wed, 20 Apr 2016)</dd>
</dl>

<h3>Log Message</h3>
<pre>REGRESSION(<a href="http://trac.webkit.org/projects/webkit/changeset/190289">r190289</a>): Spin trying to view/sign in to hbogo.com
https://bugs.webkit.org/show_bug.cgi?id=156765

Reviewed by Saam Barati.

In the op_get_by_val case, we were holding the lock on a profiled CodeBlock
when we call into handleGetById(). Changed to drop the lock before calling
handleGetById().

The bug here was that the call to handleGetById() may end up calling in to
getPredictionWithoutOSRExit() for a tail call opcode. As part of that
processing, we walk back up the stack to find the effective caller and when
found, we lock the corresponding CodeBlock to get the predicition.
That CodeBLock may be the same one locked above. There is no need anyway
to hold the CodeBlock lock when calling handleGetById().

Added a new stress test.

* dfg/DFGByteCodeParser.cpp:
(JSC::DFG::ByteCodeParser::parseBlock):
* tests/stress/regress-156765.js: Added.
(realValue):
(object.get hello):
(ok):</pre>

<h3>Modified Paths</h3>
<ul>
<li><a href="#trunkSourceJavaScriptCoreChangeLog">trunk/Source/JavaScriptCore/ChangeLog</a></li>
<li><a href="#trunkSourceJavaScriptCoredfgDFGByteCodeParsercpp">trunk/Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp</a></li>
</ul>

<h3>Added Paths</h3>
<ul>
<li><a href="#trunkSourceJavaScriptCoretestsstressregress156765js">trunk/Source/JavaScriptCore/tests/stress/regress-156765.js</a></li>
</ul>

</div>
<div id="patch">
<h3>Diff</h3>
<a id="trunkSourceJavaScriptCoreChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/ChangeLog (199782 => 199783)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/ChangeLog        2016-04-20 20:21:21 UTC (rev 199782)
+++ trunk/Source/JavaScriptCore/ChangeLog        2016-04-20 20:31:21 UTC (rev 199783)
</span><span class="lines">@@ -1,3 +1,30 @@
</span><ins>+2016-04-20  Michael Saboff  &lt;msaboff@apple.com&gt;
+
+        REGRESSION(r190289): Spin trying to view/sign in to hbogo.com
+        https://bugs.webkit.org/show_bug.cgi?id=156765
+
+        Reviewed by Saam Barati.
+
+        In the op_get_by_val case, we were holding the lock on a profiled CodeBlock
+        when we call into handleGetById(). Changed to drop the lock before calling
+        handleGetById().
+
+        The bug here was that the call to handleGetById() may end up calling in to
+        getPredictionWithoutOSRExit() for a tail call opcode. As part of that
+        processing, we walk back up the stack to find the effective caller and when
+        found, we lock the corresponding CodeBlock to get the predicition.
+        That CodeBLock may be the same one locked above. There is no need anyway
+        to hold the CodeBlock lock when calling handleGetById().
+
+        Added a new stress test.
+
+        * dfg/DFGByteCodeParser.cpp:
+        (JSC::DFG::ByteCodeParser::parseBlock):
+        * tests/stress/regress-156765.js: Added.
+        (realValue):
+        (object.get hello):
+        (ok):
+
</ins><span class="cx"> 2016-04-20  Mark Lam  &lt;mark.lam@apple.com&gt;
</span><span class="cx"> 
</span><span class="cx">         Unindent an unnecessary block in stringProtoFuncSplitFast().
</span></span></pre></div>
<a id="trunkSourceJavaScriptCoredfgDFGByteCodeParsercpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp (199782 => 199783)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp        2016-04-20 20:21:21 UTC (rev 199782)
+++ trunk/Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp        2016-04-20 20:31:21 UTC (rev 199783)
</span><span class="lines">@@ -3888,6 +3888,8 @@
</span><span class="cx">             Node* base = get(VirtualRegister(currentInstruction[2].u.operand));
</span><span class="cx">             Node* property = get(VirtualRegister(currentInstruction[3].u.operand));
</span><span class="cx">             bool compiledAsGetById = false;
</span><ins>+            GetByIdStatus getByIdStatus;
+            unsigned identifierNumber = 0;
</ins><span class="cx">             {
</span><span class="cx">                 ConcurrentJITLocker locker(m_inlineStackTop-&gt;m_profiledBlock-&gt;m_lock);
</span><span class="cx">                 ByValInfo* byValInfo = m_inlineStackTop-&gt;m_byValInfos.get(CodeOrigin(currentCodeOrigin().bytecodeIndex));
</span><span class="lines">@@ -3895,20 +3897,20 @@
</span><span class="cx">                 // At that time, there is no information.
</span><span class="cx">                 if (byValInfo &amp;&amp; byValInfo-&gt;stubInfo &amp;&amp; !byValInfo-&gt;tookSlowPath &amp;&amp; !m_inlineStackTop-&gt;m_exitProfile.hasExitSite(m_currentIndex, BadIdent)) {
</span><span class="cx">                     compiledAsGetById = true;
</span><del>-                    unsigned identifierNumber = m_graph.identifiers().ensure(byValInfo-&gt;cachedId.impl());
</del><ins>+                    identifierNumber = m_graph.identifiers().ensure(byValInfo-&gt;cachedId.impl());
</ins><span class="cx">                     UniquedStringImpl* uid = m_graph.identifiers()[identifierNumber];
</span><span class="cx"> 
</span><span class="cx">                     addToGraph(CheckIdent, OpInfo(uid), property);
</span><span class="cx"> 
</span><del>-                    GetByIdStatus getByIdStatus = GetByIdStatus::computeForStubInfo(
</del><ins>+                    getByIdStatus = GetByIdStatus::computeForStubInfo(
</ins><span class="cx">                         locker, m_inlineStackTop-&gt;m_profiledBlock,
</span><span class="cx">                         byValInfo-&gt;stubInfo, currentCodeOrigin(), uid);
</span><del>-
-                    handleGetById(currentInstruction[1].u.operand, prediction, base, identifierNumber, getByIdStatus, AccessType::Get);
</del><span class="cx">                 }
</span><span class="cx">             }
</span><span class="cx"> 
</span><del>-            if (!compiledAsGetById) {
</del><ins>+            if (compiledAsGetById)
+                handleGetById(currentInstruction[1].u.operand, prediction, base, identifierNumber, getByIdStatus, AccessType::Get);
+            else {
</ins><span class="cx">                 ArrayMode arrayMode = getArrayMode(currentInstruction[4].u.arrayProfile, Array::Read);
</span><span class="cx">                 Node* getByVal = addToGraph(GetByVal, OpInfo(arrayMode.asWord()), OpInfo(prediction), base, property);
</span><span class="cx">                 m_exitOK = false; // GetByVal must be treated as if it clobbers exit state, since FixupPhase may make it generic.
</span></span></pre></div>
<a id="trunkSourceJavaScriptCoretestsstressregress156765js"></a>
<div class="addfile"><h4>Added: trunk/Source/JavaScriptCore/tests/stress/regress-156765.js (0 => 199783)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/tests/stress/regress-156765.js                                (rev 0)
+++ trunk/Source/JavaScriptCore/tests/stress/regress-156765.js        2016-04-20 20:31:21 UTC (rev 199783)
</span><span class="lines">@@ -0,0 +1,27 @@
</span><ins>+// Regression test for https://webkit.org/b/156765. This test should not hang.
+
+&quot;use strict&quot;;
+
+let forty = 40;
+
+function realValue()
+{
+    return forty + 2;
+}
+
+var object = {
+    get hello() {
+        return realValue();
+    }
+};
+
+function ok() {
+    var value = 'hello';
+    if (object[value] + 20 !== 62)
+        throw new Error();
+}
+
+noInline(ok);
+
+for (var i = 0; i &lt; 100000; ++i)
+    ok();
</ins></span></pre>
</div>
</div>

</body>
</html>