PartitionAlloc: PCScan: Enable scanning for WTF:: partitions

This also removes default arguments for PartitionOptions. Bug: 1129751 Change-Id: I0a694f8b300a04a08fb21fd1191066556f21d03a Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2507432 Reviewed-by: Kentaro Hara <haraken@chromium.org> Reviewed-by: Michael Lippautz <mlippautz@chromium.org> Reviewed-by: Bartek Nowierski <bartekn@chromium.org> Commit-Queue: Anton Bikineev <bikineev@chromium.org> Cr-Commit-Position: refs/heads/master@{#822258}
Denger-Network · Oct 29, 2020 · baf2c5d · baf2c5d
1 parent ab05003
commit baf2c5d
Show file tree

Hide file tree

Showing 8 changed files with 50 additions and 21 deletions.
diff --git a/base/allocator/partition_allocator/memory_reclaimer_unittest.cc b/base/allocator/partition_allocator/memory_reclaimer_unittest.cc
@@ -39,7 +39,9 @@ class PartitionAllocMemoryReclaimerTest : public ::testing::Test {
     PartitionAllocGlobalInit(HandleOOM);
     PartitionAllocMemoryReclaimer::Instance()->ResetForTesting();
     allocator_ = std::make_unique<PartitionAllocator>();
-    allocator_->init();
+    allocator_->init({PartitionOptions::Alignment::kRegular,
+                      PartitionOptions::ThreadCache::kDisabled,
+                      PartitionOptions::PCScan::kAlwaysDisabled});
   }
 
   void TearDown() override {

diff --git a/base/allocator/partition_allocator/partition_alloc.h b/base/allocator/partition_allocator/partition_alloc.h
@@ -92,7 +92,7 @@ struct BASE_EXPORT PartitionAllocator {
   PartitionAllocator() = default;
   ~PartitionAllocator();
 
-  void init(PartitionOptions = {});
+  void init(PartitionOptions);
 
   ALWAYS_INLINE PartitionRoot<thread_safe>* root() { return &partition_root_; }
   ALWAYS_INLINE const PartitionRoot<thread_safe>* root() const {

diff --git a/base/allocator/partition_allocator/partition_root.cc b/base/allocator/partition_allocator/partition_root.cc
@@ -375,7 +375,8 @@ void PartitionRoot<thread_safe>::Init(PartitionOptions opts) {
   scannable = (opts.pcscan != PartitionOptions::PCScan::kAlwaysDisabled);
   // Concurrent freeing in PCScan can only safely work on thread-safe
   // partitions.
-  if (thread_safe && opts.pcscan == PartitionOptions::PCScan::kEnabled)
+  if (thread_safe &&
+      opts.pcscan == PartitionOptions::PCScan::kForcedEnabledForTesting)
     pcscan.emplace(this);
 
   // We mark the sentinel slot span as free to make sure it is skipped by our

diff --git a/base/allocator/partition_allocator/partition_root.h b/base/allocator/partition_allocator/partition_root.h
@@ -100,12 +100,12 @@ struct PartitionOptions {
     // PartitionRoot::EnablePCScan().
     kDisabledByDefault,
     // PCScan is always enabled.
-    kEnabled,
+    kForcedEnabledForTesting,
   };
 
-  Alignment alignment = Alignment::kRegular;
-  ThreadCache thread_cache = ThreadCache::kDisabled;
-  PCScan pcscan = PCScan::kAlwaysDisabled;
+  Alignment alignment;
+  ThreadCache thread_cache;
+  PCScan pcscan;
 };
 
 // Never instantiate a PartitionRoot directly, instead use
@@ -286,9 +286,7 @@ struct BASE_EXPORT PartitionRoot {
   }
 
   void EnablePCScan() {
-    // TODO(bikineev): Make CHECK once PCScan is enabled.
-    if (!scannable || pcscan.has_value())
-      return;
+    PA_CHECK(scannable && !pcscan.has_value());
     pcscan.emplace(this);
   }
 

diff --git a/base/allocator/partition_allocator/pcscan_unittest.cc b/base/allocator/partition_allocator/pcscan_unittest.cc
@@ -18,7 +18,7 @@ class PCScanTest : public testing::Test {
     PartitionAllocGlobalInit([](size_t) { LOG(FATAL) << "Out of memory"; });
     allocator_.init({PartitionOptions::Alignment::kRegular,
                      PartitionOptions::ThreadCache::kDisabled,
-                     PartitionOptions::PCScan::kEnabled});
+                     PartitionOptions::PCScan::kForcedEnabledForTesting});
   }
   ~PCScanTest() override {
     allocator_.root()->PurgeMemory(PartitionPurgeDecommitEmptySlotSpans |

diff --git a/components/gwp_asan/client/sampling_partitionalloc_shims_unittest.cc b/components/gwp_asan/client/sampling_partitionalloc_shims_unittest.cc
@@ -11,6 +11,7 @@
 #include <string>
 
 #include "base/allocator/partition_allocator/partition_alloc.h"
+#include "base/allocator/partition_allocator/partition_root.h"
 #include "base/bind_helpers.h"
 #include "base/logging.h"
 #include "base/partition_alloc_buildflags.h"
@@ -51,6 +52,11 @@ constexpr size_t kLoopIterations = kSamplingFrequency * 4;
 constexpr int kSuccess = 0;
 constexpr int kFailure = 1;
 
+constexpr base::PartitionOptions kAllocatorOptions = {
+    base::PartitionOptions::Alignment::kRegular,
+    base::PartitionOptions::ThreadCache::kDisabled,
+    base::PartitionOptions::PCScan::kAlwaysDisabled};
+
 static void HandleOOM(size_t unused_size) {
   LOG(FATAL) << "Out of memory.";
 }
@@ -79,7 +85,7 @@ MULTIPROCESS_TEST_MAIN_WITH_SETUP(
     BasicFunctionality,
     SamplingPartitionAllocShimsTest::multiprocessTestSetup) {
   base::PartitionAllocator allocator;
-  allocator.init();
+  allocator.init(kAllocatorOptions);
   for (size_t i = 0; i < kLoopIterations; i++) {
     void* ptr = allocator.root()->Alloc(1, kFakeType);
     if (GetPartitionAllocGpaForTesting().PointerIsMine(ptr))
@@ -99,7 +105,7 @@ MULTIPROCESS_TEST_MAIN_WITH_SETUP(
     Realloc,
     SamplingPartitionAllocShimsTest::multiprocessTestSetup) {
   base::PartitionAllocator allocator;
-  allocator.init();
+  allocator.init(kAllocatorOptions);
 
   void* alloc = GetPartitionAllocGpaForTesting().Allocate(base::GetPageSize());
   CHECK_NE(alloc, nullptr);
@@ -128,7 +134,7 @@ MULTIPROCESS_TEST_MAIN_WITH_SETUP(
     DifferentTypesDontOverlap,
     SamplingPartitionAllocShimsTest::multiprocessTestSetup) {
   base::PartitionAllocator allocator;
-  allocator.init();
+  allocator.init(kAllocatorOptions);
 
   std::set<void*> type1, type2;
   for (size_t i = 0; i < kLoopIterations * AllocatorState::kMaxSlots; i++) {

diff --git a/components/heap_profiling/multi_process/test_driver.cc b/components/heap_profiling/multi_process/test_driver.cc
@@ -7,6 +7,7 @@
 #include <algorithm>
 #include <string>
 
+#include "base/allocator/partition_allocator/partition_root.h"
 #include "base/bind.h"
 #include "base/command_line.h"
 #include "base/files/file_path.h"
@@ -342,7 +343,9 @@ TestDriver::TestDriver()
     : wait_for_ui_thread_(base::WaitableEvent::ResetPolicy::AUTOMATIC,
                           base::WaitableEvent::InitialState::NOT_SIGNALED) {
   base::PartitionAllocGlobalInit(HandleOOM);
-  partition_allocator_.init();
+  partition_allocator_.init({base::PartitionOptions::Alignment::kRegular,
+                             base::PartitionOptions::ThreadCache::kDisabled,
+                             base::PartitionOptions::PCScan::kAlwaysDisabled});
 }
 TestDriver::~TestDriver() {
   base::PartitionAllocGlobalUninitForTesting();

diff --git a/third_party/blink/renderer/platform/wtf/allocator/partitions.cc b/third_party/blink/renderer/platform/wtf/allocator/partitions.cc
@@ -33,6 +33,7 @@
 #include "base/allocator/partition_allocator/memory_reclaimer.h"
 #include "base/allocator/partition_allocator/oom.h"
 #include "base/allocator/partition_allocator/page_allocator.h"
+#include "base/allocator/partition_allocator/partition_alloc_features.h"
 #include "base/debug/alias.h"
 #include "base/strings/safe_sprintf.h"
 #include "base/thread_annotations.h"
@@ -75,20 +76,38 @@ bool Partitions::InitializeOnce() {
   // - BUILDFLAG(USE_PARTITION_ALLOC_AS_MALLOC): Only one thread cache at a time
   //   is supported, in this case it is already claimed by malloc().
 #if DCHECK_IS_ON() && !BUILDFLAG(USE_PARTITION_ALLOC_AS_MALLOC)
-  fast_malloc_allocator.init({base::PartitionOptions::Alignment::kRegular,
-                              base::PartitionOptions::ThreadCache::kEnabled});
+  fast_malloc_allocator.init(
+      {base::PartitionOptions::Alignment::kRegular,
+       base::PartitionOptions::ThreadCache::kEnabled,
+       base::PartitionOptions::PCScan::kDisabledByDefault});
 #else
-  fast_malloc_allocator.init();
+  fast_malloc_allocator.init(
+      {base::PartitionOptions::Alignment::kRegular,
+       base::PartitionOptions::ThreadCache::kDisabled,
+       base::PartitionOptions::PCScan::kDisabledByDefault});
 #endif
-  array_buffer_allocator.init();
-  buffer_allocator.init();
-  layout_allocator.init();
+  array_buffer_allocator.init(
+      {base::PartitionOptions::Alignment::kRegular,
+       base::PartitionOptions::ThreadCache::kDisabled,
+       base::PartitionOptions::PCScan::kAlwaysDisabled});
+  buffer_allocator.init({base::PartitionOptions::Alignment::kRegular,
+                         base::PartitionOptions::ThreadCache::kDisabled,
+                         base::PartitionOptions::PCScan::kDisabledByDefault});
+  layout_allocator.init({base::PartitionOptions::Alignment::kRegular,
+                         base::PartitionOptions::ThreadCache::kDisabled,
+                         base::PartitionOptions::PCScan::kDisabledByDefault});
 
   fast_malloc_root_ = fast_malloc_allocator.root();
   array_buffer_root_ = array_buffer_allocator.root();
   buffer_root_ = buffer_allocator.root();
   layout_root_ = layout_allocator.root();
 
+  if (base::features::IsPartitionAllocPCScanEnabled()) {
+    fast_malloc_root_->EnablePCScan();
+    buffer_root_->EnablePCScan();
+    layout_root_->EnablePCScan();
+  }
+
   initialized_ = true;
   return initialized_;
 }