0

Prepare for being stricter about buildflag values

This is a reland of commit 8f7e0bcec0,
except for the actual change to build/write_buildflag_header.py

Original change's description:
> build: Be stricter about buildflag values
>
> We don't want buildflag headers to contain arbitrary expressions
>
> A recent change had used
>
>     "ENABLE_FOO=$enable_foo_1 || $enable_foo_2",
>
> which `gn` turned into `true || false`, which write_buildflag_header.py
> wrote to a generated header verbatim. Checking
>
>     #if BUILDFLAG(ENABLE_FOO)
>
> would then expand to
>
>     #if true || false
>
> but `true` and `false` are only valid in C++, not C.
>
> (...and buildflag headers get included in resource scripts, and
> rc.exe preprocesses using C semantics, not C++ semantics, and hence
> silently does the wrong thing.)
>
> So only allow `true` and `false` (which buildflag_headers.py maps
> to 1 and 0 in its output), and string literals, for things like
> ALTERNATE_CDM_STORAGE_ID_KEY. We could also consider (re)allowing
> int literals, but those silently evaluate to truthy or falsy values
> in preprocessor expressions -- several places were checking
> `#if BUILDFLAG(CHROME_PGO)` when they morally mean
> `#if BUILDFLAG(CHROME_PGO) == 1`. Since we currently have no use
> cases for arbitrary int values in buildflags, disallow them.
> String literals don't have this problem.
>
> The main motivation is to block things like `true && false` and
> `false || true`.
>
> (Convert all `=1` / `=0` to `=true` / `=false` to make this work.
> We must support `true` and `false` for the very common
> `FOO=$foo` use case.)
>
> The CHROME_PGO issue above is fine since we also checked
> BUILDFLAG(CLANG_PROFILING_INSIDE_SANDBOX) at the same time, and
> that's only set in the profiling phase, but it's still confusing.
> So use two boolean build flags for CHROME_PGO instead. (No behavior
> change.)
>
> Bug: 403123830
> Change-Id: I7a6cedf063824b280f1fe03a07586cf77f649b29
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6371822
> Commit-Queue: Nico Weber <thakis@chromium.org>
> Owners-Override: Nico Weber <thakis@chromium.org>
> Reviewed-by: Hans Wennborg <hans@chromium.org>
> Cr-Commit-Position: refs/heads/main@{#1435044}

Bug: 403123830
Change-Id: I2dcea00d45405783848d089ce37d52b0294a7189
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6372328
Reviewed-by: Hans Wennborg <hans@chromium.org>
Owners-Override: Nico Weber <thakis@chromium.org>
Commit-Queue: Nico Weber <thakis@chromium.org>
Auto-Submit: Nico Weber <thakis@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1435395}
This commit is contained in:
Nico Weber
2025-03-20 07:39:17 -07:00
committed by Chromium LUCI CQ
parent a6326ce521
commit fdcf479f2f
15 changed files with 101 additions and 52 deletions
base
build
BUILD.gn
config
compiler
chrome
components
memory_system
safe_browsing
content
media
printing/buildflags

@ -2566,14 +2566,38 @@ if (is_linux || is_chromeos) {
buildflag_header("cfi_buildflags") {
header = "cfi_buildflags.h"
flags = [
# TODO(pcc): remove CFI_CAST_CHECK, see https://crbug.com/626794.
"CFI_CAST_CHECK=$is_cfi && $use_cfi_cast",
"CFI_DIAG=$is_cfi && $use_cfi_diag",
"CFI_ICALL_CHECK=$is_cfi && $use_cfi_icall",
"CFI_ENFORCEMENT_TRAP=$is_cfi && !$use_cfi_diag",
"CFI_ENFORCEMENT_DIAGNOSTIC=$is_cfi && $use_cfi_diag && !$use_cfi_recover",
]
flags = []
# TODO(pcc): remove CFI_CAST_CHECK, see https://crbug.com/626794.
if (is_cfi && use_cfi_cast) {
flags += [ "CFI_CAST_CHECK=true" ]
} else {
flags += [ "CFI_CAST_CHECK=false" ]
}
if (is_cfi && use_cfi_diag) {
flags += [ "CFI_DIAG=true" ]
} else {
flags += [ "CFI_DIAG=false" ]
}
if (is_cfi && use_cfi_icall) {
flags += [ "CFI_ICALL_CHECK=true" ]
} else {
flags += [ "CFI_ICALL_CHECK=false" ]
}
if (is_cfi && !use_cfi_diag) {
flags += [ "CFI_ENFORCEMENT_TRAP=true" ]
} else {
flags += [ "CFI_ENFORCEMENT_TRAP=false" ]
}
if (is_cfi && use_cfi_diag && !use_cfi_recover) {
flags += [ "CFI_ENFORCEMENT_DIAGNOSTIC=true" ]
} else {
flags += [ "CFI_ENFORCEMENT_DIAGNOSTIC=false" ]
}
}
buildflag_header("debugging_buildflags") {

@ -264,7 +264,7 @@ StackTrace::StackTrace(span<const void* const> trace)
// static
bool StackTrace::WillSymbolizeToStreamForTesting() {
#if BUILDFLAG(SYMBOL_LEVEL) == 0
#if BUILDFLAG(HAS_SYMBOLS) == 0
// Symbols are not expected to be reliable when gn args specifies
// symbol_level=0.
return false;

@ -23,13 +23,13 @@ buildflag_header("branding_buildflags") {
if (is_chrome_branded) {
flags = [
"CHROMIUM_BRANDING=0",
"GOOGLE_CHROME_BRANDING=1",
"CHROMIUM_BRANDING=false",
"GOOGLE_CHROME_BRANDING=true",
]
} else {
flags = [
"CHROMIUM_BRANDING=1",
"GOOGLE_CHROME_BRANDING=0",
"CHROMIUM_BRANDING=true",
"GOOGLE_CHROME_BRANDING=false",
]
}
@ -37,9 +37,9 @@ buildflag_header("branding_buildflags") {
assert(!is_chrome_branded,
"`is_chrome_for_testing` is incompatible with `is_chrome_branded`")
flags += [ "CHROME_FOR_TESTING=1" ]
flags += [ "CHROME_FOR_TESTING=true" ]
} else {
flags += [ "CHROME_FOR_TESTING=0" ]
flags += [ "CHROME_FOR_TESTING=false" ]
}
# Note: `GOOGLE_CHROME_FOR_TESTING_BRANDING` and `CHROMIUM_BRANDING` are not
@ -48,9 +48,9 @@ buildflag_header("branding_buildflags") {
assert(is_chrome_for_testing,
"`is_chrome_for_testing_branded` requires `is_chrome_for_testing`")
flags += [ "GOOGLE_CHROME_FOR_TESTING_BRANDING=1" ]
flags += [ "GOOGLE_CHROME_FOR_TESTING_BRANDING=true" ]
} else {
flags += [ "GOOGLE_CHROME_FOR_TESTING_BRANDING=0" ]
flags += [ "GOOGLE_CHROME_FOR_TESTING_BRANDING=false" ]
}
}

@ -3171,10 +3171,25 @@ config("default_init_stack_vars") {
buildflag_header("compiler_buildflags") {
header = "compiler_buildflags.h"
flags = [
"CLANG_PGO=$chrome_pgo_phase",
"SYMBOL_LEVEL=$symbol_level",
]
flags = []
if (chrome_pgo_phase == 1) {
flags += [ "CLANG_PGO_PROFILING=true" ]
} else {
flags += [ "CLANG_PGO_PROFILING=false" ]
}
if (chrome_pgo_phase == 2) {
flags += [ "CLANG_PGO_OPTIMIZED=true" ]
} else {
flags += [ "CLANG_PGO_OPTIMIZED=false" ]
}
if (symbol_level > 0) {
flags += [ "HAS_SYMBOLS=true" ]
} else {
flags += [ "HAS_SYMBOLS=false" ]
}
}
config("cet_shadow_stack") {

@ -113,9 +113,9 @@ buildflag_header("buildflags") {
# Android and ChromeOS don't support multiple browser processes, so they don't
# employ ProcessSingleton.
if (is_android || is_chromeos) {
flags += [ "ENABLE_PROCESS_SINGLETON=0" ]
flags += [ "ENABLE_PROCESS_SINGLETON=false" ]
} else {
flags += [ "ENABLE_PROCESS_SINGLETON=1" ]
flags += [ "ENABLE_PROCESS_SINGLETON=true" ]
}
}

@ -64,7 +64,7 @@
#include "components/rlz/rlz_tracker.h" // nogncheck crbug.com/1125897
#endif
#if BUILDFLAG(CLANG_PROFILING_INSIDE_SANDBOX) && BUILDFLAG(CLANG_PGO)
#if BUILDFLAG(CLANG_PROFILING_INSIDE_SANDBOX) && BUILDFLAG(CLANG_PGO_PROFILING)
#include "base/run_loop.h"
#include "content/public/browser/profiling_utils.h"
#endif
@ -132,13 +132,13 @@ void OnShutdownStarting(ShutdownType type) {
// TODO(crbug.com/40685224): Check if this should also be enabled for
// coverage builds.
#if BUILDFLAG(CLANG_PROFILING_INSIDE_SANDBOX) && BUILDFLAG(CLANG_PGO)
#if BUILDFLAG(CLANG_PROFILING_INSIDE_SANDBOX) && BUILDFLAG(CLANG_PGO_PROFILING)
// Wait for all the child processes to dump their profiling data without
// blocking the main thread.
base::RunLoop nested_run_loop(base::RunLoop::Type::kNestableTasksAllowed);
content::AskAllChildrenToDumpProfilingData(nested_run_loop.QuitClosure());
nested_run_loop.Run();
#endif // BUILDFLAG(CLANG_PROFILING_INSIDE_SANDBOX) && BUILDFLAG(CLANG_PGO)
#endif
// Call FastShutdown on all of the RenderProcessHosts. This will be
// a no-op in some cases, so we still need to go through the normal

@ -971,7 +971,7 @@ void ChromeBrowserMainExtraPartsMetrics::PreBrowserStart() {
// Log once here at browser start rather than at each renderer launch.
ChromeMetricsServiceAccessor::RegisterSyntheticFieldTrial("ClangPGO",
#if BUILDFLAG(CLANG_PGO)
#if BUILDFLAG(CLANG_PGO_OPTIMIZED)
#if BUILDFLAG(USE_THIN_LTO)
"EnabledWithThinLTO"
#else

@ -9,9 +9,9 @@ import("//testing/test.gni")
buildflag_header("buildflags") {
header = "buildflags.h"
if (is_chrome_branded && is_win) {
flags = [ "USE_GOOGLE_UPDATE_INTEGRATION=1" ]
flags = [ "USE_GOOGLE_UPDATE_INTEGRATION=true" ]
} else {
flags = [ "USE_GOOGLE_UPDATE_INTEGRATION=0" ]
flags = [ "USE_GOOGLE_UPDATE_INTEGRATION=false" ]
}
}

@ -13,7 +13,7 @@ namespace memory_system::features {
BASE_FEATURE(kAllocationTraceRecorder,
"AllocationTraceRecorder",
#if BUILDFLAG(CLANG_PGO) == 1 || BUILDFLAG(USE_CLANG_COVERAGE)
#if BUILDFLAG(CLANG_PGO_PROFILING) || BUILDFLAG(USE_CLANG_COVERAGE)
// If creating a profiling build include the allocation recorder
// unconditionally. This way we ensure that the recorder is covered
// by the profile even if the profiling device doesn't support MTE.
@ -30,7 +30,7 @@ BASE_FEATURE_PARAM(
kAllocationTraceRecorderForceAllProcesses,
&kAllocationTraceRecorder,
"atr_force_all_processes",
#if BUILDFLAG(CLANG_PGO) == 1 || BUILDFLAG(USE_CLANG_COVERAGE)
#if BUILDFLAG(CLANG_PGO_PROFILING) || BUILDFLAG(USE_CLANG_COVERAGE)
// If creating a profiling build include the allocation recorder
// unconditionally. This way we ensure that the recorder is covered by the
// profile even if the profiling device doesn't support MTE.

@ -25,22 +25,22 @@ buildflag_header("buildflags") {
# SAFE_BROWSING_DOWNLOAD_PROTECTION means "are SB download protection features
# available?" This is true for desktop OSes and Android.
if (safe_browsing_mode == 0) {
flags += [ "FULL_SAFE_BROWSING=0" ]
flags += [ "SAFE_BROWSING_AVAILABLE=0" ]
flags += [ "SAFE_BROWSING_DB_LOCAL=0" ]
flags += [ "SAFE_BROWSING_DB_REMOTE=0" ]
flags += [ "SAFE_BROWSING_DOWNLOAD_PROTECTION=0" ]
flags += [ "FULL_SAFE_BROWSING=false" ]
flags += [ "SAFE_BROWSING_AVAILABLE=false" ]
flags += [ "SAFE_BROWSING_DB_LOCAL=false" ]
flags += [ "SAFE_BROWSING_DB_REMOTE=false" ]
flags += [ "SAFE_BROWSING_DOWNLOAD_PROTECTION=false" ]
} else if (safe_browsing_mode == 1) {
flags += [ "FULL_SAFE_BROWSING=1" ]
flags += [ "SAFE_BROWSING_AVAILABLE=1" ]
flags += [ "SAFE_BROWSING_DB_LOCAL=1" ]
flags += [ "SAFE_BROWSING_DB_REMOTE=0" ]
flags += [ "SAFE_BROWSING_DOWNLOAD_PROTECTION=1" ]
flags += [ "FULL_SAFE_BROWSING=true" ]
flags += [ "SAFE_BROWSING_AVAILABLE=true" ]
flags += [ "SAFE_BROWSING_DB_LOCAL=true" ]
flags += [ "SAFE_BROWSING_DB_REMOTE=false" ]
flags += [ "SAFE_BROWSING_DOWNLOAD_PROTECTION=true" ]
} else if (safe_browsing_mode == 2) {
flags += [ "FULL_SAFE_BROWSING=0" ]
flags += [ "SAFE_BROWSING_AVAILABLE=1" ]
flags += [ "SAFE_BROWSING_DB_LOCAL=0" ]
flags += [ "SAFE_BROWSING_DB_REMOTE=1" ]
flags += [ "SAFE_BROWSING_DOWNLOAD_PROTECTION=1" ]
flags += [ "FULL_SAFE_BROWSING=false" ]
flags += [ "SAFE_BROWSING_AVAILABLE=true" ]
flags += [ "SAFE_BROWSING_DB_LOCAL=false" ]
flags += [ "SAFE_BROWSING_DB_REMOTE=true" ]
flags += [ "SAFE_BROWSING_DOWNLOAD_PROTECTION=true" ]
}
}

@ -39,7 +39,7 @@
#include "content/browser/devtools/protocol/visual_debugger_handler.h"
#endif
#if BUILDFLAG(CLANG_PROFILING_INSIDE_SANDBOX) && BUILDFLAG(CLANG_PGO)
#if BUILDFLAG(CLANG_PROFILING_INSIDE_SANDBOX) && BUILDFLAG(CLANG_PGO_PROFILING)
#include "content/browser/devtools/protocol/native_profiling_handler.h"
#endif
@ -220,7 +220,7 @@ bool BrowserDevToolsAgentHost::AttachSession(DevToolsSession* session) {
session->CreateAndAddHandler<protocol::TracingHandler>(
this, GetIOContext(), /* root_session */ nullptr);
#if BUILDFLAG(CLANG_PROFILING_INSIDE_SANDBOX) && BUILDFLAG(CLANG_PGO)
#if BUILDFLAG(CLANG_PROFILING_INSIDE_SANDBOX) && BUILDFLAG(CLANG_PGO_PROFILING)
session->CreateAndAddHandler<protocol::NativeProfilingHandler>();
#endif

@ -161,7 +161,7 @@ ChildProcess::~ChildProcess() {
base::ThreadPoolInstance::Get()->Shutdown();
}
#if BUILDFLAG(CLANG_PROFILING_INSIDE_SANDBOX) && BUILDFLAG(CLANG_PGO)
#if BUILDFLAG(CLANG_PROFILING_INSIDE_SANDBOX) && BUILDFLAG(CLANG_PGO_PROFILING)
// Flush the profiling data to disk. Doing this manually (vs relying on this
// being done automatically when the process exits) will ensure that this data
// doesn't get lost if the process is fast killed.

@ -401,7 +401,7 @@ class ChildThreadImpl::IOThreadState
#if BUILDFLAG(IS_POSIX)
// Take the file descriptor so that |file| does not close it.
base::ScopedFD fd(file.TakePlatformFile());
#if BUILDFLAG(CLANG_PGO) || BUILDFLAG(USE_CLANG_COVERAGE)
#if BUILDFLAG(CLANG_PGO_PROFILING) || BUILDFLAG(USE_CLANG_COVERAGE)
FILE* f = fdopen(fd.release(), "r+b");
__llvm_profile_set_file_object(f, 1);
#else

@ -62,7 +62,6 @@ buildflag_header("media_buildflags") {
"PLATFORM_HAS_OPTIONAL_HEVC_DECODE_SUPPORT=$platform_has_optional_hevc_decode_support",
"PLATFORM_HAS_OPTIONAL_HEVC_ENCODE_SUPPORT=$platform_has_optional_hevc_encode_support",
"USE_ARC_PROTECTED_MEDIA=$use_arc_protected_media",
"USE_CHROMEOS_MEDIA_ACCELERATION=$use_vaapi||$use_v4l2_codec",
"USE_CHROMEOS_PROTECTED_AV1=$use_chromeos_protected_av1",
"USE_CHROMEOS_PROTECTED_MEDIA=$use_chromeos_protected_media",
"USE_CRAS=$use_cras",
@ -71,6 +70,12 @@ buildflag_header("media_buildflags") {
"USE_OPENSLES=$media_use_opensles",
]
if (use_vaapi || use_v4l2_codec) {
flags += [ "USE_CHROMEOS_MEDIA_ACCELERATION=true" ]
} else {
flags += [ "USE_CHROMEOS_MEDIA_ACCELERATION=false" ]
}
if (enable_library_cdms) {
flags += [
"CDM_PLATFORM_SPECIFIC_PATH=\"$cdm_platform_specific_path\"",

@ -13,7 +13,6 @@ buildflag_header("buildflags") {
header = "buildflags.h"
flags = [
"ENABLE_PRINTING=$enable_printing || $enable_printing_tests",
"ENABLE_PRINT_PREVIEW=$enable_print_preview",
"ENABLE_BASIC_PRINT_DIALOG=$enable_basic_print_dialog",
"ENABLE_CONCURRENT_BASIC_PRINT_DIALOGS=$enable_concurrent_basic_print_dialogs",
@ -23,4 +22,10 @@ buildflag_header("buildflags") {
"USE_CUPS=$use_cups",
"USE_CUPS_IPP=$use_cups_ipp",
]
if (enable_printing || enable_printing_tests) {
flags += [ "ENABLE_PRINTING=true" ]
} else {
flags += [ "ENABLE_PRINTING=false" ]
}
}