Node: V8 Fatal Exception, 0x99b617

[duereg]

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)

[mscdex]

Do you have a simple example to reproduce the crash?

/cc @nodejs/v8

[mscdex]

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

[duereg]

@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]

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

[mscdex]

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

[mscdex]

FWIW

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

• https://github.com/nodejs/node/pull/10525
• https://github.com/nodejs/node/pull/10546
• https://github.com/nodejs/node/pull/10666
• https://github.com/nodejs/node/pull/10689
• https://github.com/nodejs/node/pull/10881

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

• https://github.com/nodejs/node/pull/9293
• https://github.com/nodejs/node/pull/10733
• https://github.com/nodejs/node/pull/11483

[duereg]

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]

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

Also which version of Linux?

[mscdex]

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]

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]

@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]

@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]

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 3fc6a2247f.

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

[duereg]

@mscdex reverting 3ab070d resolved the issue.

[mscdex]

/cc @targos ^

[AndrewBarba]

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]

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

[targos]

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

https://github.com/nodejs/node/blob/6803521c4dc301df9052b193cb44086d6357d049/deps/v8/src/ast/ast-numbering.cc#L368

[bnoordhuis]

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

[psmarshall]

@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]

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]

@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]

@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]

@targos updated gist with .babelrc

[targos]

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]

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]

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]

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

[gibfahn]

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]

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]

Even shorter:

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

[targos]

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

[duereg]

@targos has this been resolved in 6.10.2?

[targos]

@duereg Yes. Thanks for the report!

[targos]

Fix landed in https://github.com/nodejs/node/commit/52bdb8f2465cfb83a4b9e393ae6173e209750505