0

Revert "PA: Invert MiraclePtr Platform Enablement Logic"

This reverts commit c3e9990871.

Reason for revert: I just realize I inverted the DPD condition
causing it to be enabled in non dcheck builds.

Original change's description:
> PA: Invert MiraclePtr Platform Enablement Logic
>
> This patch inverts the build flags logic from an "allowlist" of platform
> toward a "deny list".
>
> - **Increased Confidence**: Guarantees MiraclePtr is active everywhere
>   unless explicitly disabled, reducing the risk of unexpected
>   vulnerabilities on niche platforms.
>
> - **Future-Proofing**: Potentially new platforms automatically inherit
>   MiraclePtr from the beginning, eliminating the need to "ship" it one
>   more time.
>
> - **Simplified Maintenance**: Focus shifts to maintaining a small,
>   explicit list of exceptions rather than a growing list of enabled
>   platforms.
>
> What is the "complementary"?
> -----------------------------
>
> We know from `//build/config/BUILDCONFIG` the current_os is limited to:
>   aix     , android , chromeos
>   fuchsia , ios     , linux
>   mac     , nacl    , win
>   winuwp  , zos
>
> "aix" and "zos" aren't supported by Chrome, only V8. So they can be
> ignored.
>
> Bug: 364466913
> Change-Id: Ia03f8f18a4b3bb05e4c9d304980cadcf235be3c5
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5837217
> Reviewed-by: Keishi Hattori <keishi@chromium.org>
> Commit-Queue: Arthur Sonzogni <arthursonzogni@chromium.org>
> Reviewed-by: Wez <wez@chromium.org>
> Reviewed-by: danakj <danakj@chromium.org>
> Auto-Submit: Arthur Sonzogni <arthursonzogni@chromium.org>
> Cr-Commit-Position: refs/heads/main@{#1352670}

Bug: 364466913
Change-Id: I7ed0231bfa46b2091041df6ef7c09a7726cfacc1
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5844793
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Owners-Override: Arthur Sonzogni <arthursonzogni@chromium.org>
Auto-Submit: Arthur Sonzogni <arthursonzogni@chromium.org>
Commit-Queue: Arthur Sonzogni <arthursonzogni@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1352693}
This commit is contained in:
Arthur Sonzogni
2024-09-09 13:01:14 +00:00
committed by Chromium LUCI CQ
parent 5b1dc04827
commit 19b5dd0c20
4 changed files with 53 additions and 61 deletions
base
build_overrides

@ -112,7 +112,10 @@ BASE_FEATURE(kPartitionAllocZappingByFreeFlags,
BASE_FEATURE(kPartitionAllocBackupRefPtr,
"PartitionAllocBackupRefPtr",
#if PA_BUILDFLAG(ENABLE_BACKUP_REF_PTR_FEATURE_FLAG)
#if BUILDFLAG(IS_ANDROID) || BUILDFLAG(IS_WIN) || BUILDFLAG(IS_MAC) || \
BUILDFLAG(IS_CHROMEOS_ASH) || BUILDFLAG(IS_CHROMEOS_LACROS) || \
BUILDFLAG(IS_FUCHSIA) || (BUILDFLAG(IS_LINUX) && !BUILDFLAG(IS_CASTOS)) || \
PA_BUILDFLAG(ENABLE_BACKUP_REF_PTR_FEATURE_FLAG)
FEATURE_ENABLED_BY_DEFAULT
#else
FEATURE_DISABLED_BY_DEFAULT

@ -90,8 +90,9 @@ assert(
!enable_allocator_shim_partition_alloc_dispatch_with_advanced_checks_support || use_partition_alloc_as_malloc,
"PartitionAlloc with advanced checks requires PartitionAlloc itself.")
assert(!use_allocator_shim || !is_nacl,
"The allocator shim supports every platform, except nacl")
assert(!use_allocator_shim || (is_android || is_apple || is_chromeos ||
is_fuchsia || is_linux || is_win),
"The allocator shim does not (yet) support the platform.")
if (use_allocator_shim && is_win) {
# It's hard to override CRT's malloc family in every case in the component
@ -215,15 +216,12 @@ declare_args() {
# Enable the feature flag required to activate backup ref pointers. That is to
# say `PartitionAllocBackupRefPtr`.
#
# This is meant to be modified primarily on bots. It is much easier to
# override the feature flags using a binary flag instead of updating multiple
# bots's scripts to pass command line arguments.
# This is meant to be used primarily on bots. It is much easier to override
# the feature flags using a binary flag instead of updating multiple bots's
# scripts to pass command line arguments.
#
# TODO(328104161): Remove this flag.
enable_backup_ref_ptr_feature_flag =
enable_backup_ref_ptr_support && use_raw_ptr_backup_ref_impl &&
# Platforms where BackupRefPtr hasn't shipped yet:
!is_castos && !is_ios
enable_backup_ref_ptr_feature_flag = false
# Build support for Dangling Ptr Detection (DPD) via BackupRefPtr (BRP),
# making the raw_ptr<T> implementation to RawPtrBackupRefImpl if active.
@ -241,9 +239,9 @@ declare_args() {
# Enable the feature flag required to check for dangling pointers. That is to
# say `PartitionAllocDanglingPtr`.
#
# This is meant to be modified primarily on bots. It is much easier to
# override the feature flags using a binary flag instead of updating multiple
# bots's scripts to pass command line arguments.
# This is meant to be used primarily on bots. It is much easier to override
# the feature flags using a binary flag instead of updating multiple bots's
# scripts to pass command line arguments.
#
# TODO(328104161): Remove this flag.
enable_dangling_raw_ptr_feature_flag = enable_dangling_raw_ptr_checks

@ -55,20 +55,18 @@ Having said that, we want to emphasize that dereferencing a dangling pointer
remains an Undefined Behavior.
`raw_ptr<T>` protection is enabled by default in all non-Renderer processes, on:
- Android (incl. AndroidWebView, Android WebEngine, & Android ChromeCast)
- Android (incl. AndroidWebView & Android WebEngine)
- Windows
- ChromeOS (incl. Ash & Lacros)
- macOS
- Linux
- Fuchsia
In particular, it isn't yet enabled by default on:
In particular, it isn't enabled by default on:
- iOS
- Linux CastOS (Nest hardware)
For the source of truth, both `enable_backup_ref_ptr_support` and `enable_backup_ref_ptr_feature_flag` need to enabled.
Please refer to the following files: [build_overrides/partition_alloc.gni](https://source.chromium.org/chromium/chromium/src/+/main:build_overrides/partition_alloc.gni) and [partition_alloc.gni](https://source.chromium.org/chromium/chromium/src/+/main:base/allocator/partition_allocator/partition_alloc.gni;l=5?q=partition_alloc.gni&sq=&ss=chromium)
- ChromeCast
- Aix
- Zos
[TOC]

@ -71,6 +71,9 @@ if (!is_debug || partition_alloc_optimized_debug) {
partition_alloc_remove_configs +=
[ "//build/config/compiler/pgo:default_pgo_flags" ]
# Sanitizers replace the allocator, don't use our own.
_is_using_sanitizers = is_asan || is_hwasan || is_lsan || is_tsan || is_msan
# - Component build support is disabled on all platforms except Linux. It is
# known to cause issues on some (e.g. Windows with shims, Android with
# non-universal symbol wrapping), and has not been validated on others.
@ -80,59 +83,49 @@ _disable_partition_alloc_everywhere =
# - NaCl: No plans to support it.
# - iOS: Depends on ios_partition_alloc_enabled.
_is_partition_alloc_everywhere_platform =
!is_nacl && (!is_ios || ios_partition_alloc_enabled)
use_allocator_shim_default = true # Updated below:
# Sanitizers replace the allocator, don't use our own.
if (is_asan || is_hwasan || is_lsan || is_tsan || is_msan) {
use_allocator_shim_default = false
}
# NaCl will never support the allocator shim.
if (is_nacl) {
use_allocator_shim_default = false
}
# Under Fuchsia, the allocator shim is only required for PA-E.
if (is_fuchsia && _disable_partition_alloc_everywhere) {
use_allocator_shim_default = false
if (is_ios) {
_is_partition_alloc_everywhere_platform = ios_partition_alloc_enabled
} else {
_is_partition_alloc_everywhere_platform = !is_nacl
}
# Under Windows debug build, the allocator shim is not compatible with CRT.
# NaCl in particular does seem to link some binaries statically against the
# debug CRT with "is_nacl=false".
if (is_win && is_debug) {
use_allocator_shim_default = false
# NaCl in particular does seem to link some binaries statically
# against the debug CRT with "is_nacl=false".
# Under Fuchsia, the allocator shim is only required for PA-E.
# For all other platforms & configurations, the shim is required, to replace
# the default system allocators, e.g. with PartitionAlloc.
if ((is_linux || is_chromeos || is_android || is_apple ||
(is_fuchsia && !_disable_partition_alloc_everywhere) ||
(is_win && !is_debug &&
(!is_component_build || (use_custom_libcxx && !libcxx_is_shared)))) &&
!_is_using_sanitizers) {
_default_use_allocator_shim = true
} else {
_default_use_allocator_shim = false
}
if (is_win && is_component_build && (!use_custom_libcxx || libcxx_is_shared)) {
use_allocator_shim_default = false
if (_default_use_allocator_shim && _is_partition_alloc_everywhere_platform &&
!_disable_partition_alloc_everywhere) {
_default_allocator = "partition"
} else {
_default_allocator = "none"
}
use_partition_alloc_as_malloc_default =
use_allocator_shim_default && _is_partition_alloc_everywhere_platform &&
!_disable_partition_alloc_everywhere
use_partition_alloc_as_malloc_default = _default_allocator == "partition"
use_allocator_shim_default = _default_use_allocator_shim
enable_backup_ref_ptr_support_default = use_partition_alloc_as_malloc_default
_is_brp_supported =
(is_win || is_android || is_linux || is_mac || is_ios || is_chromeos ||
is_fuchsia) && use_partition_alloc_as_malloc_default
enable_backup_ref_ptr_support_default = _is_brp_supported
enable_backup_ref_ptr_slow_checks_default = false
enable_dangling_raw_ptr_checks_default =
# The DanglingPointerDetector relies on BackupRefPtr:
enable_backup_ref_ptr_support_default &&
# As it might detect many problems, only chromium developers have the
# DanglingPointerDetector checks enabled at the moment. Please note that it
# exists official builds with DCHECK enabled, so both conditions below are
# required.
!is_official_build && !(is_debug || dcheck_always_on) &&
# Fuchsia and iOS have never been tested against DanglingPointerDetector at
# the moment.
!is_ios && !is_fuchsia &&
# Only the `android-rel` CQ bot has enforced DanglingPointerDetector checks
# at the moment. The other Android bots are not ready for it yet.
!is_android
(is_linux || is_win || is_chromeos || is_mac) && !is_official_build &&
(is_debug || dcheck_always_on)
raw_ptr_zero_on_construct_default = true
raw_ptr_zero_on_move_default = true