cppgc: Poison unmarked objects before sweeping/compacting

Poisoning unmarked objects serves two purposes:
- Prohibits finalizer from accessing other unmarked objects;
- Unpoisioning also clears potential poisoning of the embedder which
  is necessary as the sweeper and compactor modify the payload of
  objects;

Bug: chromium:1056170
Change-Id: I4346a0ab736603b3d6170b41b0e7255db1452897
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2762137
Reviewed-by: Omer Katz <omerkatz@chromium.org>
Commit-Queue: Michael Lippautz <mlippautz@chromium.org>
Cr-Commit-Position: refs/heads/master@{#73426}

BUILD.gn

@@ -4918,6 +4918,7 @@ v8_source_set("cppgc_base") {
     "src/heap/cppgc/name-trait.cc",
     "src/heap/cppgc/object-allocator.cc",
     "src/heap/cppgc/object-allocator.h",
+    "src/heap/cppgc/object-poisoner.h",
     "src/heap/cppgc/object-size-trait.cc",
     "src/heap/cppgc/object-start-bitmap.h",
     "src/heap/cppgc/page-memory.cc",

src/heap/cppgc/compactor.cc

@@ -15,6 +15,7 @@
 #include "src/heap/cppgc/heap-base.h"
 #include "src/heap/cppgc/heap-page.h"
 #include "src/heap/cppgc/heap-space.h"
+#include "src/heap/cppgc/object-poisoner.h"
 #include "src/heap/cppgc/raw-heap.h"
 #include "src/heap/cppgc/stats-collector.h"
 
@@ -371,6 +372,10 @@ void CompactSpace(NormalPageSpace* space,
                   MovableReferences& movable_references) {
   using Pages = NormalPageSpace::Pages;
 
+#ifdef V8_USE_ADDRESS_SANITIZER
+  UnmarkedObjectsPoisoner().Traverse(space);
+#endif  // V8_USE_ADDRESS_SANITIZER
+
   DCHECK(space->is_compactable());
 
   space->free_list().Clear();

src/heap/cppgc/heap-object-header.cc

@@ -7,6 +7,8 @@
 #include "include/cppgc/internal/api-constants.h"
 #include "src/base/macros.h"
 #include "src/heap/cppgc/gc-info-table.h"
+#include "src/heap/cppgc/heap-page.h"
+#include "src/heap/cppgc/sanitizers.h"
 
 namespace cppgc {
 namespace internal {
@@ -21,6 +23,13 @@ void HeapObjectHeader::CheckApiConstants() {
 }
 
 void HeapObjectHeader::Finalize() {
+#ifdef V8_USE_ADDRESS_SANITIZER
+  const size_t size =
+      IsLargeObject()
+          ? LargePage::From(BasePage::FromPayload(this))->ObjectSize()
+          : ObjectSize();
+  ASAN_UNPOISON_MEMORY_REGION(Payload(), size);
+#endif  // V8_USE_ADDRESS_SANITIZER
   const GCInfo& gc_info = GlobalGCInfoTable::GCInfoFromIndex(GetGCInfoIndex());
   if (gc_info.finalize) {
     gc_info.finalize(Payload());

src/heap/cppgc/heap-object-header.h

@@ -190,6 +190,7 @@ Address HeapObjectHeader::Payload() const {
 
 template <AccessMode mode>
 Address HeapObjectHeader::PayloadEnd() const {
+  DCHECK(!IsLargeObject());
   return reinterpret_cast<Address>(const_cast<HeapObjectHeader*>(this)) +
          GetSize<mode>();
 }

src/heap/cppgc/object-poisoner.h

@@ -0,0 +1,40 @@
+// Copyright 2021 the V8 project authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef V8_HEAP_CPPGC_OBJECT_POISONER_H_
+#define V8_HEAP_CPPGC_OBJECT_POISONER_H_
+
+#include "src/heap/cppgc/heap-object-header.h"
+#include "src/heap/cppgc/heap-page.h"
+#include "src/heap/cppgc/heap-visitor.h"
+#include "src/heap/cppgc/sanitizers.h"
+
+namespace cppgc {
+namespace internal {
+
+#ifdef V8_USE_ADDRESS_SANITIZER
+
+// Poisons the payload of unmarked objects.
+class UnmarkedObjectsPoisoner : public HeapVisitor<UnmarkedObjectsPoisoner> {
+  friend class HeapVisitor<UnmarkedObjectsPoisoner>;
+
+ private:
+  bool VisitHeapObjectHeader(HeapObjectHeader* header) {
+    if (header->IsFree() || header->IsMarked()) return true;
+
+    const size_t size =
+        header->IsLargeObject()
+            ? LargePage::From(BasePage::FromPayload(header))->ObjectSize()
+            : header->ObjectSize();
+    ASAN_POISON_MEMORY_REGION(header->Payload(), size);
+    return true;
+  }
+};
+
+#endif  // V8_USE_ADDRESS_SANITIZER
+
+}  // namespace internal
+}  // namespace cppgc
+
+#endif  // V8_HEAP_CPPGC_OBJECT_POISONER_H_

src/heap/cppgc/sweeper.cc

@@ -18,6 +18,7 @@
 #include "src/heap/cppgc/heap-page.h"
 #include "src/heap/cppgc/heap-space.h"
 #include "src/heap/cppgc/heap-visitor.h"
+#include "src/heap/cppgc/object-poisoner.h"
 #include "src/heap/cppgc/object-start-bitmap.h"
 #include "src/heap/cppgc/raw-heap.h"
 #include "src/heap/cppgc/sanitizers.h"
@@ -160,6 +161,9 @@ class DeferredFinalizationBuilder final {
       result_.unfinalized_objects.push_back({header});
       found_finalizer_ = true;
     } else {
+      // Unmarked memory may have been poisoned. In the non-concurrent case this
+      // is taken care of by finalizing a header.
+      ASAN_UNPOISON_MEMORY_REGION(header, size);
       SET_MEMORY_INACCESSIBLE(header, size);
     }
   }
@@ -478,7 +482,7 @@ class ConcurrentSweepTask final : public cppgc::JobTask,
 };
 
 // This visitor:
-// - resets linear allocation buffers and clears free lists for all spaces;
+// - clears free lists for all spaces;
 // - moves all Heap pages to local Sweeper's state (SpaceStates).
 class PrepareForSweepVisitor final
     : public HeapVisitor<PrepareForSweepVisitor> {
@@ -497,11 +501,17 @@ class PrepareForSweepVisitor final
       return true;
     DCHECK(!space->linear_allocation_buffer().size());
     space->free_list().Clear();
+#ifdef V8_USE_ADDRESS_SANITIZER
+    UnmarkedObjectsPoisoner().Traverse(space);
+#endif  // V8_USE_ADDRESS_SANITIZER
     ExtractPages(space);
     return true;
   }
 
   bool VisitLargePageSpace(LargePageSpace* space) {
+#ifdef V8_USE_ADDRESS_SANITIZER
+    UnmarkedObjectsPoisoner().Traverse(space);
+#endif  // V8_USE_ADDRESS_SANITIZER
     ExtractPages(space);
     return true;
   }