Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fatal V8 Compiler Error #12308

Closed
tristanhoy opened this issue Apr 10, 2017 · 11 comments
Closed

Fatal V8 Compiler Error #12308

tristanhoy opened this issue Apr 10, 2017 · 11 comments
Labels
confirmed-bug Issues with confirmed bugs. v8 engine Issues and PRs related to the V8 dependency.

Comments

@tristanhoy
Copy link

  • Version: 6.10.2
  • Platform: 64-bit Windows (also tested on Linux, not sure which arch)
  • Subsystem: V8 Compiler

The third line in the following script causes an error in the v8 compiler:

const sphincs = require('sphincs') // npm install sphincs@1.0.2
console.log('blah')
const keyPair = sphincs.keyPair()
blah


#
# Fatal error in ..\..\src\compiler.cc, line 786
# Check failed: !info->shared_info()->feedback_vector()->metadata()->SpecDiffersFrom( info->literal()->feedback_vector_spec()).
#

Node 7.8.0 (V8 5.5.372) is not affected.

This module is an emscripten transpile, and the issue specifically affects the optimised emcc -O3 build. The compiled, minified source is 107KB so I've made no attempts to debug any further.

An option suggested to the module author is to create a legacy build with less optimisations - so there's certainly workarounds in this case, but could this be a symptom of a deeper issue?

Are there any plans to upgrade V8 for 6.x?

@addaleax addaleax added v6.x v8 engine Issues and PRs related to the V8 dependency. labels Apr 10, 2017
@mscdex
Copy link
Contributor

mscdex commented Apr 10, 2017

/cc @nodejs/v8

@hashseed
Copy link
Member

@ripsawridge @bmeurer

@bmeurer
Copy link
Member

bmeurer commented Apr 10, 2017

Looks like one of those bugs that @ripsawridge fixed, where the feedback vector layout differed between parsing and preparsing.

@buu700
Copy link

buu700 commented Apr 10, 2017

I just had a chance to investigate this (I'm the module author), and it turns out that this actually has nothing to do with the emscripten compiler options.

The problem is triggered by a workaround I'd used to make this module work in both Node and webpack with no additional required configuration: specifically, replacing all instances of require( with eval("require")(. I feel like there must be a more elegant way to accomplish this, but that solution worked well enough and did what I needed at the time.

Also, here's my stack trace from Node 6.10.2 on Debian Jessie x86-64 if it's helpful (error message is the same as @tristanhoy's):

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

 1: V8_Fatal
 2: 0xb4548b
 3: v8::internal::Compiler::EnsureDeoptimizationSupport(v8::internal::CompilationInfo*)
 4: 0xb4d3a5
 5: 0xb4e9dd
 6: v8::internal::Compiler::CompileOptimized(v8::internal::Handle<v8::internal::JSFunction>, v8::internal::Compiler::ConcurrencyMode)
 7: v8::internal::Runtime_CompileOptimized_Concurrent(int, v8::internal::Object**, v8::internal::Isolate*)
 8: 0x2f37853092a7
Illegal instruction

@bnoordhuis
Copy link
Member

I can confirm the issue but I suspect this will be hard to fix due to the size of the changes between v6.x and v7.x.

It looks like @bmeurer was spot on, the info->shared_info()->feedback_vector() type feedback vector has an extra KEYED_LOAD_IC and a LOAD_IC that should have been a KEYED_LOAD_IC.

@bnoordhuis bnoordhuis added the confirmed-bug Issues with confirmed bugs. label Apr 12, 2017
@bmeurer
Copy link
Member

bmeurer commented Apr 13, 2017

I'm pretty sure @ripsawridge fixed that. He might be able to point you to the proper fix. He'll be back in the office next week. I'll point him to this issue.

@ripsawridge
Copy link

ripsawridge commented Apr 13, 2017 via email

@ripsawridge
Copy link

I found a few candidates:

https://bugs.chromium.org/p/chromium/issues/detail?id=620119
Fixed in June 2016, Merged to 5.2.361.33

https://bugs.chromium.org/p/chromium/issues/detail?id=608279
Fixed in May 2016, Not merged anywhere.

https://bugs.chromium.org/p/chromium/issues/detail?id=600995
Fixed in April 2016, Merged to 5.1.281.10

@ripsawridge
Copy link

Okay, the issue is https://bugs.chromium.org/p/chromium/issues/detail?id=608279.
The fix is an easy merge and the repro disappears.
(here is the CL to port: https://codereview.chromium.org/1950803002/)

@hashseed
Copy link
Member

I'll prepare a PR.

hashseed added a commit to hashseed/node that referenced this issue Apr 20, 2017
Original commit message:

    Don't treat catch scopes as possibly-shadowing for sloppy eval

    Scope analysis is over-conservative when treating variable
    resolutions as possibly-shadowed by a sloppy eval. In the attached
    bug, this comes into play since catch scopes have different behavior
    with respect to the "calls eval" in eager vs lazy compilation (in
    the latter, they are never marked as "calls eval" because
    CatchContexts don't have an associated ScopeInfo).

    This patch changes the scope-type check to also eliminate a few
    other cases where shadowing isn't possible, such as non-declaration
    block scopes.

    BUG=chromium:608279
    LOG=n

    Committed:
    https://crrev.com/75f2d65f003ebb22815489e9970913ba37234f1b
    Cr-Commit-Position: refs/heads/master@{nodejs#36046}

Fixes: nodejs#12308
MylesBorins pushed a commit that referenced this issue Apr 24, 2017
Original commit message:

    Don't treat catch scopes as possibly-shadowing for sloppy eval

    Scope analysis is over-conservative when treating variable
    resolutions as possibly-shadowed by a sloppy eval. In the attached
    bug, this comes into play since catch scopes have different behavior
    with respect to the "calls eval" in eager vs lazy compilation (in
    the latter, they are never marked as "calls eval" because
    CatchContexts don't have an associated ScopeInfo).

    This patch changes the scope-type check to also eliminate a few
    other cases where shadowing isn't possible, such as non-declaration
    block scopes.

    BUG=chromium:608279
    LOG=n

    Committed:
    https://crrev.com/75f2d65f003ebb22815489e9970913ba37234f1b
    Cr-Commit-Position: refs/heads/master@{#36046}

Fixes: #12308

PR-URL: #12535
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Myles Borins <myles.borins@gmail.com>
MylesBorins pushed a commit that referenced this issue Apr 24, 2017
Original commit message:

    Don't treat catch scopes as possibly-shadowing for sloppy eval

    Scope analysis is over-conservative when treating variable
    resolutions as possibly-shadowed by a sloppy eval. In the attached
    bug, this comes into play since catch scopes have different behavior
    with respect to the "calls eval" in eager vs lazy compilation (in
    the latter, they are never marked as "calls eval" because
    CatchContexts don't have an associated ScopeInfo).

    This patch changes the scope-type check to also eliminate a few
    other cases where shadowing isn't possible, such as non-declaration
    block scopes.

    BUG=chromium:608279
    LOG=n

    Committed:
    https://crrev.com/75f2d65f003ebb22815489e9970913ba37234f1b
    Cr-Commit-Position: refs/heads/master@{#36046}

Fixes: #12308

PR-URL: #12535
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Myles Borins <myles.borins@gmail.com>
MylesBorins pushed a commit that referenced this issue Apr 24, 2017
Original commit message:

    Don't treat catch scopes as possibly-shadowing for sloppy eval

    Scope analysis is over-conservative when treating variable
    resolutions as possibly-shadowed by a sloppy eval. In the attached
    bug, this comes into play since catch scopes have different behavior
    with respect to the "calls eval" in eager vs lazy compilation (in
    the latter, they are never marked as "calls eval" because
    CatchContexts don't have an associated ScopeInfo).

    This patch changes the scope-type check to also eliminate a few
    other cases where shadowing isn't possible, such as non-declaration
    block scopes.

    BUG=chromium:608279
    LOG=n

    Committed:
    https://crrev.com/75f2d65f003ebb22815489e9970913ba37234f1b
    Cr-Commit-Position: refs/heads/master@{#36046}

Fixes: #12308

PR-URL: #12535
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Myles Borins <myles.borins@gmail.com>
@targos
Copy link
Member

targos commented May 12, 2017

This was fixed in cd78a2bd07 and released with v6.10.3.

@targos targos closed this as completed May 12, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
confirmed-bug Issues with confirmed bugs. v8 engine Issues and PRs related to the V8 dependency.
Projects
None yet
Development

No branches or pull requests

9 participants