From 73fc4408703f20c314d9f799fa199280c4b89400 Mon Sep 17 00:00:00 2001 From: Ali Ijaz Sheikh Date: Wed, 16 Mar 2016 17:46:01 -0700 Subject: [PATCH] buffer: fix buffer alignment restriction Recent phantom weakness API changes to buffer, ebbbc5a, ending up introducing an alignment restriction on the native buffer pointers. It turns out that there are uses in the modules ecosystem that rely on the ability to create buffers with unaligned pointers (e.g. node-ffi). It turns out there is a simpler solution possible here. As a side effect this also removes the need to have to reserve the first internal field on buffers. PR-URL: https://github.com/nodejs/node/pull/5752 Reviewed-By: trevnorris - Trevor Norris Reviewed-By: bnoordhuis - Ben Noordhuis --- src/node_buffer.cc | 29 +++++++++++++++-------------- src/node_buffer.h | 4 ---- 2 files changed, 15 insertions(+), 18 deletions(-) diff --git a/src/node_buffer.cc b/src/node_buffer.cc index 6720f2c9379..4d23c4c2831 100644 --- a/src/node_buffer.cc +++ b/src/node_buffer.cc @@ -87,17 +87,20 @@ class CallbackInfo { static inline CallbackInfo* New(Isolate* isolate, Local object, FreeCallback callback, + char* data, void* hint = 0); private: static void WeakCallback(const WeakCallbackInfo&); - inline void WeakCallback(Isolate* isolate, char* const data); + inline void WeakCallback(Isolate* isolate); inline CallbackInfo(Isolate* isolate, Local object, FreeCallback callback, + char* data, void* hint); ~CallbackInfo(); Persistent 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 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 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(obj_c.Data()); + CHECK_EQ(data_, static_cast(obj_c.Data())); if (object->ByteLength() != 0) - CHECK_NE(data, nullptr); + CHECK_NE(data_, nullptr); - object->SetAlignedPointerInInternalField(kBufferInternalFieldIndex, data); - - 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& data) { CallbackInfo* self = data.GetParameter(); - self->WeakCallback( - data.GetIsolate(), - static_cast(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(sizeof(*this)); isolate->AdjustAmountOfExternalAllocatedMemory(change_in_bytes); } @@ -370,7 +371,7 @@ MaybeLocal New(Environment* env, if (!mb.FromMaybe(false)) return Local(); - CallbackInfo::New(env->isolate(), ab, callback, hint); + CallbackInfo::New(env->isolate(), ab, callback, data, hint); return scope.Escape(ui); } diff --git a/src/node_buffer.h b/src/node_buffer.h index 2ecfab33a3f..686450d984e 100644 --- a/src/node_buffer.h +++ b/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 val);