Skip to content

Commit 28280f7

Browse files
exinfinitumrvagg
authored andcommitted
src: add BE support to StringBytes::Encode()
Versions of Node.js after v0.12 have relocated byte-swapping away from the StringBytes::Encode function, thereby causing a nan test (which accesses this function directly) to fail on big-endian machines. This change re-introduces byte swapping in StringBytes::Encode, done via a call to a function in util-inl. Another change in NodeBuffer::StringSlice was necessary to avoid double byte swapping in big-endian function calls to StringSlice. PR-URL: #3410 Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: Trevor Norris <[email protected]>
1 parent 3bcfbce commit 28280f7

File tree

4 files changed

+34
-7
lines changed

4 files changed

+34
-7
lines changed

src/node_buffer.cc

+6-3
Original file line numberDiff line numberDiff line change
@@ -472,10 +472,11 @@ void StringSlice<UCS2>(const FunctionCallbackInfo<Value>& args) {
472472
// need to reorder on BE platforms. See http://nodejs.org/api/buffer.html
473473
// regarding Node's "ucs2" encoding specification.
474474
const bool aligned = (reinterpret_cast<uintptr_t>(data) % sizeof(*buf) == 0);
475-
if (IsLittleEndian() && aligned) {
476-
buf = reinterpret_cast<const uint16_t*>(data);
477-
} else {
475+
if (IsLittleEndian() && !aligned) {
478476
// Make a copy to avoid unaligned accesses in v8::String::NewFromTwoByte().
477+
// This applies ONLY to little endian platforms, as misalignment will be
478+
// handled by a byte-swapping operation in StringBytes::Encode on
479+
// big endian platforms.
479480
uint16_t* copy = new uint16_t[length];
480481
for (size_t i = 0, k = 0; i < length; i += 1, k += 2) {
481482
// Assumes that the input is little endian.
@@ -485,6 +486,8 @@ void StringSlice<UCS2>(const FunctionCallbackInfo<Value>& args) {
485486
}
486487
buf = copy;
487488
release = true;
489+
} else {
490+
buf = reinterpret_cast<const uint16_t*>(data);
488491
}
489492

490493
args.GetReturnValue().Set(StringBytes::Encode(env->isolate(), buf, length));

src/string_bytes.cc

+12-4
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66

77
#include <limits.h>
88
#include <string.h> // memcpy
9+
#include <vector>
910

1011
// When creating strings >= this length v8's gc spins up and consumes
1112
// most of the execution time. For these cases it's more performant to
@@ -406,9 +407,7 @@ size_t StringBytes::Write(Isolate* isolate,
406407
reinterpret_cast<uintptr_t>(buf) % sizeof(uint16_t);
407408
if (is_aligned) {
408409
uint16_t* const dst = reinterpret_cast<uint16_t*>(buf);
409-
for (size_t i = 0; i < nchars; i++)
410-
dst[i] = dst[i] << 8 | dst[i] >> 8;
411-
break;
410+
SwapBytes(dst, dst, nchars);
412411
}
413412

414413
ASSERT_EQ(sizeof(uint16_t), 2);
@@ -857,7 +856,16 @@ Local<Value> StringBytes::Encode(Isolate* isolate,
857856
const uint16_t* buf,
858857
size_t buflen) {
859858
Local<String> val;
860-
859+
std::vector<uint16_t> dst;
860+
if (IsBigEndian()) {
861+
// Node's "ucs2" encoding expects LE character data inside a
862+
// Buffer, so we need to reorder on BE platforms. See
863+
// http://nodejs.org/api/buffer.html regarding Node's "ucs2"
864+
// encoding specification
865+
dst.resize(buflen);
866+
SwapBytes(&dst[0], buf, buflen);
867+
buf = &dst[0];
868+
}
861869
if (buflen < EXTERN_APEX) {
862870
val = String::NewFromTwoByte(isolate,
863871
buf,

src/util-inl.h

+14
Original file line numberDiff line numberDiff line change
@@ -198,6 +198,20 @@ TypeName* Unwrap(v8::Local<v8::Object> object) {
198198
return static_cast<TypeName*>(pointer);
199199
}
200200

201+
void SwapBytes(uint16_t* dst, const uint16_t* src, size_t buflen) {
202+
for (size_t i = 0; i < buflen; i++) {
203+
// __builtin_bswap16 generates more efficient code with
204+
// g++ 4.8 on PowerPC and other big-endian archs
205+
#ifdef __GNUC__
206+
dst[i] = __builtin_bswap16(src[i]);
207+
#else
208+
dst[i] = (src[i] << 8) | (src[i] >> 8);
209+
#endif
210+
}
211+
}
212+
213+
214+
201215
} // namespace node
202216

203217
#endif // SRC_UTIL_INL_H_

src/util.h

+2
Original file line numberDiff line numberDiff line change
@@ -176,6 +176,8 @@ inline void ClearWrap(v8::Local<v8::Object> object);
176176
template <typename TypeName>
177177
inline TypeName* Unwrap(v8::Local<v8::Object> object);
178178

179+
inline void SwapBytes(uint16_t* dst, const uint16_t* src, size_t buflen);
180+
179181
class Utf8Value {
180182
public:
181183
explicit Utf8Value(v8::Isolate* isolate, v8::Local<v8::Value> value);

0 commit comments

Comments
 (0)