Skip to content

Commit

Permalink
src: remove many uses of GetBackingStore
Browse files Browse the repository at this point in the history
This replaces many uses of `GetBackingStore()->Data()` with the newly
backported `Data()`.

I only replaced the "obvious" usages here, where I could easily convince
myself that the lifetime was safe. The less obvious changes will come in
a separate PR.

Refs: nodejs#32226
Refs: nodejs#43921
  • Loading branch information
kvakil committed Jul 21, 2022
1 parent 0359430 commit 554fdf2
Show file tree
Hide file tree
Showing 14 changed files with 61 additions and 61 deletions.
7 changes: 3 additions & 4 deletions src/aliased_buffer.h
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ class AliasedBufferBase {
// allocate v8 ArrayBuffer
v8::Local<v8::ArrayBuffer> ab = v8::ArrayBuffer::New(
isolate_, size_in_bytes);
buffer_ = static_cast<NativeT*>(ab->GetBackingStore()->Data());
buffer_ = static_cast<NativeT*>(ab->Data());

// allocate v8 TypedArray
v8::Local<V8T> js_array = V8T::New(ab, byte_offset_, count);
Expand Down Expand Up @@ -119,8 +119,7 @@ class AliasedBufferBase {
// be removed when we expand the snapshot support.
DCHECK_EQ(count_, arr->Length());
DCHECK_EQ(byte_offset_, arr->ByteOffset());
uint8_t* raw =
static_cast<uint8_t*>(arr->Buffer()->GetBackingStore()->Data());
uint8_t* raw = static_cast<uint8_t*>(arr->Buffer()->Data());
buffer_ = reinterpret_cast<NativeT*>(raw + byte_offset_);
js_array_.Reset(isolate_, arr);
index_ = nullptr;
Expand Down Expand Up @@ -278,7 +277,7 @@ class AliasedBufferBase {
isolate_, new_size_in_bytes);

// allocate new native buffer
NativeT* new_buffer = static_cast<NativeT*>(ab->GetBackingStore()->Data());
NativeT* new_buffer = static_cast<NativeT*>(ab->Data());
// copy old content
memcpy(new_buffer, buffer_, old_size_in_bytes);

Expand Down
17 changes: 7 additions & 10 deletions src/js_native_api_v8.cc
Original file line number Diff line number Diff line change
Expand Up @@ -2780,7 +2780,7 @@ napi_status NAPI_CDECL napi_create_arraybuffer(napi_env env,
// Optionally return a pointer to the buffer's data, to avoid another call to
// retrieve it.
if (data != nullptr) {
*data = buffer->GetBackingStore()->Data();
*data = buffer->Data();
}

*result = v8impl::JsValueFromV8LocalValue(buffer);
Expand Down Expand Up @@ -2814,15 +2814,14 @@ napi_status NAPI_CDECL napi_get_arraybuffer_info(napi_env env,
v8::Local<v8::Value> value = v8impl::V8LocalValueFromJsValue(arraybuffer);
RETURN_STATUS_IF_FALSE(env, value->IsArrayBuffer(), napi_invalid_arg);

std::shared_ptr<v8::BackingStore> backing_store =
value.As<v8::ArrayBuffer>()->GetBackingStore();
v8::Local<v8::ArrayBuffer> ab = value.As<v8::ArrayBuffer>();

if (data != nullptr) {
*data = backing_store->Data();
*data = ab->Data();
}

if (byte_length != nullptr) {
*byte_length = backing_store->ByteLength();
*byte_length = ab->ByteLength();
}

return napi_clear_last_error(env);
Expand Down Expand Up @@ -2963,8 +2962,7 @@ napi_status NAPI_CDECL napi_get_typedarray_info(napi_env env,
}

if (data != nullptr) {
*data = static_cast<uint8_t*>(buffer->GetBackingStore()->Data()) +
array->ByteOffset();
*data = static_cast<uint8_t*>(buffer->Data()) + array->ByteOffset();
}

if (arraybuffer != nullptr) {
Expand Down Expand Up @@ -3044,8 +3042,7 @@ napi_status NAPI_CDECL napi_get_dataview_info(napi_env env,
}

if (data != nullptr) {
*data = static_cast<uint8_t*>(buffer->GetBackingStore()->Data()) +
array->ByteOffset();
*data = static_cast<uint8_t*>(buffer->Data()) + array->ByteOffset();
}

if (arraybuffer != nullptr) {
Expand Down Expand Up @@ -3256,7 +3253,7 @@ napi_status NAPI_CDECL napi_is_detached_arraybuffer(napi_env env,
v8::Local<v8::Value> value = v8impl::V8LocalValueFromJsValue(arraybuffer);

*result = value->IsArrayBuffer() &&
value.As<v8::ArrayBuffer>()->GetBackingStore()->Data() == nullptr;
value.As<v8::ArrayBuffer>()->Data() == nullptr;

return napi_clear_last_error(env);
}
3 changes: 1 addition & 2 deletions src/module_wrap.cc
Original file line number Diff line number Diff line change
Expand Up @@ -178,8 +178,7 @@ void ModuleWrap::New(const FunctionCallbackInfo<Value>& args) {
if (!args[5]->IsUndefined()) {
CHECK(args[5]->IsArrayBufferView());
Local<ArrayBufferView> cached_data_buf = args[5].As<ArrayBufferView>();
uint8_t* data = static_cast<uint8_t*>(
cached_data_buf->Buffer()->GetBackingStore()->Data());
uint8_t* data = static_cast<uint8_t*>(cached_data_buf->Buffer()->Data());
cached_data =
new ScriptCompiler::CachedData(data + cached_data_buf->ByteOffset(),
cached_data_buf->ByteLength());
Expand Down
2 changes: 1 addition & 1 deletion src/node.cc
Original file line number Diff line number Diff line change
Expand Up @@ -269,7 +269,7 @@ static void AtomicsWaitCallback(Isolate::AtomicsWaitEvent event,
", %.f) %s\n",
static_cast<int>(uv_os_getpid()),
env->thread_id(),
array_buffer->GetBackingStore()->Data(),
array_buffer->Data(),
offset_in_bytes,
value,
timeout_in_ms,
Expand Down
58 changes: 34 additions & 24 deletions src/node_buffer.cc
Original file line number Diff line number Diff line change
Expand Up @@ -244,8 +244,7 @@ bool HasInstance(Local<Object> obj) {
char* Data(Local<Value> val) {
CHECK(val->IsArrayBufferView());
Local<ArrayBufferView> ui = val.As<ArrayBufferView>();
return static_cast<char*>(ui->Buffer()->GetBackingStore()->Data()) +
ui->ByteOffset();
return static_cast<char*>(ui->Buffer()->Data()) + ui->ByteOffset();
}


Expand Down Expand Up @@ -1156,14 +1155,13 @@ static void EncodeInto(const FunctionCallbackInfo<Value>& args) {

Local<Uint8Array> dest = args[1].As<Uint8Array>();
Local<ArrayBuffer> buf = dest->Buffer();
char* write_result =
static_cast<char*>(buf->GetBackingStore()->Data()) + dest->ByteOffset();
char* write_result = static_cast<char*>(buf->Data()) + dest->ByteOffset();
size_t dest_length = dest->ByteLength();

// results = [ read, written ]
Local<Uint32Array> result_arr = args[2].As<Uint32Array>();
uint32_t* results = reinterpret_cast<uint32_t*>(
static_cast<char*>(result_arr->Buffer()->GetBackingStore()->Data()) +
static_cast<char*>(result_arr->Buffer()->Data()) +
result_arr->ByteOffset());

int nchars;
Expand Down Expand Up @@ -1227,6 +1225,27 @@ void DetachArrayBuffer(const FunctionCallbackInfo<Value>& args) {
}
}

namespace {

std::pair<void*, size_t> DecomposeBufferToParts(Local<Value> buffer) {
void* pointer;
size_t byte_length;
if (buffer->IsArrayBuffer()) {
Local<ArrayBuffer> ab = buffer.As<ArrayBuffer>();
pointer = ab->Data();
byte_length = ab->ByteLength();
} else if (buffer->IsSharedArrayBuffer()) {
Local<ArrayBuffer> ab = buffer.As<ArrayBuffer>();
pointer = ab->Data();
byte_length = ab->ByteLength();
} else {
CHECK(false); // Caller must validate.
}
return {pointer, byte_length};
}

}

void CopyArrayBuffer(const FunctionCallbackInfo<Value>& args) {
// args[0] == Destination ArrayBuffer
// args[1] == Destination ArrayBuffer Offset
Expand All @@ -1240,32 +1259,23 @@ void CopyArrayBuffer(const FunctionCallbackInfo<Value>& args) {
CHECK(args[3]->IsUint32());
CHECK(args[4]->IsUint32());

std::shared_ptr<BackingStore> destination;
std::shared_ptr<BackingStore> source;

if (args[0]->IsArrayBuffer()) {
destination = args[0].As<ArrayBuffer>()->GetBackingStore();
} else if (args[0]->IsSharedArrayBuffer()) {
destination = args[0].As<SharedArrayBuffer>()->GetBackingStore();
}
void* destination;
size_t destination_byte_length;
std::tie(destination, destination_byte_length) = DecomposeBufferToParts(args[0]);

if (args[2]->IsArrayBuffer()) {
source = args[2].As<ArrayBuffer>()->GetBackingStore();
} else if (args[0]->IsSharedArrayBuffer()) {
source = args[2].As<SharedArrayBuffer>()->GetBackingStore();
}
void* source;
size_t source_byte_length;
std::tie(source, source_byte_length) = DecomposeBufferToParts(args[2]);

uint32_t destination_offset = args[1].As<Uint32>()->Value();
uint32_t source_offset = args[3].As<Uint32>()->Value();
size_t bytes_to_copy = args[4].As<Uint32>()->Value();

CHECK_GE(destination->ByteLength() - destination_offset, bytes_to_copy);
CHECK_GE(source->ByteLength() - source_offset, bytes_to_copy);
CHECK_GE(destination_byte_length - destination_offset, bytes_to_copy);
CHECK_GE(source_byte_length - source_offset, bytes_to_copy);

uint8_t* dest =
static_cast<uint8_t*>(destination->Data()) + destination_offset;
uint8_t* src =
static_cast<uint8_t*>(source->Data()) + source_offset;
uint8_t* dest = static_cast<uint8_t*>(destination) + destination_offset;
uint8_t* src = static_cast<uint8_t*>(source) + source_offset;
memcpy(dest, src, bytes_to_copy);
}

Expand Down
4 changes: 2 additions & 2 deletions src/node_contextify.cc
Original file line number Diff line number Diff line change
Expand Up @@ -743,7 +743,7 @@ void ContextifyScript::New(const FunctionCallbackInfo<Value>& args) {
ScriptCompiler::CachedData* cached_data = nullptr;
if (!cached_data_buf.IsEmpty()) {
uint8_t* data = static_cast<uint8_t*>(
cached_data_buf->Buffer()->GetBackingStore()->Data());
cached_data_buf->Buffer()->Data());
cached_data = new ScriptCompiler::CachedData(
data + cached_data_buf->ByteOffset(), cached_data_buf->ByteLength());
}
Expand Down Expand Up @@ -1065,7 +1065,7 @@ void ContextifyContext::CompileFunction(
ScriptCompiler::CachedData* cached_data = nullptr;
if (!cached_data_buf.IsEmpty()) {
uint8_t* data = static_cast<uint8_t*>(
cached_data_buf->Buffer()->GetBackingStore()->Data());
cached_data_buf->Buffer()->Data());
cached_data = new ScriptCompiler::CachedData(
data + cached_data_buf->ByteOffset(), cached_data_buf->ByteLength());
}
Expand Down
2 changes: 1 addition & 1 deletion src/node_os.cc
Original file line number Diff line number Diff line change
Expand Up @@ -161,7 +161,7 @@ static void GetLoadAvg(const FunctionCallbackInfo<Value>& args) {
Local<Float64Array> array = args[0].As<Float64Array>();
CHECK_EQ(array->Length(), 3);
Local<ArrayBuffer> ab = array->Buffer();
double* loadavg = static_cast<double*>(ab->GetBackingStore()->Data());
double* loadavg = static_cast<double*>(ab->Data());
uv_loadavg(loadavg);
}

Expand Down
6 changes: 3 additions & 3 deletions src/node_process_methods.cc
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,7 @@ static void CPUUsage(const FunctionCallbackInfo<Value>& args) {

// Get the double array pointer from the Float64Array argument.
Local<ArrayBuffer> ab = get_fields_array_buffer(args, 0, 2);
double* fields = static_cast<double*>(ab->GetBackingStore()->Data());
double* fields = static_cast<double*>(ab->Data());

// Set the Float64Array elements to be user / system values in microseconds.
fields[0] = MICROS_PER_SEC * rusage.ru_utime.tv_sec + rusage.ru_utime.tv_usec;
Expand Down Expand Up @@ -189,7 +189,7 @@ static void MemoryUsage(const FunctionCallbackInfo<Value>& args) {

// Get the double array pointer from the Float64Array argument.
Local<ArrayBuffer> ab = get_fields_array_buffer(args, 0, 5);
double* fields = static_cast<double*>(ab->GetBackingStore()->Data());
double* fields = static_cast<double*>(ab->Data());

size_t rss;
int err = uv_resident_set_memory(&rss);
Expand Down Expand Up @@ -311,7 +311,7 @@ static void ResourceUsage(const FunctionCallbackInfo<Value>& args) {
return env->ThrowUVException(err, "uv_getrusage");

Local<ArrayBuffer> ab = get_fields_array_buffer(args, 0, 16);
double* fields = static_cast<double*>(ab->GetBackingStore()->Data());
double* fields = static_cast<double*>(ab->Data());

fields[0] = MICROS_PER_SEC * rusage.ru_utime.tv_sec + rusage.ru_utime.tv_usec;
fields[1] = MICROS_PER_SEC * rusage.ru_stime.tv_sec + rusage.ru_stime.tv_usec;
Expand Down
7 changes: 3 additions & 4 deletions src/node_wasi.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1654,10 +1654,9 @@ void WASI::_SetMemory(const FunctionCallbackInfo<Value>& args) {

uvwasi_errno_t WASI::backingStore(char** store, size_t* byte_length) {
Local<WasmMemoryObject> memory = PersistentToLocal::Strong(this->memory_);
std::shared_ptr<BackingStore> backing_store =
memory->Buffer()->GetBackingStore();
*byte_length = backing_store->ByteLength();
*store = static_cast<char*>(backing_store->Data());
Local<v8::ArrayBuffer> ab = memory->Buffer();
*byte_length = ab->ByteLength();
*store = static_cast<char*>(ab->Data());
CHECK_NOT_NULL(*store);
return UVWASI_ESUCCESS;
}
Expand Down
4 changes: 2 additions & 2 deletions src/node_wasm_web_api.cc
Original file line number Diff line number Diff line change
Expand Up @@ -105,12 +105,12 @@ void WasmStreamingObject::Push(const FunctionCallbackInfo<Value>& args) {

if (LIKELY(chunk->IsArrayBufferView())) {
Local<ArrayBufferView> view = chunk.As<ArrayBufferView>();
bytes = view->Buffer()->GetBackingStore()->Data();
bytes = view->Buffer()->Data();
offset = view->ByteOffset();
size = view->ByteLength();
} else if (LIKELY(chunk->IsArrayBuffer())) {
Local<ArrayBuffer> buffer = chunk.As<ArrayBuffer>();
bytes = buffer->GetBackingStore()->Data();
bytes = buffer->Data();
offset = 0;
size = buffer->ByteLength();
} else {
Expand Down
2 changes: 1 addition & 1 deletion src/node_worker.cc
Original file line number Diff line number Diff line change
Expand Up @@ -710,7 +710,7 @@ void Worker::GetResourceLimits(const FunctionCallbackInfo<Value>& args) {
Local<Float64Array> Worker::GetResourceLimits(Isolate* isolate) const {
Local<ArrayBuffer> ab = ArrayBuffer::New(isolate, sizeof(resource_limits_));

memcpy(ab->GetBackingStore()->Data(),
memcpy(ab->Data(),
resource_limits_,
sizeof(resource_limits_));
return Float64Array::New(ab, 0, kTotalResourceLimitCount);
Expand Down
3 changes: 1 addition & 2 deletions src/node_zlib.cc
Original file line number Diff line number Diff line change
Expand Up @@ -605,8 +605,7 @@ class ZlibStream final : public CompressionStream<ZlibContext> {
CHECK(args[4]->IsUint32Array());
Local<Uint32Array> array = args[4].As<Uint32Array>();
Local<ArrayBuffer> ab = array->Buffer();
uint32_t* write_result = static_cast<uint32_t*>(
ab->GetBackingStore()->Data());
uint32_t* write_result = static_cast<uint32_t*>(ab->Data());

CHECK(args[5]->IsFunction());
Local<Function> write_js_callback = args[5].As<Function>();
Expand Down
3 changes: 1 addition & 2 deletions src/util-inl.h
Original file line number Diff line number Diff line change
Expand Up @@ -537,8 +537,7 @@ void ArrayBufferViewContents<T, S>::Read(v8::Local<v8::ArrayBufferView> abv) {
static_assert(sizeof(T) == 1, "Only supports one-byte data at the moment");
length_ = abv->ByteLength();
if (length_ > sizeof(stack_storage_) || abv->HasBuffer()) {
data_ = static_cast<T*>(abv->Buffer()->GetBackingStore()->Data()) +
abv->ByteOffset();
data_ = static_cast<T*>(abv->Buffer()->Data()) + abv->ByteOffset();
} else {
abv->CopyContents(stack_storage_, sizeof(stack_storage_));
data_ = stack_storage_;
Expand Down
4 changes: 1 addition & 3 deletions src/util.h
Original file line number Diff line number Diff line change
Expand Up @@ -538,12 +538,10 @@ class BufferValue : public MaybeStackBuffer<char> {
#define SPREAD_BUFFER_ARG(val, name) \
CHECK((val)->IsArrayBufferView()); \
v8::Local<v8::ArrayBufferView> name = (val).As<v8::ArrayBufferView>(); \
std::shared_ptr<v8::BackingStore> name##_bs = \
name->Buffer()->GetBackingStore(); \
const size_t name##_offset = name->ByteOffset(); \
const size_t name##_length = name->ByteLength(); \
char* const name##_data = \
static_cast<char*>(name##_bs->Data()) + name##_offset; \
static_cast<char*>(name->Buffer()->Data()) + name##_offset; \
if (name##_length > 0) \
CHECK_NE(name##_data, nullptr);

Expand Down

0 comments on commit 554fdf2

Please sign in to comment.