<!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>[206844] 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/206844">206844</a></dd>
<dt>Author</dt> <dd>utatane.tea@gmail.com</dd>
<dt>Date</dt> <dd>2016-10-05 21:24:57 -0700 (Wed, 05 Oct 2016)</dd>
</dl>
<h3>Log Message</h3>
<pre>[JSC] Do not construct Simple GetByIdStatus against self-custom-accessor case
https://bugs.webkit.org/show_bug.cgi?id=162993
Reviewed by Filip Pizlo.
We accidentally created a Simple GetByIdStatus against self-custom-accessor case: the object has own custom accessor property and get_by_id hits.
If we returned such a result, the GetById will be turned to GetByOffset and it looks up incorrect thing like CustomGetterSetter object.
We do not hit this bug before since maybe there is no object that has own custom-accessor and this custom-accessor does not raise an error.
For example, "Node.prototype" has "firstChild" custom accessor. But since "Node.prototype" itself does not have Node::info(), "Node.prototype.firstChild"
access always raises an error. I guess all the custom accessors follow this pattern. This bug is uncovered when testing DOMJIT (This bug causes crash and
it can occur even if we disabled DOMJIT).
But such a assumption is not guaranteed. In this patch, we fix this by not returning Simple GetById.
* bytecode/GetByIdStatus.cpp:
(JSC::GetByIdStatus::computeFromLLInt):
(JSC::GetByIdStatus::computeForStubInfoWithoutExitSiteFeedback):
(JSC::GetByIdStatus::computeFor):</pre>
<h3>Modified Paths</h3>
<ul>
<li><a href="#trunkSourceJavaScriptCoreChangeLog">trunk/Source/JavaScriptCore/ChangeLog</a></li>
<li><a href="#trunkSourceJavaScriptCorebytecodeGetByIdStatuscpp">trunk/Source/JavaScriptCore/bytecode/GetByIdStatus.cpp</a></li>
</ul>
</div>
<div id="patch">
<h3>Diff</h3>
<a id="trunkSourceJavaScriptCoreChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/ChangeLog (206843 => 206844)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/ChangeLog 2016-10-06 04:05:35 UTC (rev 206843)
+++ trunk/Source/JavaScriptCore/ChangeLog 2016-10-06 04:24:57 UTC (rev 206844)
</span><span class="lines">@@ -1,3 +1,24 @@
</span><ins>+2016-10-05 Yusuke Suzuki <utatane.tea@gmail.com>
+
+ [JSC] Do not construct Simple GetByIdStatus against self-custom-accessor case
+ https://bugs.webkit.org/show_bug.cgi?id=162993
+
+ Reviewed by Filip Pizlo.
+
+ We accidentally created a Simple GetByIdStatus against self-custom-accessor case: the object has own custom accessor property and get_by_id hits.
+ If we returned such a result, the GetById will be turned to GetByOffset and it looks up incorrect thing like CustomGetterSetter object.
+ We do not hit this bug before since maybe there is no object that has own custom-accessor and this custom-accessor does not raise an error.
+ For example, "Node.prototype" has "firstChild" custom accessor. But since "Node.prototype" itself does not have Node::info(), "Node.prototype.firstChild"
+ access always raises an error. I guess all the custom accessors follow this pattern. This bug is uncovered when testing DOMJIT (This bug causes crash and
+ it can occur even if we disabled DOMJIT).
+
+ But such a assumption is not guaranteed. In this patch, we fix this by not returning Simple GetById.
+
+ * bytecode/GetByIdStatus.cpp:
+ (JSC::GetByIdStatus::computeFromLLInt):
+ (JSC::GetByIdStatus::computeForStubInfoWithoutExitSiteFeedback):
+ (JSC::GetByIdStatus::computeFor):
+
</ins><span class="cx"> 2016-10-05 Saam Barati <sbarati@apple.com>
</span><span class="cx">
</span><span class="cx"> PCToCodeOriginMap builder should use labelIgnoringWatchpoints() inside the DFG
</span></span></pre></div>
<a id="trunkSourceJavaScriptCorebytecodeGetByIdStatuscpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/bytecode/GetByIdStatus.cpp (206843 => 206844)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/bytecode/GetByIdStatus.cpp 2016-10-06 04:05:35 UTC (rev 206843)
+++ trunk/Source/JavaScriptCore/bytecode/GetByIdStatus.cpp 2016-10-06 04:24:57 UTC (rev 206844)
</span><span class="lines">@@ -97,10 +97,12 @@
</span><span class="cx"> if (structure->takesSlowPathInDFGForImpureProperty())
</span><span class="cx"> return GetByIdStatus(NoInformation, false);
</span><span class="cx">
</span><del>- unsigned attributesIgnored;
- PropertyOffset offset = structure->getConcurrently(uid, attributesIgnored);
</del><ins>+ unsigned attributes;
+ PropertyOffset offset = structure->getConcurrently(uid, attributes);
</ins><span class="cx"> if (!isValidOffset(offset))
</span><span class="cx"> return GetByIdStatus(NoInformation, false);
</span><ins>+ if (attributes & CustomAccessor)
+ return GetByIdStatus(NoInformation, false);
</ins><span class="cx">
</span><span class="cx"> return GetByIdStatus(Simple, false, GetByIdVariant(StructureSet(structure), offset));
</span><span class="cx"> }
</span><span class="lines">@@ -176,11 +178,13 @@
</span><span class="cx"> Structure* structure = stubInfo->u.byIdSelf.baseObjectStructure.get();
</span><span class="cx"> if (structure->takesSlowPathInDFGForImpureProperty())
</span><span class="cx"> return GetByIdStatus(slowPathState, true);
</span><del>- unsigned attributesIgnored;
</del><ins>+ unsigned attributes;
</ins><span class="cx"> GetByIdVariant variant;
</span><del>- variant.m_offset = structure->getConcurrently(uid, attributesIgnored);
</del><ins>+ variant.m_offset = structure->getConcurrently(uid, attributes);
</ins><span class="cx"> if (!isValidOffset(variant.m_offset))
</span><span class="cx"> return GetByIdStatus(slowPathState, true);
</span><ins>+ if (attributes & CustomAccessor)
+ return GetByIdStatus(slowPathState, true);
</ins><span class="cx">
</span><span class="cx"> variant.m_structureSet.add(structure);
</span><span class="cx"> bool didAppend = result.appendVariant(variant);
</span><span class="lines">@@ -343,6 +347,8 @@
</span><span class="cx"> return GetByIdStatus(TakesSlowPath); // It's probably a prototype lookup. Give up on life for now, even though we could totally be way smarter about it.
</span><span class="cx"> if (attributes & Accessor)
</span><span class="cx"> return GetByIdStatus(MakesCalls); // We could be smarter here, like strength-reducing this to a Call.
</span><ins>+ if (attributes & CustomAccessor)
+ return GetByIdStatus(TakesSlowPath);
</ins><span class="cx">
</span><span class="cx"> if (!result.appendVariant(GetByIdVariant(structure, offset)))
</span><span class="cx"> return GetByIdStatus(TakesSlowPath);
</span></span></pre>
</div>
</div>
</body>
</html>