Note: this isn't just dev tools, the tab also crashes when this kind of syntax is part of the page: http://output.jsbin.com/nunovut.

Able to reproduce this issue on Windows-10 and Ubuntu 14.04 using chrome stable #57.0.2987.110. By typing the step-2 code in the console observed crash.

By testing the same on #53.0.2766.0 it displays syntax error in the console.

Report Id: 04e601f960000000

Stack Trace:
-------------
Thread 0 CRASHED [SIGSEGV @ 0x00000010 ] MAGIC SIGNATURE THREAD
Stack Quality85%Show frame trust levels
0x00007f25596e2521	(chrome -./out/Release/../../v8/src/ast/ast.h:1645 )	v8::internal::AstNumberingVisitor::VisitAssignment(v8::internal::Assignment*)
0x00007f25596e1b3e	(chrome -./out/Release/../../v8/src/ast/ast-numbering.cc )	v8::internal::AstNumberingVisitor::VisitNoStackOverflowCheck(v8::internal::AstNode*)
0x00007f25596e1c01	(chrome -./out/Release/../../v8/src/ast/ast-numbering.cc:87 )	v8::internal::AstNumberingVisitor::VisitNoStackOverflowCheck(v8::internal::AstNode*)
0x00007f25596e2584	(chrome -./out/Release/../../v8/src/ast/ast-numbering.cc:87 )	v8::internal::AstNumberingVisitor::VisitAssignment(v8::internal::Assignment*)
0x00007f25596e1b3e	(chrome -./out/Release/../../v8/src/ast/ast-numbering.cc )	v8::internal::AstNumberingVisitor::VisitNoStackOverflowCheck(v8::internal::AstNode*)
0x00007f25596e0ddf	(chrome -./out/Release/../../v8/src/ast/ast-numbering.cc:87 )	v8::internal::AstNumberingVisitor::VisitNoStackOverflowCheck(v8::internal::AstNode*)
0x00007f25596e2efc	(chrome -./out/Release/../../v8/src/ast/ast-numbering.cc:87 )	v8::internal::AstNumberingVisitor::Renumber(v8::internal::FunctionLiteral*)
0x00007f25596e38a2	(chrome -./out/Release/../../v8/src/ast/ast-numbering.cc:663 )	v8::internal::AstNumbering::Renumber(unsigned long, v8::internal::Zone*, v8::internal::FunctionLiteral*, v8::internal::ThreadedList<v8::internal::ThreadedListZoneEntry<v8::internal::FunctionLiteral*> >*)
0x00007f25597b486c	(chrome -./out/Release/../../v8/src/compiler.cc:441 )	v8::internal::(anonymous namespace)::Renumber(v8::internal::ParseInfo*, v8::internal::ThreadedList<v8::internal::ThreadedListZoneEntry<v8::internal::FunctionLiteral*> >*)
0x00007f25597b69b6	(chrome -./out/Release/../../v8/src/compiler.cc:1133 )	v8::internal::(anonymous namespace)::GetUnoptimizedCode(v8::internal::CompilationInfo*)
0x00007f25597b507a	(chrome -./out/Release/../../v8/src/compiler.cc:1037 )	v8::internal::Compiler::Compile(v8::internal::Handle<v8::internal::JSFunction>, v8::internal::Compiler::ClearExceptionFlag)
0x00007f2559c147ca	(chrome -./out/Release/../../v8/src/runtime/runtime-compiler.cc:37 )	v8::internal::Runtime_CompileLazy(int, v8::internal::Object**, v8::internal::Isolate*)
0x00002b07e9584426		
0x00002b07e95847d7		
0x00002b07e9585f14		
0x00002b07e96a76d9		
0x00002b07e9620a82		
0x00002b07e95adb60		
0x00007f25599c6385	(chrome -./out/Release/../../v8/src/execution.cc:144 )	v8::internal::(anonymous namespace)::Invoke(v8::internal::Isolate*, bool, v8::internal::Handle<v8::internal::Object>, v8::internal::Handle<v8::internal::Object>, int, v8::internal::Handle<v8::internal::Object>*, v8::internal::Handle<v8::internal::Object>, v8::internal::Execution::MessageHandling)
0x00007f25599c60da	(chrome -./out/Release/../../v8/src/execution.cc:180 )	v8::internal::Execution::Call(v8::internal::Isolate*, v8::internal::Handle<v8::internal::Object>, v8::internal::Handle<v8::internal::Object>, int, v8::internal::Handle<v8::internal::Object>*)
0x00007f255969bb5b	(chrome -./out/Release/../../v8/src/api.cc:2038 )	v8::Script::Run(v8::Local<v8::Context>)
0x00007f255c8de3d9	(chrome -./out/Release/../../third_party/WebKit/Source/bindings/core/v8/V8ScriptRunner.cpp:536 )	blink::V8ScriptRunner::runCompiledScript(v8::Isolate*, v8::Local<v8::Script>, blink::ExecutionContext*)
0x00007f255c8beea2	(chrome -./out/Release/../../third_party/WebKit/Source/bindings/core/v8/ScriptController.cpp:150 )	blink::ScriptController::executeScriptAndReturnValue(v8::Local<v8::Context>, blink::ScriptSourceCode const&, blink::AccessControlStatus)
0x00007f255c8bff07	(chrome -./out/Release/../../third_party/WebKit/Source/bindings/core/v8/ScriptController.cpp:377 )	blink::ScriptController::evaluateScriptInMainWorld(blink::ScriptSourceCode const&, blink::AccessControlStatus, blink::ScriptController::ExecuteScriptPolicy)
0x00007f255c8c0027	(chrome -./out/Release/../../third_party/WebKit/Source/bindings/core/v8/ScriptController.cpp:350 )	blink::ScriptController::executeScriptInMainWorld(blink::ScriptSourceCode const&, blink::AccessControlStatus)
0x00007f255e69e0ca	(chrome -./out/Release/../../third_party/WebKit/Source/core/dom/ScriptLoader.cpp:546 )	blink::ScriptLoader::doExecuteScript(blink::ScriptSourceCode const&)
0x00007f255e69f3df	(chrome -./out/Release/../../third_party/WebKit/Source/core/dom/ScriptLoader.cpp:431 )	blink::ScriptLoader::prepareScript(WTF::TextPosition const&, blink::ScriptLoader::LegacyTypeSupport)
0x00007f255ce78a20	(chrome -./out/Release/../../third_party/WebKit/Source/core/html/parser/HTMLParserScriptRunner.cpp:526 )	blink::HTMLParserScriptRunner::processScriptElementInternal(blink::Element*, WTF::TextPosition const&)
0x00007f255ce7872a	(chrome -./out/Release/../../third_party/WebKit/Source/core/html/parser/HTMLParserScriptRunner.cpp:362 )	blink::HTMLParserScriptRunner::processScriptElement(blink::Element*, WTF::TextPosition const&)
0x00007f255ce6fcc5	(chrome -./out/Release/../../third_party/WebKit/Source/core/html/parser/HTMLDocumentParser.cpp:285 )	blink::HTMLDocumentParser::processTokenizedChunkFromBackgroundParser(std::unique_ptr<blink::HTMLDocumentParser::TokenizedChunk, std::default_delete<blink::HTMLDocumentParser::TokenizedChunk> >)
0x00007f255ce6df71	(chrome -./out/Release/../../third_party/WebKit/Source/core/html/parser/HTMLDocumentParser.cpp:617 )	blink::HTMLDocumentParser::pumpPendingSpeculations()
0x00007f255c7bfc54	(chrome -./out/Release/../../base/callback.h:85 )	blink::TaskHandle::Runner::run(blink::TaskHandle const&)
0x00007f255ab47d8d	(chrome -./out/Release/../../base/callback.h:68 )	base::debug::TaskAnnotator::RunTask(char const*, base::PendingTask*)
0x00007f255c8026bb	(chrome -./out/Release/../../third_party/WebKit/Source/platform/scheduler/base/task_queue_manager.cc:377 )	blink::scheduler::TaskQueueManager::ProcessTaskFromWorkQueue(blink::scheduler::internal::WorkQueue*, blink::scheduler::LazyNow, base::TimeTicks*)
0x00007f255c801b45	(chrome -./out/Release/../../third_party/WebKit/Source/platform/scheduler/base/task_queue_manager.cc:245 )	blink::scheduler::TaskQueueManager::DoWork(base::TimeTicks, bool)
0x00007f255ab47d8d	(chrome -./out/Release/../../base/callback.h:68 )	base::debug::TaskAnnotator::RunTask(char const*, base::PendingTask*)
0x00007f255aadec3f	(chrome -./out/Release/../../base/message_loop/message_loop.cc:421 )	base::MessageLoop::RunTask(base::PendingTask*)
0x00007f255aade734	(chrome -./out/Release/../../base/message_loop/message_loop.cc:430 )	base::MessageLoop::DoWork()
0x00007f255aae0254	(chrome -./out/Release/../../base/message_loop/message_pump_default.cc:33 )	base::MessagePumpDefault::Run(base::MessagePump::Delegate*)
0x00007f255aafccdf	(chrome -./out/Release/../../base/run_loop.cc:37 )	base::RunLoop::Run()
0x00007f255d7466f0	(chrome -./out/Release/../../content/renderer/renderer_main.cc:200 )	content::RendererMain(content::MainFunctionParams const&)
0x00007f255a75e97b	(chrome -./out/Release/../../content/app/content_main_runner.cc:344 )	content::RunZygote(content::MainFunctionParams const&, content::ContentMainDelegate*)
0x00007f255a75facb	(chrome -./out/Release/../../content/app/content_main_runner.cc:793 )	content::ContentMainRunnerImpl::Run()
0x00007f255a75c28d	(chrome -./out/Release/../../content/app/content_main.cc:20 )	content::ContentMain(content::ContentMainParams const&)
0x00007f25593c3d00	(chrome -./out/Release/../../chrome/app/chrome_main.cc:112 )	ChromeMain
0x00007f2551ea3f44	(libc-2.19.so -libc-start.c:287 )	__libc_start_main
0x00007f25593c3b60	(chrome + 0x00b45b60 )	_start

Investigating

Reproes w/ d8 ToT.

The failure is:

void AstNumberingVisitor::VisitReference(Expression* expr) {
  DCHECK(expr->IsProperty() || expr->IsVariableProxy()); << this fails
  // code here which assumes it's true

This is very similar to  bug 679727 :

Minimal repro for that one:
f = (e=({} = {} = 1)) => {}
f(1);

And the bug was introduced by https://chromium.googlesource.com/v8/v8/+/ad6ea932272a1f8f74535c31882aaff3fb8ee6d4 

But that one was fixed..

Going to investigate further whether this is related or just a coincidence.

... Also, this is not invalid syntax like OP claims.

function foo(x = {} = {}) {
  print(x);
}

foo();

v8$ out/Debug/d8 ../foo170.js
[object Object]

I missed that there are { } surrounding the thing so it's one level deeper. But it's valid syntax anyway:

function foo({x = {a} = {a: 1}}) {
  print(x);
  if ('a' in x) {
    print(x.a);
  }
}

foo({});
foo({x: {a: 13} });

v8$ out/Debug/d8 ../foo170.js 
[object Object]
1
[object Object]
13

This has the potential to become a RB-Stable if we get more reports.

adding cbruni@ as on-duty stab sheriff
adding machenbach@ regarding a case the correctness fuzzer missed.

If its a regression in M57 we should apply RB-Stable, if not it is not a blocker IMO.

This is not regression in M57. Some commits from mid-December are already bad.

I'm trying to bisect this but I bumped into the "drop FLAG_min_preparse_length" thing, so, some commits seemed good because the code snippet was so small. So I need to re-run my bisect.

Omg, this predates even the suspected refactoring in July (revision 37998).

Note that for testing the older revisions, it necessary to make the source file long enough, so that lazy parsing kicks in.

Current working theory is that this never worked.

The bug is that when the arrow function is lazy, default values inside the destructuring object literal thingy are not rewritten properly. I have no idea yet why, since *something* is rewritten, just not deeply enough (but why does the parser vs. preparser affect how deep?)

----------------


Debugging notes:

Parser produces this Assignment:

. VAR PROXY local[2] (mode = LET) "x"
. CONDITIONAL at -1
. . CONDITION at -1
. . . EQ_STRICT at -1
. . . . VAR PROXY local[1] (mode = TEMPORARY) ""
. . . . LITERAL undefined
. . THEN at 407
. . . DO EXPRESSION at 407
. . . . EXPRESSION STATEMENT at -1
. . . . . ASSIGN at -1
. . . . . . VAR PROXY local[0] (mode = TEMPORARY) ""
. . . . . . OBJ LITERAL at 409
. . . . . . . literal_slot = -1
. . . . IF at -1
. . . . . CONDITION at -1
. . . . . . OR at -1
. . . . . . . EQ_STRICT at -1
. . . . . . . . VAR PROXY local[0] (mode = TEMPORARY) ""
. . . . . . . . LITERAL undefined
. . . . . . . EQ_STRICT at -1
. . . . . . . . VAR PROXY local[0] (mode = TEMPORARY) ""
. . . . . . . . LITERAL null
. . . . . THEN at -1
. . . . . . EXPRESSION STATEMENT at -1
. . . . . . . THROW at -1
. . . . . . . . CALL RUNTIME NewTypeError at -1
. . . . . . . . . LITERAL 59
. . . . . . . . . LITERAL ""
. . ELSE at -1
. . . VAR PROXY local[1] (mode = TEMPORARY) ""

Which is roughly:

x = (temp1 === undefined ? do { temp0 = {}; if (temp0 === undefined || temp0 === null) { throw NewTypeError; } } : temp1);

And PreParser produces this:

. VAR PROXY local[2] (mode = LET) "x"
. CONDITIONAL at -1
. . CONDITION at -1
. . . EQ_STRICT at -1
. . . . VAR PROXY local[1] (mode = TEMPORARY) ""
. . . . LITERAL undefined
. . THEN at 407
. . . ASSIGN at 407
. . . . OBJ LITERAL at 404
. . . . . literal_slot = -1
. . . . OBJ LITERAL at 409
. . . . . literal_slot = -1
. . ELSE at -1
. . . VAR PROXY local[1] (mode = TEMPORARY) ""

Which is roughly:

x = (temp1 === undefined ? {} = {} : temp1);

The {} = {} assignment causes the crash later when we do AstNumbering::VisitAssignment.

I.e., we do rewrite some stuff but not deep enough.

Last debugging notes:

It's not about PreParser vs. Parser but *re*parsing vs. parsing - when the lazy arrow func params are reparsed, stuff goes wrong.

We do call BuildParameterInitializationBlock there too. Suspecting it has something to do with RewriteDestructuringAssignments not finding stuff to rewrite in the bad case.

Suspecting the reason is that QueueDestructuringAssignmentForRewriting is not called when the assignment in question occurs in lazy arrow function params. Going to continue from here on Monday :)

I will take a look at it too during the weekend.

> Note that for testing the older revisions, it necessary to make the source file long enough, so that lazy parsing kicks in.

@marja, in older versions there used to be a --min_preparse_length flag.  Instead of making the source file longer, I think you can force pre-parsing by setting that to something small (e.g. 1).

I have a fix. The problem is that destructuring assignments which occur in the parameter list are not rewritten by DoParseFunction. ParseArrowFunctionLiteral does some rewriting but that only concerns the FunctionState surrounding the body. The destructuring assignments in the parameters are not in that FunctionState.

Re comment 16, of course min-preparse-length works too but just making the source file long is enough is less error prone :)

The fix is here https://chromium-review.googlesource.com/459618 in case somebody wants to review.

But no rush - we have had this crash bug like... 3 years?

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

commit bc39a5148a3824ea948fd7725674945ca0b1c56a
Author: Marja Hölttä <marja@chromium.org>
Date: Tue Mar 28 08:22:46 2017

[parser] Fix crash when lazy arrow func params contain destructuring assignments.

As far as I can see, we have had this bug as long as destructuring assignments
have been there (i.e., this is not regression).

The problem was that Parser::DoParseFunction parsed the arrow function parameters
but didn't rewrite the destructuring assignments in them.

BUG= chromium:704811 

Change-Id: I0b1424e7d5103eda6efd51b403fe81a4ee235e01
Reviewed-on: https://chromium-review.googlesource.com/459618
Commit-Queue: Marja Hölttä <marja@chromium.org>
Reviewed-by: Adam Klein <adamk@chromium.org>
Cr-Commit-Position: refs/heads/master@{#44177}
[modify] https://crrev.com/bc39a5148a3824ea948fd7725674945ca0b1c56a/src/parsing/parser.cc
[add] https://crrev.com/bc39a5148a3824ea948fd7725674945ca0b1c56a/test/mjsunit/regress/regress-704811.js

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

commit 01d19f706ceb293d0449d80d0db7389cc509aea0
Author: Marja Hölttä <marja@chromium.org>
Date: Tue Apr 11 08:19:48 2017

Merged: Squashed multiple commits.

Merged: [parser] Fix crash when lazy arrow func params contain destructuring assignments.
Revision: bc39a5148a3824ea948fd7725674945ca0b1c56a

Merged: [parser] don't rewrite destructuring assignments in params for lazy top level arrow functions
Revision: 5f782db9541eca72dc6cbe3c6f5e8dc52ff7c16f

BUG= chromium:704811 , chromium:706234 , chromium:706761 , v8:6182 
LOG=N
NOTRY=true
NOPRESUBMIT=true
NOTREECHECKS=true

Change-Id: If5c04c3b9f6ac9c6879052b6a34446f895624200
Reviewed-on: https://chromium-review.googlesource.com/474746
Reviewed-by: Michael Achenbach <machenbach@chromium.org>
Cr-Commit-Position: refs/branch-heads/5.8@{#58}
Cr-Branched-From: eda659cc5e307f20ac1ad542ba12ab32eaf4c7ef-refs/heads/5.8.283@{#1}
Cr-Branched-From: 4310cd02d2160b1457baed81a2f40063eb264a21-refs/heads/master@{#43429}
[modify] https://crrev.com/01d19f706ceb293d0449d80d0db7389cc509aea0/src/ast/scopes.cc
[modify] https://crrev.com/01d19f706ceb293d0449d80d0db7389cc509aea0/src/parsing/parser-base.h
[modify] https://crrev.com/01d19f706ceb293d0449d80d0db7389cc509aea0/src/parsing/parser.cc
[add] https://crrev.com/01d19f706ceb293d0449d80d0db7389cc509aea0/test/mjsunit/regress/regress-704811.js
[add] https://crrev.com/01d19f706ceb293d0449d80d0db7389cc509aea0/test/mjsunit/regress/regress-706234-2.js
[add] https://crrev.com/01d19f706ceb293d0449d80d0db7389cc509aea0/test/mjsunit/regress/regress-706234.js