Bug 171456

Summary: Deep nesting is leading to ReferenceError for hoisted function
Product: WebKit Reporter: GSkachkov <gskachkov>
Component: JavaScriptCoreAssignee: GSkachkov <gskachkov>
Status: RESOLVED FIXED    
Severity: Normal CC: buildbot, joepeck, keith_miller, mark.lam, msaboff, saam, ysuzuki
Priority: P2    
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 163208    
Attachments:
Description Flags
Patch ysuzuki: review+

GSkachkov
Reported 2017-04-28 15:43:29 PDT
function boo () { { function foo() {} } if (true) { { { { { { { { { { let x = 0; print(x); } } } } } } } } } } print(!!foo); // ReferenceError: Can't find variable: foo, but should be true }; boo(); Error occurs because we do not copy hosting candidates when we increase buffer for scope stack, so we need to add , m_sloppyModeHoistableFunctionCandidates(WTFMove(other.m_sloppyModeHoistableFunctionCandidates)) to Scope constructor This error present in latest Safari and STP
Attachments
Patch (9.34 KB, patch)
2017-04-29 03:22 PDT, GSkachkov 		ysuzuki: review+
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
GSkachkov
Comment 1 2017-04-28 15:45:48 PDT
Will prepare patch tomorrow with tests.
Saam Barati
Comment 2 2017-04-28 16:08:55 PDT
(In reply to GSkachkov from comment #0) > function boo () { > { > function foo() {} > } > if (true) { > { > { > { > { > { > { > { > { > { > let x = 0; > print(x); > } > } > } > } > } > } > } > } > } > } Oh man this is glorious. Nice find. > print(!!foo); // ReferenceError: Can't find variable: foo, but should be > true > }; > boo(); > > Error occurs because we do not copy hosting candidates when we increase > buffer for scope stack, so we need to add , > m_sloppyModeHoistableFunctionCandidates(WTFMove(other. > m_sloppyModeHoistableFunctionCandidates)) to Scope constructor > > This error present in latest Safari and STP
GSkachkov
Comment 3 2017-04-29 03:22:45 PDT
Created attachment 308663 [details] Patch Fix
Yusuke Suzuki
Comment 4 2017-04-29 06:03:52 PDT
Comment on attachment 308663 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=308663&action=review Nice! r=me with comment. > Source/JavaScriptCore/parser/Parser.h:228 > { Listing all the members are not a scalable way. Can we just do `Scope(Scope&&) = default;` here?
GSkachkov
Comment 5 2017-04-29 14:27:01 PDT
(In reply to Yusuke Suzuki from comment #4) > Comment on attachment 308663 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=308663&action=review > > Nice! r=me with comment. > > > Source/JavaScriptCore/parser/Parser.h:228 > > { > > Listing all the members are not a scalable way. Can we just do > `Scope(Scope&&) = default;` here? Done. Thanks for review!
GSkachkov
Comment 6 2017-04-29 14:27:41 PDT
Patch landed https://trac.webkit.org/r215977
GSkachkov
Comment 7 2017-04-29 14:27:55 PDT
Comment on attachment 308663 [details] Patch Clear flags
Joseph Pecoraro
Comment 8 2017-04-29 16:22:51 PDT
Nice!
Note You need to log in before you can comment on or make changes to this bug.