[inspector] Make `ArrayBuffer.[[ArrayBufferData]]` deterministic.

Previously the internal `[[ArrayBufferData]]` property for `ArrayBuffer`
objects reported by the inspector (and used by the DevTools front-end to
identify `ArrayBuffer`s and `WebAssembly.Memory`s using the same backing
store) simply contained a hex string representation of the backing store
pointer. However that unnecessarily leaks internal addresses and more
importantly is not deterministic, which complicates tests (just blew up
on layout tests).

This CL introduces an automatically incremented `BackingStore::id()`,
which is used instead now and is deterministic.

Bug: chromium:1199701, chromium:1163802, chromium:1249961
Change-Id: I8ee47009cd825cfdbe00230f617c87c90508ab2a
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3162144
Commit-Queue: Benedikt Meurer <bmeurer@chromium.org>
Reviewed-by: Jakob Kummerow <jkummerow@chromium.org>
Cr-Commit-Position: refs/heads/main@{#76893}

src/objects/backing-store.cc

@@ -54,6 +54,8 @@ constexpr size_t kAddressSpaceLimit = 0xC0000000;  // 3 GiB
 
 std::atomic<uint64_t> reserved_address_space_{0};
 
+std::atomic<uint32_t> next_backing_store_id_{1};
+
 // Allocation results are reported to UMA
 //
 // See wasm_memory_allocation_result in counters.h
@@ -145,6 +147,34 @@ void BackingStore::Clear() {
   type_specific_data_.v8_api_array_buffer_allocator = nullptr;
 }
 
+BackingStore::BackingStore(void* buffer_start, size_t byte_length,
+                           size_t max_byte_length, size_t byte_capacity,
+                           SharedFlag shared, ResizableFlag resizable,
+                           bool is_wasm_memory, bool free_on_destruct,
+                           bool has_guard_regions, bool custom_deleter,
+                           bool empty_deleter)
+    : buffer_start_(buffer_start),
+      byte_length_(byte_length),
+      max_byte_length_(max_byte_length),
+      byte_capacity_(byte_capacity),
+      id_(next_backing_store_id_.fetch_add(1)),
+      is_shared_(shared == SharedFlag::kShared),
+      is_resizable_(resizable == ResizableFlag::kResizable),
+      is_wasm_memory_(is_wasm_memory),
+      holds_shared_ptr_to_allocator_(false),
+      free_on_destruct_(free_on_destruct),
+      has_guard_regions_(has_guard_regions),
+      globally_registered_(false),
+      custom_deleter_(custom_deleter),
+      empty_deleter_(empty_deleter) {
+  // TODO(v8:11111): RAB / GSAB - Wasm integration.
+  DCHECK_IMPLIES(is_wasm_memory_, !is_resizable_);
+  DCHECK_IMPLIES(is_resizable_, !custom_deleter_);
+  DCHECK_IMPLIES(is_resizable_, free_on_destruct_);
+  DCHECK_IMPLIES(!is_wasm_memory && !is_resizable_,
+                 byte_length_ == max_byte_length_);
+}
+
 BackingStore::~BackingStore() {
   GlobalBackingStoreRegistry::Unregister(this);
 

src/objects/backing-store.h

@@ -163,44 +163,29 @@ class V8_EXPORT_PRIVATE BackingStore : public BackingStoreBase {
     return byte_length();
   }
 
+  uint32_t id() const { return id_; }
+
  private:
   friend class GlobalBackingStoreRegistry;
 
   BackingStore(void* buffer_start, size_t byte_length, size_t max_byte_length,
                size_t byte_capacity, SharedFlag shared, ResizableFlag resizable,
                bool is_wasm_memory, bool free_on_destruct,
-               bool has_guard_regions, bool custom_deleter, bool empty_deleter)
-      : buffer_start_(buffer_start),
-        byte_length_(byte_length),
-        max_byte_length_(max_byte_length),
-        byte_capacity_(byte_capacity),
-        is_shared_(shared == SharedFlag::kShared),
-        is_resizable_(resizable == ResizableFlag::kResizable),
-        is_wasm_memory_(is_wasm_memory),
-        holds_shared_ptr_to_allocator_(false),
-        free_on_destruct_(free_on_destruct),
-        has_guard_regions_(has_guard_regions),
-        globally_registered_(false),
-        custom_deleter_(custom_deleter),
-        empty_deleter_(empty_deleter) {
-    // TODO(v8:11111): RAB / GSAB - Wasm integration.
-    DCHECK_IMPLIES(is_wasm_memory_, !is_resizable_);
-    DCHECK_IMPLIES(is_resizable_, !custom_deleter_);
-    DCHECK_IMPLIES(is_resizable_, free_on_destruct_);
-    DCHECK_IMPLIES(!is_wasm_memory && !is_resizable_,
-                   byte_length_ == max_byte_length_);
-  }
+               bool has_guard_regions, bool custom_deleter, bool empty_deleter);
   BackingStore(const BackingStore&) = delete;
   BackingStore& operator=(const BackingStore&) = delete;
   void SetAllocatorFromIsolate(Isolate* isolate);
 
   void* buffer_start_ = nullptr;
-  std::atomic<size_t> byte_length_{0};
+  std::atomic<size_t> byte_length_;
   // Max byte length of the corresponding JSArrayBuffer(s).
-  size_t max_byte_length_ = 0;
+  size_t max_byte_length_;
   // Amount of the memory allocated
-  size_t byte_capacity_ = 0;
-
+  size_t byte_capacity_;
+  // Unique ID of this backing store. Currently only used by DevTools, to
+  // identify stores used by several ArrayBuffers or WebAssembly memories
+  // (reported by the inspector as [[ArrayBufferData]] internal property)
+  uint32_t id_;
   struct DeleterInfo {
     v8::BackingStore::DeleterCallback callback;
     void* data;

src/runtime/runtime-debug.cc

@@ -335,16 +335,15 @@ MaybeHandle<JSArray> Runtime::GetInternalProperties(Isolate* isolate,
                              "[[ArrayBufferByteLength]]"),
                          isolate->factory()->NewNumberFromSize(byte_length));
 
-      // Use the backing store pointer as a unique ID
-      base::EmbeddedVector<char, 32> buffer_data_vec;
-      int len =
-          SNPrintF(buffer_data_vec, V8PRIxPTR_FMT,
-                   reinterpret_cast<Address>(js_array_buffer->backing_store()));
+      auto backing_store = js_array_buffer->GetBackingStore();
+      Handle<Object> array_buffer_data =
+          backing_store
+              ? isolate->factory()->NewNumberFromUint(backing_store->id())
+              : isolate->factory()->null_value();
       result = ArrayList::Add(
           isolate, result,
           isolate->factory()->NewStringFromAsciiChecked("[[ArrayBufferData]]"),
-          isolate->factory()->InternalizeUtf8String(
-              buffer_data_vec.SubVector(0, len)));
+          array_buffer_data);
 
       Handle<Symbol> memory_symbol =
           isolate->factory()->array_buffer_wasm_memory_symbol();

test/inspector/debugger/get-properties-paused-expected.txt

@@ -61,7 +61,7 @@ Running test: testArrayBuffer
 Running test: testArrayBufferWithBrokenUintCtor
 Internal properties
   [[ArrayBufferByteLength]] number 7
-  [[ArrayBufferData]] string 0x...
+  [[ArrayBufferData]] number 2
   [[Int8Array]] object undefined
   [[Prototype]] object undefined
   [[Uint8Array]] object undefined

test/inspector/debugger/get-properties-paused.js

@@ -96,11 +96,7 @@ let { Protocol } = InspectorTest.start('Checks Runtime.getProperties method whil
       for (var i = 0; i < internalPropertyArray.length; i++) {
         var p = internalPropertyArray[i];
         var v = p.value;
-        if (p.name === "[[ArrayBufferData]]")
-          // Hex value for pointer is non-deterministic
-          InspectorTest.log(`  ${p.name} ${v.type} ${v.value.substr(0, 2)}...`);
-        else
-          InspectorTest.log(`  ${p.name} ${v.type} ${v.value}`);
+        InspectorTest.log(`  ${p.name} ${v.type} ${v.value}`);
       }
     }
 

test/inspector/runtime/get-properties-expected.txt

@@ -176,7 +176,7 @@ Running test: testDetachedArrayBuffer
 Running test: testArrayBufferWithBrokenUintCtor
 Internal properties
   [[ArrayBufferByteLength]] number 7
-  [[ArrayBufferData]] string 0x...
+  [[ArrayBufferData]] number 4
   [[Int8Array]] object undefined
   [[Prototype]] object undefined
   [[Uint8Array]] object undefined

test/inspector/runtime/get-properties.js

@@ -155,11 +155,7 @@ async function logGetPropertiesResult(objectId, flags = { ownProperties: true })
     for (var i = 0; i < array.length; i++) {
       var p = array[i];
       var v = p.value;
-      if (p.name == "[[ArrayBufferData]]")
-        // Hex value for pointer is non-deterministic
-        InspectorTest.log(`  ${p.name} ${v.type} ${v.value.substr(0, 2)}...`);
-      else
-        InspectorTest.log(`  ${p.name} ${v.type} ${v.value}`);
+      InspectorTest.log(`  ${p.name} ${v.type} ${v.value}`);
     }
   }