- Revision
- 204305
- Author
- sbar...@apple.com
- Date
- 2016-08-09 15:03:28 -0700 (Tue, 09 Aug 2016)
Log Message
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/_javascript_Core:
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:
Modified Paths
Diff
Modified: trunk/Source/_javascript_Core/ChangeLog (204304 => 204305)
--- trunk/Source/_javascript_Core/ChangeLog 2016-08-09 21:54:58 UTC (rev 204304)
+++ trunk/Source/_javascript_Core/ChangeLog 2016-08-09 22:03:28 UTC (rev 204305)
@@ -1,3 +1,26 @@
+2016-08-09 Saam Barati <sbar...@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:
+
2016-08-08 Mark Lam <mark....@apple.com>
ASSERTION FAILED: hasInlineStorage() in JSFinalObject::visitChildren().
Modified: trunk/Source/_javascript_Core/parser/Parser.cpp (204304 => 204305)
--- trunk/Source/_javascript_Core/parser/Parser.cpp 2016-08-09 21:54:58 UTC (rev 204304)
+++ trunk/Source/_javascript_Core/parser/Parser.cpp 2016-08-09 22:03:28 UTC (rev 204305)
@@ -1939,6 +1939,8 @@
{
RELEASE_ASSERT(isFunctionParseMode(mode));
+ ScopeRef parentScope = currentScope();
+
bool upperScopeIsGenerator = currentScope()->isGenerator();
AutoPopScopeRef functionScope(this, pushScope());
functionScope->setSourceParseMode(mode);
@@ -1954,6 +1956,9 @@
FunctionBodyType functionBodyType;
auto loadCachedFunction = [&] () -> bool {
+ if (UNLIKELY(!Options::useSourceProviderCache()))
+ return false;
+
ASSERT(parametersStart != -1);
ASSERT(startColumn != -1);
@@ -2133,8 +2138,11 @@
// as their own scope).
UniquedStringImplPtrSet nonLocalCapturesFromParameterExpressions;
functionScope->forEachUsedVariable([&] (UniquedStringImpl* impl) {
- if (!functionScope->hasDeclaredParameter(impl))
+ if (!functionScope->hasDeclaredParameter(impl)) {
nonLocalCapturesFromParameterExpressions.add(impl);
+ if (TreeBuilder::NeedsFreeVariableInfo)
+ parentScope->addClosedVariableCandidateUnconditionally(impl);
+ }
});
auto performParsingFunctionBody = [&] {
Modified: trunk/Source/_javascript_Core/parser/Parser.h (204304 => 204305)
--- trunk/Source/_javascript_Core/parser/Parser.h 2016-08-09 21:54:58 UTC (rev 204304)
+++ trunk/Source/_javascript_Core/parser/Parser.h 2016-08-09 22:03:28 UTC (rev 204305)
@@ -539,6 +539,11 @@
if (usedVariablesContains(m_vm->propertyNames->arguments.impl()))
setInnerArrowFunctionUsesArguments();
}
+
+ void addClosedVariableCandidateUnconditionally(UniquedStringImpl* impl)
+ {
+ m_closedVariableCandidates.add(impl);
+ }
void collectFreeVariables(Scope* nestedScope, bool shouldTrackClosedVariables)
{
Modified: trunk/Source/_javascript_Core/runtime/Options.h (204304 => 204305)
--- trunk/Source/_javascript_Core/runtime/Options.h 2016-08-09 21:54:58 UTC (rev 204304)
+++ trunk/Source/_javascript_Core/runtime/Options.h 2016-08-09 22:03:28 UTC (rev 204305)
@@ -379,6 +379,8 @@
\
v(bool, reportLLIntStats, false, Configurable, "Reports LLInt statistics") \
v(optionString, llintStatsFile, nullptr, Configurable, "File to collect LLInt statistics in") \
+ \
+ 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.") \
enum OptionEquivalence {
SameOption,
Modified: trunk/Tools/ChangeLog (204304 => 204305)
--- trunk/Tools/ChangeLog 2016-08-09 21:54:58 UTC (rev 204304)
+++ trunk/Tools/ChangeLog 2016-08-09 22:03:28 UTC (rev 204305)
@@ -1,3 +1,13 @@
+2016-08-09 Saam Barati <sbar...@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:
+
2016-08-09 Alexey Proskuryakov <a...@apple.com>
Make directory reading code in iOSSimulatorDevices() more strict
Modified: trunk/Tools/Scripts/run-jsc-stress-tests (204304 => 204305)
--- 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)
@@ -800,7 +800,7 @@
end
def runNoCJITValidatePhases
- run("no-cjit-validate-phases", "--validateBytecode=true", "--validateGraphAtEachPhase=true", *NO_CJIT_OPTIONS)
+ run("no-cjit-validate-phases", "--validateBytecode=true", "--validateGraphAtEachPhase=true", "--useSourceProviderCache=false", *NO_CJIT_OPTIONS)
end
def runDefault
