[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}

src/d8.cc

@@ -66,137 +66,104 @@ namespace v8 {
 
 namespace {
 
-const int MB = 1024 * 1024;
-const int kMaxWorkers = 50;
-const int kMaxSerializerMemoryUsage = 1 * MB;  // Arbitrary maximum for testing.
-
-#define USE_VM 1
-#define VM_THRESHOLD 65536
-// TODO(titzer): allocations should fail if >= 2gb because of
-// array buffers storing the lengths as a SMI internally.
-#define TWO_GB (2u * 1024u * 1024u * 1024u)
+const int kMB = 1024 * 1024;
+const size_t kTwoGB = 2u * 1024u * 1024u * 1024u;
 
-// Forwards memory reservation and protection functions to the V8 default
-// allocator. Used by ShellArrayBufferAllocator and MockArrayBufferAllocator.
+const int kMaxWorkers = 50;
+const int kMaxSerializerMemoryUsage =
+    1 * kMB;  // Arbitrary maximum for testing.
+
+// The ArrayBuffer allocator for the shell. It implements kNormal allocations
+// and delegates kReservation allocations to the default V8 allocator. It also
+// tracks its allocations so it doesn't free externalized ArrayBuffers that it
+// didn't allocate.
+// TODO(bbudge) Figure out why foreign externalized ArrayBuffers are passed into
+// these allocators in mjsunit/wasm/worker-memory.js.
 class ArrayBufferAllocatorBase : public v8::ArrayBuffer::Allocator {
-  std::unique_ptr<Allocator> allocator_ =
-      std::unique_ptr<Allocator>(NewDefaultAllocator());
-
- public:
-  void* Reserve(size_t length) override { return allocator_->Reserve(length); }
-
-  void Free(void*, size_t) override = 0;
-
-  void Free(void* data, size_t length, AllocationMode mode) override {
-    switch (mode) {
-      case AllocationMode::kNormal: {
-        return Free(data, length);
-      }
-      case AllocationMode::kReservation: {
-        return allocator_->Free(data, length, mode);
-      }
-    }
-  }
-
-  void SetProtection(void* data, size_t length,
-                     Protection protection) override {
-    allocator_->SetProtection(data, length, protection);
-  }
-};
-
-class ShellArrayBufferAllocator : public ArrayBufferAllocatorBase {
  public:
   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;
-    }
+    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
-    void* data = AllocateUninitialized(length);
-    return data == nullptr ? data : memset(data, 0, length);
+    MSAN_MEMORY_IS_INITIALIZED(data, alloc_length);
+
+    allocations_.insert(std::make_pair(data, alloc_length));
+    return data;
   }
+
   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
+    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
-    return __linux_malloc(length);
+    // 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
-    return malloc(length);
+    void* data = malloc(alloc_length);
 #endif
+    allocations_.insert(std::make_pair(data, alloc_length));
+    return data;
   }
-  using ArrayBufferAllocatorBase::Free;
+
   void Free(void* data, size_t length) override {
-#if USE_VM
-    if (RoundToPageSize(&length)) {
-      CHECK(base::OS::Free(data, length));
-      return;
+    if (allocations_.find(data) != allocations_.end()) {
+      DCHECK_EQ(GetAllocLength(length), allocations_[data]);
+      free(data);
+      allocations_.erase(data);
     }
-#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;
+
+  void Free(void* data, size_t length, AllocationMode mode) override {
+    size_t alloc_length = GetAllocLength(length);
+    if (mode == AllocationMode::kNormal) {
+      Free(data, alloc_length);
+    } else {
+      allocator_->Free(data, alloc_length, mode);
     }
-    return false;
   }
-#if USE_VM
-  void* VirtualMemoryAllocate(size_t length) {
+
+  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:
+  virtual size_t GetAllocLength(size_t length) const {
     size_t page_size = base::OS::AllocatePageSize();
     size_t alloc_size = RoundUp(length, page_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;
+    return alloc_size;
   }
-#endif
 };
 
 class MockArrayBufferAllocator : public ArrayBufferAllocatorBase {
-  const size_t kAllocationLimit = 10 * MB;
-  size_t get_actual_length(size_t length) const {
+ private:
+  size_t GetAllocLength(size_t length) const override {
+    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
@@ -300,7 +267,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 * MB);
+  return base::Thread::Options(name, 2 * kMB);
 }
 
 }  // namespace
@@ -2570,7 +2537,7 @@ void SourceGroup::JoinThread() {
 }
 
 ExternalizedContents::~ExternalizedContents() {
-  Shell::array_buffer_allocator->Free(data_, size_);
+  Shell::array_buffer_allocator->Free(base_, length_, mode_);
 }
 
 void SerializationDataQueue::Enqueue(std::unique_ptr<SerializationData> data) {
@@ -3479,3 +3446,6 @@ int main(int argc, char* argv[]) {
   return v8::Shell::Main(argc, argv);
 }
 #endif
+
+#undef CHECK
+#undef DCHECK

src/d8.h

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