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: nodejs#32226 Refs: nodejs#43921
kvakil · Jul 21, 2022 · 261134e · 261134e
1 parent 0359430
commit 261134e
Show file tree

Hide file tree

Showing 14 changed files with 72 additions and 75 deletions.
diff --git a/src/aliased_buffer.h b/src/aliased_buffer.h
@@ -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);
@@ -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;
@@ -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);
 

diff --git 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<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);
@@ -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) {
@@ -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) {
@@ -3255,8 +3252,8 @@ 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;
+  *result =
+      value->IsArrayBuffer() && value.As<v8::ArrayBuffer>()->Data() == nullptr;
 
   return napi_clear_last_error(env);
 }
diff --git a/src/module_wrap.cc b/src/module_wrap.cc
@@ -178,8 +178,8 @@ 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());

diff --git a/src/node.cc b/src/node.cc
@@ -266,10 +266,10 @@ static void AtomicsWaitCallback(Isolate::AtomicsWaitEvent event,
 
   fprintf(stderr,
           "(node:%d) [Thread %" PRIu64 "] Atomics.wait(%p + %zx, %" PRId64
-              ", %.f) %s\n",
+          ", %.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,

diff --git a/src/node_buffer.cc b/src/node_buffer.cc
@@ -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();
 }
 
 
@@ -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;
@@ -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};
+}
+
+}  // namespace
+
 void CopyArrayBuffer(const FunctionCallbackInfo<Value>& args) {
   // args[0] == Destination ArrayBuffer
   // args[1] == Destination ArrayBuffer Offset
@@ -1240,32 +1259,24 @@ void CopyArrayBuffer(const FunctionCallbackInfo<Value>& args) {
   CHECK(args[3]->IsUint32());
   CHECK(args[4]->IsUint32());
 
-  std::shared_ptr<BackingStore> destination;
-  std::shared_ptr<BackingStore> source;
+  void* destination;
+  size_t destination_byte_length;
+  std::tie(destination, destination_byte_length) =
+      DecomposeBufferToParts(args[0]);
 
-  if (args[0]->IsArrayBuffer()) {
-    destination = args[0].As<ArrayBuffer>()->GetBackingStore();
-  } else if (args[0]->IsSharedArrayBuffer()) {
-    destination = args[0].As<SharedArrayBuffer>()->GetBackingStore();
-  }
-
-  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);
 }
 

diff --git a/src/node_contextify.cc b/src/node_contextify.cc
@@ -742,8 +742,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());
+    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());
   }
@@ -1064,8 +1063,7 @@ void ContextifyContext::CompileFunction(
   // Read cache from cached data buffer
   ScriptCompiler::CachedData* cached_data = nullptr;
   if (!cached_data_buf.IsEmpty()) {
-    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());
   }

diff --git a/src/node_os.cc b/src/node_os.cc
@@ -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);
 }
 

diff --git a/src/node_process_methods.cc b/src/node_process_methods.cc
@@ -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;
@@ -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);
@@ -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;

diff --git a/src/node_wasi.cc b/src/node_wasi.cc
@@ -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;
 }

diff --git a/src/node_wasm_web_api.cc b/src/node_wasm_web_api.cc
@@ -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 {

diff --git a/src/node_worker.cc b/src/node_worker.cc
@@ -710,9 +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(),
-         resource_limits_,
-         sizeof(resource_limits_));
+  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
@@ -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>();

diff --git a/src/util-inl.h b/src/util-inl.h
@@ -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_;

diff --git a/src/util.h b/src/util.h
@@ -535,17 +535,14 @@ class BufferValue : public MaybeStackBuffer<char> {
   inline std::string ToString() const { return std::string(out(), length()); }
 };
 
-#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;                  \
-  if (name##_length > 0)                                                      \
-    CHECK_NE(name##_data, nullptr);
+#define SPREAD_BUFFER_ARG(val, name)                                           \
+  CHECK((val)->IsArrayBufferView());                                           \
+  v8::Local<v8::ArrayBufferView> name = (val).As<v8::ArrayBufferView>();       \
+  const size_t name##_offset = name->ByteOffset();                             \
+  const size_t name##_length = name->ByteLength();                             \
+  char* const name##_data =                                                    \
+      static_cast<char*>(name->Buffer()->Data()) + name##_offset;              \
+  if (name##_length > 0) CHECK_NE(name##_data, nullptr);
 
 // Use this when a variable or parameter is unused in order to explicitly
 // silence a compiler warning about that.