PA: Remove no-op free() feature scaffolding

This CL is intended to be a no-op, as this feature was already launched
in efdfb40bf1e9fba88f9001bf8c7da65c3ed3d60d.

This CL

*   removes the `base::Feature` controlling no-op `free()`,

*   strips all `MakeFreeNoOp()` calls save (the launched) one,

*   moves the TODO onto the DPD feature, and

*   moves `MakeFreeNoOp()` into `partition_alloc_support`.

Fixed: 40802063
Change-Id: I1d5e4f1c31287a3eb109961f92d348377f2fa75e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5937487
Reviewed-by: Keishi Hattori <[email protected]>
Reviewed-by: Gabriel Charette <[email protected]>
Commit-Queue: Kalvin Lee <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1373881}


CrOS-Libchrome-Original-Commit: 8529bddee489dc7c0d2cb1a57f9653a453382050

Author: Kalvin Lee

base/allocator/partition_alloc_features.cc

@@ -56,6 +56,10 @@ const base::FeatureParam<UnretainedDanglingPtrMode>
         &kUnretainedDanglingPtrModeOption,
 };
 
+// Note: DPD conflicts with no-op `free()` (see
+// `base::allocator::MakeFreeNoOp()`). No-op `free()` stands down in the
+// presence of DPD, but hypothetically fully launching DPD should prompt
+// a rethink of no-op `free()`.
 BASE_FEATURE(kPartitionAllocDanglingPtr,
              "PartitionAllocDanglingPtr",
 #if PA_BUILDFLAG(ENABLE_DANGLING_RAW_PTR_FEATURE_FLAG)
@@ -434,60 +438,6 @@ BASE_FEATURE(kUsePoolOffsetFreelists,
              base::FEATURE_DISABLED_BY_DEFAULT);
 #endif
 
-BASE_FEATURE(kPartitionAllocMakeFreeNoOpOnShutdown,
-             "PartitionAllocMakeFreeNoOpOnShutdown",
-#if PA_BUILDFLAG(IS_CHROMEOS)
-             FEATURE_ENABLED_BY_DEFAULT
-#else
-             FEATURE_DISABLED_BY_DEFAULT
-#endif
-);
-
-constexpr FeatureParam<WhenFreeBecomesNoOp>::Option
-    kPartitionAllocMakeFreeNoOpOnShutdownOptions[] = {
-        {WhenFreeBecomesNoOp::kBeforePreShutdown, "before-preshutdown"},
-        {WhenFreeBecomesNoOp::kBeforeHaltingStartupTracingController,
-         "before-halting-startup-tracing-controller"},
-        {
-            WhenFreeBecomesNoOp::kBeforeShutDownThreads,
-            "before-shutdown-threads",
-        },
-        {
-            WhenFreeBecomesNoOp::kInShutDownThreads,
-            "in-shutdown-threads",
-        },
-        {
-            WhenFreeBecomesNoOp::kAfterShutDownThreads,
-            "after-shutdown-threads",
-        },
-};
-
-const base::FeatureParam<WhenFreeBecomesNoOp>
-    kPartitionAllocMakeFreeNoOpOnShutdownParam{
-        &kPartitionAllocMakeFreeNoOpOnShutdown, "callsite",
-        WhenFreeBecomesNoOp::kBeforePreShutdown,
-        &kPartitionAllocMakeFreeNoOpOnShutdownOptions};
-
-void MakeFreeNoOp(WhenFreeBecomesNoOp callsite) {
-  CHECK(base::FeatureList::GetInstance());
-  // Ignoring `free()` during Shutdown would allow developers to introduce new
-  // dangling pointers. So we want to avoid ignoring free when it is enabled.
-  // Note: For now, the DanglingPointerDetector is only enabled on 5 bots, and
-  // on linux non-official configuration.
-  // TODO(b/40802063): Reconsider this decision after the experiment.
-#if PA_BUILDFLAG(ENABLE_DANGLING_RAW_PTR_CHECKS)
-  if (base::FeatureList::IsEnabled(features::kPartitionAllocDanglingPtr)) {
-    return;
-  }
-#endif  // PA_BUILDFLAG(ENABLE_DANGLING_RAW_PTR_CHECKS)
-#if PA_BUILDFLAG(USE_ALLOCATOR_SHIM)
-  if (base::FeatureList::IsEnabled(kPartitionAllocMakeFreeNoOpOnShutdown) &&
-      kPartitionAllocMakeFreeNoOpOnShutdownParam.Get() == callsite) {
-    allocator_shim::InsertNoOpOnFreeAllocatorShimOnShutDown();
-  }
-#endif  // PA_BUILDFLAG(USE_ALLOCATOR_SHIM)
-}
-
 BASE_FEATURE(kPartitionAllocAdjustSizeWhenInForeground,
              "PartitionAllocAdjustSizeWhenInForeground",
 #if BUILDFLAG(IS_MAC)

base/allocator/partition_alloc_features.h

@@ -133,24 +133,6 @@ enum class BucketDistributionMode : uint8_t {
   kDenser,
 };
 
-// Parameter for 'kPartitionAllocMakeFreeNoOpOnShutdown' feature which
-// controls when free() becomes a no-op during Shutdown()
-enum class WhenFreeBecomesNoOp {
-  kBeforePreShutdown,
-  kBeforeHaltingStartupTracingController,
-  kBeforeShutDownThreads,
-  kInShutDownThreads,
-  kAfterShutDownThreads,
-};
-
-// Inserts a no-op on 'free()' allocator shim at the front of the
-// dispatch chain if called from the appropriate callsite.
-BASE_EXPORT void MakeFreeNoOp(WhenFreeBecomesNoOp callsite);
-
-BASE_EXPORT BASE_DECLARE_FEATURE(kPartitionAllocMakeFreeNoOpOnShutdown);
-extern const BASE_EXPORT base::FeatureParam<WhenFreeBecomesNoOp>
-    kPartitionAllocMakeFreeNoOpOnShutdownParam;
-
 BASE_EXPORT BASE_DECLARE_FEATURE(kPartitionAllocBackupRefPtr);
 extern const BASE_EXPORT base::FeatureParam<BackupRefPtrEnabledProcesses>
     kBackupRefPtrEnabledProcessesParam;

base/allocator/partition_alloc_support.cc

@@ -58,6 +58,7 @@
 #include "partition_alloc/pointers/raw_ptr.h"
 #include "partition_alloc/shim/allocator_shim.h"
 #include "partition_alloc/shim/allocator_shim_default_dispatch_to_partition_alloc.h"
+#include "partition_alloc/shim/allocator_shim_dispatch_to_noop_on_free.h"
 #include "partition_alloc/stack/stack.h"
 #include "partition_alloc/thread_cache.h"
 
@@ -782,6 +783,22 @@ void ReconfigurePartitionForKnownProcess(const std::string& process_type) {
   // experiments.
 }
 
+void MakeFreeNoOp() {
+  // Ignoring `free()` during Shutdown would allow developers to introduce new
+  // dangling pointers. So we want to avoid ignoring free when it is enabled.
+  // Note: For now, the DanglingPointerDetector is only enabled on 5 bots, and
+  // on linux non-official configuration.
+#if PA_BUILDFLAG(ENABLE_DANGLING_RAW_PTR_CHECKS)
+  CHECK(base::FeatureList::GetInstance());
+  if (base::FeatureList::IsEnabled(features::kPartitionAllocDanglingPtr)) {
+    return;
+  }
+#endif  // PA_BUILDFLAG(ENABLE_DANGLING_RAW_PTR_CHECKS)
+#if PA_BUILDFLAG(USE_ALLOCATOR_SHIM)
+  allocator_shim::InsertNoOpOnFreeAllocatorShimOnShutDown();
+#endif  // PA_BUILDFLAG(USE_ALLOCATOR_SHIM)
+}
+
 PartitionAllocSupport* PartitionAllocSupport::Get() {
   static auto* singleton = new PartitionAllocSupport();
   return singleton;

base/allocator/partition_alloc_support.h

@@ -37,6 +37,13 @@ BASE_EXPORT std::map<std::string, std::string> ProposeSyntheticFinchTrials();
 BASE_EXPORT void InstallDanglingRawPtrChecks();
 BASE_EXPORT void InstallUnretainedDanglingRawPtrChecks();
 
+// Once called, makes `free()` do nothing. This is done to reduce
+// shutdown hangs on CrOS.
+// Does nothing if Dangling Pointer Detector (`docs/dangling_ptr.md`)
+// is not active.
+// Does nothing if allocator shim support is not built.
+BASE_EXPORT void MakeFreeNoOp();
+
 // Allows to re-configure PartitionAlloc at run-time.
 class BASE_EXPORT PartitionAllocSupport {
  public: