V8 Fatal Exception, 0x99b617 #11977

duereg · 2017-03-21T21:49:48Z

Version: Node, 6.10.1
Platform: Linux, version 4.4.27

#
# Fatal error in , line 0
# unreachable code
#

==== C stack trace ===============================

 1: V8_Fatal
 2: 0x99b617
 3: v8::internal::AstNumberingVisitor::VisitArrayLiteral(v8::internal::ArrayLiteral*)
 4: v8::internal::AstNumberingVisitor::VisitConditional(v8::internal::Conditional*)
 5: v8::internal::AstNumberingVisitor::VisitObjectLiteral(v8::internal::ObjectLiteral*)
 6: v8::internal::AstNumberingVisitor::VisitAssignment(v8::internal::Assignment*)
 7: v8::internal::AstNumberingVisitor::VisitBlock(v8::internal::Block*)
 8: v8::internal::AstNumberingVisitor::VisitBlock(v8::internal::Block*)
 9: v8::internal::AstNumberingVisitor::Renumber(v8::internal::FunctionLiteral*)
10: v8::internal::AstNumbering::Renumber(v8::internal::Isolate*, v8::internal::Zone*, v8::internal::FunctionLiteral*)
11: v8::internal::Compiler::Analyze(v8::internal::ParseInfo*)
12: 0xb4e248
13: 0xb55966
14: v8::internal::Compiler::Compile(v8::internal::Handle<v8::internal::JSFunction>, v8::internal::Compiler::ClearExceptionFlag)
15: v8::internal::Accessors::FunctionLengthGetter(v8::Local<v8::Name>, v8::PropertyCallbackInfo<v8::Value> const&)
16: 0x72825232722
Illegal instruction (core dumped)

The text was updated successfully, but these errors were encountered:

mscdex · 2017-03-21T22:05:34Z

Do you have a simple example to reproduce the crash?

/cc @nodejs/v8

mscdex · 2017-03-21T22:08:32Z

Additionally, you may want to file an issue over at https://bugs.chromium.org/p/v8/.

duereg · 2017-03-21T22:48:57Z

@mscdex I could probably come up with a simple use case. I'll file an issue with the v8 folks. This only happens in 6.10.1 as far as I can see (downgrading to 6.9 resolved the issue).

duereg · 2017-03-21T23:00:05Z

https://bugs.chromium.org/p/v8/issues/detail?id=6131

mscdex · 2017-03-21T23:06:33Z

By 6.9 do you mean 6.9.5 or 6.9.0? Also, does 6.10.0 exhibit the same problem?

mscdex · 2017-03-21T23:14:32Z

FWIW

These V8-related PRs were merged in v6.10.0:

These V8-related PRs were merged in v6.10.1:

duereg · 2017-03-22T16:48:00Z

6.10.0 does no exhibit the same problem. 6.9.0 and 6.9.5 also do not exhibit the same problem. I have a reproducible test case, but I need to streamline it to just the minimum elements. This looks to be isolated to 6.10.1.

In the v8 issue, the chromium team stated that the version of v8 used by 6.10.1 is no longer supported, and this issue is not present in later version of v8.

https://bugs.chromium.org/p/v8/issues/detail?id=6131#c2

gibfahn · 2017-03-22T16:58:00Z

@duereg if you could include a way of reproducing that would be helpful.

Also which version of Linux?

mscdex · 2017-03-22T16:58:47Z

If you're feeling adventurous, you could try reverting each of those 3 V8-related commits, one at a time, and see which one is causing the problem.

duereg · 2017-03-22T20:06:23Z

I have a way of reproducing - I can reproduce it locally on my Mac, as well on our CI environment, which is:

Platform: Linux, version 4.4.27

So I believe it might be platform agnostic.

duereg · 2017-03-22T20:08:10Z

@gibfahn I'm going to try to create a simple gist to recreate tonight.
@mscdex I'm never run node locally from a non-binary. Depends on how adventurous I'm feeling, I might try this weekend

gibfahn · 2017-03-22T23:13:19Z

@duereg cheers. FWIW, it should be just ./configure && make -j4 to rebuild, then use the ./node symlink in the directory to run your testcase.

mscdex · 2017-03-22T23:50:20Z

If you want to test reverting one of the V8 commits, just checkout the v6.10.1 tag, then git revert <commit hash>, recompile, and test the newly compiled executable with your script.

The commits to try reverting for #9293 are:

47cbb88
0eb4f1d
a38d091
daa56a6

The commit to try reverting for #10733 is 3fc6a22.

The commit to try reverting for #11483 is 3ab070d.

duereg · 2017-03-23T19:48:56Z

@mscdex reverting 3ab070d resolved the issue.

mscdex · 2017-03-23T21:20:48Z

/cc @targos ^

AndrewBarba · 2017-03-23T23:06:52Z

We just ran into this as well. Upgrading from 6.10.0 to 6.10.1. Downgrading to 6.10.0 fixes the issue

bnoordhuis · 2017-03-24T09:26:19Z

@duereg Did you have a chance to put that gist together?

targos · 2017-03-24T09:29:31Z

I ran the oca test with a debug build. here is the stack trace:

Fatal error in ../deps/v8/src/ast/ast-numbering.cc, line 368

#
# Fatal error in ../deps/v8/src/ast/ast-numbering.cc, line 368
# unreachable code
#

==== C stack trace ===============================

 1: V8_Fatal
 2: 0x12e3e9d
 3: v8::internal::Spread::Accept(v8::internal::AstVisitor*)
 4: v8::internal::AstNumberingVisitor::Visit(v8::internal::AstNode*)
 5: v8::internal::AstNumberingVisitor::VisitArrayLiteral(v8::internal::ArrayLiteral*)
 6: v8::internal::ArrayLiteral::Accept(v8::internal::AstVisitor*)
 7: v8::internal::AstNumberingVisitor::Visit(v8::internal::AstNode*)
 8: v8::internal::AstNumberingVisitor::VisitRewritableExpression(v8::internal::RewritableExpression*)
 9: v8::internal::RewritableExpression::Accept(v8::internal::AstVisitor*)
10: v8::internal::AstNumberingVisitor::Visit(v8::internal::AstNode*)
11: v8::internal::AstNumberingVisitor::VisitBinaryOperation(v8::internal::BinaryOperation*)
12: v8::internal::BinaryOperation::Accept(v8::internal::AstVisitor*)
13: v8::internal::AstNumberingVisitor::Visit(v8::internal::AstNode*)
14: v8::internal::AstNumberingVisitor::VisitAssignment(v8::internal::Assignment*)
15: v8::internal::Assignment::Accept(v8::internal::AstVisitor*)
16: v8::internal::AstNumberingVisitor::Visit(v8::internal::AstNode*)
17: v8::internal::AstNumberingVisitor::VisitExpressionStatement(v8::internal::ExpressionStatement*)
18: v8::internal::ExpressionStatement::Accept(v8::internal::AstVisitor*)
19: v8::internal::AstNumberingVisitor::Visit(v8::internal::AstNode*)
20: v8::internal::AstNumberingVisitor::VisitStatements(v8::internal::ZoneList<v8::internal::Statement*>*)
21: v8::internal::AstNumberingVisitor::VisitBlock(v8::internal::Block*)
22: v8::internal::Block::Accept(v8::internal::AstVisitor*)
23: v8::internal::AstNumberingVisitor::Visit(v8::internal::AstNode*)
24: v8::internal::AstNumberingVisitor::VisitStatements(v8::internal::ZoneList<v8::internal::Statement*>*)
25: v8::internal::AstNumberingVisitor::VisitBlock(v8::internal::Block*)
26: v8::internal::Block::Accept(v8::internal::AstVisitor*)
27: v8::internal::AstNumberingVisitor::Visit(v8::internal::AstNode*)
28: v8::internal::AstNumberingVisitor::VisitStatements(v8::internal::ZoneList<v8::internal::Statement*>*)
29: v8::internal::AstNumberingVisitor::Renumber(v8::internal::FunctionLiteral*)
30: v8::internal::AstNumbering::Renumber(v8::internal::Isolate*, v8::internal::Zone*, v8::internal::FunctionLiteral*)
31: 0x15fdb21
32: v8::internal::Compiler::Analyze(v8::internal::ParseInfo*)
33: 0x15fd237
34: 0x15fd597
35: 0x15feee9
36: v8::internal::Compiler::Compile(v8::internal::Handle<v8::internal::JSFunction>, v8::internal::Compiler::ClearExceptionFlag)
37: 0x1a788de
38: v8::internal::Runtime_CompileLazy(int, v8::internal::Object**, v8::internal::Isolate*)
39: 0xa362e0092a7

node/deps/v8/src/ast/ast-numbering.cc

Line 368 in 6803521

void AstNumberingVisitor::VisitSpread(Spread* node) { UNREACHABLE(); }

bnoordhuis · 2017-03-24T11:23:50Z

We probably need v8/v8@a328143e and possibly v8/v8@96220730 too.

psmarshall · 2017-03-24T12:49:52Z

@bnoordhuis Those two commits aren't bugfixes so I don't think they are needed to fix this problem. Although they provide an implementation for AstNumberingVisitor::VisitSpread which is where the crash occurs, the idea would be to avoid reaching VisitSpread() in the first place.

targos · 2017-03-24T13:02:43Z

Yeah I don't think those will fix the problem. It looks like the crash happens with an array literal spread, not a spread call.
It would be really useful to have a small testcase with the syntax that triggers this.

duereg · 2017-03-24T14:57:09Z

@targos @mscdex @bnoordhuis

Sorry for the delay on the gist... I was trying to figure out a way to recreate the issue without mocha, but I could not, so I created a simple test suite.

https://gist.github.com/duereg/8e398f05e962af0965053d8ddf827165

targos · 2017-03-24T15:12:42Z

@duereg thank you. what is your babel config and version?

Edit: I don't know how to reproduce. I tried to install babel-register and babel-plugin-transform-es2015-modules-commonjs

duereg · 2017-03-24T15:26:51Z

@targos updated gist with .babelrc

targos · 2017-03-24T15:42:22Z

I'm sorry, I still can't reproduce.
--compilers js:babel/register doesn't work with babel 6 and if I replace it with --compilers js:babel-register there is no crash.

duereg · 2017-03-24T17:41:56Z

Our mocha runner is wrapped in a gulp process. I was hoping to not have to figure that out as well. I'll update the gist to take that into account.

duereg · 2017-03-24T17:42:13Z

I also cannot get that test case to fail without the gulp runner. The issue seems to lie with the instrumentation code that calculates test coverage, that sits atop of all of this. :/

duereg · 2017-03-24T19:17:28Z

@targos updated gist. Download, install, run, everything explodes

gibfahn · 2017-03-24T23:19:41Z

Reproducible for me (macOS Sierra):

Stack Trace

==== C stack trace ===============================

 1: V8_Fatal
 2: v8::internal::AstNumberingVisitor::VisitSpread(v8::internal::Spread*)
 3: v8::internal::AstNumberingVisitor::VisitArrayLiteral(v8::internal::ArrayLiteral*)
 4: v8::internal::AstNumberingVisitor::VisitRewritableExpression(v8::internal::RewritableExpression*)
 5: v8::internal::AstNumberingVisitor::VisitBinaryOperation(v8::internal::BinaryOperation*)
 6: v8::internal::AstNumberingVisitor::VisitConditional(v8::internal::Conditional*)
 7: v8::internal::AstNumberingVisitor::VisitObjectLiteral(v8::internal::ObjectLiteral*)
 8: v8::internal::AstNumberingVisitor::VisitAssignment(v8::internal::Assignment*)
 9: v8::internal::AstNumberingVisitor::VisitExpressionStatement(v8::internal::ExpressionStatement*)
10: v8::internal::AstNumberingVisitor::VisitBlock(v8::internal::Block*)
11: v8::internal::AstNumberingVisitor::Renumber(v8::internal::FunctionLiteral*)
12: v8::internal::AstNumbering::Renumber(v8::internal::Isolate*, v8::internal::Zone*, v8::internal::FunctionLiteral*)
13: v8::internal::Compiler::Analyze(v8::internal::ParseInfo*)
14: v8::internal::(anonymous namespace)::GetUnoptimizedCodeCommon(v8::internal::CompilationInfo*)
15: v8::internal::Compiler::Compile(v8::internal::Handle<v8::internal::JSFunction>, v8::internal::Compiler::ClearExceptionFlag)
16: v8::internal::Runtime_CompileLazy(int, v8::internal::Object**, v8::internal::Isolate*)
17: 0x15fb64a092a7
18: 0x15fb64a34338
19: 0x15fb65055791
20: 0x15fb65063a3a
[1]    35429 illegal hardware instruction (core dumped)  gulp

EDIT: Indeed, it doesn't fail if you take out Istanbul, i.e. with:

this diff

diff --git a/gulpfile.babel.js b/gulpfile.babel.js
index d28b64e..0fa981a 100644
--- a/gulpfile.babel.js
+++ b/gulpfile.babel.js
@@ -1,23 +1,12 @@
 import gulp from 'gulp';
-import istanbul from 'gulp-istanbul';
 import {Instrumenter} from 'isparta';
 import mocha from 'gulp-mocha';
 
 function runTestsWithCoverage(cb) {
   return gulp.src(['./brokenNodeTest.js'])
-    .pipe(istanbul({ // Covering files
-      instrumenter: Instrumenter,
-      includeUntested: true
-    }))
-    .pipe(istanbul.hookRequire()) // Force `require` to return covered files
     .on('finish', function finish() {
       gulp.src(["./brokenNodeTest.spec.js"], {read: false})
         .pipe(mocha({reporter: 'spec', timeout: 15000}))
-        .pipe(istanbul.writeReports({
-          dir: "coverage",
-          reportOpts: {dir: "coverage"},
-          reporters: ['text', 'text-summary', 'json', 'html']
-        }))
         .on('end', console.error)
         .on('error', console.error);
     });

targos · 2017-03-25T09:38:27Z

Thank you. Here is a reduced testcase based on this:

'use strict';
let a = 0;
const DEFAULT_FLAGS = [1];
const query = {
  flags: true ? (a++,
  [...DEFAULT_FLAGS, 2]) : (a++,
  DEFAULT_FLAGS)
};

targos · 2017-03-25T11:22:36Z

Even shorter:

(42, [...[0], 1]);

targos · 2017-03-25T12:34:47Z

The crash also happens if I build V8 at the commit that I backported. I'm going to bisect.

Original commit message: Fix classifier related bug [email protected] BUG=chromium:621111 LOG=N Review-Url: https://codereview.chromium.org/2086513002 Cr-Commit-Position: refs/heads/master@{nodejs#37150} Fixes: nodejs#11977

Original commit message: Fix classifier related bug [email protected] BUG=chromium:621111 LOG=N Review-Url: https://codereview.chromium.org/2086513002 Cr-Commit-Position: refs/heads/master@{#37150} Fixes: #11977 PR-URL: #12037 Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: Myles Borins <[email protected]>

duereg · 2017-04-06T22:55:15Z

@targos has this been resolved in 6.10.2?

targos · 2017-04-07T08:16:04Z

@duereg Yes. Thanks for the report!

targos · 2017-04-07T08:17:27Z

Fix landed in 52bdb8f

Fishrock123 added the v8 engine Issues and PRs related to the V8 dependency. label Mar 21, 2017

mscdex added the v6.x label Mar 21, 2017

Saviio mentioned this issue Mar 24, 2017

bugfix teambition/ReactiveDB#65

Merged

This was referenced Mar 24, 2017

V6.x crash with illegal spread as single arrow parameter #12017

Closed

V6.x crash with holes following spreads in array literals #12018

Closed

targos mentioned this issue Mar 25, 2017

v6.x: backport V8 fixes for parser bugs #12037

Closed

3 tasks

MylesBorins mentioned this issue Mar 28, 2017

Regressions in v4.8.1 and v6.10.1 nodejs/Release#193

Closed

STRML mentioned this issue Mar 30, 2017

Remove Node 6.10.1 and revert to 6.10.0 nodesource/distributions#436

Closed

targos closed this as completed Apr 7, 2017

V8 Fatal Exception, 0x99b617 #11977

V8 Fatal Exception, 0x99b617 #11977

Comments

duereg commented Mar 21, 2017 • edited Loading

mscdex commented Mar 21, 2017 • edited Loading

mscdex commented Mar 21, 2017

duereg commented Mar 21, 2017

duereg commented Mar 21, 2017

mscdex commented Mar 21, 2017

mscdex commented Mar 21, 2017 • edited Loading

duereg commented Mar 22, 2017

gibfahn commented Mar 22, 2017

mscdex commented Mar 22, 2017

duereg commented Mar 22, 2017

duereg commented Mar 22, 2017

gibfahn commented Mar 22, 2017

mscdex commented Mar 22, 2017

duereg commented Mar 23, 2017 • edited Loading

mscdex commented Mar 23, 2017

AndrewBarba commented Mar 23, 2017

bnoordhuis commented Mar 24, 2017

targos commented Mar 24, 2017

bnoordhuis commented Mar 24, 2017

psmarshall commented Mar 24, 2017

targos commented Mar 24, 2017

duereg commented Mar 24, 2017 • edited Loading

targos commented Mar 24, 2017 • edited Loading

duereg commented Mar 24, 2017

targos commented Mar 24, 2017

duereg commented Mar 24, 2017

duereg commented Mar 24, 2017 • edited Loading

duereg commented Mar 24, 2017

gibfahn commented Mar 24, 2017 • edited by targos Loading

targos commented Mar 25, 2017 • edited Loading

targos commented Mar 25, 2017

targos commented Mar 25, 2017

duereg commented Apr 6, 2017

targos commented Apr 7, 2017

targos commented Apr 7, 2017

duereg commented Mar 21, 2017 •

edited

Loading

mscdex commented Mar 21, 2017 •

edited

Loading

mscdex commented Mar 21, 2017 •

edited

Loading

duereg commented Mar 23, 2017 •

edited

Loading

duereg commented Mar 24, 2017 •

edited

Loading

targos commented Mar 24, 2017 •

edited

Loading

duereg commented Mar 24, 2017 •

edited

Loading

gibfahn commented Mar 24, 2017 •

edited by targos

Loading

targos commented Mar 25, 2017 •

edited

Loading