Repository navigation
Buffer.allocUnsafe causes "Conditional jump..." on valgrind #26464
Description
Activity
- addedbufferIssues and PRs related to the buffer subsystem.Issues and PRs related to the buffer subsystem.
on Mar 6, 2019 Thanks for the report. I'd say that's expected given that checkBounds() checks that
buf[offset] === undefined, which has been observed to be a little faster than checking thatoffset < buf.length.V8's runtime loads the byte at
buf[offset]. That's a random value if you useBuffer.allocUnsafe()because it doesn't zero the memory.I'm not sure if this is something we actually want to fix because there's no real bug.
I suppose the strongest argument for addressing it is that it can't be suppressed because the comparison takes place in runtime-generated code - the address and even the exact layout of the code can change between invocations.
Reacted by Anna Henningsenbuf[offset] === undefined, which has been observed to be a little faster than checking thatoffset < buf.length.Is that really true? (I'm amazed that it amounts to something noticeable - I'm curious to test it myself)
I'm not sure if this is something we actually want to fix because there's no real bug.
I'm trying to understand why it's not a real bug, my best guess is that no matter what random memory value is read it won't cause Buffer
[]operator to returnundefinedvalue as long as it's inside the array bounds. Am I getting this right?I suppose the strongest argument for addressing it is that it can't be suppressed because the comparison takes place in runtime-generated code - the address and even the exact layout of the code can change between invocations.
IMHO, almost any fix to this will have, at least, the 'penalty' of changing to checking
offset<buf.length.
So if someone is already usingallocUnsafeI can deduce 2 things about them: performance sensitive and know what they are doing (because working with uninitialized memory/buffer intentionally can lead to much worse bugs).
So I would suggest not checking bounds if usingallocUnsafebecause as the name states it's 'unsafe' - you're on your own.Is that really true? (I'm amazed that it amounts to something noticeable - I'm curious to test it myself)
I was able to replace multiple error checks with
buf[offset] === undefinedand newer V8 versions have a nice OOB protection that works relatively fast. Ifbuf[offset]is indeed undefined the original checks are executed to detect what exactly is wrong (e.g., wrong input value, out of bounds, float instead of integer...).my best guess is that no matter what random memory value is read it won't cause Buffer [] operator to return undefined value as long as it's inside the array bounds. Am I getting this right?
Yes, that's right.
I am going to close this since it does not really seem to be a bug.
Please feel free to comment in case this should be reopened.
Working with valgrind to clean a giant networking application I was getting "Conditional jump or move depends on uninitialised value(s)" with js stacks (see below), took a long time but managed to zero in on reason.
Running the following code through valgrind will trigger the error
>valgrind --gen-suppressions=yes --leak-check=full /var/lib/nave/global/10.13.0/bin/node a.jsI've managed to trace the cause to
lib/internal/buffer.js:checkBoundsaccessing the buffer that isn't initialized.Buffer.allocUnsafeused bywsnpm moduleShachar