Skip to content

Commit

Permalink
buffer: fix buffer alignment restriction
Browse files Browse the repository at this point in the history
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: #5752
Reviewed-By: trevnorris - Trevor Norris <trev.norris@gmail.com>
Reviewed-By: bnoordhuis - Ben Noordhuis <info@bnoordhuis.nl>
  • Loading branch information
ofrobots committed Mar 19, 2016
1 parent 5f1eb43 commit 73fc440
Show file tree
Hide file tree
Showing 2 changed files with 15 additions and 18 deletions.
29 changes: 15 additions & 14 deletions src/node_buffer.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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);
};
Expand All @@ -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));
Expand All @@ -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);
}
Expand Down Expand Up @@ -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);
}

Expand Down
4 changes: 0 additions & 4 deletions src/node_buffer.h
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down

0 comments on commit 73fc440

Please sign in to comment.