Skip to content

Commit 8ca6279

Browse files
committed
[MERGE #6157 @boingoing] Names declared in the body scope of a function can be incorrectly captured by a function in the param scope
Merge pull request #6157 from boingoing:fix_paramscopecapturesbodydecl Names declared in the body scope of a function can be incorrectly captured by a function in the param scope ```javascript function foo(a = (()=>x)()) { ()=>eval(""); var x; } ``` In above function foo, the name `x` should not be visible to the lambda assigned to `a`. However, foo has merged body and param scope so when we restore the ScopeInfo for foo in order to defer-parse the lambda in param scope we will set the enclosing scope of the lambda to be foo's body scope. This means we will bind the name `x` to the decl of `x` in the body scope of foo. This causes us to try and load `x` from a slot, but we don't initialize this slot value before running the lambda because we call `DefineUserVars` after we emit the default arguments. Use a parse node flag to mark functions located in param scope and construct their ScopeInfo chain correctly nesting such functions in the param scope of their parent. Fixes: https://microsoft.visualstudio.com/OS/_workitems/edit/18926499/
2 parents c60faa4 + 784bff5 commit 8ca6279

File tree

5 files changed

+50
-1
lines changed

5 files changed

+50
-1
lines changed

lib/Parser/Parse.cpp

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5310,6 +5310,7 @@ ParseNodeFnc * Parser::ParseFncDeclInternal(ushort flags, LPCOLESTR pNameHint, c
53105310
pnodeFnc->SetIsModule(fModule);
53115311
pnodeFnc->SetIsClassConstructor((flags & fFncClassConstructor) != 0);
53125312
pnodeFnc->SetIsBaseClassConstructor((flags & fFncBaseClassConstructor) != 0);
5313+
pnodeFnc->SetIsDeclaredInParamScope(this->m_currentScope && this->m_currentScope->GetScopeType() == ScopeType_Parameter);
53135314
pnodeFnc->SetHomeObjLocation(Js::Constants::NoRegister);
53145315

53155316
IdentPtr pFncNamePid = nullptr;

lib/Parser/ptree.h

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -474,7 +474,7 @@ enum FncFlags : uint
474474
kFunctionIsStaticMember = 1 << 24,
475475
kFunctionIsGenerator = 1 << 25, // Function is an ES6 generator function
476476
kFunctionAsmjsMode = 1 << 26,
477-
// Free = 1 << 27,
477+
kFunctionIsDeclaredInParamScope = 1 << 27, // Function is declared in parameter scope (ex: inside default argument)
478478
kFunctionIsAsync = 1 << 28, // function is async
479479
kFunctionHasDirectSuper = 1 << 29, // super()
480480
kFunctionIsDefaultModuleExport = 1 << 30, // function is the default export of a module
@@ -612,6 +612,7 @@ class ParseNodeFnc : public ParseNode
612612
void SetHasHomeObj(bool set = true) { SetFlags(kFunctionHasHomeObj, set); }
613613
void SetUsesArguments(bool set = true) { SetFlags(kFunctionUsesArguments, set); }
614614
void SetIsDefaultModuleExport(bool set = true) { SetFlags(kFunctionIsDefaultModuleExport, set); }
615+
void SetIsDeclaredInParamScope(bool set = true) { SetFlags(kFunctionIsDeclaredInParamScope, set); }
615616
void SetNestedFuncEscapes(bool set = true) { nestedFuncEscapes = set; }
616617
void SetCanBeDeferred(bool set = true) { canBeDeferred = set; }
617618
void ResetBodyAndParamScopeMerged() { isBodyAndParamScopeMerged = false; }
@@ -652,6 +653,7 @@ class ParseNodeFnc : public ParseNode
652653
bool HasHomeObj() const { return HasFlags(kFunctionHasHomeObj); }
653654
bool UsesArguments() const { return HasFlags(kFunctionUsesArguments); }
654655
bool IsDefaultModuleExport() const { return HasFlags(kFunctionIsDefaultModuleExport); }
656+
bool IsDeclaredInParamScope() const { return HasFlags(kFunctionIsDeclaredInParamScope); }
655657
bool NestedFuncEscapes() const { return nestedFuncEscapes; }
656658
bool CanBeDeferred() const { return canBeDeferred; }
657659
bool IsBodyAndParamScopeMerged() { return isBodyAndParamScopeMerged; }

lib/Runtime/ByteCode/ScopeInfo.cpp

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -162,6 +162,21 @@ namespace Js
162162
Scope* currentScope = byteCodeGenerator->GetCurrentScope();
163163
Assert(currentScope->GetFunc() == funcInfo);
164164

165+
if (funcInfo->root->IsDeclaredInParamScope()) {
166+
Assert(currentScope->GetScopeType() == ScopeType_FunctionBody);
167+
Assert(currentScope->GetEnclosingScope());
168+
169+
FuncInfo* func = currentScope->GetEnclosingScope()->GetFunc();
170+
Assert(func);
171+
172+
if (func->IsBodyAndParamScopeMerged())
173+
{
174+
currentScope = func->GetParamScope();
175+
Assert(currentScope->GetScopeType() == ScopeType_Parameter);
176+
Assert(!currentScope->GetMustInstantiate());
177+
}
178+
}
179+
165180
while (currentScope->GetFunc() == funcInfo)
166181
{
167182
currentScope = currentScope->GetEnclosingScope();

test/Bugs/bug_OS18926499.js

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
//-------------------------------------------------------------------------------------------------------
2+
// Copyright (C) Microsoft. All rights reserved.
3+
// Licensed under the MIT license. See LICENSE.txt file in the project root for full license information.
4+
//-------------------------------------------------------------------------------------------------------
5+
6+
function foo(a = (()=>+x)()) {
7+
function bar() { eval(''); }
8+
var x;
9+
}
10+
11+
try {
12+
foo();
13+
console.log('fail');
14+
} catch {
15+
console.log("pass");
16+
}
17+
18+
function foo2(a = () => x) { var x = 1; return a(); }
19+
20+
try {
21+
foo2()
22+
console.log('fail');
23+
} catch {
24+
console.log('pass');
25+
}

test/Bugs/rlexe.xml

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -582,4 +582,10 @@
582582
<compile-flags>-args summary -endargs</compile-flags>
583583
</default>
584584
</test>
585+
<test>
586+
<default>
587+
<files>bug_OS18926499.js</files>
588+
<compile-flags>-force:deferparse</compile-flags>
589+
</default>
590+
</test>
585591
</regress-exe>

0 commit comments

Comments
 (0)