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

src: fix unaligned access in ucs2 string encoder #127

Merged
merged 1 commit into from
Dec 9, 2014

Conversation

bnoordhuis
Copy link
Member

Seen with g++ 4.9.2 on x86_64 Linux: a SIGSEGV is generated when the
input to v8::String::NewFromTwoByte() is not suitably aligned.

g++ 4.9.2 emits SSE instructions for copy loops. That requires aligned
input but that was something StringBytes::Encode() did not enforce until
now. Make a properly aligned copy before handing off the input to V8.

We could, as an optimization, check that the pointer is aligned on a
two-byte boundary but that is technically still UB; pointers-to-char
are allowed to alias other pointers but the reverse is not true:
a pointer-to-uint16_t that aliases a pointer-to-char is in violation
of the pointer aliasing rules.

See https://code.google.com/p/v8/issues/detail?id=3694

R=@trevnorris? Please disregard the first commit, that's #125.

Seen with g++ 4.9.2 on x86_64 Linux: a SIGSEGV is generated when the
input to v8::String::NewFromTwoByte() is not suitably aligned.

g++ 4.9.2 emits SSE instructions for copy loops.  That requires aligned
input but that was something StringBytes::Encode() did not enforce until
now.  Make a properly aligned copy before handing off the input to V8.

We could, as an optimization, check that the pointer is aligned on a
two-byte boundary but that is technically still UB; pointers-to-char
are allowed to alias other pointers but the reverse is not true:
a pointer-to-uint16_t that aliases a pointer-to-char is in violation
of the pointer aliasing rules.

See https://code.google.com/p/v8/issues/detail?id=3694

Fixes segfaulting test simple/test-stream2-writable.

PR-URL: nodejs#127
Reviewed-by: Trevor Norris <trev.norris@gmail.com>
@trevnorris
Copy link
Contributor

LGTM

@bnoordhuis bnoordhuis merged commit 535fec8 into nodejs:v0.12 Dec 9, 2014
@bnoordhuis bnoordhuis deleted the fix-unaligned-access branch December 9, 2014 20:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants