Able to reproduce this issue on Windows 10, Ubuntu 14.04  with chrome version 55.0.2883.87 and Mac 10.12.2 on chrome stable version 55.0.2883.95 and also in current canary version #57.0.2977.0
Issue is broken in M54.

Bisect Info:
===========
Good build : 54.0.2806.0,  Revision Range -407375
Bad build  : 54.0.2807.0,  Revision Range -407480

After executing the per-revision bisect script , i got the following CL's between good and bad build versions
===========================================
https://chromium.googlesource.com/chromium/src/+log/8cac25c661e66d12de27fb04d33605b7dce0ecd3..29b30bf368f96d99ea5067b66a9ac2b662f4510b


The suspecting Change Log is :
-----------
https://chromium.googlesource.com/v8/v8/+/682a41db33afada6052997f8d2ee5f20c3069787

From the above CL suspecting the below change
---------------------------
Review-Url: https://codereview.chromium.org/2173343002

dpranke@- Could you please look into this issue, if it's related to your change?  if not could you please help us to reassign this issue to the right owner.

I'd be kinda surprised if this caused the issue. I think this change would either be fine, or nothing would work at all.

Michael, thoughts?

Why not https://chromium.googlesource.com/v8/v8/+/ad6ea932272a1f8f74535c31882aaff3fb8ee6d4 from that V8 roll?

Jochen could you route this to an owner? nikolaos on cc.

Marja, can you triage please

Minimal repro case:

f = (e=({} = {} = 1)) => {}
f(1);

and nikolaos@'s commit is indeed the culprit. It's weird, the commit is supposedly just refactoring with no functional changes.


Investigating further.

I can take a look at it in the afternoon.  Sorry, I'm in a class right now.

Minimal repro case I found:

out.gn/x64.debug/d8 -e "((e = ({} = {} = {})) => {})()"

It seems that two {} assignments are necessary and the arrow function must
have a body (not an expression) for it to crash.

Guessing that the bug is that InitializerRewriter is now a subclass of AstTravelsalVisitor instead of AstExpressionVisitor, and now it does something differently so that ast numbering is confused later. (AstNumberingVisitor is still a subclass of AstVisitor; that didn't change by the CL.)

It's curious that this only happens for lazy arrow function params.

I'm trying to compile the alleged culprit (commit ad6ea93), back from July,
and gclient sync fails with a Python error:

...
________ running '/usr/bin/python
v8/build/linux/sysroot_scripts/install-sysroot.py --running-as-hook' in
'/data/nickie/gaa'
Installing Debian Wheezy amd64 root image:
/data/nickie/gaa/v8/build/linux/debian_wheezy_amd64-sysroot
Downloading
https://commondatastorage.googleapis.com/chrome-linux-sysroot/toolchain/c52471d9dec240c8d0a88fa98aa1eefeee32e22f/debian_wheezy_amd64_sysroot.tgz
...
OSError: [Errno 2] No such file or directory
Error: Command '/usr/bin/python
v8/build/linux/sysroot_scripts/install-sysroot.py --running-as-hook'
returned non-zero exit status 1 in /data/nickie/gaa

I also tried to use an older version of depot_tools (from July) but this
did not work either.

Any idea what's happening?

Some more debugging (but, nikolaos@, if you have time to take over, that'd be great):

Indeed, when renumbering, we get these expressions in AstNumberingVisitor::VisitReference:

Good:
VAR PROXY local[2] (mode = LET) "e"
VAR PROXY local[0] (mode = TEMPORARY) ""
VAR PROXY local[1] (mode = TEMPORARY) ""

Bad:
VAR PROXY local[1] (mode = LET) "e"
VAR PROXY local[0] (mode = TEMPORARY) ""
OBJ LITERAL at 13
. literal_index = 1

nikolaos@; I confirmed that the example fails at revision 37998 and succeeds at the previous revision 37997 so I'm pretty confident that it's the culprit.

Sorry, no idea about the gclient sync error; I was able to run gclient sync on both revisions (the culprit and the previous one) just fine. :/

Hmm, I wonder what file is not found.

I thought it was the debian_wheezy_amd64_sysroot.tgz
<https://commondatastorage.googleapis.com/chrome-linux-sysroot/toolchain/c52471d9dec240c8d0a88fa98aa1eefeee32e22f/debian_wheezy_amd64_sysroot.tgz>
that
was missing, but it seems to be there...  Anyway, I'm on it and I hope I'll
be able to compile it.

Great that you got it working. I think the error message is coming from being unable to open a *local* file, not being unable to download from the link.

I don't understand this bug yet but I'd like to. Maybe you can already tell me what's wrong, based on this:

In the good case, we call Parser::PatternRewriter::RewriteDestructuringAssignment twice (probably because of the foo = bar = baz construct), in the bad case only once.

The two calls in the good cases are coming from:

1)

#0  v8::internal::InitializerRewriter::VisitExpression (this=0x7fffffffb188, expr=0x1930800)
    at ../../src/parsing/parser.cc:4646
#1  0x0000000000db8e96 in v8::internal::AstExpressionVisitor::VisitRewritableExpression (
    this=0x7fffffffb188, expr=0x1930800) at ../../src/ast/ast-expression-visitor.cc:161
#2  0x0000000000db861e in v8::internal::AstTraversalVisitor::Visit (this=0x7fffffffb188, node=0x1930800)
    at ../../src/ast/ast.h:3066
#3  0x0000000000db8017 in v8::internal::AstExpressionVisitor::Run (this=0x7fffffffb188)
    at ../../src/ast/ast-expression-visitor.cc:23
#4  0x0000000000aaec5f in v8::internal::Parser::RewriteParameterInitializer (this=0x7fffffffc988, 
    expr=0x1930800, scope=0x1930850) at ../../src/parsing/parser.cc:4667
#5  0x0000000000aaef72 in v8::internal::Parser::BuildParameterInitializationBlock (this=0x7fffffffc988, 
    parameters=..., ok=0x7fffffffc1bf) at ../../src/parsing/parser.cc:4701
#6  0x0000000000aad3ba in v8::internal::Parser::ParseEagerFunctionBody (this=0x7fffffffc988, 
    function_name=0x0, pos=-1, parameters=..., kind=v8::internal::kArrowFunction, 
    function_type=v8::internal::FunctionLiteral::kAnonymousExpression, ok=0x7fffffffc1bf)
    at ../../src/parsing/parser.cc:4915
#7  0x0000000000ac310e in v8::internal::ParserTraits::ParseEagerFunctionBody (this=0x7fffffffc988, 
    name=0x0, pos=-1, parameters=..., kind=v8::internal::kArrowFunction, 
    function_type=v8::internal::FunctionLiteral::kAnonymousExpression, ok=0x7fffffffc1bf)
    at ../../src/parsing/parser.h:1175
#8  0x0000000000a9849a in v8::internal::ParserBase<v8::internal::ParserTraits>::ParseArrowFunctionLiteral (this=0x7fffffffc988, accept_IN=true, formal_parameters=..., is_async=false, formals_classifier=..., 
    ok=0x7fffffffc1bf) at ../../src/parsing/parser-base.h:3376

2)

#0  v8::internal::InitializerRewriter::VisitExpression (this=0x7fffffffb188, expr=0x1931720)
    at ../../src/parsing/parser.cc:4646
#1  0x0000000000db8706 in v8::internal::AstExpressionVisitor::VisitDoExpression (this=0x7fffffffb188, 
    expr=0x1931720) at ../../src/ast/ast-expression-visitor.cc:36
#2  0x0000000000db85ff in v8::internal::AstTraversalVisitor::Visit (this=0x7fffffffb188, node=0x1931720)
    at ../../src/ast/ast.h:3066
#3  0x0000000000dc975f in v8::internal::AstTraversalVisitor::VisitRewritableExpression (
    this=0x7fffffffb188, expr=0x1930800) at ../../src/ast/ast.cc:1219
#4  0x0000000000db8ea6 in v8::internal::AstExpressionVisitor::VisitRewritableExpression (
    this=0x7fffffffb188, expr=0x1930800) at ../../src/ast/ast-expression-visitor.cc:162
#5  0x0000000000db861e in v8::internal::AstTraversalVisitor::Visit (this=0x7fffffffb188, node=0x1930800)
    at ../../src/ast/ast.h:3066
#6  0x0000000000db8017 in v8::internal::AstExpressionVisitor::Run (this=0x7fffffffb188)
    at ../../src/ast/ast-expression-visitor.cc:23
#7  0x0000000000aaec5f in v8::internal::Parser::RewriteParameterInitializer (this=0x7fffffffc988, 
    expr=0x1930800, scope=0x1930850) at ../../src/parsing/parser.cc:4667
#8  0x0000000000aaef72 in v8::internal::Parser::BuildParameterInitializationBlock (this=0x7fffffffc988, 
    parameters=..., ok=0x7fffffffc1bf) at ../../src/parsing/parser.cc:4701
#9  0x0000000000aad3ba in v8::internal::Parser::ParseEagerFunctionBody (this=0x7fffffffc988, 
    function_name=0x0, pos=-1, parameters=..., kind=v8::internal::kArrowFunction, 
    function_type=v8::internal::FunctionLiteral::kAnonymousExpression, ok=0x7fffffffc1bf)
    at ../../src/parsing/parser.cc:4915
#10 0x0000000000ac310e in v8::internal::ParserTraits::ParseEagerFunctionBody (this=0x7fffffffc988, 
    name=0x0, pos=-1, parameters=..., kind=v8::internal::kArrowFunction, 
    function_type=v8::internal::FunctionLiteral::kAnonymousExpression, ok=0x7fffffffc1bf)
    at ../../src/parsing/parser.h:1175
#11 0x0000000000a9849a in v8::internal::ParserBase<v8::internal::ParserTraits>::ParseArrowFunctionLiteral (this=0x7fffffffc988, accept_IN=true, formal_parameters=..., is_async=false, formals_classifier=..., 
    ok=0x7fffffffc1bf) at ../../src/parsing/parser-base.h:3376

So maybe it's VisitRewritableExpression where the difference is?

Iow, maybe VisitRewritableExpression doesn't now recurse properly if there's another RewritableExpression inside it?

Aha, suspecting that either InitializerRewriter::VisitRewritableExpression should recurse properly, or we should have InitializerRewriter overwrite VisitExpression instead of VisitRewritableExpression. Preparing a fix...

Some more details about this:

State before the refactoring:

InitializerRewriter is a subclass of AstVisitor. It overrides VisitExpression. And AstVisitor::VisitRewritableExpression does this:

void AstExpressionVisitor::VisitRewritableExpression(
    RewritableExpression* expr) {
  VisitExpression(expr); << this will be InitializerRewriter::VisitExpression
  AstTraversalVisitor::VisitRewritableExpression(expr); << recurse!
}

After the refactoring, InitializerRewriter is a subclass over AstTraversalVisitor, and overrides VisitRewritableExpression directly. And it doesn't recurse.

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

commit aff64e9dfa436f62555325daf788e81f59b810fb
Author: marja <marja@chromium.org>
Date: Thu Jan 12 15:52:00 2017

Parser: Fix InitializerRewriter.

The bug was caused by AstTraversalVisitor refactoring:
https://codereview.chromium.org/2169833002/

InitializerRewriter::VisitRewritableExpression in parser.cc didn't recurse; so
it fails when a rewritable expression contains another rewritable expression.

See the bug for more details.

BUG= chromium:679727 

Review-Url: https://codereview.chromium.org/2629623002
Cr-Commit-Position: refs/heads/master@{#42274}

[modify] https://crrev.com/aff64e9dfa436f62555325daf788e81f59b810fb/src/parsing/parser.cc
[add] https://crrev.com/aff64e9dfa436f62555325daf788e81f59b810fb/test/mjsunit/regress/regress-679727.js

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

commit aff64e9dfa436f62555325daf788e81f59b810fb
Author: marja <marja@chromium.org>
Date: Thu Jan 12 15:52:00 2017

Parser: Fix InitializerRewriter.

The bug was caused by AstTraversalVisitor refactoring:
https://codereview.chromium.org/2169833002/

InitializerRewriter::VisitRewritableExpression in parser.cc didn't recurse; so
it fails when a rewritable expression contains another rewritable expression.

See the bug for more details.

BUG= chromium:679727 

Review-Url: https://codereview.chromium.org/2629623002
Cr-Commit-Position: refs/heads/master@{#42274}

[modify] https://crrev.com/aff64e9dfa436f62555325daf788e81f59b810fb/src/parsing/parser.cc
[add] https://crrev.com/aff64e9dfa436f62555325daf788e81f59b810fb/test/mjsunit/regress/regress-679727.js

Thanks Marja, for taking care of this.

Alright, fixed. Thanks for the bug report, OP, this was a good bug report. And a great catch!

We should merge this fix back to at least 56 when it has Canary coverage.

Now there's probably canary coverage; how do we merge it back?

(Otoh, there were quite many stable revisions already where this was broken... half a year worth of stable revisions, so, around 4 +- 1.)