buffer: fix buffer alignment restriction

src/node_buffer.cc

@@ -87,17 +87,20 @@ class CallbackInfo {
   static inline CallbackInfo* New(Isolate* isolate,
                                   Local<ArrayBuffer> object,
                                   FreeCallback callback,
+                                  char* data,
                                   void* hint = 0);
  private:
   static void WeakCallback(const WeakCallbackInfo<CallbackInfo>&);
-  inline void WeakCallback(Isolate* isolate, char* const data);
+  inline void WeakCallback(Isolate* isolate);
   inline CallbackInfo(Isolate* isolate,
                       Local<ArrayBuffer> object,
                       FreeCallback callback,
+                      char* data,
                       void* hint);
   ~CallbackInfo();
   Persistent<ArrayBuffer> persistent_;
   FreeCallback const callback_;
+  char* const data_;
   void* const hint_;
   DISALLOW_COPY_AND_ASSIGN(CallbackInfo);
 };
@@ -111,27 +114,27 @@ void CallbackInfo::Free(char* data, void*) {
 CallbackInfo* CallbackInfo::New(Isolate* isolate,
                                 Local<ArrayBuffer> object,
                                 FreeCallback callback,
+                                char* data,
                                 void* hint) {
-  return new CallbackInfo(isolate, object, callback, hint);
+  return new CallbackInfo(isolate, object, callback, data, hint);
 }
 
 
 CallbackInfo::CallbackInfo(Isolate* isolate,
                            Local<ArrayBuffer> object,
                            FreeCallback callback,
+                           char* data,
                            void* hint)
     : persistent_(isolate, object),
       callback_(callback),
+      data_(data),
       hint_(hint) {
   ArrayBuffer::Contents obj_c = object->GetContents();
-  char* const data = static_cast<char*>(obj_c.Data());
+  CHECK_EQ(data_, static_cast<char*>(obj_c.Data()));
   if (object->ByteLength() != 0)
-    CHECK_NE(data, nullptr);
-
-  object->SetAlignedPointerInInternalField(kBufferInternalFieldIndex, data);
+    CHECK_NE(data_, nullptr);
 
-  persistent_.SetWeak(this, WeakCallback,
-                      v8::WeakCallbackType::kInternalFields);
+  persistent_.SetWeak(this, WeakCallback, v8::WeakCallbackType::kParameter);
   persistent_.SetWrapperClassId(BUFFER_ID);
   persistent_.MarkIndependent();
   isolate->AdjustAmountOfExternalAllocatedMemory(sizeof(*this));
@@ -146,15 +149,13 @@ CallbackInfo::~CallbackInfo() {
 void CallbackInfo::WeakCallback(
     const WeakCallbackInfo<CallbackInfo>& data) {
   CallbackInfo* self = data.GetParameter();
-  self->WeakCallback(
-      data.GetIsolate(),
-      static_cast<char*>(data.GetInternalField(kBufferInternalFieldIndex)));
+  self->WeakCallback(data.GetIsolate());
   delete self;
 }
 
 
-void CallbackInfo::WeakCallback(Isolate* isolate, char* const data) {
-  callback_(data, hint_);
+void CallbackInfo::WeakCallback(Isolate* isolate) {
+  callback_(data_, hint_);
   int64_t change_in_bytes = -static_cast<int64_t>(sizeof(*this));
   isolate->AdjustAmountOfExternalAllocatedMemory(change_in_bytes);
 }
@@ -370,7 +371,7 @@ MaybeLocal<Object> New(Environment* env,
   if (!mb.FromMaybe(false))
     return Local<Object>();
 
-  CallbackInfo::New(env->isolate(), ab, callback, hint);
+  CallbackInfo::New(env->isolate(), ab, callback, data, hint);
   return scope.Escape(ui);
 }
 

src/node_buffer.h

@@ -13,10 +13,6 @@ namespace Buffer {
 static const unsigned int kMaxLength =
     sizeof(int32_t) == sizeof(intptr_t) ? 0x3fffffff : 0x7fffffff;
 
-// Buffers have two internal fields, the first of which is reserved for use by
-// Node.
-static const unsigned int kBufferInternalFieldIndex = 0;
-
 NODE_EXTERN typedef void (*FreeCallback)(char* data, void* hint);
 
 NODE_EXTERN bool HasInstance(v8::Local<v8::Value> val);

test/addons/buffer-free-callback/binding.cc

@@ -13,9 +13,16 @@ static void FreeCallback(char* data, void* hint) {
 void Alloc(const v8::FunctionCallbackInfo<v8::Value>& args) {
   v8::Isolate* isolate = args.GetIsolate();
   alive++;
+
+  uintptr_t alignment = args[1]->IntegerValue();
+  uintptr_t offset = args[2]->IntegerValue();
+
+  uintptr_t static_offset = reinterpret_cast<uintptr_t>(buf) % alignment;
+  char* aligned = buf + (alignment - static_offset) + offset;
+
   args.GetReturnValue().Set(node::Buffer::New(
         isolate,
-        buf,
+        aligned,
         args[0]->IntegerValue(),
         FreeCallback,
         nullptr).ToLocalChecked());

test/addons/buffer-free-callback/test.js

@@ -4,8 +4,8 @@
 require('../../common');
 var binding = require('./build/Release/binding');
 
-function check(size) {
-  var buf = binding.alloc(size);
+function check(size, alignment, offset) {
+  var buf = binding.alloc(size, alignment, offset);
   var slice = buf.slice(size >>> 1);
 
   buf = null;
@@ -16,7 +16,20 @@ function check(size) {
   gc();
 }
 
-check(64);
+check(64, 1, 0);
+
+// Buffers can have weird sizes.
+check(97, 1, 0);
+
+// Buffers can be unaligned
+check(64,  8, 0);
+check(64, 16, 0);
+check(64,  8, 1);
+check(64, 16, 1);
+check(97,  8, 1);
+check(97, 16, 1);
+check(97,  8, 3);
+check(97, 16, 3);
 
 // Empty ArrayBuffer does not allocate data, worth checking
-check(0);
+check(0, 1, 0);