Skip to content

V8 Fatal Error when trying to convert a buffered integer to string #649

Closed
Closed
V8 Fatal Error when trying to convert a buffered integer to string#649
Assignees
bnoordhuis
Labels
bufferIssues and PRs related to the buffer subsystem.confirmed-bugIssues and PRs for confirmed bugs.v8 engineIssues and PRs related to the V8 dependency.

Description

@kevinmartin
kevinmartin
opened on Jan 29, 2015

IO.js Version 1.0.4

$ iojs
> Date.now();
1422508723712
> Buffer(Date.now());
<Buffer 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ... >
> Buffer(Date.now()).toString('utf8');


#
# Fatal error in ../deps/v8/src/handles.h, line 48
# CHECK(location_ != NULL) failed
#

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

 1: V8_Fatal
 2: v8::String::NewFromUtf8(v8::Isolate*, char const*, v8::String::NewStringType, int)
 3: node::StringBytes::Encode(v8::Isolate*, char const*, unsigned long, node::encoding)
 4: node::Buffer::Utf8Slice(v8::FunctionCallbackInfo<v8::Value> const&)
 5: v8::internal::FunctionCallbackArguments::Call(void (*)(v8::FunctionCallbackInfo<v8::Value> const&))
 6: ??
 7: ??
Illegal instruction

Activity

  1. jonathanong commented on Jan 29, 2015

    @jonathanong
    jonathanong
    on Jan 29, 2015
    Contributor

    Date.now() is an integer, so you're creating a buffer with Date.now() bytes... which is gigantic. I think you want new Buffer(String(Date.now()))

  2. kevinmartin commented on Jan 29, 2015

    @kevinmartin
    kevinmartin
    on Jan 29, 2015
    Author

    I understand what's wrong, haha. I'm just wondering why its giving a scary V8 error like that instead of a generic Javascript error.

    In Node 0.10.31:

    $ node
> Buffer(5).toString('utf8')
'\u0000 \u0000\u0000\u0000'
> Buffer(Date.now()).toString('utf8')
TypeError: Bad argument
    at new Buffer (buffer.js:194:21)
    at Buffer (buffer.js:158:12)
    at repl:1:2
    at REPLServer.self.eval (repl.js:110:21)
    at Interface.<anonymous> (repl.js:239:12)
    at Interface.emit (events.js:95:17)
    at Interface._onLine (readline.js:202:10)
    at Interface._line (readline.js:531:8)
    at Interface._ttyWrite (readline.js:760:14)
    at ReadStream.onkeypress (readline.js:99:10)

  3. brendanashworth commented on Jan 29, 2015

    @brendanashworth
    brendanashworth
    on Jan 29, 2015
    Contributor

    @kevinmartin well, it does have an extraordinary length.

    > var buf = new Buffer(Date.now())
undefined
> buf.length
878611666

  4. targos commented on Jan 29, 2015

    @targos
    targos
    on Jan 29, 2015
    Member

    this issue reminds me of similar crash I had last year on node. I just retried it on iojs and it is still crashing.

    test.js :

    var fs = require('fs');
var zlib = require('zlib');

fs.readFile('./Compound_006625001_006650000.sdf.gz', function (err, content) {
  zlib.gunzip(content, function (err, sdf) {
    sdf.toString();
  });
});

    The gzipped file : ftp://ftp.ncbi.nlm.nih.gov/pubchem/Compound/CURRENT-Full/SDF/Compound_006625001_006650000.sdf.gz

    > iojs test.js
#
# Fatal error in ../deps/v8/src/handles.h, line 48
# CHECK(location_ != NULL) failed
#

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

 1: V8_Fatal
 2: v8::String::NewFromUtf8(v8::Isolate*, char const*, v8::String::NewStringType, int)
 3: node::StringBytes::Encode(v8::Isolate*, char const*, unsigned long, node::encoding)
 4: node::Buffer::Utf8Slice(v8::FunctionCallbackInfo<v8::Value> const&)
 5: v8::internal::FunctionCallbackArguments::Call(void (*)(v8::FunctionCallbackInfo<v8::Value> const&))
 6: ??
 7: ??
zsh: illegal hardware instruction (core dumped)  iojs test.js

    The uncompressed file is valid utf-8.
    Could it be a v8 limit on the size (it is 276,9 MB) ?

  5. Fishrock123
    assigned
    Fishrock123
    and
    bnoordhuis
    and unassigned
    Fishrock123
    on Jan 29, 2015

  6. bnoordhuis commented on Jan 29, 2015

    @bnoordhuis
    bnoordhuis
    on Jan 29, 2015
    Member

    There is a logic error in lib/buffer.js: the length argument to the constructor gets coerced to uint32 with length >>> 0, which makes it bypass the length < kMaxLength check because e.g. (1422546646261 >>> 0) < 0x3fffffff.

    In short, confirmed. :-)

  7. Fishrock123
    added
    bufferIssues and PRs related to the buffer subsystem.
    confirmed-bugIssues and PRs for confirmed bugs.
    on Jan 29, 2015

  8. cjb commented on Jan 29, 2015

    @cjb
    cjb
    on Jan 29, 2015

    I think @bnoordhuis found a bug, but I don't think it was this bug. The length > kMaxLength after length >>> 0 check he mentions is part of SlowBuffer(), and that's not called at all in this stack trace.

    We just have a plain Buffer (for which util.isNumber(subject) returns true), and when it hits the if (this.length > kMaxLength) check at lib/buffer.js:52 it has a length of 921032252 and a kMaxLength of 1073741823, so it's under the limit at 85% of kMaxLength.

  9. bnoordhuis commented on Jan 29, 2015

    @bnoordhuis
    bnoordhuis
    on Jan 29, 2015
    Member

    See #657 for a proposed mitigation. It's not perfect because the real issue lies in V8, it aborts when internal operations OOM. For example, Buffer(0x3fffffff-1).toString() still hits the assert.

  10. bnoordhuis commented on Jan 29, 2015

    @bnoordhuis
    bnoordhuis
    on Jan 29, 2015
    Member

    V8 bug report: https://code.google.com/p/v8/issues/detail?id=3853

  11. 11 remaining items

  12. trevnorris commented on Mar 13, 2015

    @trevnorris
    trevnorris
    on Mar 13, 2015
    Contributor

    Also, the cutoff point that's currently in there was chosen after meticulous benchmarking. Here's a basic graph showing what happens to allocation time as string size increases: http://trevnorris.github.io/nodeconf.eu/#/17

  13. bnoordhuis commented on Mar 13, 2015

    @bnoordhuis
    bnoordhuis
    on Mar 13, 2015
    Member

    External strings can only be one or two byte strings. Thus excluding the possibility of an external UTF8 string.

    True, but internal strings aren't UTF-8 either, V8 always decodes them to ASCII or UTF-16.

  14. petkaantonov commented on Mar 13, 2015

    @petkaantonov
    petkaantonov
    on Mar 13, 2015
    Contributor

    True, but internal strings aren't UTF-8 either, V8 always decodes them to ASCII or UTF-16.

    Yes, we are not gaining anything by outsourcing this work to v8. We can do it ourselves and make it external.

  15. trevnorris commented on Mar 13, 2015

    @trevnorris
    trevnorris
    on Mar 13, 2015
    Contributor

    Sure, but do we really want to implement our own encoding parser so we can externalize all strings?

  16. petkaantonov commented on Mar 13, 2015

    @petkaantonov
    petkaantonov
    on Mar 13, 2015
    Contributor

    We don't have to implement our own, even though it's a fun task.

  17. Fishrock123
    added
    v8 engineIssues and PRs related to the V8 dependency.
    on Apr 8, 2015
  18. jasnell
    added a commit that references this issue on Dec 6, 2016

    test: fix test for buffer regression #649

    a3a3937
  19. addaleax
    added 2 commits that reference this issue on Dec 8, 2016

    test: fix test for buffer regression #649

    bb91747

    test: fix test for buffer regression nodejs#649

    dd4586b
  20. MylesBorins
    added a commit that references this issue on Dec 21, 2016

    test: fix test for buffer regression #649

    0b62370
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

bufferIssues and PRs related to the buffer subsystem.confirmed-bugIssues and PRs for confirmed bugs.v8 engineIssues and PRs related to the V8 dependency.

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions