Revert "[D8] Clean up ArrayBuffer Allocators in shell."

This reverts commit 0c2faa0633.

Reason for revert: seems to make win asan flaky:
https://build.chromium.org/p/client.v8/builders/V8%20Win32%20ASAN/builds/1094
https://build.chromium.org/p/client.v8/builders/V8%20Win32%20ASAN/builds/1095

Original change's description:
> [D8] Clean up ArrayBuffer Allocators in shell.
> 
> - Reworks the class hierarchy in d8.cc to conform to the allocator API.
>   In particular, allocators should malloc/free or equivalent unless
>   v8::ArrayBuffer::Allocator::Reserve is called.
> - Modifies ExternalizedContents to remember the allocation mode.
> - ArrayAllocatorBase now tracks its allocations to make sure it doesn't
>   call Free on externalized array buffers it didn't allocate.
> 
> Bug: chromium:756050
> Change-Id: Ic2d07d36358f1b4fa542bea27f93d1d51a1757e1
> Reviewed-on: https://chromium-review.googlesource.com/807355
> Commit-Queue: Bill Budge <bbudge@chromium.org>
> Reviewed-by: Ben Titzer <titzer@chromium.org>
> Reviewed-by: Eric Holk <eholk@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#49893}

TBR=bbudge@chromium.org,titzer@chromium.org,eholk@chromium.org

Change-Id: I0f3ecc10de843102e7681285f0300cf98704c92c
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: chromium:756050
Reviewed-on: https://chromium-review.googlesource.com/810904
Reviewed-by: Michael Achenbach <machenbach@chromium.org>
Commit-Queue: Michael Achenbach <machenbach@chromium.org>
Cr-Commit-Position: refs/heads/master@{#49895}

src/d8.cc

@@ -66,104 +66,137 @@ namespace v8 {
 
 namespace {
 
-const int kMB = 1024 * 1024;
+const int MB = 1024 * 1024;
-const size_t kTwoGB = 2u * 1024u * 1024u * 1024u;
-
 const int kMaxWorkers = 50;
-const int kMaxSerializerMemoryUsage =
+const int kMaxSerializerMemoryUsage = 1 * MB;  // Arbitrary maximum for testing.
-    1 * kMB;  // Arbitrary maximum for testing.
 
-// The ArrayBuffer allocator for the shell. It implements kNormal allocations
+#define USE_VM 1
-// and delegates kReservation allocations to the default V8 allocator. It also
+#define VM_THRESHOLD 65536
-// tracks its allocations so it doesn't free externalized ArrayBuffers that it
+// TODO(titzer): allocations should fail if >= 2gb because of
-// didn't allocate.
+// array buffers storing the lengths as a SMI internally.
-// TODO(bbudge) Figure out why foreign externalized ArrayBuffers are passed into
+#define TWO_GB (2u * 1024u * 1024u * 1024u)
-// these allocators in mjsunit/wasm/worker-memory.js.
+
+// Forwards memory reservation and protection functions to the V8 default
+// allocator. Used by ShellArrayBufferAllocator and MockArrayBufferAllocator.
 class ArrayBufferAllocatorBase : public v8::ArrayBuffer::Allocator {
+  std::unique_ptr<Allocator> allocator_ =
+      std::unique_ptr<Allocator>(NewDefaultAllocator());
+
  public:
-  void* Allocate(size_t length) override {
+  void* Reserve(size_t length) override { return allocator_->Reserve(length); }
-    size_t alloc_length = GetAllocLength(length);
-    // TODO(titzer): allocations should fail if >= 2gb because array buffers
-    // store their lengths as a SMI internally.
-    if (alloc_length > kTwoGB) return nullptr;
-#if V8_OS_AIX && _LINUX_SOURCE_COMPAT
-    // Work around for GCC bug on AIX
-    // See: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=79839
-    void* data = __linux_calloc(alloc_length, 1);
-#else
-    void* data = calloc(alloc_length, 1);
-#endif
-    MSAN_MEMORY_IS_INITIALIZED(data, alloc_length);
 
-    allocations_.insert(std::make_pair(data, alloc_length));
+  void Free(void*, size_t) override = 0;
-    return data;
-  }
-
-  void* AllocateUninitialized(size_t length) override {
-    size_t alloc_length = GetAllocLength(length);
-    // TODO(titzer): allocations should fail if >= 2gb because array buffers
-    // store their lengths as a SMI internally.
-    if (alloc_length > kTwoGB) return nullptr;
-#if V8_OS_AIX && _LINUX_SOURCE_COMPAT
-    // Work around for GCC bug on AIX
-    // See: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=79839
-    void* data = __linux_malloc(alloc_length);
-#else
-    void* data = malloc(alloc_length);
-#endif
-    allocations_.insert(std::make_pair(data, alloc_length));
-    return data;
-  }
-
-  void Free(void* data, size_t length) override {
-    if (allocations_.find(data) != allocations_.end()) {
-      DCHECK_EQ(GetAllocLength(length), allocations_[data]);
-      free(data);
-      allocations_.erase(data);
-    }
-  }
 
   void Free(void* data, size_t length, AllocationMode mode) override {
-    size_t alloc_length = GetAllocLength(length);
+    switch (mode) {
-    if (mode == AllocationMode::kNormal) {
+      case AllocationMode::kNormal: {
-      Free(data, alloc_length);
+        return Free(data, length);
-    } else {
+      }
-      allocator_->Free(data, alloc_length, mode);
+      case AllocationMode::kReservation: {
+        return allocator_->Free(data, length, mode);
+      }
     }
   }
 
-  void* Reserve(size_t length) override {
-    return allocator_->Reserve(GetAllocLength(length));
-  }
-
   void SetProtection(void* data, size_t length,
                      Protection protection) override {
     allocator_->SetProtection(data, length, protection);
   }
-
- private:
-  virtual size_t GetAllocLength(size_t length) const { return length; }
-
-  std::unique_ptr<Allocator> allocator_ =
-      std::unique_ptr<Allocator>(NewDefaultAllocator());
-  std::unordered_map<void*, size_t> allocations_;
 };
 
 class ShellArrayBufferAllocator : public ArrayBufferAllocatorBase {
- private:
+ public:
-  virtual size_t GetAllocLength(size_t length) const {
+  void* Allocate(size_t length) override {
+#if USE_VM
+    if (RoundToPageSize(&length)) {
+      void* data = VirtualMemoryAllocate(length);
+#if DEBUG
+      if (data) {
+        // In debug mode, check the memory is zero-initialized.
+        size_t limit = length / sizeof(uint64_t);
+        uint64_t* ptr = reinterpret_cast<uint64_t*>(data);
+        for (size_t i = 0; i < limit; i++) {
+          DCHECK_EQ(0u, ptr[i]);
+        }
+      }
+#endif
+      return data;
+    }
+#endif
+    void* data = AllocateUninitialized(length);
+    return data == nullptr ? data : memset(data, 0, length);
+  }
+  void* AllocateUninitialized(size_t length) override {
+#if USE_VM
+    if (RoundToPageSize(&length)) return VirtualMemoryAllocate(length);
+#endif
+// Work around for GCC bug on AIX
+// See: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=79839
+#if V8_OS_AIX && _LINUX_SOURCE_COMPAT
+    return __linux_malloc(length);
+#else
+    return malloc(length);
+#endif
+  }
+  using ArrayBufferAllocatorBase::Free;
+  void Free(void* data, size_t length) override {
+#if USE_VM
+    if (RoundToPageSize(&length)) {
+      CHECK(base::OS::Free(data, length));
+      return;
+    }
+#endif
+    free(data);
+  }
+  // If {length} is at least {VM_THRESHOLD}, round up to next page size and
+  // return {true}. Otherwise return {false}.
+  bool RoundToPageSize(size_t* length) {
+    size_t page_size = base::OS::AllocatePageSize();
+    if (*length >= VM_THRESHOLD && *length < TWO_GB) {
+      *length = RoundUp(*length, page_size);
+      return true;
+    }
+    return false;
+  }
+#if USE_VM
+  void* VirtualMemoryAllocate(size_t length) {
     size_t page_size = base::OS::AllocatePageSize();
     size_t alloc_size = RoundUp(length, page_size);
-    return alloc_size;
+    void* address = base::OS::Allocate(nullptr, alloc_size, page_size,
+                                       base::OS::MemoryPermission::kReadWrite);
+    if (address != nullptr) {
+#if defined(LEAK_SANITIZER)
+      __lsan_register_root_region(address, alloc_size);
+#endif
+      MSAN_MEMORY_IS_INITIALIZED(address, alloc_size);
+    }
+    return address;
   }
+#endif
 };
 
 class MockArrayBufferAllocator : public ArrayBufferAllocatorBase {
- private:
+  const size_t kAllocationLimit = 10 * MB;
-  size_t GetAllocLength(size_t length) const override {
+  size_t get_actual_length(size_t length) const {
-    const size_t kAllocationLimit = 10 * kMB;
     return length > kAllocationLimit ? base::OS::AllocatePageSize() : length;
   }
+
+ public:
+  void* Allocate(size_t length) override {
+    const size_t actual_length = get_actual_length(length);
+    void* data = AllocateUninitialized(actual_length);
+    return data == nullptr ? data : memset(data, 0, actual_length);
+  }
+  void* AllocateUninitialized(size_t length) override {
+    return malloc(get_actual_length(length));
+  }
+  void Free(void* p, size_t) override { free(p); }
+  void Free(void* data, size_t length, AllocationMode mode) override {
+    ArrayBufferAllocatorBase::Free(data, get_actual_length(length), mode);
+  }
+  void* Reserve(size_t length) override {
+    return ArrayBufferAllocatorBase::Reserve(get_actual_length(length));
+  }
 };
 
 // Predictable v8::Platform implementation. Background tasks and idle tasks are
@@ -267,7 +300,7 @@ base::Thread::Options GetThreadOptions(const char* name) {
   // which is not enough to parse the big literal expressions used in tests.
   // The stack size should be at least StackGuard::kLimitSize + some
   // OS-specific padding for thread startup code.  2Mbytes seems to be enough.
-  return base::Thread::Options(name, 2 * kMB);
+  return base::Thread::Options(name, 2 * MB);
 }
 
 }  // namespace
@@ -2537,7 +2570,7 @@ void SourceGroup::JoinThread() {
 }
 
 ExternalizedContents::~ExternalizedContents() {
-  Shell::array_buffer_allocator->Free(base_, length_, mode_);
+  Shell::array_buffer_allocator->Free(data_, size_);
 }
 
 void SerializationDataQueue::Enqueue(std::unique_ptr<SerializationData> data) {
@@ -3446,6 +3479,3 @@ int main(int argc, char* argv[]) {
   return v8::Shell::Main(argc, argv);
 }
 #endif
-
-#undef CHECK
-#undef DCHECK

src/d8.h

@@ -149,36 +149,28 @@ class SourceGroup {
 class ExternalizedContents {
  public:
   explicit ExternalizedContents(const ArrayBuffer::Contents& contents)
-      : base_(contents.AllocationBase()),
+      : data_(contents.Data()), size_(contents.ByteLength()) {}
-        length_(contents.AllocationLength()),
-        mode_(contents.AllocationMode()) {}
   explicit ExternalizedContents(const SharedArrayBuffer::Contents& contents)
-      : base_(contents.AllocationBase()),
+      : data_(contents.Data()), size_(contents.ByteLength()) {}
-        length_(contents.AllocationLength()),
-        mode_(contents.AllocationMode()) {}
   ExternalizedContents(ExternalizedContents&& other)
-      : base_(other.base_), length_(other.length_), mode_(other.mode_) {
+      : data_(other.data_), size_(other.size_) {
-    other.base_ = nullptr;
+    other.data_ = nullptr;
-    other.length_ = 0;
+    other.size_ = 0;
-    other.mode_ = ArrayBuffer::Allocator::AllocationMode::kNormal;
   }
   ExternalizedContents& operator=(ExternalizedContents&& other) {
     if (this != &other) {
-      base_ = other.base_;
+      data_ = other.data_;
-      length_ = other.length_;
+      size_ = other.size_;
-      mode_ = other.mode_;
+      other.data_ = nullptr;
-      other.base_ = nullptr;
+      other.size_ = 0;
-      other.length_ = 0;
-      other.mode_ = ArrayBuffer::Allocator::AllocationMode::kNormal;
     }
     return *this;
   }
   ~ExternalizedContents();
 
  private:
-  void* base_;
+  void* data_;
-  size_t length_;
+  size_t size_;
-  ArrayBuffer::Allocator::AllocationMode mode_;
 
   DISALLOW_COPY_AND_ASSIGN(ExternalizedContents);
 };