Skip to content

Commit 3e0459c

Browse files
sigatrevsigatrev
committed
[1.7>master] [MERGE chakra-core#3538 @sigatrev] OS:10898061 fix bug with cached scopes and default/destrctured arguments
Merge pull request chakra-core#3538 from sigatrev:CachedScopeChecks When cached scopes are used with default function arguments or destructured parameters, any variable references in the values of the default argument or destructured params would erroneously see the merged body and param scope
2 parents b6eb1f1 + 911cfcb commit 3e0459c

5 files changed

Lines changed: 54 additions & 1 deletion

File tree

‎lib/Parser/Parse.cpp‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6319,6 +6319,7 @@ void Parser::ParseFncFormals(ParseNodePtr pnodeFnc, ParseNodePtr pnodeParentFnc,
63196319
{
63206320
// Mark that the function has a non simple parameter list before parsing the pattern since the pattern can have function definitions.
63216321
this->GetCurrentFunctionNode()->sxFnc.SetHasNonSimpleParameterList();
6322+
this->GetCurrentFunctionNode()->sxFnc.SetHasDestructuredParams();
63226323

63236324
ParseNodePtr *const ppnodeVarSave = m_ppnodeVar;
63246325
m_ppnodeVar = &pnodeFnc->sxFnc.pnodeVars;

‎lib/Parser/ptree.h‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -182,7 +182,7 @@ enum FncFlags : uint
182182
kFunctionIsAccessor = 1 << 6, // function is a property getter or setter
183183
kFunctionHasNonThisStmt = 1 << 7,
184184
kFunctionStrictMode = 1 << 8,
185-
// Free = 1 << 9,
185+
kFunctionHasDestructuredParams = 1 << 9,
186186
kFunctionIsModule = 1 << 10, // function is a module body
187187
// Free = 1 << 11,
188188
kFunctionHasWithStmt = 1 << 12, // function (or child) uses with
@@ -296,6 +296,7 @@ struct PnFnc
296296
void SetChildCallsEval(bool set = true) { SetFlags(kFunctionChildCallsEval, set); }
297297
void SetDeclaration(bool set = true) { SetFlags(kFunctionDeclaration, set); }
298298
void SetHasDefaultArguments(bool set = true) { SetFlags(kFunctionHasDefaultArguments, set); }
299+
void SetHasDestructuredParams(bool set = true) { SetFlags(kFunctionHasDestructuredParams, set); }
299300
void SetHasHeapArguments(bool set = true) { SetFlags(kFunctionHasHeapArguments, set); }
300301
void SetHasAnyWriteToFormals(bool set = true) { SetFlags((uint)kFunctionHasAnyWriteToFormals, set); }
301302
void SetHasNonSimpleParameterList(bool set = true) { SetFlags(kFunctionHasNonSimpleParameterList, set); }
@@ -330,6 +331,7 @@ struct PnFnc
330331
bool GetAsmjsMode() const { return HasFlags(kFunctionAsmjsMode); }
331332
bool GetStrictMode() const { return HasFlags(kFunctionStrictMode); }
332333
bool HasDefaultArguments() const { return HasFlags(kFunctionHasDefaultArguments); }
334+
bool HasDestructuredParams() const { return HasFlags(kFunctionHasDestructuredParams); }
333335
bool HasHeapArguments() const { return true; /* HasFlags(kFunctionHasHeapArguments); Disabling stack arguments. Always return HeapArguments as True */ }
334336
bool HasAnyWriteToFormals() const { return HasFlags((uint)kFunctionHasAnyWriteToFormals); }
335337
bool HasOnlyThisStmts() const { return !HasFlags(kFunctionHasNonThisStmt); }

‎lib/Runtime/ByteCode/ByteCodeEmitter.cpp‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4011,6 +4011,8 @@ void ByteCodeGenerator::StartEmitFunction(ParseNode *pnodeFnc)
40114011
funcInfo->frameObjRegister != Js::Constants::NoRegister &&
40124012
!ApplyEnclosesArgs(pnodeFnc, this) &&
40134013
funcInfo->IsBodyAndParamScopeMerged() && // There is eval in the param scope
4014+
!pnodeFnc->sxFnc.HasDefaultArguments() &&
4015+
!pnodeFnc->sxFnc.HasDestructuredParams() &&
40144016
(PHASE_FORCE(Js::CachedScopePhase, funcInfo->byteCodeFunction) || !IsInDebugMode())
40154017
#if ENABLE_TTD
40164018
&& !funcInfo->GetParsedFunctionBody()->GetScriptContext()->GetThreadContext()->IsRuntimeInTTDMode()

‎test/es6/bug_OS10898061.js‎

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,42 @@
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 = b )
7+
{
8+
eval("");
9+
var b;
10+
}
11+
12+
function bar( {a:a = b} )
13+
{
14+
eval("");
15+
var b;
16+
}
17+
18+
19+
function test()
20+
{
21+
try
22+
{
23+
// foo should throw a ReferenceError: 'b' is not defined.
24+
foo();
25+
return false;
26+
}
27+
catch( a )
28+
{}
29+
30+
try
31+
{
32+
// bar should throw a ReferenceError: 'b' is not defined.
33+
bar({});
34+
return false;
35+
}
36+
catch( a )
37+
{}
38+
39+
return true;
40+
}
41+
42+
WScript.Echo(test() ? "PASSED" : "FAILED");

‎test/es6/rlexe.xml‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1473,4 +1473,10 @@
14731473
<tags>BugFix</tags>
14741474
</default>
14751475
</test>
1476+
<test>
1477+
<default>
1478+
<files>bug_OS10898061.js</files>
1479+
<tags>BugFix</tags>
1480+
</default>
1481+
</test>
14761482
</regress-exe>

0 commit comments

Comments
 (0)