PA: Upgrade DCHECKs to CHECKs in FreeWithSize, guarded by a Finch flag Upgrade `PA_DCHECKs` to `PA_CHECKs` within `GetSlotStartAndSlotSpan`. These stricter checks are active when the Finch feature flag (which is enabled by default) is active, allowing for investigation of potential inconsistencies in production. When this Finch flag is active, add a prefetch for slot_span in `FreeWithSizeNoHooks` to potentially mitigate performance impact from the added `PA_CHECKs`. Bug: 410190984 Change-Id: Ibd34674f52e0dbc91c06ebf6b135d0a20f3ef099 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/7236054 Reviewed-by: Keishi Hattori <[email protected]> Reviewed-by: Stephen Nusko <[email protected]> Commit-Queue: Ayumi Ono <[email protected]> Cr-Commit-Position: refs/heads/main@{#1556536} NOKEYCHECK=True GitOrigin-RevId: 986b8d2d082366e6bb951fa94ccb501d091f0c06
diff --git a/src/partition_alloc/partition_root.cc b/src/partition_alloc/partition_root.cc index 55d30b5..24c73e4 100644 --- a/src/partition_alloc/partition_root.cc +++ b/src/partition_alloc/partition_root.cc
@@ -1162,6 +1162,8 @@ settings.enable_free_with_size = (opts.free_with_size == PartitionOptions::kEnabled); + settings.enable_strict_free_size_check = + (opts.strict_free_size_check == PartitionOptions::kEnabled); initialized = true; }
diff --git a/src/partition_alloc/partition_root.h b/src/partition_alloc/partition_root.h index befbb5b..8b5d89a 100644 --- a/src/partition_alloc/partition_root.h +++ b/src/partition_alloc/partition_root.h
@@ -213,6 +213,7 @@ #endif EnableToggle free_with_size = kDisabled; + EnableToggle strict_free_size_check = kEnabled; }; constexpr PartitionOptions::PartitionOptions() = default; @@ -298,6 +299,7 @@ #endif bool enable_free_with_size = false; + bool enable_strict_free_size_check = true; }; Settings settings; @@ -2158,10 +2160,20 @@ // determined without using `slot_span`. auto bucket_index = SizeToBucketIndex(raw_size, this->GetBucketDistribution()); - PA_DCHECK(bucket_index == - static_cast<uint16_t>(slot_span->bucket - this->buckets)); auto slot_size = BucketIndexLookup::GetBucketSize(bucket_index); - PA_DCHECK(slot_size == slot_span->bucket->slot_size); + if (settings.enable_strict_free_size_check) { + // TODO(crbug.com/410190984): Remove this prefetch & CHECKS once the + // PA_CHECK of the given size against the slot span metadata is replaced + // with a PA_DCHECK. + PA_PREFETCH(slot_span); + PA_CHECK(bucket_index == + static_cast<uint16_t>(slot_span->bucket - this->buckets)); + PA_CHECK(slot_size == slot_span->bucket->slot_size); + } else { + PA_DCHECK(bucket_index == + static_cast<uint16_t>(slot_span->bucket - this->buckets)); + PA_DCHECK(slot_size == slot_span->bucket->slot_size); + } return internal::BucketSizeDetails{ .bucket_index = bucket_index, .slot_size = slot_size,
diff --git a/src/partition_alloc/shim/allocator_shim.h b/src/partition_alloc/shim/allocator_shim.h index ce10230..2a9bdf8 100644 --- a/src/partition_alloc/shim/allocator_shim.h +++ b/src/partition_alloc/shim/allocator_shim.h
@@ -151,6 +151,8 @@ using EnableFreeWithSize = partition_alloc::internal::base::StrongAlias<class EnableFreeWithSizeTag, bool>; +using EnableStrictFreeSizeCheck = partition_alloc::internal::base:: + StrongAlias<class EnableStrictFreeSizeCheckTag, bool>; enum class BucketDistribution : uint8_t { kNeutral, kDenser }; using EventuallyZeroFreedMemory = partition_alloc::internal::base:: StrongAlias<class EventuallyZeroFreedMemoryTag, bool>; @@ -171,7 +173,8 @@ partition_alloc::internal::SchedulerLoopQuarantineConfig scheduler_loop_quarantine_for_advanced_memory_safety_checks_config, EventuallyZeroFreedMemory eventually_zero_freed_memory, - EnableFreeWithSize enable_free_with_size); + EnableFreeWithSize enable_free_with_size, + EnableStrictFreeSizeCheck enable_strict_free_size_check); PA_COMPONENT_EXPORT(ALLOCATOR_SHIM) uint32_t GetMainPartitionRootExtrasSize();
diff --git a/src/partition_alloc/shim/allocator_shim_default_dispatch_to_partition_alloc.cc b/src/partition_alloc/shim/allocator_shim_default_dispatch_to_partition_alloc.cc index 0a339b4..260bd45 100644 --- a/src/partition_alloc/shim/allocator_shim_default_dispatch_to_partition_alloc.cc +++ b/src/partition_alloc/shim/allocator_shim_default_dispatch_to_partition_alloc.cc
@@ -681,7 +681,8 @@ partition_alloc::internal::SchedulerLoopQuarantineConfig scheduler_loop_quarantine_for_advanced_memory_safety_checks_config, EventuallyZeroFreedMemory eventually_zero_freed_memory, - EnableFreeWithSize enable_free_with_size) { + EnableFreeWithSize enable_free_with_size, + EnableStrictFreeSizeCheck enable_strict_free_size_check) { // Calling Get() is actually important, even if the return value isn't // used, because it has a side effect of initializing the variable, if it // wasn't already. @@ -724,6 +725,10 @@ enable_free_with_size ? partition_alloc::PartitionOptions::kEnabled : partition_alloc::PartitionOptions::kDisabled; + opts.strict_free_size_check = + enable_strict_free_size_check + ? partition_alloc::PartitionOptions::kEnabled + : partition_alloc::PartitionOptions::kDisabled; return opts; }()); partition_alloc::PartitionRoot* new_root = new_main_allocator->root();
diff --git a/src/partition_alloc/shim/allocator_shim_default_dispatch_to_partition_alloc.h b/src/partition_alloc/shim/allocator_shim_default_dispatch_to_partition_alloc.h index afd9736..22bc1b5 100644 --- a/src/partition_alloc/shim/allocator_shim_default_dispatch_to_partition_alloc.h +++ b/src/partition_alloc/shim/allocator_shim_default_dispatch_to_partition_alloc.h
@@ -195,6 +195,8 @@ auto eventually_zero_freed_memory = EventuallyZeroFreedMemory(false); auto enable_free_with_size = allocator_shim::EnableFreeWithSize( PA_BUILDFLAG(SHIM_SUPPORTS_SIZED_DEALLOC)); + auto enable_strict_free_size_check = + allocator_shim::EnableStrictFreeSizeCheck(true); ConfigurePartitions( enable_brp, brp_extra_extras_size, enable_memory_tagging, @@ -202,7 +204,8 @@ scheduler_loop_quarantine_global_config, scheduler_loop_quarantine_thread_local_config, scheduler_loop_quarantine_for_advanced_memory_safety_checks_config, - eventually_zero_freed_memory, enable_free_with_size); + eventually_zero_freed_memory, enable_free_with_size, + enable_strict_free_size_check); } #endif // PA_BUILDFLAG(USE_PARTITION_ALLOC_AS_MALLOC)