Use new arraybuffer deleter interface in d8

With this cl we start using the custom deleter to free externalized
array buffers. This also allows us to keep wasm memories registered
with the wasm memory tracker and thereby to propagate that a memory
is wasm allocated over postMessage calls.

Bug: v8:8073, chromium:836800
Cq-Include-Trybots: luci.chromium.try:linux_chromium_rel_ng
Change-Id: I57e3ea44d9c6633ada7996677dd1de4da810ab64
Reviewed-on: https://chromium-review.googlesource.com/1186681Reviewed-by: Michael Starzinger <mstarzinger@chromium.org>
Commit-Queue: Stephan Herhut <herhut@chromium.org>
Cr-Commit-Position: refs/heads/master@{#55361}

src/api.cc

@@ -7472,17 +7472,7 @@ v8::ArrayBuffer::Contents v8::ArrayBuffer::Externalize() {
                   "ArrayBuffer already externalized");
   self->set_is_external(true);
 
-  // We need to capture the contents before releasing the allocation from the
-  // Wasm tracker, because otherwise we will not correctly capture the
-  // allocation data.
   const v8::ArrayBuffer::Contents contents = GetContents();
-  if (self->is_wasm_memory()) {
-    // Since this is being externalized, the Wasm Allocation Tracker can no
-    // longer track it.
-    //
-    // TODO(eholk): Find a way to track this across externalization
-    self->StopTrackingWasmMemory(isolate);
-  }
   isolate->heap()->UnregisterArrayBuffer(*self);
 
   // A regular copy is good enough. No move semantics needed.
@@ -7733,17 +7723,7 @@ v8::SharedArrayBuffer::Contents v8::SharedArrayBuffer::Externalize() {
                   "SharedArrayBuffer already externalized");
   self->set_is_external(true);
 
-  // We need to capture the contents before releasing the allocation from the
-  // Wasm tracker, because otherwise we will not correctly capture the
-  // allocation data.
   const v8::SharedArrayBuffer::Contents contents = GetContents();
-  if (self->is_wasm_memory()) {
-    // Since this is being externalized, the Wasm Allocation Tracker can no
-    // longer track it.
-    //
-    // TODO(eholk): Find a way to track this across externalization
-    self->StopTrackingWasmMemory(isolate);
-  }
   isolate->heap()->UnregisterArrayBuffer(*self);
 
   // A regular copy is good enough. No move semantics needed.
@@ -7817,9 +7797,11 @@ Local<SharedArrayBuffer> v8::SharedArrayBuffer::New(
   ENTER_V8_NO_SCRIPT_NO_EXCEPTION(i_isolate);
   i::Handle<i::JSArrayBuffer> obj =
       i_isolate->factory()->NewJSArrayBuffer(i::SharedFlag::kShared);
+  bool is_wasm_memory =
+      i_isolate->wasm_engine()->memory_tracker()->IsWasmMemory(data);
   i::JSArrayBuffer::Setup(obj, i_isolate,
                           mode == ArrayBufferCreationMode::kExternalized, data,
-                          byte_length, i::SharedFlag::kShared);
+                          byte_length, i::SharedFlag::kShared, is_wasm_memory);
   return Utils::ToLocalShared(obj);
 }
 

src/d8.cc

@@ -2567,12 +2567,8 @@ void SourceGroup::JoinThread() {
 }
 
 ExternalizedContents::~ExternalizedContents() {
-  if (base_ != nullptr) {
-    if (mode_ == ArrayBuffer::Allocator::AllocationMode::kReservation) {
-      CHECK(i::FreePages(base_, length_));
-    } else {
-      Shell::array_buffer_allocator->Free(base_, length_);
-    }
+  if (data_ != nullptr) {
+    deleter_(data_, length_, deleter_data_);
   }
 }
 

src/d8.h

@@ -132,38 +132,45 @@ class SourceGroup {
 class ExternalizedContents {
  public:
   explicit ExternalizedContents(const ArrayBuffer::Contents& contents)
-      : base_(contents.AllocationBase()),
-        length_(contents.AllocationLength()),
-        mode_(contents.AllocationMode()) {}
+      : data_(contents.Data()),
+        length_(contents.ByteLength()),
+        deleter_(contents.Deleter()),
+        deleter_data_(contents.DeleterData()) {}
   explicit ExternalizedContents(const SharedArrayBuffer::Contents& contents)
-      : base_(contents.AllocationBase()),
-        length_(contents.AllocationLength()),
-        mode_(contents.AllocationMode()) {}
+      : data_(contents.Data()),
+        length_(contents.ByteLength()),
+        deleter_(contents.Deleter()),
+        deleter_data_(contents.DeleterData()) {}
   ExternalizedContents(ExternalizedContents&& other) V8_NOEXCEPT
-      : base_(other.base_),
+      : data_(other.data_),
         length_(other.length_),
-        mode_(other.mode_) {
-    other.base_ = nullptr;
+        deleter_(other.deleter_),
+        deleter_data_(other.deleter_data_) {
+    other.data_ = nullptr;
     other.length_ = 0;
-    other.mode_ = ArrayBuffer::Allocator::AllocationMode::kNormal;
+    other.deleter_ = nullptr;
+    other.deleter_data_ = nullptr;
   }
   ExternalizedContents& operator=(ExternalizedContents&& other) V8_NOEXCEPT {
     if (this != &other) {
-      base_ = other.base_;
+      data_ = other.data_;
       length_ = other.length_;
-      mode_ = other.mode_;
-      other.base_ = nullptr;
+      deleter_ = other.deleter_;
+      deleter_data_ = other.deleter_data_;
+      other.data_ = nullptr;
       other.length_ = 0;
-      other.mode_ = ArrayBuffer::Allocator::AllocationMode::kNormal;
+      other.deleter_ = nullptr;
+      other.deleter_data_ = nullptr;
     }
     return *this;
   }
   ~ExternalizedContents();
 
  private:
-  void* base_;
+  void* data_;
   size_t length_;
-  ArrayBuffer::Allocator::AllocationMode mode_;
+  ArrayBuffer::Contents::DeleterCallback deleter_;
+  void* deleter_data_;
 
   DISALLOW_COPY_AND_ASSIGN(ExternalizedContents);
 };

src/wasm/module-compiler.cc

@@ -963,13 +963,9 @@ MaybeHandle<WasmInstanceObject> InstanceBuilder::Build() {
     Handle<JSArrayBuffer> memory = memory_.ToHandleChecked();
     memory->set_is_neuterable(false);
 
-    DCHECK_IMPLIES(use_trap_handler(),
-                   module_->origin == kAsmJsOrigin ||
-                       memory->is_wasm_memory() ||
-                       memory->backing_store() == nullptr ||
-                       // TODO(836800) Remove once is_wasm_memory transfers over
-                       // post-message.
-                       (enabled_.threads && memory->is_shared()));
+    DCHECK_IMPLIES(use_trap_handler(), module_->origin == kAsmJsOrigin ||
+                                           memory->is_wasm_memory() ||
+                                           memory->backing_store() == nullptr);
   } else if (initial_pages > 0 || use_trap_handler()) {
     // We need to unconditionally create a guard region if using trap handlers,
     // even when the size is zero to prevent null-dereference issues