Revert "Use new arraybuffer deleter interface in d8"

This reverts commit 524215be.

Reason for revert: Breaks cfi:
https://ci.chromium.org/p/v8/builders/luci.v8.ci/V8%20Linux64%20-%20cfi/16422

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}

TBR=mstarzinger@chromium.org,herhut@chromium.org

Change-Id: I64c4e76d8d68bad8df4ba3297c099b9b44eabc7c
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: v8:8073, chromium:836800
Cq-Include-Trybots: luci.chromium.try:linux_chromium_rel_ng
Reviewed-on: https://chromium-review.googlesource.com/1187241Reviewed-by: Michael Achenbach <machenbach@chromium.org>
Commit-Queue: Michael Achenbach <machenbach@chromium.org>
Cr-Commit-Position: refs/heads/master@{#55366}

src/api.cc

@@ -7472,7 +7472,17 @@ 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.
@@ -7723,7 +7733,17 @@ 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.
@@ -7797,11 +7817,9 @@ 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, is_wasm_memory);
+                          byte_length, i::SharedFlag::kShared);
   return Utils::ToLocalShared(obj);
 }
 

src/d8.cc

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

src/d8.h

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

src/wasm/module-compiler.cc

@@ -992,9 +992,13 @@ 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);
+    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()));
   } 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