Reland "Use new arraybuffer deleter interface in d8"

This is a reland of 524215be

Original change's description:
> 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/1186681
> Reviewed-by: Michael Starzinger <mstarzinger@chromium.org>
> Commit-Queue: Stephan Herhut <herhut@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#55361}

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

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.
@@ -7505,13 +7495,15 @@ v8::ArrayBuffer::Contents::Contents(void* data, size_t byte_length,
   DCHECK_LE(byte_length_, allocation_length_);
 }
 
-void WasmMemoryDeleter(void* buffer, size_t lenght,
-                       internal::wasm::WasmEngine* engine) {
+void WasmMemoryDeleter(void* buffer, size_t lenght, void* info) {
+  internal::wasm::WasmEngine* engine =
+      reinterpret_cast<internal::wasm::WasmEngine*>(info);
   CHECK(engine->memory_tracker()->FreeMemoryIfIsWasmMemory(nullptr, buffer));
 }
 
-void ArrayBufferDeleter(void* buffer, size_t length,
-                        ArrayBufferAllocator* allocator) {
+void ArrayBufferDeleter(void* buffer, size_t length, void* info) {
+  v8::ArrayBuffer::Allocator* allocator =
+      reinterpret_cast<v8::ArrayBuffer::Allocator*>(info);
   allocator->Free(buffer, length);
 }
 
@@ -7523,9 +7515,7 @@ v8::ArrayBuffer::Contents v8::ArrayBuffer::GetContents() {
       self->allocation_length(),
       self->is_wasm_memory() ? Allocator::AllocationMode::kReservation
                              : Allocator::AllocationMode::kNormal,
-      self->is_wasm_memory()
-          ? reinterpret_cast<Contents::DeleterCallback>(WasmMemoryDeleter)
-          : reinterpret_cast<Contents::DeleterCallback>(ArrayBufferDeleter),
+      self->is_wasm_memory() ? WasmMemoryDeleter : ArrayBufferDeleter,
       self->is_wasm_memory()
           ? static_cast<void*>(self->GetIsolate()->wasm_engine())
           : static_cast<void*>(self->GetIsolate()->array_buffer_allocator()));
@@ -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

@@ -992,13 +992,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