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 error when comparing long strings #12573

Closed
jdmnd opened this issue Apr 21, 2017 · 9 comments
Closed

Fatal error when comparing long strings #12573

jdmnd opened this issue Apr 21, 2017 · 9 comments
Labels
v8 engine Issues and PRs related to the V8 dependency.

Comments

@jdmnd
Copy link

jdmnd commented Apr 21, 2017

  • Version: v7.9.0
  • Platform: Darwin 15.6.0 Darwin Kernel Version 15.6.0: Mon Jan 9 23:07:29 PST 2017; root:xnu-3248.60.11.2.1~1/RELEASE_X86_64 x86_64
  • Subsystem: ?

The following code snippet causes node to crash:

('a'.repeat(268435441)) === ('a'.repeat(268435441))

Note: The crash appears to only affect strings of length exactly 268435441 (2^28 - 15)

Output:

#
# Fatal error in ../deps/v8/src/handles.h, line 210
# Check failed: (location_) != nullptr.
#

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

    0   node                                0x0000000100bcb703 v8::base::debug::StackTrace::StackTrace() + 19
    1   node                                0x0000000100bc8889 V8_Fatal + 233
    2   node                                0x0000000100679c6f v8::internal::String::SlowFlatten(v8::internal::Handle<v8::internal::ConsString>, v8::internal::PretenureFlag) + 1039
    3   node                                0x000000010069ed06 v8::internal::String::SlowEquals(v8::internal::Handle<v8::internal::String>, v8::internal::Handle<v8::internal::String>) + 678
    4   node                                0x00000001008848d1 v8::internal::Runtime_StringEqual(int, v8::internal::Object**, v8::internal::Isolate*) + 337
    5   ???                                 0x00001c377a3063a7 0x0 + 31024598770599
Illegal instruction: 4
@mscdex
Copy link
Contributor

mscdex commented Apr 21, 2017

/cc @nodejs/v8

@mscdex mscdex added v7.x v8 engine Issues and PRs related to the V8 dependency. labels Apr 21, 2017
@vsemozhetbyt
Copy link
Contributor

vsemozhetbyt commented Apr 21, 2017

@jdmnd @mscdex See #9489 and #10687

@bmeurer
Copy link
Member

bmeurer commented Apr 21, 2017

Doesn't reproduce on latest V8, probably fixed since then.

@mscdex
Copy link
Contributor

mscdex commented Apr 21, 2017

@vsemozhetbyt Getting an 'illegal instruction' error is different than the errors received in those two linked issues. At least in both of those cases, the error can be caught. This error cannot be caught.

@TimothyGu
Copy link
Member

I can reproduce this on v7.9.0, but not on master.

@bmeurer
Copy link
Member

bmeurer commented Apr 21, 2017

Reproduces on V8 5.5. The string created by S.p.repeat has an invalid length, which crashes in flattening. Somehow S.p.repeat should already throw, but doesn't.

@bmeurer
Copy link
Member

bmeurer commented Apr 23, 2017

Manually bisected to crrev.com/2653623002, you'll probably need to merge that.

@hashseed
Copy link
Member

The V8 fix landed in 5.8.

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

    [crankshaft] Fix string addition to check for max length of cons string.

    BUG=chromium:678917

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

Fixes: nodejs#12573
targos pushed a commit that referenced this issue May 12, 2017
Original commit message:

    [crankshaft] Fix string addition to check for max length of cons string.

    BUG=chromium:678917

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

PR-URL: #12696
Fixes: #12573
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: Michaël Zasso <[email protected]>
@targos
Copy link
Member

targos commented May 12, 2017

The fix just landed on v7.x-staging and should be in the next 7.x release.

@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
v8 engine Issues and PRs related to the V8 dependency.
Projects
None yet
Development

No branches or pull requests

7 participants