Skip to content

Commit

Permalink
src: add BE support to StringBytes::Encode()
Browse files Browse the repository at this point in the history
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 <info@bnoordhuis.nl>
Reviewed-By: Trevor Norris <trev.norris@gmail.com>
  • Loading branch information
exinfinitum authored and rvagg committed Dec 5, 2015
1 parent 1fe4d30 commit 7e18f2e
Show file tree
Hide file tree
Showing 4 changed files with 34 additions and 7 deletions.
9 changes: 6 additions & 3 deletions src/node_buffer.cc
Original file line number Diff line number Diff line change
Expand Up @@ -477,10 +477,11 @@ void StringSlice<UCS2>(const FunctionCallbackInfo<Value>& args) {
// need to reorder on BE platforms. See http://nodejs.org/api/buffer.html
// regarding Node's "ucs2" encoding specification.
const bool aligned = (reinterpret_cast<uintptr_t>(data) % sizeof(*buf) == 0);
if (IsLittleEndian() && aligned) {
buf = reinterpret_cast<const uint16_t*>(data);
} else {
if (IsLittleEndian() && !aligned) {
// Make a copy to avoid unaligned accesses in v8::String::NewFromTwoByte().
// This applies ONLY to little endian platforms, as misalignment will be
// handled by a byte-swapping operation in StringBytes::Encode on
// big endian platforms.
uint16_t* copy = new uint16_t[length];
for (size_t i = 0, k = 0; i < length; i += 1, k += 2) {
// Assumes that the input is little endian.
Expand All @@ -490,6 +491,8 @@ void StringSlice<UCS2>(const FunctionCallbackInfo<Value>& args) {
}
buf = copy;
release = true;
} else {
buf = reinterpret_cast<const uint16_t*>(data);
}

args.GetReturnValue().Set(StringBytes::Encode(env->isolate(), buf, length));
Expand Down
16 changes: 12 additions & 4 deletions src/string_bytes.cc
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@

#include <limits.h>
#include <string.h> // memcpy
#include <vector>

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

ASSERT_EQ(sizeof(uint16_t), 2);
Expand Down Expand Up @@ -857,7 +856,16 @@ Local<Value> StringBytes::Encode(Isolate* isolate,
const uint16_t* buf,
size_t buflen) {
Local<String> val;

std::vector<uint16_t> dst;
if (IsBigEndian()) {
// Node's "ucs2" encoding expects LE character data inside a
// Buffer, so we need to reorder on BE platforms. See
// http://nodejs.org/api/buffer.html regarding Node's "ucs2"
// encoding specification
dst.resize(buflen);
SwapBytes(&dst[0], buf, buflen);
buf = &dst[0];
}
if (buflen < EXTERN_APEX) {
val = String::NewFromTwoByte(isolate,
buf,
Expand Down
14 changes: 14 additions & 0 deletions src/util-inl.h
Original file line number Diff line number Diff line change
Expand Up @@ -198,6 +198,20 @@ TypeName* Unwrap(v8::Local<v8::Object> object) {
return static_cast<TypeName*>(pointer);
}

void SwapBytes(uint16_t* dst, const uint16_t* src, size_t buflen) {
for (size_t i = 0; i < buflen; i++) {
// __builtin_bswap16 generates more efficient code with
// g++ 4.8 on PowerPC and other big-endian archs
#ifdef __GNUC__
dst[i] = __builtin_bswap16(src[i]);
#else
dst[i] = (src[i] << 8) | (src[i] >> 8);
#endif
}
}



} // namespace node

#endif // SRC_UTIL_INL_H_
2 changes: 2 additions & 0 deletions src/util.h
Original file line number Diff line number Diff line change
Expand Up @@ -176,6 +176,8 @@ inline void ClearWrap(v8::Local<v8::Object> object);
template <typename TypeName>
inline TypeName* Unwrap(v8::Local<v8::Object> object);

inline void SwapBytes(uint16_t* dst, const uint16_t* src, size_t buflen);

class Utf8Value {
public:
explicit Utf8Value(v8::Isolate* isolate, v8::Local<v8::Value> value);
Expand Down

0 comments on commit 7e18f2e

Please sign in to comment.