src: redo unaligned access workaround

Introduce two-byte overloads of node::Encode() and StringBytes::Encode()
that ensure that the input is suitably aligned.

Revisits commit 535fec8 from yesterday.

diff --git a/src/node.cc b/src/node.cc
index a48e228..e2a5066 100644 (file)
--- a/src/node.cc
+++ b/src/node.cc
@@ -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

diff --git a/src/node.h b/src/node.h
index afca6bf..80bb20d 100644 (file)
--- a/src/node.h
+++ b/src/node.h
@@ -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)
@@ -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

diff --git a/src/node_buffer.cc b/src/node_buffer.cc
index 4c1caeb..17477c0 100644 (file)
--- a/src/node_buffer.cc
+++ b/src/node_buffer.cc
@@ -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);
 }

diff --git a/src/node_crypto.cc b/src/node_crypto.cc
index 918c2b9..84625ac 100644 (file)
--- a/src/node_crypto.cc
+++ b/src/node_crypto.cc
@@ -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;

diff --git a/src/string_bytes.cc b/src/string_bytes.cc
index f989240..e646087 100644 (file)
--- a/src/string_bytes.cc
+++ b/src/string_bytes.cc
@@ -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));
@@ -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];
@@ -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

diff --git a/src/string_bytes.h b/src/string_bytes.h
index 10fee30..a67bd45 100644 (file)
--- a/src/string_bytes.h
+++ b/src/string_bytes.h
@@ -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, ...)",