cppgc: Clear object memory on sweep

We clear during sweep so that we are guaranteed the in-construction bit
of newly allocated objects is always 0. The lock sweeping uses for
synchronization assures no data races between clearing and concurrent
marking.

The only exception to that is debug builds that zap on sweep and clear
on allocation. This makes it so that dangling references will most
likely crash in debug builds.

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

src/heap/cppgc/object-allocator.h

@@ -118,7 +118,14 @@ void* ObjectAllocator::AllocateObjectOnSpace(NormalPageSpace* space,
   }
 
   void* raw = current_lab.Allocate(size);
-  SET_MEMORY_ACCESIBLE(raw, size);
+#if !defined(V8_USE_MEMORY_SANITIZER) && !defined(V8_USE_ADDRESS_SANITIZER) && \
+    DEBUG
+  // For debug builds, unzap only the payload.
+  SET_MEMORY_ACCESSIBLE(static_cast<char*>(raw) + sizeof(HeapObjectHeader),
+                        size - sizeof(HeapObjectHeader));
+#else
+  SET_MEMORY_ACCESSIBLE(raw, size);
+#endif
   auto* header = new (raw) HeapObjectHeader(size, gcinfo);
 
   NormalPage::From(BasePage::FromPayload(header))

src/heap/cppgc/sanitizers.h

@@ -47,27 +47,32 @@
 
 // API for newly allocated or reclaimed memory.
 #if defined(V8_USE_MEMORY_SANITIZER)
-#define SET_MEMORY_ACCESIBLE(address, size) \
-  MSAN_UNPOISON(address, size);             \
-  memset((address), 0, (size))
-#define SET_MEMORY_INACCESIBLE(address, size) MSAN_POISON((address), (size))
-#elif DEBUG || defined(V8_USE_ADDRESS_SANITIZER)
-#define SET_MEMORY_ACCESIBLE(address, size)   \
-  ASAN_UNPOISON_MEMORY_REGION(address, size); \
-  memset((address), 0, (size))
-#define SET_MEMORY_INACCESIBLE(address, size)      \
-  ::cppgc::internal::ZapMemory((address), (size)); \
+#define SET_MEMORY_ACCESSIBLE(address, size) MSAN_UNPOISON(address, size);
+#define SET_MEMORY_INACCESSIBLE(address, size) \
+  memset((address), 0, (size));                \
+  MSAN_POISON((address), (size))
+#elif defined(V8_USE_ADDRESS_SANITIZER)
+#define SET_MEMORY_ACCESSIBLE(address, size) \
+  ASAN_UNPOISON_MEMORY_REGION(address, size);
+#define SET_MEMORY_INACCESSIBLE(address, size) \
+  memset((address), 0, (size));                \
   ASAN_POISON_MEMORY_REGION(address, size)
+#elif DEBUG
+#define SET_MEMORY_ACCESSIBLE(address, size) memset((address), 0, (size))
+#define SET_MEMORY_INACCESSIBLE(address, size) \
+  ::cppgc::internal::ZapMemory((address), (size));
 #else
-#define SET_MEMORY_ACCESIBLE(address, size) memset((address), 0, (size))
-#define SET_MEMORY_INACCESIBLE(address, size) ((void)(address), (void)(size))
+#define SET_MEMORY_ACCESSIBLE(address, size) ((void)(address), (void)(size))
+#define SET_MEMORY_INACCESSIBLE(address, size) memset((address), 0, (size))
 #endif
 
 namespace cppgc {
 namespace internal {
 
 inline void ZapMemory(void* address, size_t size) {
-  static constexpr uint8_t kZappedValue = 0xcd;
+  // 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);
 }
 

src/heap/cppgc/sweeper.cc

@@ -127,7 +127,7 @@ class InlinedFinalizationBuilder final {
 
   void AddFinalizer(HeapObjectHeader* header, size_t size) {
     header->Finalize();
-    SET_MEMORY_INACCESIBLE(header, size);
+    SET_MEMORY_INACCESSIBLE(header, size);
   }
 
   void AddFreeListEntry(Address start, size_t size) {
@@ -153,7 +153,7 @@ class DeferredFinalizationBuilder final {
       result_.unfinalized_objects.push_back({header});
       found_finalizer_ = true;
     } else {
-      SET_MEMORY_INACCESIBLE(header, size);
+      SET_MEMORY_INACCESSIBLE(header, size);
     }
   }
 
@@ -191,7 +191,7 @@ typename FinalizationBuilder::ResultType SweepNormalPage(NormalPage* page) {
     const size_t size = header->GetSize();
     // Check if this is a free list entry.
     if (header->IsFree<kAtomicAccess>()) {
-      SET_MEMORY_INACCESIBLE(header, std::min(kFreeListEntrySize, size));
+      SET_MEMORY_INACCESSIBLE(header, std::min(kFreeListEntrySize, size));
       begin += size;
       continue;
     }
@@ -273,7 +273,9 @@ class SweepFinalizer final {
 
     // Call finalizers.
     for (HeapObjectHeader* object : page_state->unfinalized_objects) {
+      const size_t size = object->GetSize();
       object->Finalize();
+      SET_MEMORY_INACCESSIBLE(object, size);
     }
 
     // Unmap page if empty.