marja: Your CL https://chromium.googlesource.com/v8/v8/+/bc39a5148a3824ea948fd7725674945ca0b1c56a is the only one in the blame range for this. Can you please investigate, thanks!

Reproes with plain d8.

A smaller repro:

function foo() {
  eval('var f = ([x=[]=[]]) => {}; f()');
}
foo();

It's important for the repro that the code is inside a function and evalled. Otherwise it doesn't repro...

So here the arrow function parameters are [x] with a default value [] = [] for x.

Update: I'm investigating this and... a possible (partial) solution would be to disable lazy parsing for arrow funcs with non-simple params. Maybe.

*But* I'm bumping into some weird behavior where I try to make the arrow func eager, but still, we end up reparsing & recompiling it when calling it.

And that behavior only happens when the arrow func params are non-simple.

Example 1:

Script:
let f = () => { }; f();

$ d8 --trace-parse --no-lazy ../foo299.js
[parsing script: ../foo299.js - took 0.053 ms]

Example 2:

Script:
let f = ([a=0]) => { }; f();

$ d8 --trace-parse --no-lazy ../foo299.js 
[parsing script: ../foo299.js - took 0.067 ms]
[parsing function: f - took 0.011 ms] <<<<< whaaaattt, why is the function lazy???

The previous comment was confused. In that case, the reparsing is coming from here:

#0  v8::internal::parsing::ParseFunction (info=0x7fffffffc9e0, shared_info=..., isolate=0x55555558c170)
    at ../../src/parsing/parsing.cc:58
#1  0x00007ffff7739c9c in v8::internal::(anonymous namespace)::RenderCallSite (isolate=0x55555558c170, 
    object=..., hint=0x7fffffffcf14) at ../../src/runtime/runtime-internal.cc:394
#2  0x00007ffff773963c in v8::internal::Runtime::ThrowIteratorError (isolate=0x55555558c170, object=...)
    at ../../src/runtime/runtime-internal.cc:433
#3  0x00007ffff7445a3d in v8::internal::LoadIC::Load (this=0x7fffffffcff8, object=..., name=...)
    at ../../src/ic/ic.cc:437
#4  0x00007ffff7457751 in v8::internal::__RT_impl_Runtime_LoadIC_Miss (args=..., isolate=0x55555558c170)
    at ../../src/ic/ic.cc:2366


What this means is: even if I disable lazy parsing in that case, there's still this code path which won't work.

After a debugging session w/ verwaest@ we concluded that the problem is that the NewTemporary call in BuildIteratorClose goes to the wrong Scope.

There are two possible solutions:
1) push the right scope into the scope chain when doing rewriting
2) pipe the scope in the rewritable assignment through properly

I'll work on a fix...

For completeness, here's the stack trace for the problem:

libv8.so(v8::internal::DeclarationScope::AddLocal(v8::internal::Variable*)+0xab) [0x7f061065955b]
libv8.so(v8::internal::Scope::NewTemporary(v8::internal::AstRawString const*)+0x6f) [0x7f061065915f]
libv8.so(v8::internal::Parser::BuildIteratorCloseForCompletion(v8::internal::Scope*, v8::internal::ZoneList<v8::internal::Statement*>*, v8::internal::Variable*, v8::internal::Expression*, v8::internal::IteratorType)+0x3e) [0x7f0610cfe5ee]
libv8.so(v8::internal::Parser::FinalizeIteratorUse(v8::internal::Scope*, v8::internal::Variable*, v8::internal::Expression*, v8::internal::Variable*, v8::internal::Block*, v8::internal::Block*, v8::internal::IteratorType)+0x374) [0x7f0610cfdfd4]
libv8.so(+0x91d066) [0x7f0610d1d066] << missing, but this is PatternRewriter::VisitArrayLiteral
libv8.so(+0x91ab43) [0x7f0610d1ab43]
libv8.so(v8::internal::Parser::RewriteDestructuringAssignment(v8::internal::RewritableExpression*, v8::internal::Scope*)+0x6c) [0x7f0610d19d5c]
libv8.so(+0x8e7d04) [0x7f0610ce7d04]
libv8.so(v8::internal::Parser::DoParseFunction(v8::internal::ParseInfo*, v8::internal::AstRawString const*)+0xc03) [0x7f0610ce9283]

Looks like BuildIteratorCloseForCompletion already gets the correct scope as a param, it just doesn't use it...

This is a serious security regression. If you are not able to fix this quickly, please revert the change that introduced it.

If this doesn't affect a release branch, or has not been properly classified for severity, please update the Security_Impact or Security_Severity labels, and remove the ReleaseBlock label. To disable this altogether, apply ReleaseBlock-NA.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot

cc caitp

Fix proposals (2 alternatives) at

https://chromium-review.googlesource.com/c/v8/v8/+/650352

and

https://chromium-review.googlesource.com/c/v8/v8/+/650297

The following revision refers to this bug:
  https://chromium.googlesource.com/v8/v8.git/+/138fbdb4f4f6bf21002c2b5c0d3d5a69bbc61bd0

commit 138fbdb4f4f6bf21002c2b5c0d3d5a69bbc61bd0
Author: Marja Hölttä <marja@chromium.org>
Date: Thu Sep 07 13:06:44 2017

[parser] Fix arrow funcs w/ destructuring params again. [Alternative fix]

What happened:
- When rewriting in DoParseFunction, the relevant function scope is no longer in
the scope stack.
- The correct scope is given to the PatternRewriter.
- PatternRewriter called to Parser::BuildIteratorCloseForCompletion.
- BuildIteratorCloseForCompletion would just call NewTemporary (which creates
a new temporary in Parser's current scope) instead of using the scope passed to
it and calling NewTemporary on it.
- Normally this went unnoticed, since it doesn't matter that much where the
temporary is.
- But in the lazy arrow func case, the Parser's scope at that point was the
already-resolved outer scope, and a DCHECK detected this problem.

Kudos & thanks to verwaest@ for a debugging session :)

BUG= chromium:761831 

Change-Id: I1e8474ce927be0330f4ba4efc0fc08fdcc328809
Reviewed-on: https://chromium-review.googlesource.com/650297
Commit-Queue: Marja Hölttä <marja@chromium.org>
Reviewed-by: Toon Verwaest <verwaest@chromium.org>
Reviewed-by: Adam Klein <adamk@chromium.org>
Cr-Commit-Position: refs/heads/master@{#47877}
[modify] https://crrev.com/138fbdb4f4f6bf21002c2b5c0d3d5a69bbc61bd0/src/parsing/parser.cc
[modify] https://crrev.com/138fbdb4f4f6bf21002c2b5c0d3d5a69bbc61bd0/src/parsing/parser.h
[modify] https://crrev.com/138fbdb4f4f6bf21002c2b5c0d3d5a69bbc61bd0/src/parsing/pattern-rewriter.cc
[add] https://crrev.com/138fbdb4f4f6bf21002c2b5c0d3d5a69bbc61bd0/test/mjsunit/regress/regress-761831.js

ClusterFuzz has detected this issue as fixed in range 47876:47877.

Detailed report: https://clusterfuzz.com/testcase?key=6010193094901760

Fuzzer: inferno_js_fuzzer_c
Job Type: linux_asan_d8_dbg
Platform Id: linux

Crash Type: DCHECK failure
Crash Address: 
Crash State:
  !already_resolved_ in scopes.cc
  v8::internal::DeclarationScope::AddLocal
  v8::internal::Scope::NewTemporary
  
Sanitizer: address (ASAN)

Regressed: https://clusterfuzz.com/revisions?job=linux_asan_d8_dbg&range=44176:44177
Fixed: https://clusterfuzz.com/revisions?job=linux_asan_d8_dbg&range=47876:47877

Reproducer Testcase: https://clusterfuzz.com/download?testcase_id=6010193094901760

See https://github.com/google/clusterfuzz-tools for more information.

If you suspect that the result above is incorrect, try re-doing that job on the test case report page.

ClusterFuzz testcase 6010193094901760 is verified as fixed, so closing issue as verified.

If this is incorrect, please add ClusterFuzz-Wrong label and re-open the issue.

This bug requires manual review: M62 has already been promoted to the beta branch, so this requires manual review
Please contact the milestone owner if you have questions.
Owners: amineer@(Android), cmasso@(iOS), bhthompson@(ChromeOS), abdulsyed@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot

Approving merge for M62. Branch:3202

The following revision refers to this bug:
  https://chromium.googlesource.com/v8/v8.git/+/9cef1320e4b95c29292133270e8a7d34c885d54b

commit 9cef1320e4b95c29292133270e8a7d34c885d54b
Author: Marja Hölttä <marja@chromium.org>
Date: Mon Sep 18 10:36:01 2017

Merged: [parser] Fix arrow funcs w/ destructuring params again. [Alternative fix]

What happened:
- When rewriting in DoParseFunction, the relevant function scope is no longer in
the scope stack.
- The correct scope is given to the PatternRewriter.
- PatternRewriter called to Parser::BuildIteratorCloseForCompletion.
- BuildIteratorCloseForCompletion would just call NewTemporary (which creates
a new temporary in Parser's current scope) instead of using the scope passed to
it and calling NewTemporary on it.
- Normally this went unnoticed, since it doesn't matter that much where the
temporary is.
- But in the lazy arrow func case, the Parser's scope at that point was the
already-resolved outer scope, and a DCHECK detected this problem.

Kudos & thanks to verwaest@ for a debugging session :)

BUG= chromium:761831 

NOTRY=true
NOPRESUBMIT=true
NOTREECHECKS=true

Change-Id: I1e8474ce927be0330f4ba4efc0fc08fdcc328809
Reviewed-on: https://chromium-review.googlesource.com/650297
Commit-Queue: Marja Hölttä <marja@chromium.org>
Reviewed-by: Toon Verwaest <verwaest@chromium.org>
Reviewed-by: Adam Klein <adamk@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#47877}(cherry picked from commit 138fbdb4f4f6bf21002c2b5c0d3d5a69bbc61bd0)
Reviewed-on: https://chromium-review.googlesource.com/670599
Reviewed-by: Michael Achenbach <machenbach@chromium.org>
Cr-Commit-Position: refs/branch-heads/6.2@{#25}
Cr-Branched-From: efa2ac4129d30c7c72e84c16af3d20b44829f990-refs/heads/6.2.414@{#1}
Cr-Branched-From: a861ebb762a60bf5cc2a274faee3620abfb06311-refs/heads/master@{#47693}
[modify] https://crrev.com/9cef1320e4b95c29292133270e8a7d34c885d54b/src/parsing/parser.cc
[modify] https://crrev.com/9cef1320e4b95c29292133270e8a7d34c885d54b/src/parsing/parser.h
[modify] https://crrev.com/9cef1320e4b95c29292133270e8a7d34c885d54b/src/parsing/pattern-rewriter.cc
[add] https://crrev.com/9cef1320e4b95c29292133270e8a7d34c885d54b/test/mjsunit/regress/regress-761831.js

This issue has been approved for a merge. Please merge the fix to any appropriate branches as soon as possible!

If all merges have been completed, please remove any remaining Merge-Approved labels from this issue.

Thanks for your time! To disable nags, add the Disable-Nags label.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot

Umm, this was already merged to 62 so not sure what's missing.

Just needs relabeling.

This bug has been closed for more than 14 weeks. Removing security view restrictions.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot