<!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>[187487] 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/187487">187487</a></dd>
<dt>Author</dt> <dd>fpizlo@apple.com</dd>
<dt>Date</dt> <dd>2015-07-28 09:55:21 -0700 (Tue, 28 Jul 2015)</dd>
</dl>
<h3>Log Message</h3>
<pre>DFG::safeToExecute() cases for GetByOffset/PutByOffset don't handle clobbered structure abstract values correctly
https://bugs.webkit.org/show_bug.cgi?id=147354
Reviewed by Michael Saboff.
If m_structure.isClobbered(), it means that we had a side effect that clobbered
the abstract value but it may recover back to its original value at the next
invalidation point. Since the invalidation point hasn't been reached yet, we need
to conservatively treat the clobbered state as if it was top. At the invalidation
point, the clobbered set will return back to being unclobbered.
In addition to fixing the bug, this introduces isInfinite(), which should be used
in places where it's tempting to just use isTop().
* dfg/DFGSafeToExecute.h:
(JSC::DFG::safeToExecute): Fix the bug.
* dfg/DFGStructureAbstractValue.cpp:
(JSC::DFG::StructureAbstractValue::contains): Switch to using isInfinite().
(JSC::DFG::StructureAbstractValue::isSubsetOf): Switch to using isInfinite().
(JSC::DFG::StructureAbstractValue::isSupersetOf): Switch to using isInfinite().
(JSC::DFG::StructureAbstractValue::overlaps): Switch to using isInfinite().
* dfg/DFGStructureAbstractValue.h:
(JSC::DFG::StructureAbstractValue::isFinite): New convenience method.
(JSC::DFG::StructureAbstractValue::isInfinite): New convenience method.
(JSC::DFG::StructureAbstractValue::onlyStructure): Switch to using isInfinite().</pre>
<h3>Modified Paths</h3>
<ul>
<li><a href="#trunkSourceJavaScriptCoreChangeLog">trunk/Source/JavaScriptCore/ChangeLog</a></li>
<li><a href="#trunkSourceJavaScriptCoredfgDFGSafeToExecuteh">trunk/Source/JavaScriptCore/dfg/DFGSafeToExecute.h</a></li>
<li><a href="#trunkSourceJavaScriptCoredfgDFGStructureAbstractValuecpp">trunk/Source/JavaScriptCore/dfg/DFGStructureAbstractValue.cpp</a></li>
<li><a href="#trunkSourceJavaScriptCoredfgDFGStructureAbstractValueh">trunk/Source/JavaScriptCore/dfg/DFGStructureAbstractValue.h</a></li>
</ul>
</div>
<div id="patch">
<h3>Diff</h3>
<a id="trunkSourceJavaScriptCoreChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/ChangeLog (187486 => 187487)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/ChangeLog 2015-07-28 16:29:44 UTC (rev 187486)
+++ trunk/Source/JavaScriptCore/ChangeLog 2015-07-28 16:55:21 UTC (rev 187487)
</span><span class="lines">@@ -1,3 +1,31 @@
</span><ins>+2015-07-27 Filip Pizlo <fpizlo@apple.com>
+
+ DFG::safeToExecute() cases for GetByOffset/PutByOffset don't handle clobbered structure abstract values correctly
+ https://bugs.webkit.org/show_bug.cgi?id=147354
+
+ Reviewed by Michael Saboff.
+
+ If m_structure.isClobbered(), it means that we had a side effect that clobbered
+ the abstract value but it may recover back to its original value at the next
+ invalidation point. Since the invalidation point hasn't been reached yet, we need
+ to conservatively treat the clobbered state as if it was top. At the invalidation
+ point, the clobbered set will return back to being unclobbered.
+
+ In addition to fixing the bug, this introduces isInfinite(), which should be used
+ in places where it's tempting to just use isTop().
+
+ * dfg/DFGSafeToExecute.h:
+ (JSC::DFG::safeToExecute): Fix the bug.
+ * dfg/DFGStructureAbstractValue.cpp:
+ (JSC::DFG::StructureAbstractValue::contains): Switch to using isInfinite().
+ (JSC::DFG::StructureAbstractValue::isSubsetOf): Switch to using isInfinite().
+ (JSC::DFG::StructureAbstractValue::isSupersetOf): Switch to using isInfinite().
+ (JSC::DFG::StructureAbstractValue::overlaps): Switch to using isInfinite().
+ * dfg/DFGStructureAbstractValue.h:
+ (JSC::DFG::StructureAbstractValue::isFinite): New convenience method.
+ (JSC::DFG::StructureAbstractValue::isInfinite): New convenience method.
+ (JSC::DFG::StructureAbstractValue::onlyStructure): Switch to using isInfinite().
+
</ins><span class="cx"> 2015-07-27 Yusuke Suzuki <utatane.tea@gmail.com>
</span><span class="cx">
</span><span class="cx"> [ES6] Implement Reflect.enumerate
</span></span></pre></div>
<a id="trunkSourceJavaScriptCoredfgDFGSafeToExecuteh"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/dfg/DFGSafeToExecute.h (187486 => 187487)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/dfg/DFGSafeToExecute.h 2015-07-28 16:29:44 UTC (rev 187486)
+++ trunk/Source/JavaScriptCore/dfg/DFGSafeToExecute.h 2015-07-28 16:55:21 UTC (rev 187487)
</span><span class="lines">@@ -329,7 +329,7 @@
</span><span class="cx"> case GetGetterSetterByOffset:
</span><span class="cx"> case PutByOffset: {
</span><span class="cx"> StructureAbstractValue& value = state.forNode(node->child1()).m_structure;
</span><del>- if (value.isTop())
</del><ins>+ if (value.isInfinite())
</ins><span class="cx"> return false;
</span><span class="cx"> PropertyOffset offset = node->storageAccessData().offset;
</span><span class="cx"> for (unsigned i = value.size(); i--;) {
</span></span></pre></div>
<a id="trunkSourceJavaScriptCoredfgDFGStructureAbstractValuecpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/dfg/DFGStructureAbstractValue.cpp (187486 => 187487)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/dfg/DFGStructureAbstractValue.cpp 2015-07-28 16:29:44 UTC (rev 187486)
+++ trunk/Source/JavaScriptCore/dfg/DFGStructureAbstractValue.cpp 2015-07-28 16:55:21 UTC (rev 187487)
</span><span class="lines">@@ -289,7 +289,7 @@
</span><span class="cx"> {
</span><span class="cx"> SAMPLE("StructureAbstractValue contains");
</span><span class="cx">
</span><del>- if (isTop() || isClobbered())
</del><ins>+ if (isInfinite())
</ins><span class="cx"> return true;
</span><span class="cx">
</span><span class="cx"> return m_set.contains(structure);
</span><span class="lines">@@ -299,7 +299,7 @@
</span><span class="cx"> {
</span><span class="cx"> SAMPLE("StructureAbstractValue isSubsetOf set");
</span><span class="cx">
</span><del>- if (isTop() || isClobbered())
</del><ins>+ if (isInfinite())
</ins><span class="cx"> return false;
</span><span class="cx">
</span><span class="cx"> return m_set.isSubsetOf(other);
</span><span class="lines">@@ -332,7 +332,7 @@
</span><span class="cx"> {
</span><span class="cx"> SAMPLE("StructureAbstractValue isSupersetOf set");
</span><span class="cx">
</span><del>- if (isTop() || isClobbered())
</del><ins>+ if (isInfinite())
</ins><span class="cx"> return true;
</span><span class="cx">
</span><span class="cx"> return m_set.isSupersetOf(other);
</span><span class="lines">@@ -342,7 +342,7 @@
</span><span class="cx"> {
</span><span class="cx"> SAMPLE("StructureAbstractValue overlaps set");
</span><span class="cx">
</span><del>- if (isTop() || isClobbered())
</del><ins>+ if (isInfinite())
</ins><span class="cx"> return true;
</span><span class="cx">
</span><span class="cx"> return m_set.overlaps(other);
</span><span class="lines">@@ -352,7 +352,7 @@
</span><span class="cx"> {
</span><span class="cx"> SAMPLE("StructureAbstractValue overlaps value");
</span><span class="cx">
</span><del>- if (other.isTop() || other.isClobbered())
</del><ins>+ if (other.isInfinite())
</ins><span class="cx"> return true;
</span><span class="cx">
</span><span class="cx"> return overlaps(other.m_set);
</span></span></pre></div>
<a id="trunkSourceJavaScriptCoredfgDFGStructureAbstractValueh"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/dfg/DFGStructureAbstractValue.h (187486 => 187487)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/dfg/DFGStructureAbstractValue.h 2015-07-28 16:29:44 UTC (rev 187486)
+++ trunk/Source/JavaScriptCore/dfg/DFGStructureAbstractValue.h 2015-07-28 16:55:21 UTC (rev 187487)
</span><span class="lines">@@ -123,6 +123,14 @@
</span><span class="cx"> // m_set by looking at reachability as this would mean that the new set is TOP. Instead they
</span><span class="cx"> // literally assume that the set is just m_set rather than m_set plus TOP.
</span><span class="cx"> bool isClobbered() const { return m_set.getReservedFlag(); }
</span><ins>+
+ // A finite structure abstract value is one where enumerating over it will yield all
+ // of the structures that the value may have right now. This is true so long as we're
+ // neither top nor clobbered.
+ bool isFinite() const { return !isTop() && !isClobbered(); }
+
+ // An infinite structure abstract value may currently have any structure.
+ bool isInfinite() const { return !isFinite(); }
</ins><span class="cx">
</span><span class="cx"> bool add(Structure* structure);
</span><span class="cx">
</span><span class="lines">@@ -190,7 +198,7 @@
</span><span class="cx"> // meaningfully special, like for transitions.
</span><span class="cx"> Structure* onlyStructure() const
</span><span class="cx"> {
</span><del>- if (isTop() || isClobbered())
</del><ins>+ if (isInfinite())
</ins><span class="cx"> return nullptr;
</span><span class="cx"> return m_set.onlyStructure();
</span><span class="cx"> }
</span></span></pre>
</div>
</div>
</body>
</html>