cppgc: Establish invariant that free objects are inaccessible

Enforcing this invariant allows for assuming that free memory is left
untouched.

Bug: chromium:1056170
Change-Id: Ia225a31bbe6d394b8310ce512ed4f76f78e5c177
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3017808
Commit-Queue: Michael Lippautz <mlippautz@chromium.org>
Reviewed-by: Anton Bikineev <bikineev@chromium.org>
Cr-Commit-Position: refs/heads/master@{#75669}

src/base/sanitizer/asan.h

@@ -22,6 +22,17 @@
 
 #define DISABLE_ASAN __attribute__((no_sanitize_address))
 
+// Check that all bytes in a memory region are poisoned. This is different from
+// `__asan_region_is_poisoned()` which only requires a single byte in the region
+// to be poisoned.
+#define ASAN_CHECK_MEMORY_REGION_IS_POISONED(start, size)                     \
+  do {                                                                        \
+    for (size_t i = 0; i < size; i++) {                                       \
+      CHECK(__asan_address_is_poisoned(reinterpret_cast<const char*>(start) + \
+                                       i));                                   \
+    }                                                                         \
+  } while (0)
+
 #else  // !V8_USE_ADDRESS_SANITIZER
 
 #define DISABLE_ASAN
@@ -36,6 +47,9 @@
 #define ASAN_UNPOISON_MEMORY_REGION(start, size) \
   ASAN_POISON_MEMORY_REGION(start, size)
 
+#define ASAN_CHECK_MEMORY_REGION_IS_POISONED(start, size) \
+  ASAN_POISON_MEMORY_REGION(start, size)
+
 #endif  // !V8_USE_ADDRESS_SANITIZER
 
 #endif  // V8_BASE_SANITIZER_ASAN_H_

src/heap/cppgc/heap-page.cc

@@ -12,6 +12,7 @@
 #include "src/heap/cppgc/heap-object-header.h"
 #include "src/heap/cppgc/heap-space.h"
 #include "src/heap/cppgc/heap.h"
+#include "src/heap/cppgc/memory.h"
 #include "src/heap/cppgc/object-start-bitmap.h"
 #include "src/heap/cppgc/page-memory.h"
 #include "src/heap/cppgc/raw-heap.h"
@@ -118,6 +119,25 @@ NormalPage* NormalPage::Create(PageBackend& page_backend,
   auto* normal_page = new (memory) NormalPage(*space.raw_heap()->heap(), space);
   normal_page->SynchronizedStore();
   normal_page->heap().stats_collector()->NotifyAllocatedMemory(kPageSize);
+  // Memory is zero initialized as
+  // a) memory retrieved from the OS is zeroed;
+  // b) memory retrieved from the page pool was swept and thus is zeroed except
+  //    for the first header which will anyways serve as header again.
+  //
+  // The following is a subset of SetMemoryInaccessible() to establish the
+  // invariant that memory is in the same state as it would be after sweeping.
+  // This allows to return newly allocated pages to go into that LAB and back
+  // into the free list.
+  Address begin = normal_page->PayloadStart() + sizeof(HeapObjectHeader);
+  const size_t size = normal_page->PayloadSize() - sizeof(HeapObjectHeader);
+#if defined(V8_USE_MEMORY_SANITIZER)
+  MSAN_ALLOCATED_UNINITIALIZED_MEMORY(begin, size);
+#elif defined(V8_USE_ADDRESS_SANITIZER)
+  ASAN_POISON_MEMORY_REGION(begin, size);
+#elif DEBUG
+  cppgc::internal::ZapMemory(begin, size);
+#endif  // Release builds.
+  CheckMemoryIsInaccessible(begin, size);
   return normal_page;
 }
 

src/heap/cppgc/memory.h

@@ -12,6 +12,7 @@
 #include "src/base/macros.h"
 #include "src/base/sanitizer/asan.h"
 #include "src/base/sanitizer/msan.h"
+#include "src/heap/cppgc/globals.h"
 
 namespace cppgc {
 namespace internal {
@@ -19,13 +20,26 @@ namespace internal {
 V8_NOINLINE DISABLE_ASAN void NoSanitizeMemset(void* address, char c,
                                                size_t bytes);
 
-inline void ZapMemory(void* address, size_t size) {
+static constexpr uint8_t kZappedValue = 0xdc;
+
+V8_INLINE void ZapMemory(void* address, size_t size) {
   // The lowest bit of the zapped value should be 0 so that zapped object are
   // never viewed as fully constructed objects.
-  static constexpr uint8_t kZappedValue = 0xdc;
   memset(address, kZappedValue, size);
 }
 
+V8_INLINE void CheckMemoryIsZapped(const void* address, size_t size) {
+  for (size_t i = 0; i < size; i++) {
+    CHECK_EQ(kZappedValue, reinterpret_cast<ConstAddress>(address)[i]);
+  }
+}
+
+V8_INLINE void CheckMemoryIsZero(const void* address, size_t size) {
+  for (size_t i = 0; i < size; i++) {
+    CHECK_EQ(0, reinterpret_cast<ConstAddress>(address)[i]);
+  }
+}
+
 // Together `SetMemoryAccessible()` and `SetMemoryInaccessible()` form the
 // memory access model for allocation and free.
 V8_INLINE void SetMemoryAccessible(void* address, size_t size) {
@@ -70,6 +84,61 @@ V8_INLINE void SetMemoryInaccessible(void* address, size_t size) {
 #endif  // Release builds.
 }
 
+constexpr bool CheckMemoryIsInaccessibleIsNoop() {
+#if defined(V8_USE_MEMORY_SANITIZER)
+
+  return true;
+
+#elif defined(V8_USE_ADDRESS_SANITIZER)
+
+  return false;
+
+#elif DEBUG
+
+  return false;
+
+#else  // Release builds.
+
+  return true;
+
+#endif  // Release builds.
+}
+
+V8_INLINE void CheckMemoryIsInaccessible(const void* address, size_t size) {
+#if defined(V8_USE_MEMORY_SANITIZER)
+
+  static_assert(CheckMemoryIsInaccessibleIsNoop(),
+                "CheckMemoryIsInaccessibleIsNoop() needs to reflect "
+                "CheckMemoryIsInaccessible().");
+  // Unable to check that memory is marked as uninitialized by MSAN.
+
+#elif defined(V8_USE_ADDRESS_SANITIZER)
+
+  static_assert(!CheckMemoryIsInaccessibleIsNoop(),
+                "CheckMemoryIsInaccessibleIsNoop() needs to reflect "
+                "CheckMemoryIsInaccessible().");
+  ASAN_CHECK_MEMORY_REGION_IS_POISONED(address, size);
+  ASAN_UNPOISON_MEMORY_REGION(address, size);
+  CheckMemoryIsZero(address, size);
+  ASAN_POISON_MEMORY_REGION(address, size);
+
+#elif DEBUG
+
+  static_assert(!CheckMemoryIsInaccessibleIsNoop(),
+                "CheckMemoryIsInaccessibleIsNoop() needs to reflect "
+                "CheckMemoryIsInaccessible().");
+  CheckMemoryIsZapped(address, size);
+
+#else  // Release builds.
+
+  static_assert(CheckMemoryIsInaccessibleIsNoop(),
+                "CheckMemoryIsInaccessibleIsNoop() needs to reflect "
+                "CheckMemoryIsInaccessible().");
+  // No check in release builds.
+
+#endif  // Release builds.
+}
+
 }  // namespace internal
 }  // namespace cppgc
 

src/heap/cppgc/object-allocator.cc

@@ -13,6 +13,7 @@
 #include "src/heap/cppgc/heap-space.h"
 #include "src/heap/cppgc/heap-visitor.h"
 #include "src/heap/cppgc/heap.h"
+#include "src/heap/cppgc/memory.h"
 #include "src/heap/cppgc/object-start-bitmap.h"
 #include "src/heap/cppgc/page-memory.h"
 #include "src/heap/cppgc/stats-collector.h"
@@ -54,6 +55,8 @@ void MarkRangeAsYoung(BasePage* page, Address begin, Address end) {
 }
 
 void AddToFreeList(NormalPageSpace& space, Address start, size_t size) {
+  // No need for SetMemoryInaccessible() as LAB memory is retrieved as free
+  // inaccessible memory.
   space.free_list().Add({start, size});
   NormalPage::From(BasePage::FromPayload(start))
       ->object_start_bitmap()

src/heap/cppgc/sweeper.cc

@@ -203,6 +203,7 @@ typename FinalizationBuilder::ResultType SweepNormalPage(NormalPage* page) {
     // Check if this is a free list entry.
     if (header->IsFree<kAtomicAccess>()) {
       SetMemoryInaccessible(header, std::min(kFreeListEntrySize, size));
+      CheckMemoryIsInaccessible(header, size);
       begin += size;
       continue;
     }