cppgc: Add LSAN support for pages containing managed objects

- Move LsanPageAllocator to base;
- Use LsanPageAllocator in PageBackend that serves managed C++
  objects;
- Remove spurious TODO for GCInfoTable which should not use the
  LSAN-aware backend;

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

BUILD.gn

@@ -3969,7 +3969,6 @@ v8_source_set("v8_base_without_compiler") {
     "src/runtime/runtime-typedarray.cc",
     "src/runtime/runtime-weak-refs.cc",
     "src/runtime/runtime.cc",
-    "src/sanitizer/lsan-page-allocator.cc",
     "src/snapshot/code-serializer.cc",
     "src/snapshot/context-deserializer.cc",
     "src/snapshot/context-serializer.cc",
@@ -4615,6 +4614,8 @@ v8_component("v8_libbase") {
     "src/base/safe_conversions.h",
     "src/base/safe_conversions_arm_impl.h",
     "src/base/safe_conversions_impl.h",
+    "src/base/sanitizer/lsan-page-allocator.cc",
+    "src/base/sanitizer/lsan-page-allocator.h",
     "src/base/small-vector.h",
     "src/base/sys-info.cc",
     "src/base/sys-info.h",

src/sanitizer/lsan-page-allocator.cc → src/base/sanitizer/lsan-page-allocator.cc

@@ -2,7 +2,7 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#include "src/sanitizer/lsan-page-allocator.h"
+#include "src/base/sanitizer/lsan-page-allocator.h"
 
 #include "include/v8-platform.h"
 #include "src/base/logging.h"

src/sanitizer/lsan-page-allocator.h → src/base/sanitizer/lsan-page-allocator.h

@@ -2,8 +2,8 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#ifndef V8_SANITIZER_LSAN_PAGE_ALLOCATOR_H_
-#define V8_SANITIZER_LSAN_PAGE_ALLOCATOR_H_
+#ifndef V8_BASE_SANITIZER_LSAN_PAGE_ALLOCATOR_H_
+#define V8_BASE_SANITIZER_LSAN_PAGE_ALLOCATOR_H_
 
 #include "include/v8-platform.h"
 #include "src/base/compiler-specific.h"
@@ -56,4 +56,4 @@ class LsanPageAllocator : public v8::PageAllocator {
 
 }  // namespace base
 }  // namespace v8
-#endif  // V8_SANITIZER_LSAN_PAGE_ALLOCATOR_H_
+#endif  // V8_BASE_SANITIZER_LSAN_PAGE_ALLOCATOR_H_

src/heap/cppgc/gc-info-table.cc

@@ -35,8 +35,9 @@ PageAllocator* GetAllocator(PageAllocator* page_allocator) {
         default_page_allocator;
     page_allocator = default_page_allocator.get();
   }
-  // TODO(chromium:1056170): Wrap page_allocator into LsanPageAllocator when
-  // running with LEAK_SANITIZER.
+  // No need to introduce LSAN support for PageAllocator, as `GCInfoTable` is
+  // already a leaky object and the table payload (`GCInfoTable::table_`) should
+  // not refer to dynamically allocated objects.
   return page_allocator;
 }
 

src/heap/cppgc/heap-base.cc

@@ -5,8 +5,8 @@
 #include "src/heap/cppgc/heap-base.h"
 
 #include "include/cppgc/heap-consistency.h"
-#include "src/base/bounded-page-allocator.h"
 #include "src/base/platform/platform.h"
+#include "src/base/sanitizer/lsan-page-allocator.h"
 #include "src/heap/base/stack.h"
 #include "src/heap/cppgc/globals.h"
 #include "src/heap/cppgc/heap-object-header.h"
@@ -59,13 +59,16 @@ HeapBase::HeapBase(
     std::unique_ptr<MetricRecorder> histogram_recorder)
     : raw_heap_(this, custom_spaces),
       platform_(std::move(platform)),
+#if defined(LEAK_SANITIZER)
+      lsan_page_allocator_(std::make_unique<v8::base::LsanPageAllocator>(
+          platform_->GetPageAllocator())),
+#endif  // LEAK_SANITIZER
 #if defined(CPPGC_CAGED_HEAP)
-      caged_heap_(this, platform_->GetPageAllocator()),
+      caged_heap_(this, page_allocator()),
       page_backend_(std::make_unique<PageBackend>(&caged_heap_.allocator())),
-#else
-      page_backend_(
-          std::make_unique<PageBackend>(platform_->GetPageAllocator())),
-#endif
+#else   // !CPPGC_CAGED_HEAP
+      page_backend_(std::make_unique<PageBackend>(page_allocator())),
+#endif  // !CPPGC_CAGED_HEAP
       stats_collector_(std::make_unique<StatsCollector>(
           std::move(histogram_recorder), platform_.get())),
       stack_(std::make_unique<heap::base::Stack>(
@@ -82,6 +85,14 @@ HeapBase::HeapBase(
 
 HeapBase::~HeapBase() = default;
 
+PageAllocator* HeapBase::page_allocator() const {
+#if defined(LEAK_SANITIZER)
+  return lsan_page_allocator_.get();
+#else   // !LEAK_SANITIZER
+  return platform_->GetPageAllocator();
+#endif  // !LEAK_SANITIZER
+}
+
 size_t HeapBase::ObjectPayloadSize() const {
   return ObjectSizeCounter().GetSize(const_cast<RawHeap*>(&raw_heap()));
 }

src/heap/cppgc/heap-base.h

@@ -27,6 +27,12 @@
 #include "src/heap/cppgc/caged-heap.h"
 #endif
 
+namespace v8 {
+namespace base {
+class LsanPageAllocator;
+}  // namespace base
+}  // namespace v8
+
 namespace heap {
 namespace base {
 class Stack;
@@ -189,11 +195,18 @@ class V8_EXPORT_PRIVATE HeapBase : public cppgc::HeapHandle {
 
   void ExecutePreFinalizers();
 
+  PageAllocator* page_allocator() const;
+
   RawHeap raw_heap_;
   std::shared_ptr<cppgc::Platform> platform_;
+
+#if defined(LEAK_SANITIZER)
+  std::unique_ptr<v8::base::LsanPageAllocator> lsan_page_allocator_;
+#endif  // LEAK_SANITIZER
+
 #if defined(CPPGC_CAGED_HEAP)
   CagedHeap caged_heap_;
-#endif
+#endif  // CPPGC_CAGED_HEAP
   std::unique_ptr<PageBackend> page_backend_;
 
   std::unique_ptr<StatsCollector> stats_collector_;

src/utils/allocation.cc

@@ -12,9 +12,9 @@
 #include "src/base/logging.h"
 #include "src/base/page-allocator.h"
 #include "src/base/platform/platform.h"
+#include "src/base/sanitizer/lsan-page-allocator.h"
 #include "src/flags/flags.h"
 #include "src/init/v8.h"
-#include "src/sanitizer/lsan-page-allocator.h"
 #include "src/utils/memcopy.h"
 #include "src/utils/vector.h"
 

test/unittests/BUILD.gn

@@ -118,6 +118,7 @@ v8_source_set("cppgc_unittests_sources") {
     "heap/cppgc/page-memory-unittest.cc",
     "heap/cppgc/persistent-family-unittest.cc",
     "heap/cppgc/prefinalizer-unittest.cc",
+    "heap/cppgc/sanitizer-unittest.cc",
     "heap/cppgc/source-location-unittest.cc",
     "heap/cppgc/stack-unittest.cc",
     "heap/cppgc/stats-collector-scopes-unittest.cc",

test/unittests/heap/cppgc/sanitizer-unittest.cc

+// 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.
+
+#include "include/cppgc/allocation.h"
+#include "src/base/macros.h"
+#include "test/unittests/heap/cppgc/tests.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+#if defined(LEAK_SANITIZER)
+#include <sanitizer/lsan_interface.h>
+#endif  // LEAK_SANITIZER
+
+namespace cppgc {
+namespace internal {
+
+#if defined(LEAK_SANITIZER)
+
+using LsanTest = testing::TestWithHeap;
+
+class GCed final : public GarbageCollected<GCed> {
+ public:
+  void Trace(cppgc::Visitor*) const {}
+  std::unique_ptr<int> dummy{std::make_unique<int>(17)};
+};
+
+TEST_F(LsanTest, LeakDetectionDoesNotFindMemoryRetainedFromManaged) {
+  auto* o = MakeGarbageCollected<GCed>(GetAllocationHandle());
+  __lsan_do_leak_check();
+  USE(o);
+}
+
+#endif  // LEAK_SANITIZER
+
+}  // namespace internal
+}  // namespace cppgc