Skip to content

Commit

Permalink
src: redo unaligned access workaround
Browse files Browse the repository at this point in the history
Introduce two-byte overloads of node::Encode() and StringBytes::Encode()
that ensure that the input is suitably aligned.

Revisits commit 535fec8 from yesterday.
  • Loading branch information
bnoordhuis committed Dec 14, 2014
1 parent a60056d commit 56fde66
Show file tree
Hide file tree
Showing 6 changed files with 100 additions and 35 deletions.
12 changes: 7 additions & 5 deletions src/node.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1219,13 +1219,15 @@ enum encoding ParseEncoding(Isolate* isolate,
}

Local<Value> Encode(Isolate* isolate,
const void* buf,
const char* buf,
size_t len,
enum encoding encoding) {
return StringBytes::Encode(isolate,
static_cast<const char*>(buf),
len,
encoding);
CHECK_NE(encoding, UCS2);
return StringBytes::Encode(isolate, buf, len, encoding);
}

Local<Value> Encode(Isolate* isolate, const uint16_t* buf, size_t len) {
return StringBytes::Encode(isolate, buf, len);
}

// Returns -1 if the handle was not valid for decoding
Expand Down
18 changes: 16 additions & 2 deletions src/node.h
Original file line number Diff line number Diff line change
Expand Up @@ -142,6 +142,7 @@ NODE_EXTERN v8::Handle<v8::Value> MakeCallback(
#endif

#include <assert.h>
#include <stdint.h>

#ifndef NODE_STRINGIFY
#define NODE_STRINGIFY(n) NODE_STRINGIFY_HELPER(n)
Expand Down Expand Up @@ -267,16 +268,29 @@ NODE_DEPRECATED("Use FatalException(isolate, ...)",
return FatalException(v8::Isolate::GetCurrent(), try_catch);
})

// Don't call with encoding=UCS2.
NODE_EXTERN v8::Local<v8::Value> Encode(v8::Isolate* isolate,
const void* buf,
const char* buf,
size_t len,
enum encoding encoding = BINARY);

NODE_EXTERN v8::Local<v8::Value> Encode(v8::Isolate* isolate,
const uint16_t* buf,
size_t len);

NODE_DEPRECATED("Use Encode(isolate, ...)",
inline v8::Local<v8::Value> Encode(
const void* buf,
size_t len,
enum encoding encoding = BINARY) {
return Encode(v8::Isolate::GetCurrent(), buf, len, encoding);
v8::Isolate* isolate = v8::Isolate::GetCurrent();
if (encoding == UCS2) {
assert(reinterpret_cast<uintptr_t>(buf) % sizeof(uint16_t) == 0 &&
"UCS2 buffer must be aligned on two-byte boundary.");
const uint16_t* that = static_cast<const uint16_t*>(buf);
return Encode(isolate, that, len / sizeof(*that));
}
return Encode(isolate, static_cast<const char*>(buf), len, encoding);
})

// Returns -1 if the handle was not valid for decoding
Expand Down
34 changes: 34 additions & 0 deletions src/node_buffer.cc
Original file line number Diff line number Diff line change
Expand Up @@ -264,6 +264,40 @@ void StringSlice(const FunctionCallbackInfo<Value>& args) {
}


template <>
void StringSlice<UCS2>(const FunctionCallbackInfo<Value>& args) {
Environment* env = Environment::GetCurrent(args);

ARGS_THIS(args.This())
SLICE_START_END(args[0], args[1], obj_length)
length /= 2;

const char* data = obj_data + start;
const uint16_t* buf;
bool release = false;

if (reinterpret_cast<uintptr_t>(data) % sizeof(*buf) == 0) {
buf = reinterpret_cast<const uint16_t*>(data);
} else {
// Make a copy to avoid unaligned accesses in v8::String::NewFromTwoByte().
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.
const uint8_t lo = static_cast<uint8_t>(data[k + 0]);
const uint8_t hi = static_cast<uint8_t>(data[k + 1]);
copy[i] = lo | hi << 8;
}
buf = copy;
release = true;
}

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

if (release)
delete[] buf;
}


void BinarySlice(const FunctionCallbackInfo<Value>& args) {
StringSlice<BINARY>(args);
}
Expand Down
4 changes: 2 additions & 2 deletions src/node_crypto.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1447,8 +1447,8 @@ void SSLWrap<Base>::GetSession(const FunctionCallbackInfo<Value>& args) {
int slen = i2d_SSL_SESSION(sess, nullptr);
CHECK_GT(slen, 0);

unsigned char* sbuf = new unsigned char[slen];
unsigned char* p = sbuf;
char* sbuf = new char[slen];
unsigned char* p = reinterpret_cast<unsigned char*>(sbuf);
i2d_SSL_SESSION(sess, &p);
args.GetReturnValue().Set(Encode(env->isolate(), sbuf, slen, BUFFER));
delete[] sbuf;
Expand Down
62 changes: 36 additions & 26 deletions src/string_bytes.cc
Original file line number Diff line number Diff line change
Expand Up @@ -690,6 +690,7 @@ Local<Value> StringBytes::Encode(Isolate* isolate,
enum encoding encoding) {
EscapableHandleScope scope(isolate);

CHECK_NE(encoding, UCS2);
CHECK_LE(buflen, Buffer::kMaxLength);
if (!buflen && encoding != BUFFER)
return scope.Escape(String::Empty(isolate));
Expand Down Expand Up @@ -747,32 +748,6 @@ Local<Value> StringBytes::Encode(Isolate* isolate,
break;
}

case UCS2: {
// 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. Note that we have to make a copy
// to avoid pointer aliasing and unaligned access, something
// that we can't guarantee by just reinterpret_casting |buf|.
size_t srclen = buflen / 2;
uint16_t* src = new uint16_t[srclen];
for (size_t i = 0, k = 0; i < srclen; i += 1, k += 2) {
const uint8_t lo = static_cast<uint8_t>(buf[k + 0]);
const uint8_t hi = static_cast<uint8_t>(buf[k + 1]);
src[i] = lo | hi << 8;
}
if (buflen < EXTERN_APEX) {
val = String::NewFromTwoByte(isolate,
src,
String::kNormalString,
srclen);
} else {
val = ExternTwoByteString::NewFromCopy(isolate, src, srclen);
}
delete[] src;
break;
}

case HEX: {
size_t dlen = buflen * 2;
char* dst = new char[dlen];
Expand All @@ -796,4 +771,39 @@ Local<Value> StringBytes::Encode(Isolate* isolate,
return scope.Escape(val);
}


Local<Value> StringBytes::Encode(Isolate* isolate,
const uint16_t* buf,
size_t buflen) {
const uint16_t* src = buf;

// 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.
if (IsBigEndian()) {
// Inefficient, see StringSlice<UCS2>() in src/node_buffer.cc;
// this is potentially the second copy of the actual input.
uint16_t* copy = new uint16_t[buflen];
for (size_t i = 0; i < buflen; i += 1)
copy[i] = buf[i] << 8 | buf[i] >> 8;
src = copy;
}

Local<String> val;
if (buflen < EXTERN_APEX) {
val = String::NewFromTwoByte(isolate,
src,
String::kNormalString,
buflen);
} else {
val = ExternTwoByteString::NewFromCopy(isolate, src, buflen);
}

if (src != buf)
delete[] src;

return val;
}

} // namespace node
5 changes: 5 additions & 0 deletions src/string_bytes.h
Original file line number Diff line number Diff line change
Expand Up @@ -71,11 +71,16 @@ class StringBytes {
int* chars_written = nullptr);

// Take the bytes in the src, and turn it into a Buffer or String.
// Don't call with encoding=UCS2.
static v8::Local<v8::Value> Encode(v8::Isolate* isolate,
const char* buf,
size_t buflen,
enum encoding encoding);

static v8::Local<v8::Value> Encode(v8::Isolate* isolate,
const uint16_t* buf,
size_t buflen);

// Deprecated legacy interface

NODE_DEPRECATED("Use IsValidString(isolate, ...)",
Expand Down

0 comments on commit 56fde66

Please sign in to comment.