<!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>[204305] 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/204305">204305</a></dd>
<dt>Author</dt> <dd>sbarati@apple.com</dd>
<dt>Date</dt> <dd>2016-08-09 15:03:28 -0700 (Tue, 09 Aug 2016)</dd>
</dl>
<h3>Log Message</h3>
<pre>Parser<LexerType>::parseFunctionInfo() has the wrong info about captured vars when a function is not cached.
https://bugs.webkit.org/show_bug.cgi?id=160671
<rdar://problem/27756112>
Reviewed by Mark Lam.
Source/JavaScriptCore:
There was a bug in our captured variable analysis when a function has a default
parameter expression that is a function that captures something from the parent scope.
The bug was that we were relying on the SourceProviderCache to succeed for the
analysis to work. This is obviously wrong. I've fixed this to work regardless
of getting a cache hit. To prevent future bugs that rely on the success of the
SourceProviderCache, I've made the validate testing mode disable the SourceProviderCache
* parser/Parser.cpp:
(JSC::Parser<LexerType>::parseFunctionInfo):
* parser/Parser.h:
(JSC::Scope::setInnerArrowFunctionUsesEvalAndUseArgumentsIfNeeded):
(JSC::Scope::addClosedVariableCandidateUnconditionally):
(JSC::Scope::collectFreeVariables):
* runtime/Options.h:
Tools:
* Scripts/run-jsc-stress-tests:</pre>
<h3>Modified Paths</h3>
<ul>
<li><a href="#trunkSourceJavaScriptCoreChangeLog">trunk/Source/JavaScriptCore/ChangeLog</a></li>
<li><a href="#trunkSourceJavaScriptCoreparserParsercpp">trunk/Source/JavaScriptCore/parser/Parser.cpp</a></li>
<li><a href="#trunkSourceJavaScriptCoreparserParserh">trunk/Source/JavaScriptCore/parser/Parser.h</a></li>
<li><a href="#trunkSourceJavaScriptCoreruntimeOptionsh">trunk/Source/JavaScriptCore/runtime/Options.h</a></li>
<li><a href="#trunkToolsChangeLog">trunk/Tools/ChangeLog</a></li>
<li><a href="#trunkToolsScriptsrunjscstresstests">trunk/Tools/Scripts/run-jsc-stress-tests</a></li>
</ul>
</div>
<div id="patch">
<h3>Diff</h3>
<a id="trunkSourceJavaScriptCoreChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/ChangeLog (204304 => 204305)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/ChangeLog 2016-08-09 21:54:58 UTC (rev 204304)
+++ trunk/Source/JavaScriptCore/ChangeLog 2016-08-09 22:03:28 UTC (rev 204305)
</span><span class="lines">@@ -1,3 +1,26 @@
</span><ins>+2016-08-09 Saam Barati <sbarati@apple.com>
+
+ Parser<LexerType>::parseFunctionInfo() has the wrong info about captured vars when a function is not cached.
+ https://bugs.webkit.org/show_bug.cgi?id=160671
+ <rdar://problem/27756112>
+
+ Reviewed by Mark Lam.
+
+ There was a bug in our captured variable analysis when a function has a default
+ parameter expression that is a function that captures something from the parent scope.
+ The bug was that we were relying on the SourceProviderCache to succeed for the
+ analysis to work. This is obviously wrong. I've fixed this to work regardless
+ of getting a cache hit. To prevent future bugs that rely on the success of the
+ SourceProviderCache, I've made the validate testing mode disable the SourceProviderCache
+
+ * parser/Parser.cpp:
+ (JSC::Parser<LexerType>::parseFunctionInfo):
+ * parser/Parser.h:
+ (JSC::Scope::setInnerArrowFunctionUsesEvalAndUseArgumentsIfNeeded):
+ (JSC::Scope::addClosedVariableCandidateUnconditionally):
+ (JSC::Scope::collectFreeVariables):
+ * runtime/Options.h:
+
</ins><span class="cx"> 2016-08-08 Mark Lam <mark.lam@apple.com>
</span><span class="cx">
</span><span class="cx"> ASSERTION FAILED: hasInlineStorage() in JSFinalObject::visitChildren().
</span></span></pre></div>
<a id="trunkSourceJavaScriptCoreparserParsercpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/parser/Parser.cpp (204304 => 204305)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/parser/Parser.cpp 2016-08-09 21:54:58 UTC (rev 204304)
+++ trunk/Source/JavaScriptCore/parser/Parser.cpp 2016-08-09 22:03:28 UTC (rev 204305)
</span><span class="lines">@@ -1939,6 +1939,8 @@
</span><span class="cx"> {
</span><span class="cx"> RELEASE_ASSERT(isFunctionParseMode(mode));
</span><span class="cx">
</span><ins>+ ScopeRef parentScope = currentScope();
+
</ins><span class="cx"> bool upperScopeIsGenerator = currentScope()->isGenerator();
</span><span class="cx"> AutoPopScopeRef functionScope(this, pushScope());
</span><span class="cx"> functionScope->setSourceParseMode(mode);
</span><span class="lines">@@ -1954,6 +1956,9 @@
</span><span class="cx"> FunctionBodyType functionBodyType;
</span><span class="cx">
</span><span class="cx"> auto loadCachedFunction = [&] () -> bool {
</span><ins>+ if (UNLIKELY(!Options::useSourceProviderCache()))
+ return false;
+
</ins><span class="cx"> ASSERT(parametersStart != -1);
</span><span class="cx"> ASSERT(startColumn != -1);
</span><span class="cx">
</span><span class="lines">@@ -2133,8 +2138,11 @@
</span><span class="cx"> // as their own scope).
</span><span class="cx"> UniquedStringImplPtrSet nonLocalCapturesFromParameterExpressions;
</span><span class="cx"> functionScope->forEachUsedVariable([&] (UniquedStringImpl* impl) {
</span><del>- if (!functionScope->hasDeclaredParameter(impl))
</del><ins>+ if (!functionScope->hasDeclaredParameter(impl)) {
</ins><span class="cx"> nonLocalCapturesFromParameterExpressions.add(impl);
</span><ins>+ if (TreeBuilder::NeedsFreeVariableInfo)
+ parentScope->addClosedVariableCandidateUnconditionally(impl);
+ }
</ins><span class="cx"> });
</span><span class="cx">
</span><span class="cx"> auto performParsingFunctionBody = [&] {
</span></span></pre></div>
<a id="trunkSourceJavaScriptCoreparserParserh"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/parser/Parser.h (204304 => 204305)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/parser/Parser.h 2016-08-09 21:54:58 UTC (rev 204304)
+++ trunk/Source/JavaScriptCore/parser/Parser.h 2016-08-09 22:03:28 UTC (rev 204305)
</span><span class="lines">@@ -539,6 +539,11 @@
</span><span class="cx"> if (usedVariablesContains(m_vm->propertyNames->arguments.impl()))
</span><span class="cx"> setInnerArrowFunctionUsesArguments();
</span><span class="cx"> }
</span><ins>+
+ void addClosedVariableCandidateUnconditionally(UniquedStringImpl* impl)
+ {
+ m_closedVariableCandidates.add(impl);
+ }
</ins><span class="cx">
</span><span class="cx"> void collectFreeVariables(Scope* nestedScope, bool shouldTrackClosedVariables)
</span><span class="cx"> {
</span></span></pre></div>
<a id="trunkSourceJavaScriptCoreruntimeOptionsh"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/runtime/Options.h (204304 => 204305)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/runtime/Options.h 2016-08-09 21:54:58 UTC (rev 204304)
+++ trunk/Source/JavaScriptCore/runtime/Options.h 2016-08-09 22:03:28 UTC (rev 204305)
</span><span class="lines">@@ -379,6 +379,8 @@
</span><span class="cx"> \
</span><span class="cx"> v(bool, reportLLIntStats, false, Configurable, "Reports LLInt statistics") \
</span><span class="cx"> v(optionString, llintStatsFile, nullptr, Configurable, "File to collect LLInt statistics in") \
</span><ins>+ \
+ v(bool, useSourceProviderCache, true, Normal, "If false, the parser will not use the source provider cache. It's good to verify everything works when this is false. Because the cache is so successful, it can mask bugs.") \
</ins><span class="cx">
</span><span class="cx"> enum OptionEquivalence {
</span><span class="cx"> SameOption,
</span></span></pre></div>
<a id="trunkToolsChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/Tools/ChangeLog (204304 => 204305)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Tools/ChangeLog 2016-08-09 21:54:58 UTC (rev 204304)
+++ trunk/Tools/ChangeLog 2016-08-09 22:03:28 UTC (rev 204305)
</span><span class="lines">@@ -1,3 +1,13 @@
</span><ins>+2016-08-09 Saam Barati <sbarati@apple.com>
+
+ Parser<LexerType>::parseFunctionInfo() has the wrong info about captured vars when a function is not cached.
+ https://bugs.webkit.org/show_bug.cgi?id=160671
+ <rdar://problem/27756112>
+
+ Reviewed by Mark Lam.
+
+ * Scripts/run-jsc-stress-tests:
+
</ins><span class="cx"> 2016-08-09 Alexey Proskuryakov <ap@apple.com>
</span><span class="cx">
</span><span class="cx"> Make directory reading code in iOSSimulatorDevices() more strict
</span></span></pre></div>
<a id="trunkToolsScriptsrunjscstresstests"></a>
<div class="modfile"><h4>Modified: trunk/Tools/Scripts/run-jsc-stress-tests (204304 => 204305)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Tools/Scripts/run-jsc-stress-tests 2016-08-09 21:54:58 UTC (rev 204304)
+++ trunk/Tools/Scripts/run-jsc-stress-tests 2016-08-09 22:03:28 UTC (rev 204305)
</span><span class="lines">@@ -800,7 +800,7 @@
</span><span class="cx"> end
</span><span class="cx">
</span><span class="cx"> def runNoCJITValidatePhases
</span><del>- run("no-cjit-validate-phases", "--validateBytecode=true", "--validateGraphAtEachPhase=true", *NO_CJIT_OPTIONS)
</del><ins>+ run("no-cjit-validate-phases", "--validateBytecode=true", "--validateGraphAtEachPhase=true", "--useSourceProviderCache=false", *NO_CJIT_OPTIONS)
</ins><span class="cx"> end
</span><span class="cx">
</span><span class="cx"> def runDefault
</span></span></pre>
</div>
</div>
</body>
</html>