[heap] Fix ArrayBufferTracker teardown ordering

The byte_length of ArrayBuffers may be a heap number. This length is
needed for freeing a buffer during tear down, implying that ArrayBuffers
need to be freed before regular space tear down can remove actual pages.

Bug: v8:7623
Change-Id: Iab91843e48c50276a2e110915f69cf9e6c24ef8f
Reviewed-on: https://chromium-review.googlesource.com/997776
Commit-Queue: Michael Lippautz <mlippautz@chromium.org>
Reviewed-by: Hannes Payer <hpayer@chromium.org>
Cr-Commit-Position: refs/heads/master@{#52426}

src/heap/array-buffer-tracker.cc

@@ -136,5 +136,25 @@ bool ArrayBufferTracker::IsTracked(JSArrayBuffer* buffer) {
   }
 }
 
+void ArrayBufferTracker::TearDown(Heap* heap) {
+  // ArrayBuffers can only be found in NEW_SPACE and OLD_SPACE.
+  for (Page* p : *heap->old_space()) {
+    FreeAll(p);
+  }
+  NewSpace* new_space = heap->new_space();
+  if (new_space->to_space().is_committed()) {
+    for (Page* p : new_space->to_space()) {
+      FreeAll(p);
+    }
+  }
+#ifdef DEBUG
+  if (new_space->from_space().is_committed()) {
+    for (Page* p : new_space->from_space()) {
+      DCHECK(!p->contains_array_buffers());
+    }
+  }
+#endif  // DEBUG
+}
+
 }  // namespace internal
 }  // namespace v8

src/heap/array-buffer-tracker.h

@@ -57,6 +57,9 @@ class ArrayBufferTracker : public AllStatic {
 
   // Returns whether a buffer is currently tracked.
   static bool IsTracked(JSArrayBuffer* buffer);
+
+  // Tears down the tracker and frees up all registered array buffers.
+  static void TearDown(Heap* heap);
 };
 
 // LocalArrayBufferTracker tracks internalized array buffers.

src/heap/heap.cc

@@ -6163,6 +6163,11 @@ void Heap::TearDown() {
 
   external_string_table_.TearDown();
 
+  // Tear down all ArrayBuffers before tearing down the heap since  their
+  // byte_length may be a HeapNumber which is required for freeing the backing
+  // store.
+  ArrayBufferTracker::TearDown(this);
+
   delete tracer_;
   tracer_ = nullptr;
 

src/heap/spaces.cc

@@ -1472,7 +1472,6 @@ bool PagedSpace::HasBeenSetUp() { return true; }
 void PagedSpace::TearDown() {
   for (auto it = begin(); it != end();) {
     Page* page = *(it++);  // Will be erased.
-    ArrayBufferTracker::FreeAll(page);
     heap()->memory_allocator()->Free<MemoryAllocator::kFull>(page);
   }
   anchor_.set_next_page(&anchor_);
@@ -2419,9 +2418,6 @@ void SemiSpace::SetUp(size_t initial_capacity, size_t maximum_capacity) {
 void SemiSpace::TearDown() {
   // Properly uncommit memory to keep the allocator counters in sync.
   if (is_committed()) {
-    for (Page* p : *this) {
-      ArrayBufferTracker::FreeAll(p);
-    }
     Uncommit();
   }
   current_capacity_ = maximum_capacity_ = 0;