<!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>[282821] trunk</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/282821">282821</a></dd>
<dt>Author</dt> <dd>justin_michaud@apple.com</dd>
<dt>Date</dt> <dd>2021-09-21 09:06:01 -0700 (Tue, 21 Sep 2021)</dd>
</dl>

<h3>Log Message</h3>
<pre>Differential testing: live statement don't execute
https://bugs.webkit.org/show_bug.cgi?id=229939

Reviewed by Saam Barati.

JSTests:

* stress/in-by-val-should-throw.js: Added.
(doesThrow):
(noInline.doesThrow.noFTL.doesThrow.blackbox):
(noInline.blackbox.doesNotThrow):
(noInline.doesNotThrow.noFTL.doesNotThrow.main):

Source/JavaScriptCore:

In statements are supposed to throw if they are applied to a non-object. We incorrectly converted
InByVals into HasIndexedProperty any time they were a cell, so we silently converted non-objects. Before converting
an InByVal, we first speculate that the base is an object now.

We do not always require an object edge for HasIndexedProperty because enumerator next() does not
throw if it encounters a cell that requires conversion during the call to toObject (for example, a
string literal). That is, we should silently convert the string during enumeration, but not for an
In statement, and so HasIndexedProperty is prepared to handle both cases.

* dfg/DFGFixupPhase.cpp:
(JSC::DFG::FixupPhase::fixupNode):
(JSC::DFG::FixupPhase::convertToHasIndexedProperty):</pre>

<h3>Modified Paths</h3>
<ul>
<li><a href="#trunkJSTestsChangeLog">trunk/JSTests/ChangeLog</a></li>
<li><a href="#trunkSourceJavaScriptCoreChangeLog">trunk/Source/JavaScriptCore/ChangeLog</a></li>
<li><a href="#trunkSourceJavaScriptCoredfgDFGFixupPhasecpp">trunk/Source/JavaScriptCore/dfg/DFGFixupPhase.cpp</a></li>
<li><a href="#trunkSourceJavaScriptCoredfgDFGSpeculativeJITcpp">trunk/Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp</a></li>
<li><a href="#trunkSourceJavaScriptCoreftlFTLLowerDFGToB3cpp">trunk/Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp</a></li>
</ul>

<h3>Added Paths</h3>
<ul>
<li><a href="#trunkJSTestsstressinbyvalshouldthrowjs">trunk/JSTests/stress/in-by-val-should-throw.js</a></li>
</ul>

</div>
<div id="patch">
<h3>Diff</h3>
<a id="trunkJSTestsChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/JSTests/ChangeLog (282820 => 282821)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/JSTests/ChangeLog  2021-09-21 15:55:59 UTC (rev 282820)
+++ trunk/JSTests/ChangeLog     2021-09-21 16:06:01 UTC (rev 282821)
</span><span class="lines">@@ -1,3 +1,16 @@
</span><ins>+2021-09-21  Justin Michaud  <justin_michaud@apple.com>
+
+        Differential testing: live statement don't execute
+        https://bugs.webkit.org/show_bug.cgi?id=229939
+
+        Reviewed by Saam Barati.
+
+        * stress/in-by-val-should-throw.js: Added.
+        (doesThrow):
+        (noInline.doesThrow.noFTL.doesThrow.blackbox):
+        (noInline.blackbox.doesNotThrow):
+        (noInline.doesNotThrow.noFTL.doesNotThrow.main):
+
</ins><span class="cx"> 2021-09-20  Mikhail R. Gadelha  <mikhail@igalia.com>
</span><span class="cx"> 
</span><span class="cx">         Skip stress/json-stringify-stack-overflow.js only on memory limited systems
</span></span></pre></div>
<a id="trunkJSTestsstressinbyvalshouldthrowjs"></a>
<div class="addfile"><h4>Added: trunk/JSTests/stress/in-by-val-should-throw.js (0 => 282821)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/JSTests/stress/in-by-val-should-throw.js                           (rev 0)
+++ trunk/JSTests/stress/in-by-val-should-throw.js      2021-09-21 16:06:01 UTC (rev 282821)
</span><span class="lines">@@ -0,0 +1,85 @@
</span><ins>+function doesThrow() {
+    try {
+        1 in ""
+        return false
+    } catch(v45) {
+        return true
+    }
+}
+noInline(doesThrow)
+noFTL(doesThrow)
+
+function doesThrowFTL() {
+    try {
+        1 in ""
+        return false
+    } catch(v45) {
+        return true
+    }
+}
+noInline(doesThrowFTL)
+
+function blackbox() {
+    return { }
+}
+noInline(blackbox)
+
+function doesNotThrow() {
+    try {
+        1 in blackbox()
+        return false
+    } catch(v45) {
+        return true
+    }
+}
+noInline(doesNotThrow)
+noFTL(doesNotThrow)
+
+function trickster(o) {
+    try {
+        1 in o
+        return false
+    } catch(v45) {
+        return true
+    }
+}
+noInline(trickster)
+
+// Does not throw
+function enumeratorTest(o) {
+    let sum = 0
+    for (let i in o)
+        sum += o[i]
+    return sum
+}
+noInline(enumeratorTest)
+noInline(enumeratorTest)
+
+let indexedObject = []
+indexedObject.length = 10
+indexedObject.fill(1)
+
+function main() {
+    for (let j = 0; j < 50000; j++) {
+        if (!doesThrow())
+            throw new Error("Should throw!")
+        if (!doesThrowFTL())
+            throw new Error("Should throw!")
+        if (doesNotThrow())
+            throw new Error("Should not throw!")
+        
+            let o = {}
+        o["a" + j] = 0
+        if (trickster(o))
+            throw new Error("Should not throw!")
+        
+        enumeratorTest(indexedObject)
+    }
+    if (!trickster(""))
+        throw new Error("Should throw!")
+    enumeratorTest("")
+}
+noDFG(main)
+noFTL(main)
+noInline(main)
+main()
</ins><span class="cx">\ No newline at end of file
</span></span></pre></div>
<a id="trunkSourceJavaScriptCoreChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/ChangeLog (282820 => 282821)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/ChangeLog    2021-09-21 15:55:59 UTC (rev 282820)
+++ trunk/Source/JavaScriptCore/ChangeLog       2021-09-21 16:06:01 UTC (rev 282821)
</span><span class="lines">@@ -1,3 +1,23 @@
</span><ins>+2021-09-21  Justin Michaud  <justin_michaud@apple.com>
+
+        Differential testing: live statement don't execute
+        https://bugs.webkit.org/show_bug.cgi?id=229939
+
+        Reviewed by Saam Barati.
+
+        In statements are supposed to throw if they are applied to a non-object. We incorrectly converted
+        InByVals into HasIndexedProperty any time they were a cell, so we silently converted non-objects. Before converting
+        an InByVal, we first speculate that the base is an object now.
+
+        We do not always require an object edge for HasIndexedProperty because enumerator next() does not
+        throw if it encounters a cell that requires conversion during the call to toObject (for example, a
+        string literal). That is, we should silently convert the string during enumeration, but not for an
+        In statement, and so HasIndexedProperty is prepared to handle both cases.
+
+        * dfg/DFGFixupPhase.cpp:
+        (JSC::DFG::FixupPhase::fixupNode):
+        (JSC::DFG::FixupPhase::convertToHasIndexedProperty):
+
</ins><span class="cx"> 2021-09-21  Mikhail R. Gadelha  <mikhail@igalia.com>
</span><span class="cx"> 
</span><span class="cx">         Prevent test from accessing FP registers if they are not available (e.g., arm softFP)
</span></span></pre></div>
<a id="trunkSourceJavaScriptCoredfgDFGFixupPhasecpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/dfg/DFGFixupPhase.cpp (282820 => 282821)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/dfg/DFGFixupPhase.cpp        2021-09-21 15:55:59 UTC (rev 282820)
+++ trunk/Source/JavaScriptCore/dfg/DFGFixupPhase.cpp   2021-09-21 16:06:01 UTC (rev 282821)
</span><span class="lines">@@ -2077,7 +2077,8 @@
</span><span class="cx">         }
</span><span class="cx"> 
</span><span class="cx">         case InByVal: {
</span><del>-            if (node->child2()->shouldSpeculateInt32()) {
</del><ins>+            if (node->child1()->shouldSpeculateObject()
+                && node->child2()->shouldSpeculateInt32()) {
</ins><span class="cx">                 convertToHasIndexedProperty(node);
</span><span class="cx">                 break;
</span><span class="cx">             }
</span><span class="lines">@@ -4132,7 +4133,7 @@
</span><span class="cx">         if (arrayMode.isJSArrayWithOriginalStructure() && arrayMode.speculation() == Array::InBounds)
</span><span class="cx">             setSaneChainIfPossible(node, Array::InBoundsSaneChain);
</span><span class="cx"> 
</span><del>-        fixEdge<CellUse>(m_graph.varArgChild(node, 0));
</del><ins>+        fixEdge<ObjectUse>(m_graph.varArgChild(node, 0));
</ins><span class="cx">         fixEdge<Int32Use>(m_graph.varArgChild(node, 1));
</span><span class="cx">     }
</span><span class="cx"> 
</span></span></pre></div>
<a id="trunkSourceJavaScriptCoredfgDFGSpeculativeJITcpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp (282820 => 282821)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp    2021-09-21 15:55:59 UTC (rev 282820)
+++ trunk/Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp       2021-09-21 16:06:01 UTC (rev 282821)
</span><span class="lines">@@ -15183,12 +15183,16 @@
</span><span class="cx"> 
</span><span class="cx"> void SpeculativeJIT::compileHasIndexedProperty(Node* node, S_JITOperation_GCZ slowPathOperation, const ScopedLambda<std::tuple<GPRReg, GPRReg>()>& prefix)
</span><span class="cx"> {
</span><del>-    SpeculateCellOperand base(this, m_graph.varArgChild(node, 0));
</del><ins>+    auto baseEdge = m_graph.varArgChild(node, 0);
+    SpeculateCellOperand base(this, baseEdge);
</ins><span class="cx"> 
</span><span class="cx">     GPRReg baseGPR = base.gpr();
</span><span class="cx">     GPRReg indexGPR = InvalidGPRReg;
</span><span class="cx">     GPRReg resultGPR = InvalidGPRReg;
</span><span class="cx"> 
</span><ins>+    if (baseEdge.useKind() == ObjectUse)
+        speculateObject(baseEdge, baseGPR);
+
</ins><span class="cx">     MacroAssembler::JumpList slowCases;
</span><span class="cx">     ArrayMode mode = node->arrayMode();
</span><span class="cx">     switch (mode.type()) {
</span></span></pre></div>
<a id="trunkSourceJavaScriptCoreftlFTLLowerDFGToB3cpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp (282820 => 282821)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp      2021-09-21 15:55:59 UTC (rev 282820)
+++ trunk/Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp 2021-09-21 16:06:01 UTC (rev 282821)
</span><span class="lines">@@ -13182,9 +13182,13 @@
</span><span class="cx">     LValue compileHasIndexedPropertyImpl(LValue index, S_JITOperation_GCZ slowPathOperation)
</span><span class="cx">     {
</span><span class="cx">         JSGlobalObject* globalObject = m_graph.globalObjectFor(m_origin.semantic);
</span><del>-        LValue base = lowCell(m_graph.varArgChild(m_node, 0));
</del><span class="cx">         ArrayMode mode = m_node->arrayMode();
</span><span class="cx"> 
</span><ins>+        auto baseEdge = m_graph.varArgChild(m_node, 0);
+        LValue base = lowCell(baseEdge);
+        if (baseEdge.useKind() == ObjectUse)
+            speculateObject(baseEdge, base);
+
</ins><span class="cx">         switch (m_node->arrayMode().type()) {
</span><span class="cx">         case Array::Int32:
</span><span class="cx">         case Array::Contiguous: {
</span></span></pre>
</div>
</div>

</body>
</html>