diff --git a/BUILD.gn b/BUILD.gn index cd4380096d..66999559e0 100644 --- a/BUILD.gn +++ b/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", diff --git a/src/sanitizer/lsan-page-allocator.cc b/src/base/sanitizer/lsan-page-allocator.cc similarity index 97% rename from src/sanitizer/lsan-page-allocator.cc rename to src/base/sanitizer/lsan-page-allocator.cc index 7794e0b734..bb52eb368f 100644 --- a/src/sanitizer/lsan-page-allocator.cc +++ b/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" diff --git a/src/sanitizer/lsan-page-allocator.h b/src/base/sanitizer/lsan-page-allocator.h similarity index 92% rename from src/sanitizer/lsan-page-allocator.h rename to src/base/sanitizer/lsan-page-allocator.h index f86ffd98e8..3c5f2784cf 100644 --- a/src/sanitizer/lsan-page-allocator.h +++ b/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_ diff --git a/src/heap/cppgc/gc-info-table.cc b/src/heap/cppgc/gc-info-table.cc index 6b177848cb..7462ba8a21 100644 --- a/src/heap/cppgc/gc-info-table.cc +++ b/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; } diff --git a/src/heap/cppgc/heap-base.cc b/src/heap/cppgc/heap-base.cc index 60ae6d8be9..9330fd42d6 100644 --- a/src/heap/cppgc/heap-base.cc +++ b/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 histogram_recorder) : raw_heap_(this, custom_spaces), platform_(std::move(platform)), +#if defined(LEAK_SANITIZER) + lsan_page_allocator_(std::make_unique( + 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(&caged_heap_.allocator())), -#else - page_backend_( - std::make_unique(platform_->GetPageAllocator())), -#endif +#else // !CPPGC_CAGED_HEAP + page_backend_(std::make_unique(page_allocator())), +#endif // !CPPGC_CAGED_HEAP stats_collector_(std::make_unique( std::move(histogram_recorder), platform_.get())), stack_(std::make_unique( @@ -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(&raw_heap())); } diff --git a/src/heap/cppgc/heap-base.h b/src/heap/cppgc/heap-base.h index f9bdb95c04..894307e9fe 100644 --- a/src/heap/cppgc/heap-base.h +++ b/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 platform_; + +#if defined(LEAK_SANITIZER) + std::unique_ptr lsan_page_allocator_; +#endif // LEAK_SANITIZER + #if defined(CPPGC_CAGED_HEAP) CagedHeap caged_heap_; -#endif +#endif // CPPGC_CAGED_HEAP std::unique_ptr page_backend_; std::unique_ptr stats_collector_; diff --git a/src/utils/allocation.cc b/src/utils/allocation.cc index 90e9808fe2..f25a70752b 100644 --- a/src/utils/allocation.cc +++ b/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" diff --git a/test/unittests/BUILD.gn b/test/unittests/BUILD.gn index 47bd8fe27d..a669355c49 100644 --- a/test/unittests/BUILD.gn +++ b/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", diff --git a/test/unittests/heap/cppgc/sanitizer-unittest.cc b/test/unittests/heap/cppgc/sanitizer-unittest.cc new file mode 100644 index 0000000000..4f6f853800 --- /dev/null +++ b/test/unittests/heap/cppgc/sanitizer-unittest.cc @@ -0,0 +1,36 @@ +// 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 +#endif // LEAK_SANITIZER + +namespace cppgc { +namespace internal { + +#if defined(LEAK_SANITIZER) + +using LsanTest = testing::TestWithHeap; + +class GCed final : public GarbageCollected { + public: + void Trace(cppgc::Visitor*) const {} + std::unique_ptr dummy{std::make_unique(17)}; +}; + +TEST_F(LsanTest, LeakDetectionDoesNotFindMemoryRetainedFromManaged) { + auto* o = MakeGarbageCollected(GetAllocationHandle()); + __lsan_do_leak_check(); + USE(o); +} + +#endif // LEAK_SANITIZER + +} // namespace internal +} // namespace cppgc