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)
