Skip to content

Commit 535fec8

Browse files
committed
src: fix unaligned access in ucs2 string encoder
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: #127 Reviewed-by: Trevor Norris <trev.norris@gmail.com>
1 parent 4efc02a commit 535fec8

File tree

1 file changed

+19
-19
lines changed

1 file changed

+19
-19
lines changed

src/string_bytes.cc

+19-19
Original file line numberDiff line numberDiff line change
@@ -748,28 +748,28 @@ Local<Value> StringBytes::Encode(Isolate* isolate,
748748
}
749749

750750
case UCS2: {
751-
const uint16_t* out = reinterpret_cast<const uint16_t*>(buf);
752-
uint16_t* dst = nullptr;
753-
if (IsBigEndian()) {
754-
// Node's "ucs2" encoding expects LE character data inside a
755-
// Buffer, so we need to reorder on BE platforms. See
756-
// http://nodejs.org/api/buffer.html regarding Node's "ucs2"
757-
// encoding specification
758-
dst = new uint16_t[buflen / 2];
759-
for (size_t i = 0; i < buflen / 2; i++) {
760-
dst[i] = (out[i] << 8) | (out[i] >> 8);
761-
}
762-
out = dst;
751+
// Node's "ucs2" encoding expects LE character data inside a
752+
// Buffer, so we need to reorder on BE platforms. See
753+
// http://nodejs.org/api/buffer.html regarding Node's "ucs2"
754+
// encoding specification. Note that we have to make a copy
755+
// to avoid pointer aliasing and unaligned access, something
756+
// that we can't guarantee by just reinterpret_casting |buf|.
757+
size_t srclen = buflen / 2;
758+
uint16_t* src = new uint16_t[srclen];
759+
for (size_t i = 0, k = 0; i < srclen; i += 1, k += 2) {
760+
const uint8_t lo = static_cast<uint8_t>(buf[k + 0]);
761+
const uint8_t hi = static_cast<uint8_t>(buf[k + 1]);
762+
src[i] = lo | hi << 8;
763763
}
764-
if (buflen < EXTERN_APEX)
764+
if (buflen < EXTERN_APEX) {
765765
val = String::NewFromTwoByte(isolate,
766-
out,
766+
src,
767767
String::kNormalString,
768-
buflen / 2);
769-
else
770-
val = ExternTwoByteString::NewFromCopy(isolate, out, buflen / 2);
771-
if (dst)
772-
delete[] dst;
768+
srclen);
769+
} else {
770+
val = ExternTwoByteString::NewFromCopy(isolate, src, srclen);
771+
}
772+
delete[] src;
773773
break;
774774
}
775775

0 commit comments

Comments
 (0)