From 554fdf27c6d4f0d31b6eb3b7f30166109a1a1d30 Mon Sep 17 00:00:00 2001 From: Keyhan Vakil Date: Thu, 21 Jul 2022 04:16:07 +0000 Subject: [PATCH] src: remove many uses of GetBackingStore 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: https://github.com/nodejs/node/issues/32226 Refs: https://github.com/nodejs/node/pull/43921 --- src/aliased_buffer.h | 7 ++--- src/js_native_api_v8.cc | 17 +++++------ src/module_wrap.cc | 3 +- src/node.cc | 2 +- src/node_buffer.cc | 58 ++++++++++++++++++++++--------------- src/node_contextify.cc | 4 +-- src/node_os.cc | 2 +- src/node_process_methods.cc | 6 ++-- src/node_wasi.cc | 7 ++--- src/node_wasm_web_api.cc | 4 +-- src/node_worker.cc | 2 +- src/node_zlib.cc | 3 +- src/util-inl.h | 3 +- src/util.h | 4 +-- 14 files changed, 61 insertions(+), 61 deletions(-) diff --git a/src/aliased_buffer.h b/src/aliased_buffer.h index 2cbc70aaa7c37c..29b5c77a042881 100644 --- a/src/aliased_buffer.h +++ b/src/aliased_buffer.h @@ -50,7 +50,7 @@ class AliasedBufferBase { // allocate v8 ArrayBuffer v8::Local ab = v8::ArrayBuffer::New( isolate_, size_in_bytes); - buffer_ = static_cast(ab->GetBackingStore()->Data()); + buffer_ = static_cast(ab->Data()); // allocate v8 TypedArray v8::Local js_array = V8T::New(ab, byte_offset_, count); @@ -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(arr->Buffer()->GetBackingStore()->Data()); + uint8_t* raw = static_cast(arr->Buffer()->Data()); buffer_ = reinterpret_cast(raw + byte_offset_); js_array_.Reset(isolate_, arr); index_ = nullptr; @@ -278,7 +277,7 @@ class AliasedBufferBase { isolate_, new_size_in_bytes); // allocate new native buffer - NativeT* new_buffer = static_cast(ab->GetBackingStore()->Data()); + NativeT* new_buffer = static_cast(ab->Data()); // copy old content memcpy(new_buffer, buffer_, old_size_in_bytes); diff --git a/src/js_native_api_v8.cc b/src/js_native_api_v8.cc index 5f8e21e0b58c8a..b8a9fb127793cb 100644 --- a/src/js_native_api_v8.cc +++ b/src/js_native_api_v8.cc @@ -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); @@ -2814,15 +2814,14 @@ napi_status NAPI_CDECL napi_get_arraybuffer_info(napi_env env, v8::Local value = v8impl::V8LocalValueFromJsValue(arraybuffer); RETURN_STATUS_IF_FALSE(env, value->IsArrayBuffer(), napi_invalid_arg); - std::shared_ptr backing_store = - value.As()->GetBackingStore(); + v8::Local ab = value.As(); 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); @@ -2963,8 +2962,7 @@ napi_status NAPI_CDECL napi_get_typedarray_info(napi_env env, } if (data != nullptr) { - *data = static_cast(buffer->GetBackingStore()->Data()) + - array->ByteOffset(); + *data = static_cast(buffer->Data()) + array->ByteOffset(); } if (arraybuffer != nullptr) { @@ -3044,8 +3042,7 @@ napi_status NAPI_CDECL napi_get_dataview_info(napi_env env, } if (data != nullptr) { - *data = static_cast(buffer->GetBackingStore()->Data()) + - array->ByteOffset(); + *data = static_cast(buffer->Data()) + array->ByteOffset(); } if (arraybuffer != nullptr) { @@ -3256,7 +3253,7 @@ napi_status NAPI_CDECL napi_is_detached_arraybuffer(napi_env env, v8::Local value = v8impl::V8LocalValueFromJsValue(arraybuffer); *result = value->IsArrayBuffer() && - value.As()->GetBackingStore()->Data() == nullptr; + value.As()->Data() == nullptr; return napi_clear_last_error(env); } diff --git a/src/module_wrap.cc b/src/module_wrap.cc index 50ce8d510cb1a4..9f95668307be4d 100644 --- a/src/module_wrap.cc +++ b/src/module_wrap.cc @@ -178,8 +178,7 @@ void ModuleWrap::New(const FunctionCallbackInfo& args) { if (!args[5]->IsUndefined()) { CHECK(args[5]->IsArrayBufferView()); Local cached_data_buf = args[5].As(); - uint8_t* data = static_cast( - cached_data_buf->Buffer()->GetBackingStore()->Data()); + uint8_t* data = static_cast(cached_data_buf->Buffer()->Data()); cached_data = new ScriptCompiler::CachedData(data + cached_data_buf->ByteOffset(), cached_data_buf->ByteLength()); diff --git a/src/node.cc b/src/node.cc index b54abb3af80d4d..cb52ee2a42b79c 100644 --- a/src/node.cc +++ b/src/node.cc @@ -269,7 +269,7 @@ static void AtomicsWaitCallback(Isolate::AtomicsWaitEvent event, ", %.f) %s\n", static_cast(uv_os_getpid()), env->thread_id(), - array_buffer->GetBackingStore()->Data(), + array_buffer->Data(), offset_in_bytes, value, timeout_in_ms, diff --git a/src/node_buffer.cc b/src/node_buffer.cc index 5b2186feb8c707..d10baa052ecd94 100644 --- a/src/node_buffer.cc +++ b/src/node_buffer.cc @@ -244,8 +244,7 @@ bool HasInstance(Local obj) { char* Data(Local val) { CHECK(val->IsArrayBufferView()); Local ui = val.As(); - return static_cast(ui->Buffer()->GetBackingStore()->Data()) + - ui->ByteOffset(); + return static_cast(ui->Buffer()->Data()) + ui->ByteOffset(); } @@ -1156,14 +1155,13 @@ static void EncodeInto(const FunctionCallbackInfo& args) { Local dest = args[1].As(); Local buf = dest->Buffer(); - char* write_result = - static_cast(buf->GetBackingStore()->Data()) + dest->ByteOffset(); + char* write_result = static_cast(buf->Data()) + dest->ByteOffset(); size_t dest_length = dest->ByteLength(); // results = [ read, written ] Local result_arr = args[2].As(); uint32_t* results = reinterpret_cast( - static_cast(result_arr->Buffer()->GetBackingStore()->Data()) + + static_cast(result_arr->Buffer()->Data()) + result_arr->ByteOffset()); int nchars; @@ -1227,6 +1225,27 @@ void DetachArrayBuffer(const FunctionCallbackInfo& args) { } } +namespace { + +std::pair DecomposeBufferToParts(Local buffer) { + void* pointer; + size_t byte_length; + if (buffer->IsArrayBuffer()) { + Local ab = buffer.As(); + pointer = ab->Data(); + byte_length = ab->ByteLength(); + } else if (buffer->IsSharedArrayBuffer()) { + Local ab = buffer.As(); + pointer = ab->Data(); + byte_length = ab->ByteLength(); + } else { + CHECK(false); // Caller must validate. + } + return {pointer, byte_length}; +} + +} + void CopyArrayBuffer(const FunctionCallbackInfo& args) { // args[0] == Destination ArrayBuffer // args[1] == Destination ArrayBuffer Offset @@ -1240,32 +1259,23 @@ void CopyArrayBuffer(const FunctionCallbackInfo& args) { CHECK(args[3]->IsUint32()); CHECK(args[4]->IsUint32()); - std::shared_ptr destination; - std::shared_ptr source; - - if (args[0]->IsArrayBuffer()) { - destination = args[0].As()->GetBackingStore(); - } else if (args[0]->IsSharedArrayBuffer()) { - destination = args[0].As()->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()->GetBackingStore(); - } else if (args[0]->IsSharedArrayBuffer()) { - source = args[2].As()->GetBackingStore(); - } + void* source; + size_t source_byte_length; + std::tie(source, source_byte_length) = DecomposeBufferToParts(args[2]); uint32_t destination_offset = args[1].As()->Value(); uint32_t source_offset = args[3].As()->Value(); size_t bytes_to_copy = args[4].As()->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(destination->Data()) + destination_offset; - uint8_t* src = - static_cast(source->Data()) + source_offset; + uint8_t* dest = static_cast(destination) + destination_offset; + uint8_t* src = static_cast(source) + source_offset; memcpy(dest, src, bytes_to_copy); } diff --git a/src/node_contextify.cc b/src/node_contextify.cc index f383b6def93bbe..b075b916127f8d 100644 --- a/src/node_contextify.cc +++ b/src/node_contextify.cc @@ -743,7 +743,7 @@ void ContextifyScript::New(const FunctionCallbackInfo& args) { ScriptCompiler::CachedData* cached_data = nullptr; if (!cached_data_buf.IsEmpty()) { uint8_t* data = static_cast( - 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()); } @@ -1065,7 +1065,7 @@ void ContextifyContext::CompileFunction( ScriptCompiler::CachedData* cached_data = nullptr; if (!cached_data_buf.IsEmpty()) { uint8_t* data = static_cast( - 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()); } diff --git a/src/node_os.cc b/src/node_os.cc index 046a6106ccd0e5..eea6304158d29a 100644 --- a/src/node_os.cc +++ b/src/node_os.cc @@ -161,7 +161,7 @@ static void GetLoadAvg(const FunctionCallbackInfo& args) { Local array = args[0].As(); CHECK_EQ(array->Length(), 3); Local ab = array->Buffer(); - double* loadavg = static_cast(ab->GetBackingStore()->Data()); + double* loadavg = static_cast(ab->Data()); uv_loadavg(loadavg); } diff --git a/src/node_process_methods.cc b/src/node_process_methods.cc index 350a7094baad59..26a0633b2f0493 100644 --- a/src/node_process_methods.cc +++ b/src/node_process_methods.cc @@ -116,7 +116,7 @@ static void CPUUsage(const FunctionCallbackInfo& args) { // Get the double array pointer from the Float64Array argument. Local ab = get_fields_array_buffer(args, 0, 2); - double* fields = static_cast(ab->GetBackingStore()->Data()); + double* fields = static_cast(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; @@ -189,7 +189,7 @@ static void MemoryUsage(const FunctionCallbackInfo& args) { // Get the double array pointer from the Float64Array argument. Local ab = get_fields_array_buffer(args, 0, 5); - double* fields = static_cast(ab->GetBackingStore()->Data()); + double* fields = static_cast(ab->Data()); size_t rss; int err = uv_resident_set_memory(&rss); @@ -311,7 +311,7 @@ static void ResourceUsage(const FunctionCallbackInfo& args) { return env->ThrowUVException(err, "uv_getrusage"); Local ab = get_fields_array_buffer(args, 0, 16); - double* fields = static_cast(ab->GetBackingStore()->Data()); + double* fields = static_cast(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; diff --git a/src/node_wasi.cc b/src/node_wasi.cc index 965a619c8d4acd..4bb3fe2ccc39d5 100644 --- a/src/node_wasi.cc +++ b/src/node_wasi.cc @@ -1654,10 +1654,9 @@ void WASI::_SetMemory(const FunctionCallbackInfo& args) { uvwasi_errno_t WASI::backingStore(char** store, size_t* byte_length) { Local memory = PersistentToLocal::Strong(this->memory_); - std::shared_ptr backing_store = - memory->Buffer()->GetBackingStore(); - *byte_length = backing_store->ByteLength(); - *store = static_cast(backing_store->Data()); + Local ab = memory->Buffer(); + *byte_length = ab->ByteLength(); + *store = static_cast(ab->Data()); CHECK_NOT_NULL(*store); return UVWASI_ESUCCESS; } diff --git a/src/node_wasm_web_api.cc b/src/node_wasm_web_api.cc index 67437034bbee34..6fe5a51b4c4202 100644 --- a/src/node_wasm_web_api.cc +++ b/src/node_wasm_web_api.cc @@ -105,12 +105,12 @@ void WasmStreamingObject::Push(const FunctionCallbackInfo& args) { if (LIKELY(chunk->IsArrayBufferView())) { Local view = chunk.As(); - bytes = view->Buffer()->GetBackingStore()->Data(); + bytes = view->Buffer()->Data(); offset = view->ByteOffset(); size = view->ByteLength(); } else if (LIKELY(chunk->IsArrayBuffer())) { Local buffer = chunk.As(); - bytes = buffer->GetBackingStore()->Data(); + bytes = buffer->Data(); offset = 0; size = buffer->ByteLength(); } else { diff --git a/src/node_worker.cc b/src/node_worker.cc index c70ccd6bd43edd..05ee35ba712ae4 100644 --- a/src/node_worker.cc +++ b/src/node_worker.cc @@ -710,7 +710,7 @@ void Worker::GetResourceLimits(const FunctionCallbackInfo& args) { Local Worker::GetResourceLimits(Isolate* isolate) const { Local 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); diff --git a/src/node_zlib.cc b/src/node_zlib.cc index 5930ffd7a8ae8e..04c176095f8af2 100644 --- a/src/node_zlib.cc +++ b/src/node_zlib.cc @@ -605,8 +605,7 @@ class ZlibStream final : public CompressionStream { CHECK(args[4]->IsUint32Array()); Local array = args[4].As(); Local ab = array->Buffer(); - uint32_t* write_result = static_cast( - ab->GetBackingStore()->Data()); + uint32_t* write_result = static_cast(ab->Data()); CHECK(args[5]->IsFunction()); Local write_js_callback = args[5].As(); diff --git a/src/util-inl.h b/src/util-inl.h index 327893618525f8..caadba9dae2caa 100644 --- a/src/util-inl.h +++ b/src/util-inl.h @@ -537,8 +537,7 @@ void ArrayBufferViewContents::Read(v8::Local 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(abv->Buffer()->GetBackingStore()->Data()) + - abv->ByteOffset(); + data_ = static_cast(abv->Buffer()->Data()) + abv->ByteOffset(); } else { abv->CopyContents(stack_storage_, sizeof(stack_storage_)); data_ = stack_storage_; diff --git a/src/util.h b/src/util.h index a48071b093db97..e63990cc81c4a9 100644 --- a/src/util.h +++ b/src/util.h @@ -538,12 +538,10 @@ class BufferValue : public MaybeStackBuffer { #define SPREAD_BUFFER_ARG(val, name) \ CHECK((val)->IsArrayBufferView()); \ v8::Local name = (val).As(); \ - std::shared_ptr 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(name##_bs->Data()) + name##_offset; \ + static_cast(name->Buffer()->Data()) + name##_offset; \ if (name##_length > 0) \ CHECK_NE(name##_data, nullptr);