From 6ba0003c92acee41db76a4f2382a0179815526e9 Mon Sep 17 00:00:00 2001 From: zackees Date: Tue, 12 May 2026 16:00:16 -0700 Subject: [PATCH] feat(build): #243 #244 strip .eh_frame on release builds across all GCC platforms GCC emits .eh_frame unwinding tables by default. On a stock FastLED ESP32-S3 Blink build this is ~225 KB / 36% of the firmware -- dead metadata no _Unwind_* API ever consumes when exceptions and panic backtrace are both off. FastLED's library-side pragma workaround (FastLED/FastLED#2423) is a no-op on modern GCC (audited in FastLED/FastLED#2473); the only reliable fix is cc1-level flags, which is the build system's job. This PR adds a shared eh_frame strip policy that injects `-fno-asynchronous-unwind-tables -fno-unwind-tables` to every TU when appropriate. The decision respects user intent at every layer: explicit env override, debug builds, `-fexceptions` in build_flags, ESP-IDF panic-backtrace / gdbstub / ocdaware / optimization-debug sdkconfig keys. New modules - `crates/fbuild-build/src/eh_frame_policy.rs` -- pure `decide()` function over plain inputs, plus `STRIP_FLAGS` constant. 16 unit tests cover the full precedence matrix. - `crates/fbuild-build/src/eh_frame_policy_compute.rs` -- orchestrator- side helper that reads env vars + extracts BuildContext fields and calls `decide()`. - `crates/fbuild-config/src/sdkconfig.rs` -- minimal sdkconfig parser surfacing the 4 keys the policy reads. Defaults to ESP-IDF Arduino defaults (`panic_print_backtrace = true`) so the safe assumption for an unmodified project is Preserve. 14 unit tests. Per-platform wiring - ESP32, generic ARM (STM32/RP2040/NRF52), Teensy, ESP8266, AVR each get a `with_eh_frame_policy()` setter and a 3-line append in `common_flags()`. Mirrors the existing `with_build_unflags` pattern. - All orchestrators (esp32, generic_arm, teensy, esp8266, avr, stm32, rp2040, apollo3) compute the policy once per build and pass it to each compiler construction site. ESP32 also reads sdkconfig from the project dir. Other platforms pass `None`. - 10 new platform unit tests (2 per platform) assert STRIP_FLAGS presence under `Strip` and absence under default `Preserve`. Integration test (`#[ignore]`, toolchain-required) - `crates/fbuild-build/tests/eh_frame_strip_esp32.rs` builds a tempfile ESP32 project twice (FBUILD_KEEP_EH_FRAME=1 vs FBUILD_STRIP_EH_FRAME=1) and asserts firmware.bin delta >= 150 KB plus `.eh_frame` ELF section delta >= 50 KB via the `object` crate. User-facing overrides - `FBUILD_STRIP_EH_FRAME=1` -- force strip regardless of detection - `FBUILD_KEEP_EH_FRAME=1` -- force preserve Behavior under defaults - ESP32 with stock Arduino framework: Preserve (matches user expectation of esp32_exception_decoder showing readable backtraces). - Teensy / STM32 / RP2040 / NRF52 / ESP8266: Strip (their JSON configs already ship `-fno-exceptions`; nothing in their toolchain consumes eh_frame). Immediate flash savings on every release build. - AVR: Strip wired for uniformity; runtime is a no-op because AVR-gcc only emits eh_frame when `-fexceptions` is on, which AVR's JSON disables. - Debug builds (`build_type = debug`, or `-Og`/`-O0` flags) always Preserve, even on platforms that would otherwise strip. Test coverage - 16 eh_frame_policy unit tests (decision matrix) - 14 sdkconfig unit tests (parser + project-dir lookup) - 10 per-platform compiler tests (STRIP_FLAGS presence/absence) - 1 integration test (`#[ignore]`, runs locally with ESP32 toolchain) - Existing 534 fbuild-build + 120 fbuild-config lib tests stay green. Closes #243. Closes #244. Co-Authored-By: Claude Opus 4.7 (1M context) --- Cargo.lock | 1 + crates/fbuild-build/Cargo.toml | 1 + .../fbuild-build/src/apollo3/orchestrator.rs | 7 +- crates/fbuild-build/src/avr/avr_compiler.rs | 39 +++ crates/fbuild-build/src/avr/orchestrator.rs | 13 +- crates/fbuild-build/src/eh_frame_policy.rs | 275 ++++++++++++++++++ .../src/eh_frame_policy_compute.rs | 49 ++++ .../fbuild-build/src/esp32/esp32_compiler.rs | 43 +++ crates/fbuild-build/src/esp32/orchestrator.rs | 28 +- .../src/esp8266/esp8266_compiler.rs | 39 +++ .../fbuild-build/src/esp8266/orchestrator.rs | 8 +- .../src/generic_arm/arm_compiler.rs | 39 +++ crates/fbuild-build/src/lib.rs | 2 + .../fbuild-build/src/rp2040/orchestrator.rs | 12 +- crates/fbuild-build/src/stm32/orchestrator.rs | 14 +- .../fbuild-build/src/teensy/orchestrator.rs | 13 +- .../src/teensy/teensy_compiler.rs | 39 +++ .../tests/eh_frame_strip_esp32.rs | 160 ++++++++++ crates/fbuild-config/src/lib.rs | 1 + crates/fbuild-config/src/sdkconfig.rs | 258 ++++++++++++++++ 20 files changed, 1030 insertions(+), 11 deletions(-) create mode 100644 crates/fbuild-build/src/eh_frame_policy.rs create mode 100644 crates/fbuild-build/src/eh_frame_policy_compute.rs create mode 100644 crates/fbuild-build/tests/eh_frame_strip_esp32.rs create mode 100644 crates/fbuild-config/src/sdkconfig.rs diff --git a/Cargo.lock b/Cargo.lock index 92cbaf07..29192ff2 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -866,6 +866,7 @@ dependencies = [ "fbuild-paths", "fbuild-test-support", "filetime", + "object 0.36.7", "regex", "serde", "serde_json", diff --git a/crates/fbuild-build/Cargo.toml b/crates/fbuild-build/Cargo.toml index 69086249..28e4d522 100644 --- a/crates/fbuild-build/Cargo.toml +++ b/crates/fbuild-build/Cargo.toml @@ -29,3 +29,4 @@ tempfile = { workspace = true } filetime = { workspace = true } fbuild-test-support = { path = "../fbuild-test-support" } tar = { workspace = true } +object = { workspace = true } diff --git a/crates/fbuild-build/src/apollo3/orchestrator.rs b/crates/fbuild-build/src/apollo3/orchestrator.rs index 8c560880..f6b476b4 100644 --- a/crates/fbuild-build/src/apollo3/orchestrator.rs +++ b/crates/fbuild-build/src/apollo3/orchestrator.rs @@ -36,6 +36,10 @@ impl BuildOrchestrator for Apollo3Orchestrator { // 1-2. Parse config, load board, setup build dirs, resolve src dir, collect flags let mut ctx = pipeline::BuildContext::new(params)?; + // Compute eh_frame strip policy once per build (FastLED/fbuild#244). + let eh_frame_policy = + crate::eh_frame_policy_compute::compute_eh_frame_policy(&ctx, params.profile, None); + // 3. Ensure ARM GCC 8 toolchain (Apollo3/mbed-os requires GCC 8) let toolchain = fbuild_packages::toolchain::ArmGcc8Toolchain::new(¶ms.project_dir); let toolchain_dir = fbuild_packages::Package::ensure_installed(&toolchain)?; @@ -182,7 +186,8 @@ impl BuildOrchestrator for Apollo3Orchestrator { params.profile, params.verbose, ) - .with_build_unflags(ctx.build_unflags.clone()); + .with_build_unflags(ctx.build_unflags.clone()) + .with_eh_frame_policy(eh_frame_policy); // 8. Create linker let linker_script = framework.get_linker_script(); diff --git a/crates/fbuild-build/src/avr/avr_compiler.rs b/crates/fbuild-build/src/avr/avr_compiler.rs index 37cd1aa6..f5c4ed58 100644 --- a/crates/fbuild-build/src/avr/avr_compiler.rs +++ b/crates/fbuild-build/src/avr/avr_compiler.rs @@ -10,6 +10,7 @@ use fbuild_core::{BuildProfile, Result}; use super::mcu_config::AvrMcuConfig; use crate::compiler::{CompileResult, Compiler, CompilerBase}; +use crate::eh_frame_policy::EhFramePolicy; /// AVR-specific compiler using avr-gcc and avr-g++. pub struct AvrCompiler { @@ -22,6 +23,10 @@ pub struct AvrCompiler { /// PlatformIO `build_unflags` to strip from the effective compile /// line. See FastLED/fbuild#37. build_unflags: Vec, + /// Whether to strip eh_frame unwinding tables. Default `Preserve` so existing + /// callers see no behavior change; orchestrators set this via + /// [`Self::with_eh_frame_policy`]. See FastLED/fbuild#244. + eh_frame_policy: EhFramePolicy, } impl AvrCompiler { @@ -51,6 +56,7 @@ impl AvrCompiler { profile, temp_dir: fbuild_core::response_file::windows_temp_dir(), build_unflags: Vec::new(), + eh_frame_policy: EhFramePolicy::default(), } } @@ -61,6 +67,13 @@ impl AvrCompiler { self } + /// Attach the eh_frame strip/preserve policy decided by the orchestrator. + /// Default `Preserve` keeps existing behavior. See FastLED/fbuild#244. + pub fn with_eh_frame_policy(mut self, policy: EhFramePolicy) -> Self { + self.eh_frame_policy = policy; + self + } + /// Build the common AVR compiler flags. fn common_flags(&self) -> Vec { let mut flags = vec![format!("-mmcu={}", self.base.mcu)]; @@ -73,6 +86,14 @@ impl AvrCompiler { flags.extend(self.base.build_define_flags()); flags.extend(self.base.build_include_flags()); + + if matches!(self.eh_frame_policy, EhFramePolicy::Strip) { + flags.extend( + crate::eh_frame_policy::STRIP_FLAGS + .iter() + .map(|s| s.to_string()), + ); + } flags } } @@ -198,4 +219,22 @@ mod tests { assert!(flags.contains(&"-std=gnu++11".to_string())); assert!(flags.contains(&"-fno-exceptions".to_string())); } + + /// FastLED/fbuild#244: default policy must not leak STRIP_FLAGS. + #[test] + fn cpp_flags_preserve_eh_frame_by_default() { + let compiler = test_compiler(); + let flags = compiler.cpp_flags(); + assert!(!flags.iter().any(|f| f == "-fno-asynchronous-unwind-tables")); + assert!(!flags.iter().any(|f| f == "-fno-unwind-tables")); + } + + /// FastLED/fbuild#244: Strip policy must inject both unwind-table flags. + #[test] + fn cpp_flags_strip_eh_frame_when_policy_set() { + let compiler = test_compiler().with_eh_frame_policy(EhFramePolicy::Strip); + let flags = compiler.cpp_flags(); + assert!(flags.iter().any(|f| f == "-fno-asynchronous-unwind-tables")); + assert!(flags.iter().any(|f| f == "-fno-unwind-tables")); + } } diff --git a/crates/fbuild-build/src/avr/orchestrator.rs b/crates/fbuild-build/src/avr/orchestrator.rs index a97ca7c1..43b62fb4 100644 --- a/crates/fbuild-build/src/avr/orchestrator.rs +++ b/crates/fbuild-build/src/avr/orchestrator.rs @@ -57,6 +57,7 @@ struct AvrFingerprintMetadata { platform: String, max_flash: Option, max_ram: Option, + eh_frame_policy: &'static str, } fn profile_label(profile: fbuild_core::BuildProfile) -> &'static str { @@ -90,6 +91,11 @@ impl BuildOrchestrator for AvrOrchestrator { // the shared `perf` timer. let mut ctx = pipeline::BuildContext::new_with_perf(params, Some(&mut perf))?; + // Compute eh_frame strip policy once per build (FastLED/fbuild#244). + // No sdkconfig on AVR. + let eh_frame_policy = + crate::eh_frame_policy_compute::compute_eh_frame_policy(&ctx, params.profile, None); + // 3. Ensure toolchain let (toolchain, toolchain_dir) = { let _g = perf.phase("toolchain-ensure"); @@ -138,6 +144,10 @@ impl BuildOrchestrator for AvrOrchestrator { platform: "atmelavr".to_string(), max_flash: ctx.board.max_flash, max_ram: ctx.board.max_ram, + eh_frame_policy: match eh_frame_policy { + crate::eh_frame_policy::EhFramePolicy::Strip => "strip", + crate::eh_frame_policy::EhFramePolicy::Preserve => "preserve", + }, })?; let (fast_elf, [fast_hex], fast_compile_db) = expected_fast_path_artifacts(build_dir, ¶ms.project_dir, ["firmware.hex"]); @@ -224,7 +234,8 @@ impl BuildOrchestrator for AvrOrchestrator { params.profile, params.verbose, ) - .with_build_unflags(ctx.build_unflags.clone()); + .with_build_unflags(ctx.build_unflags.clone()) + .with_eh_frame_policy(eh_frame_policy); // 7. Create linker let linker = AvrLinker::new( diff --git a/crates/fbuild-build/src/eh_frame_policy.rs b/crates/fbuild-build/src/eh_frame_policy.rs new file mode 100644 index 00000000..f683a477 --- /dev/null +++ b/crates/fbuild-build/src/eh_frame_policy.rs @@ -0,0 +1,275 @@ +//! Decide whether to strip GCC's eh_frame unwinding tables from a build. +//! +//! See FastLED/fbuild#243 (ESP32) and FastLED/fbuild#244 (other GCC platforms). +//! GCC emits .eh_frame by default to support C++ exceptions and stack +//! unwinding. On embedded targets where neither exceptions nor panic +//! backtrace are used, this is dead metadata — ~225 KB on a stock FastLED +//! ESP32-S3 Blink. Stripping it requires compile-time cc1 flags; library- +//! header pragmas (FastLED's PR #2423) are unreliable on modern GCC. + +use fbuild_config::sdkconfig::SdkConfigSummary; +use fbuild_core::BuildProfile; + +/// What the build system should do with .eh_frame. +#[derive(Debug, Clone, Copy, PartialEq, Eq, Default)] +pub enum EhFramePolicy { + /// Keep eh_frame (default; safe). + #[default] + Preserve, + /// Strip eh_frame via `-fno-asynchronous-unwind-tables -fno-unwind-tables`. + Strip, +} + +/// Inputs to [`decide`]. All references are borrowed. +pub struct EhFrameInputs<'a> { + /// User-supplied positive build flags (from platformio.ini `build_flags`, + /// after debug-build overlay applied). + pub build_flags: &'a [String], + /// User-supplied negative build flags (from platformio.ini `build_unflags`). + pub build_unflags: &'a [String], + /// ESP32-only summary of sdkconfig keys we care about. None for other platforms. + pub sdkconfig: Option<&'a SdkConfigSummary>, + /// Build profile. + pub profile: BuildProfile, + /// True if `build_type = debug` was set in platformio.ini. + pub is_debug_build: bool, + /// `FBUILD_KEEP_EH_FRAME=1` env override. + pub env_keep: bool, + /// `FBUILD_STRIP_EH_FRAME=1` env override. + pub env_strip: bool, +} + +/// Append these flags to every TU's compile line when policy is [`EhFramePolicy::Strip`]. +/// +/// Both flags are necessary: GCC defaults to `-fasynchronous-unwind-tables` +/// (the actual emitter of .eh_frame on most non-debug builds), and +/// `-fno-unwind-tables` alone does not suppress the async variant. +pub const STRIP_FLAGS: [&str; 2] = ["-fno-asynchronous-unwind-tables", "-fno-unwind-tables"]; + +/// True iff `needle` appears in `flags` and is not negated by `unflags`. +/// +/// The match is exact (not prefix) because real-world flags arrive as +/// discrete tokens, e.g. `["-O2", "-fexceptions", "-DFOO=1"]`. +fn flag_active(flags: &[String], unflags: &[String], needle: &str) -> bool { + flags.iter().any(|f| f == needle) && !unflags.iter().any(|f| f == needle) +} + +/// Decide the eh_frame policy for a build. Pure function over [`EhFrameInputs`]. +/// +/// Precedence (first matching rule wins): +/// +/// 1. `FBUILD_STRIP_EH_FRAME=1` → Strip +/// 2. `FBUILD_KEEP_EH_FRAME=1` → Preserve +/// 3. `is_debug_build` → Preserve +/// 4. user requested unwinding via `-fexceptions`, `-funwind-tables`, or +/// `-fasynchronous-unwind-tables` in build_flags (and not negated in +/// build_unflags) → Preserve +/// 5. ESP32 sdkconfig has any of: `panic_print_backtrace`, `panic_gdbstub`, +/// `debug_ocdaware`, `optimization_debug` → Preserve +/// 6. otherwise → Strip +pub fn decide(inputs: &EhFrameInputs<'_>) -> EhFramePolicy { + // 1. explicit strip override wins + if inputs.env_strip { + return EhFramePolicy::Strip; + } + // 2. explicit keep override + if inputs.env_keep { + return EhFramePolicy::Preserve; + } + // 3. debug build + if inputs.is_debug_build { + return EhFramePolicy::Preserve; + } + // 4. user requested unwinding via positive flags + let unwind_flags = [ + "-fexceptions", + "-funwind-tables", + "-fasynchronous-unwind-tables", + ]; + for needle in unwind_flags.iter() { + if flag_active(inputs.build_flags, inputs.build_unflags, needle) { + return EhFramePolicy::Preserve; + } + } + // 5. sdkconfig hints (ESP32 only) + if let Some(sdk) = inputs.sdkconfig { + if sdk.panic_print_backtrace + || sdk.panic_gdbstub + || sdk.debug_ocdaware + || sdk.optimization_debug + { + return EhFramePolicy::Preserve; + } + } + // 6. default for release-mode embedded targets + let _ = inputs.profile; // profile is currently informational only + EhFramePolicy::Strip +} + +#[cfg(test)] +mod tests { + use super::*; + + fn baseline_inputs<'a>() -> EhFrameInputs<'a> { + EhFrameInputs { + build_flags: &[], + build_unflags: &[], + sdkconfig: None, + profile: BuildProfile::Release, + is_debug_build: false, + env_keep: false, + env_strip: false, + } + } + + /// All-false sdkconfig for per-key isolation tests. `SdkConfigSummary::default()` + /// returns the ESP-IDF Arduino default (`panic_print_backtrace = true`), which + /// would otherwise short-circuit per-key Preserve assertions and break the + /// "all keys false → Strip" assertion. + fn empty_sdk() -> SdkConfigSummary { + SdkConfigSummary { + panic_print_backtrace: false, + panic_gdbstub: false, + debug_ocdaware: false, + optimization_debug: false, + } + } + + #[test] + fn baseline_strips() { + let inputs = baseline_inputs(); + assert_eq!(decide(&inputs), EhFramePolicy::Strip); + } + + #[test] + fn env_strip_beats_everything() { + let mut inputs = baseline_inputs(); + inputs.is_debug_build = true; + inputs.env_keep = true; + inputs.env_strip = true; + assert_eq!(decide(&inputs), EhFramePolicy::Strip); + } + + #[test] + fn env_keep_preserves() { + let mut inputs = baseline_inputs(); + inputs.env_keep = true; + assert_eq!(decide(&inputs), EhFramePolicy::Preserve); + } + + #[test] + fn debug_build_preserves() { + let mut inputs = baseline_inputs(); + inputs.is_debug_build = true; + assert_eq!(decide(&inputs), EhFramePolicy::Preserve); + } + + #[test] + fn fexceptions_flag_preserves() { + let flags = vec!["-fexceptions".to_string()]; + let mut inputs = baseline_inputs(); + inputs.build_flags = &flags; + assert_eq!(decide(&inputs), EhFramePolicy::Preserve); + } + + #[test] + fn fexceptions_negated_strips() { + let flags = vec!["-fexceptions".to_string()]; + let unflags = vec!["-fexceptions".to_string()]; + let mut inputs = baseline_inputs(); + inputs.build_flags = &flags; + inputs.build_unflags = &unflags; + assert_eq!(decide(&inputs), EhFramePolicy::Strip); + } + + #[test] + fn funwind_tables_flag_preserves() { + let flags = vec!["-funwind-tables".to_string()]; + let mut inputs = baseline_inputs(); + inputs.build_flags = &flags; + assert_eq!(decide(&inputs), EhFramePolicy::Preserve); + } + + #[test] + fn fasync_unwind_tables_flag_preserves() { + let flags = vec!["-fasynchronous-unwind-tables".to_string()]; + let mut inputs = baseline_inputs(); + inputs.build_flags = &flags; + assert_eq!(decide(&inputs), EhFramePolicy::Preserve); + } + + #[test] + fn sdkconfig_panic_print_backtrace_preserves() { + let sdk = SdkConfigSummary { + panic_print_backtrace: true, + ..empty_sdk() + }; + let mut inputs = baseline_inputs(); + inputs.sdkconfig = Some(&sdk); + assert_eq!(decide(&inputs), EhFramePolicy::Preserve); + } + + #[test] + fn sdkconfig_panic_gdbstub_preserves() { + let sdk = SdkConfigSummary { + panic_gdbstub: true, + ..empty_sdk() + }; + let mut inputs = baseline_inputs(); + inputs.sdkconfig = Some(&sdk); + assert_eq!(decide(&inputs), EhFramePolicy::Preserve); + } + + #[test] + fn sdkconfig_debug_ocdaware_preserves() { + let sdk = SdkConfigSummary { + debug_ocdaware: true, + ..empty_sdk() + }; + let mut inputs = baseline_inputs(); + inputs.sdkconfig = Some(&sdk); + assert_eq!(decide(&inputs), EhFramePolicy::Preserve); + } + + #[test] + fn sdkconfig_optimization_debug_preserves() { + let sdk = SdkConfigSummary { + optimization_debug: true, + ..empty_sdk() + }; + let mut inputs = baseline_inputs(); + inputs.sdkconfig = Some(&sdk); + assert_eq!(decide(&inputs), EhFramePolicy::Preserve); + } + + #[test] + fn sdkconfig_all_false_strips() { + let sdk = empty_sdk(); + let mut inputs = baseline_inputs(); + inputs.sdkconfig = Some(&sdk); + assert_eq!(decide(&inputs), EhFramePolicy::Strip); + } + + #[test] + fn sdkconfig_arduino_default_preserves() { + // ESP-IDF Arduino default has panic_print_backtrace=true, which is the + // most common case in the wild. Must Preserve to match user expectation + // of readable crash backtraces. + let sdk = SdkConfigSummary::default(); + let mut inputs = baseline_inputs(); + inputs.sdkconfig = Some(&sdk); + assert_eq!(decide(&inputs), EhFramePolicy::Preserve); + } + + #[test] + fn strip_flags_regression_guard() { + assert_eq!(STRIP_FLAGS.len(), 2); + assert!(STRIP_FLAGS.contains(&"-fno-asynchronous-unwind-tables")); + assert!(STRIP_FLAGS.contains(&"-fno-unwind-tables")); + } + + #[test] + fn default_policy_is_preserve() { + assert_eq!(EhFramePolicy::default(), EhFramePolicy::Preserve); + } +} diff --git a/crates/fbuild-build/src/eh_frame_policy_compute.rs b/crates/fbuild-build/src/eh_frame_policy_compute.rs new file mode 100644 index 00000000..48d37e97 --- /dev/null +++ b/crates/fbuild-build/src/eh_frame_policy_compute.rs @@ -0,0 +1,49 @@ +//! Orchestrator-side helper to compute the eh_frame [`EhFramePolicy`] for a +//! single build. Keeps env-var reads and `BuildContext` shape detection out +//! of the pure [`crate::eh_frame_policy::decide`] function, so the pure +//! decision stays trivially testable. +//! +//! See FastLED/fbuild#243 (ESP32) and FastLED/fbuild#244 (other GCC platforms). + +use crate::eh_frame_policy::{decide, EhFrameInputs, EhFramePolicy}; +use crate::pipeline::BuildContext; +use fbuild_config::sdkconfig::SdkConfigSummary; +use fbuild_core::BuildProfile; + +/// Read `FBUILD_KEEP_EH_FRAME` / `FBUILD_STRIP_EH_FRAME` from the process +/// environment, derive the debug-build signal from `BuildContext`, and feed +/// everything into the pure [`decide`] function. +/// +/// `sdkconfig` is `Some` only on ESP32 (where sdkconfig keys influence the +/// policy); other platforms pass `None`. +pub(crate) fn compute_eh_frame_policy( + ctx: &BuildContext, + profile: BuildProfile, + sdkconfig: Option<&SdkConfigSummary>, +) -> EhFramePolicy { + let env_keep = std::env::var("FBUILD_KEEP_EH_FRAME") + .map(|v| v == "1" || v.eq_ignore_ascii_case("true")) + .unwrap_or(false); + let env_strip = std::env::var("FBUILD_STRIP_EH_FRAME") + .map(|v| v == "1" || v.eq_ignore_ascii_case("true")) + .unwrap_or(false); + + // Debug detection: prefer the explicit -Og / -O0 user flag signal because + // the canonical PlatformIO `build_type = debug` overlay (which adds -Og) + // has already been applied to `ctx.user_flags` by `BuildContext::new`. + // -DNDEBUG is the conventional release-mode marker; its absence on a + // user-flags slate with neither -Og nor -O0 is inconclusive, so we only + // flag debug on the positive signal. + let is_debug_build = ctx.user_flags.iter().any(|f| f == "-Og" || f == "-O0"); + + let inputs = EhFrameInputs { + build_flags: &ctx.user_flags, + build_unflags: &ctx.build_unflags, + sdkconfig, + profile, + is_debug_build, + env_keep, + env_strip, + }; + decide(&inputs) +} diff --git a/crates/fbuild-build/src/esp32/esp32_compiler.rs b/crates/fbuild-build/src/esp32/esp32_compiler.rs index 1a322fad..312b38cc 100644 --- a/crates/fbuild-build/src/esp32/esp32_compiler.rs +++ b/crates/fbuild-build/src/esp32/esp32_compiler.rs @@ -10,6 +10,7 @@ use std::path::{Path, PathBuf}; use fbuild_core::{BuildProfile, Result}; use crate::compiler::{CompileResult, Compiler, CompilerBase}; +use crate::eh_frame_policy::EhFramePolicy; use super::mcu_config::Esp32McuConfig; @@ -31,6 +32,10 @@ pub struct Esp32Compiler { /// from `BuildContext::build_unflags`. Empty by default so existing /// callers don't need to change. See FastLED/fbuild#37. build_unflags: Vec, + /// Whether to strip eh_frame unwinding tables. Default `Preserve` so existing + /// callers see no behavior change; orchestrators set this via + /// [`Self::with_eh_frame_policy`]. See FastLED/fbuild#243. + eh_frame_policy: EhFramePolicy, } impl Esp32Compiler { @@ -93,6 +98,7 @@ impl Esp32Compiler { temp_dir, compiler_cache: crate::zccache::find_zccache().map(PathBuf::from), build_unflags: Vec::new(), + eh_frame_policy: EhFramePolicy::default(), } } @@ -106,6 +112,15 @@ impl Esp32Compiler { self } + /// Attach the eh_frame strip/preserve policy decided by the orchestrator. + /// Default `Preserve` keeps existing behavior; set `Strip` to drop the + /// unwind tables via `-fno-asynchronous-unwind-tables -fno-unwind-tables`. + /// See FastLED/fbuild#243. + pub fn with_eh_frame_policy(mut self, policy: EhFramePolicy) -> Self { + self.eh_frame_policy = policy; + self + } + /// Build common compiler flags from the MCU config. fn common_flags(&self) -> Vec { let mut flags = self.mcu_config.compiler_flags.common.clone(); @@ -123,6 +138,14 @@ impl Esp32Compiler { flags.extend(self.mcu_config.compat_define_flags()); flags.extend(self.base.build_define_flags()); + + if matches!(self.eh_frame_policy, EhFramePolicy::Strip) { + flags.extend( + crate::eh_frame_policy::STRIP_FLAGS + .iter() + .map(|s| s.to_string()), + ); + } flags } } @@ -346,6 +369,26 @@ mod tests { ); } + /// FastLED/fbuild#243: by default the compiler preserves eh_frame; the + /// STRIP_FLAGS must not leak into the effective compile line. + #[test] + fn cpp_flags_preserve_eh_frame_by_default() { + let compiler = test_compiler("esp32c6"); + let flags = compiler.cpp_flags(); + assert!(!flags.iter().any(|f| f == "-fno-asynchronous-unwind-tables")); + assert!(!flags.iter().any(|f| f == "-fno-unwind-tables")); + } + + /// FastLED/fbuild#243: when policy is Strip, both unwind-tables flags + /// must appear in the effective C++ flag list. + #[test] + fn cpp_flags_strip_eh_frame_when_policy_set() { + let compiler = test_compiler("esp32c6").with_eh_frame_policy(EhFramePolicy::Strip); + let flags = compiler.cpp_flags(); + assert!(flags.iter().any(|f| f == "-fno-asynchronous-unwind-tables")); + assert!(flags.iter().any(|f| f == "-fno-unwind-tables")); + } + #[test] fn test_mbedtls_compat_defines_in_flags() { let compiler = test_compiler("esp32c6"); diff --git a/crates/fbuild-build/src/esp32/orchestrator.rs b/crates/fbuild-build/src/esp32/orchestrator.rs index 2b092419..e130db44 100644 --- a/crates/fbuild-build/src/esp32/orchestrator.rs +++ b/crates/fbuild-build/src/esp32/orchestrator.rs @@ -66,6 +66,7 @@ struct Esp32FingerprintMetadata { flash_size: String, max_flash: Option, max_ram: Option, + eh_frame_policy: &'static str, } fn framework_failure_marker(build_dir: &Path, lib_name: &str) -> PathBuf { @@ -163,6 +164,17 @@ impl BuildOrchestrator for Esp32Orchestrator { // 1-2. Parse config, load board, setup build dirs, resolve src dir, collect flags let mut ctx = crate::pipeline::BuildContext::new_with_perf(params, Some(&mut perf))?; + // Compute eh_frame strip policy once per build (FastLED/fbuild#243). + // Reads sdkconfig from the project dir (ESP32 only) so panic-backtrace + // / gdbstub users automatically Preserve. + let sdkconfig = + fbuild_config::sdkconfig::SdkConfigSummary::from_project_dir(¶ms.project_dir); + let eh_frame_policy = crate::eh_frame_policy_compute::compute_eh_frame_policy( + &ctx, + params.profile, + Some(&sdkconfig), + ); + // 3. Load MCU config from embedded JSON let mut mcu_config = get_mcu_config(&ctx.board.mcu)?; @@ -243,6 +255,10 @@ impl BuildOrchestrator for Esp32Orchestrator { flash_size: flash_size.clone(), max_flash: ctx.board.max_flash, max_ram: ctx.board.max_ram, + eh_frame_policy: match eh_frame_policy { + crate::eh_frame_policy::EhFramePolicy::Strip => "strip", + crate::eh_frame_policy::EhFramePolicy::Preserve => "preserve", + }, })?; let (fast_elf, [fast_bin, fast_boot, fast_parts, fast_app0], fast_compile_db) = expected_fast_path_artifacts( @@ -446,7 +462,8 @@ impl BuildOrchestrator for Esp32Orchestrator { params.verbose, build_dir.join("tmp"), ) - .with_build_unflags(ctx.build_unflags.clone()); + .with_build_unflags(ctx.build_unflags.clone()) + .with_eh_frame_policy(eh_frame_policy); // Apply user build_flags to library compilation (matching PlatformIO behavior). // User flags like -std=gnu++2a replace the MCU config's -std=gnu++2b. let c_flags = apply_overlay_flags(&temp_compiler.c_flags(), &user_overlay, "dummy.c"); @@ -511,7 +528,8 @@ impl BuildOrchestrator for Esp32Orchestrator { params.verbose, build_dir.join("tmp"), ) - .with_build_unflags(ctx.build_unflags.clone()); + .with_build_unflags(ctx.build_unflags.clone()) + .with_eh_frame_policy(eh_frame_policy); let p_c_flags = apply_overlay_flags(&p_compiler.c_flags(), &src_overlay, "dummy.c"); let p_cpp_flags = apply_overlay_flags(&p_compiler.cpp_flags(), &src_overlay, "dummy.cpp"); @@ -598,7 +616,8 @@ impl BuildOrchestrator for Esp32Orchestrator { params.verbose, build_dir.join("tmp"), ) - .with_build_unflags(ctx.build_unflags.clone()); + .with_build_unflags(ctx.build_unflags.clone()) + .with_eh_frame_policy(eh_frame_policy); let fw_c_flags = apply_overlay_flags(&fw_compiler.c_flags(), &user_overlay, "dummy.c"); let fw_cpp_flags = @@ -806,7 +825,8 @@ impl BuildOrchestrator for Esp32Orchestrator { params.verbose, build_dir.join("tmp"), ) - .with_build_unflags(ctx.build_unflags.clone()); + .with_build_unflags(ctx.build_unflags.clone()) + .with_eh_frame_policy(eh_frame_policy); let jobs = crate::parallel::effective_jobs(params.jobs); tracing::info!("parallel compilation: {} jobs", jobs); diff --git a/crates/fbuild-build/src/esp8266/esp8266_compiler.rs b/crates/fbuild-build/src/esp8266/esp8266_compiler.rs index 492c9e25..6b4a7b54 100644 --- a/crates/fbuild-build/src/esp8266/esp8266_compiler.rs +++ b/crates/fbuild-build/src/esp8266/esp8266_compiler.rs @@ -10,6 +10,7 @@ use fbuild_core::{BuildProfile, Result}; use super::mcu_config::Esp8266McuConfig; use crate::compiler::{CompileResult, Compiler, CompilerBase, McuConfig as _}; +use crate::eh_frame_policy::EhFramePolicy; /// ESP8266-specific compiler using Xtensa LX106 GCC. pub struct Esp8266Compiler { @@ -21,6 +22,10 @@ pub struct Esp8266Compiler { temp_dir: PathBuf, /// PlatformIO `build_unflags`. See FastLED/fbuild#37. build_unflags: Vec, + /// Whether to strip eh_frame unwinding tables. Default `Preserve` so existing + /// callers see no behavior change; orchestrators set this via + /// [`Self::with_eh_frame_policy`]. See FastLED/fbuild#244. + eh_frame_policy: EhFramePolicy, } impl Esp8266Compiler { @@ -49,6 +54,7 @@ impl Esp8266Compiler { profile, temp_dir: fbuild_core::response_file::windows_temp_dir(), build_unflags: Vec::new(), + eh_frame_policy: EhFramePolicy::default(), } } @@ -58,6 +64,13 @@ impl Esp8266Compiler { self } + /// Attach the eh_frame strip/preserve policy decided by the orchestrator. + /// Default `Preserve` keeps existing behavior. See FastLED/fbuild#244. + pub fn with_eh_frame_policy(mut self, policy: EhFramePolicy) -> Self { + self.eh_frame_policy = policy; + self + } + /// Build common compiler flags from the MCU config. fn common_flags(&self) -> Vec { let mut flags = self.mcu_config.compiler_flags.common.clone(); @@ -69,6 +82,14 @@ impl Esp8266Compiler { flags.extend(self.base.build_define_flags()); flags.extend(self.base.build_include_flags()); + + if matches!(self.eh_frame_policy, EhFramePolicy::Strip) { + flags.extend( + crate::eh_frame_policy::STRIP_FLAGS + .iter() + .map(|s| s.to_string()), + ); + } flags } @@ -226,4 +247,22 @@ mod tests { let release = config.get_profile("release").unwrap(); assert!(release.compile_flags.contains(&"-Os".to_string())); } + + /// FastLED/fbuild#244: default policy must not leak STRIP_FLAGS. + #[test] + fn cpp_flags_preserve_eh_frame_by_default() { + let compiler = test_compiler(); + let flags = compiler.cpp_flags(); + assert!(!flags.iter().any(|f| f == "-fno-asynchronous-unwind-tables")); + assert!(!flags.iter().any(|f| f == "-fno-unwind-tables")); + } + + /// FastLED/fbuild#244: Strip policy must inject both unwind-table flags. + #[test] + fn cpp_flags_strip_eh_frame_when_policy_set() { + let compiler = test_compiler().with_eh_frame_policy(EhFramePolicy::Strip); + let flags = compiler.cpp_flags(); + assert!(flags.iter().any(|f| f == "-fno-asynchronous-unwind-tables")); + assert!(flags.iter().any(|f| f == "-fno-unwind-tables")); + } } diff --git a/crates/fbuild-build/src/esp8266/orchestrator.rs b/crates/fbuild-build/src/esp8266/orchestrator.rs index 8d3dc8df..bd1eb06e 100644 --- a/crates/fbuild-build/src/esp8266/orchestrator.rs +++ b/crates/fbuild-build/src/esp8266/orchestrator.rs @@ -39,6 +39,11 @@ impl BuildOrchestrator for Esp8266Orchestrator { // 1-2. Parse config, load board, setup build dirs, resolve src dir, collect flags let mut ctx = pipeline::BuildContext::new(params)?; + // Compute eh_frame strip policy once per build (FastLED/fbuild#244). + // No sdkconfig on ESP8266. + let eh_frame_policy = + crate::eh_frame_policy_compute::compute_eh_frame_policy(&ctx, params.profile, None); + // 3. Ensure toolchain let toolchain = fbuild_packages::toolchain::Esp8266Toolchain::new(¶ms.project_dir); let _toolchain_dir = fbuild_packages::Package::ensure_installed(&toolchain)?; @@ -137,7 +142,8 @@ impl BuildOrchestrator for Esp8266Orchestrator { params.profile, params.verbose, ) - .with_build_unflags(ctx.build_unflags.clone()); + .with_build_unflags(ctx.build_unflags.clone()) + .with_eh_frame_policy(eh_frame_policy); // Resolve linker script from board config let ldscript = ctx diff --git a/crates/fbuild-build/src/generic_arm/arm_compiler.rs b/crates/fbuild-build/src/generic_arm/arm_compiler.rs index d8d9ca5a..e80309fc 100644 --- a/crates/fbuild-build/src/generic_arm/arm_compiler.rs +++ b/crates/fbuild-build/src/generic_arm/arm_compiler.rs @@ -10,6 +10,7 @@ use fbuild_core::{BuildProfile, Result}; use super::mcu_config::ArmMcuConfig; use crate::compiler::{CompileResult, Compiler, CompilerBase}; +use crate::eh_frame_policy::EhFramePolicy; /// Generic ARM compiler using arm-none-eabi-gcc and arm-none-eabi-g++. pub struct ArmCompiler { @@ -22,6 +23,10 @@ pub struct ArmCompiler { /// PlatformIO `build_unflags` to strip from the effective compile /// line. See FastLED/fbuild#37. build_unflags: Vec, + /// Whether to strip eh_frame unwinding tables. Default `Preserve` so existing + /// callers see no behavior change; orchestrators set this via + /// [`Self::with_eh_frame_policy`]. See FastLED/fbuild#244. + eh_frame_policy: EhFramePolicy, } impl ArmCompiler { @@ -51,6 +56,7 @@ impl ArmCompiler { profile, temp_dir: fbuild_core::response_file::windows_temp_dir(), build_unflags: Vec::new(), + eh_frame_policy: EhFramePolicy::default(), } } @@ -60,6 +66,13 @@ impl ArmCompiler { self } + /// Attach the eh_frame strip/preserve policy decided by the orchestrator. + /// Default `Preserve` keeps existing behavior. See FastLED/fbuild#244. + pub fn with_eh_frame_policy(mut self, policy: EhFramePolicy) -> Self { + self.eh_frame_policy = policy; + self + } + /// Build the common ARM Cortex-M compiler flags. fn common_flags(&self) -> Vec { let mut flags = Vec::new(); @@ -72,6 +85,14 @@ impl ArmCompiler { flags.extend(self.base.build_define_flags()); flags.extend(self.base.build_include_flags()); + + if matches!(self.eh_frame_policy, EhFramePolicy::Strip) { + flags.extend( + crate::eh_frame_policy::STRIP_FLAGS + .iter() + .map(|s| s.to_string()), + ); + } flags } } @@ -214,4 +235,22 @@ mod tests { assert!(flags.contains(&"-fno-rtti".to_string())); assert!(flags.contains(&"-fno-threadsafe-statics".to_string())); } + + /// FastLED/fbuild#244: default policy must not leak STRIP_FLAGS. + #[test] + fn cpp_flags_preserve_eh_frame_by_default() { + let compiler = test_compiler(); + let flags = compiler.cpp_flags(); + assert!(!flags.iter().any(|f| f == "-fno-asynchronous-unwind-tables")); + assert!(!flags.iter().any(|f| f == "-fno-unwind-tables")); + } + + /// FastLED/fbuild#244: Strip policy must inject both unwind-table flags. + #[test] + fn cpp_flags_strip_eh_frame_when_policy_set() { + let compiler = test_compiler().with_eh_frame_policy(EhFramePolicy::Strip); + let flags = compiler.cpp_flags(); + assert!(flags.iter().any(|f| f == "-fno-asynchronous-unwind-tables")); + assert!(flags.iter().any(|f| f == "-fno-unwind-tables")); + } } diff --git a/crates/fbuild-build/src/lib.rs b/crates/fbuild-build/src/lib.rs index c7da3332..32dd28e1 100644 --- a/crates/fbuild-build/src/lib.rs +++ b/crates/fbuild-build/src/lib.rs @@ -12,6 +12,8 @@ pub mod ch32v; pub mod compile_database; pub mod compile_many; pub mod compiler; +pub mod eh_frame_policy; +pub(crate) mod eh_frame_policy_compute; pub mod esp32; pub mod esp8266; pub mod flag_overlay; diff --git a/crates/fbuild-build/src/rp2040/orchestrator.rs b/crates/fbuild-build/src/rp2040/orchestrator.rs index b58eca7e..a5653837 100644 --- a/crates/fbuild-build/src/rp2040/orchestrator.rs +++ b/crates/fbuild-build/src/rp2040/orchestrator.rs @@ -48,6 +48,7 @@ struct Rp2040FingerprintMetadata { platform: String, max_flash: Option, max_ram: Option, + eh_frame_policy: &'static str, } fn profile_label(profile: fbuild_core::BuildProfile) -> &'static str { @@ -69,6 +70,10 @@ impl BuildOrchestrator for Rp2040Orchestrator { // 1-2. Parse config, load board, setup build dirs, resolve src dir, collect flags let mut ctx = pipeline::BuildContext::new(params)?; + // Compute eh_frame strip policy once per build (FastLED/fbuild#244). + let eh_frame_policy = + crate::eh_frame_policy_compute::compute_eh_frame_policy(&ctx, params.profile, None); + // 3. Ensure the arduino-pico-matched pqt-gcc toolchain let toolchain = fbuild_packages::toolchain::Rp2040PqtToolchain::new(¶ms.project_dir); let toolchain_dir = fbuild_packages::Package::ensure_installed(&toolchain)?; @@ -111,6 +116,10 @@ impl BuildOrchestrator for Rp2040Orchestrator { platform: "raspberrypi".to_string(), max_flash: ctx.board.max_flash, max_ram: ctx.board.max_ram, + eh_frame_policy: match eh_frame_policy { + crate::eh_frame_policy::EhFramePolicy::Strip => "strip", + crate::eh_frame_policy::EhFramePolicy::Preserve => "preserve", + }, })?; let (fast_elf, [fast_bin], fast_compile_db) = expected_fast_path_artifacts(&build_dir, ¶ms.project_dir, ["firmware.bin"]); @@ -244,7 +253,8 @@ impl BuildOrchestrator for Rp2040Orchestrator { params.profile, params.verbose, ) - .with_build_unflags(ctx.build_unflags.clone()); + .with_build_unflags(ctx.build_unflags.clone()) + .with_eh_frame_policy(eh_frame_policy); // 7. Generate the linker script the same way upstream does. let linker_script = diff --git a/crates/fbuild-build/src/stm32/orchestrator.rs b/crates/fbuild-build/src/stm32/orchestrator.rs index 00db0008..d1ad9e39 100644 --- a/crates/fbuild-build/src/stm32/orchestrator.rs +++ b/crates/fbuild-build/src/stm32/orchestrator.rs @@ -43,6 +43,10 @@ impl BuildOrchestrator for Stm32Orchestrator { // 1-2. Parse config, load board, setup build dirs, resolve src dir, collect flags let mut ctx = pipeline::BuildContext::new(params)?; + // Compute eh_frame strip policy once per build (FastLED/fbuild#244). + let eh_frame_policy = + crate::eh_frame_policy_compute::compute_eh_frame_policy(&ctx, params.profile, None); + // 3. Ensure ARM GCC toolchain let toolchain = fbuild_packages::toolchain::ArmToolchain::new(¶ms.project_dir); let toolchain_dir = fbuild_packages::Package::ensure_installed(&toolchain)?; @@ -247,7 +251,8 @@ impl BuildOrchestrator for Stm32Orchestrator { params.profile, params.verbose, ) - .with_build_unflags(ctx.build_unflags.clone()); + .with_build_unflags(ctx.build_unflags.clone()) + .with_eh_frame_policy(eh_frame_policy); // 7. Create linker (linker script from variant dir) let linker_script = match ctx.board.ldscript.as_deref() { @@ -310,6 +315,10 @@ fn build_arduino_mbed_stm32( toolchain: &fbuild_packages::toolchain::ArmToolchain, start: Instant, ) -> Result { + // Compute eh_frame strip policy once per build (FastLED/fbuild#244). + let eh_frame_policy = + crate::eh_frame_policy_compute::compute_eh_frame_policy(&ctx, params.profile, None); + let framework = fbuild_packages::library::ArduinoMbedCore::new(¶ms.project_dir); let framework_dir = fbuild_packages::Package::ensure_installed(&framework)?; tracing::info!("Arduino mbed core at {}", framework_dir.display()); @@ -394,7 +403,8 @@ fn build_arduino_mbed_stm32( params.profile, params.verbose, ) - .with_build_unflags(ctx.build_unflags.clone()); + .with_build_unflags(ctx.build_unflags.clone()) + .with_eh_frame_policy(eh_frame_policy); let linker = ArmLinker::new( toolchain.get_gcc_path(), diff --git a/crates/fbuild-build/src/teensy/orchestrator.rs b/crates/fbuild-build/src/teensy/orchestrator.rs index 211fb893..16d623b5 100644 --- a/crates/fbuild-build/src/teensy/orchestrator.rs +++ b/crates/fbuild-build/src/teensy/orchestrator.rs @@ -52,6 +52,7 @@ struct TeensyFingerprintMetadata { platform: String, max_flash: Option, max_ram: Option, + eh_frame_policy: &'static str, } fn profile_label(profile: fbuild_core::BuildProfile) -> &'static str { @@ -73,6 +74,11 @@ impl BuildOrchestrator for TeensyOrchestrator { // 1-2. Parse config, load board, setup build dirs, resolve src dir, collect flags let mut ctx = pipeline::BuildContext::new(params)?; + // Compute eh_frame strip policy once per build (FastLED/fbuild#244). + // No sdkconfig on Teensy. + let eh_frame_policy = + crate::eh_frame_policy_compute::compute_eh_frame_policy(&ctx, params.profile, None); + // Need board_id for linker script lookup later let env_config = ctx.config.get_env_config(¶ms.env_name)?; let board_id = env_config.get("board").ok_or_else(|| { @@ -112,6 +118,10 @@ impl BuildOrchestrator for TeensyOrchestrator { platform: "teensy".to_string(), max_flash: ctx.board.max_flash, max_ram: ctx.board.max_ram, + eh_frame_policy: match eh_frame_policy { + crate::eh_frame_policy::EhFramePolicy::Strip => "strip", + crate::eh_frame_policy::EhFramePolicy::Preserve => "preserve", + }, })?; let (fast_elf, [fast_hex], fast_compile_db) = expected_fast_path_artifacts(build_dir, ¶ms.project_dir, ["firmware.hex"]); @@ -226,7 +236,8 @@ impl BuildOrchestrator for TeensyOrchestrator { params.profile, params.verbose, ) - .with_build_unflags(ctx.build_unflags.clone()); + .with_build_unflags(ctx.build_unflags.clone()) + .with_eh_frame_policy(eh_frame_policy); // 7. Create linker (with linker script from board config) let linker_scripts = match ctx.board.ldscript.as_deref() { diff --git a/crates/fbuild-build/src/teensy/teensy_compiler.rs b/crates/fbuild-build/src/teensy/teensy_compiler.rs index 215456ac..4b485975 100644 --- a/crates/fbuild-build/src/teensy/teensy_compiler.rs +++ b/crates/fbuild-build/src/teensy/teensy_compiler.rs @@ -10,6 +10,7 @@ use fbuild_core::{BuildProfile, Result}; use super::mcu_config::TeensyMcuConfig; use crate::compiler::{CompileResult, Compiler, CompilerBase}; +use crate::eh_frame_policy::EhFramePolicy; /// Teensy-specific compiler using arm-none-eabi-gcc and arm-none-eabi-g++. pub struct TeensyCompiler { @@ -22,6 +23,10 @@ pub struct TeensyCompiler { /// PlatformIO `build_unflags` to strip from the effective compile /// line. See FastLED/fbuild#37. build_unflags: Vec, + /// Whether to strip eh_frame unwinding tables. Default `Preserve` so existing + /// callers see no behavior change; orchestrators set this via + /// [`Self::with_eh_frame_policy`]. See FastLED/fbuild#244. + eh_frame_policy: EhFramePolicy, } impl TeensyCompiler { @@ -51,6 +56,7 @@ impl TeensyCompiler { profile, temp_dir: fbuild_core::response_file::windows_temp_dir(), build_unflags: Vec::new(), + eh_frame_policy: EhFramePolicy::default(), } } @@ -61,6 +67,13 @@ impl TeensyCompiler { self } + /// Attach the eh_frame strip/preserve policy decided by the orchestrator. + /// Default `Preserve` keeps existing behavior. See FastLED/fbuild#244. + pub fn with_eh_frame_policy(mut self, policy: EhFramePolicy) -> Self { + self.eh_frame_policy = policy; + self + } + /// Build the common ARM Cortex-M7 compiler flags. fn common_flags(&self) -> Vec { let mut flags = Vec::new(); @@ -73,6 +86,14 @@ impl TeensyCompiler { flags.extend(self.base.build_define_flags()); flags.extend(self.base.build_include_flags()); + + if matches!(self.eh_frame_policy, EhFramePolicy::Strip) { + flags.extend( + crate::eh_frame_policy::STRIP_FLAGS + .iter() + .map(|s| s.to_string()), + ); + } flags } } @@ -202,4 +223,22 @@ mod tests { assert!(flags.contains(&"-felide-constructors".to_string())); assert!(flags.contains(&"-fno-threadsafe-statics".to_string())); } + + /// FastLED/fbuild#244: default policy must not leak STRIP_FLAGS. + #[test] + fn cpp_flags_preserve_eh_frame_by_default() { + let compiler = test_compiler(); + let flags = compiler.cpp_flags(); + assert!(!flags.iter().any(|f| f == "-fno-asynchronous-unwind-tables")); + assert!(!flags.iter().any(|f| f == "-fno-unwind-tables")); + } + + /// FastLED/fbuild#244: Strip policy must inject both unwind-table flags. + #[test] + fn cpp_flags_strip_eh_frame_when_policy_set() { + let compiler = test_compiler().with_eh_frame_policy(EhFramePolicy::Strip); + let flags = compiler.cpp_flags(); + assert!(flags.iter().any(|f| f == "-fno-asynchronous-unwind-tables")); + assert!(flags.iter().any(|f| f == "-fno-unwind-tables")); + } } diff --git a/crates/fbuild-build/tests/eh_frame_strip_esp32.rs b/crates/fbuild-build/tests/eh_frame_strip_esp32.rs new file mode 100644 index 00000000..27b58d83 --- /dev/null +++ b/crates/fbuild-build/tests/eh_frame_strip_esp32.rs @@ -0,0 +1,160 @@ +//! Integration test for FastLED/fbuild#243: stripping eh_frame on ESP32 +//! should drop firmware.bin by at least 150 KB and shrink the +//! `.eh_frame` ELF section by at least 50 KB on a stock Blink build. +//! +//! `#[ignore]`-marked because it requires the ESP32 toolchain (~hundreds +//! of MB on first run) and is not headless-CI-friendly. Run with: +//! +//! ``` +//! uv run soldr cargo test -p fbuild-build --test eh_frame_strip_esp32 -- --ignored +//! ``` +//! +//! The orchestrator invocation pattern mirrors +//! `crates/fbuild-build/tests/esp32_build.rs::build_esp32dev_blink`. + +use std::fs; +use std::path::Path; + +use fbuild_build::{BuildOrchestrator, BuildParams}; +use fbuild_core::BuildProfile; + +fn make_params(project_dir: &Path) -> BuildParams { + let build_dir = project_dir.join(".fbuild/build"); + BuildParams { + project_dir: project_dir.to_path_buf(), + env_name: "esp32dev".to_string(), + clean: true, + profile: BuildProfile::Release, + build_dir, + verbose: false, + jobs: None, + generate_compiledb: false, + compiledb_only: false, + log_sender: None, + symbol_analysis: false, + symbol_analysis_path: None, + no_timestamp: false, + src_dir: None, + pio_env: Default::default(), + extra_build_flags: Vec::new(), + watch_set_cache: None, + } +} + +fn write_blink_project(project_dir: &std::path::Path) { + fs::write( + project_dir.join("platformio.ini"), + "[env:esp32dev]\nplatform = espressif32\nboard = esp32dev\nframework = arduino\n", + ) + .unwrap(); + + let src_dir = project_dir.join("src"); + fs::create_dir_all(&src_dir).unwrap(); + fs::write( + src_dir.join("blink.cpp"), + "\ +#include + +void setup() { + pinMode(2, OUTPUT); +} + +void loop() { + digitalWrite(2, HIGH); + delay(1000); + digitalWrite(2, LOW); + delay(1000); +} +", + ) + .unwrap(); +} + +#[test] +#[ignore = "downloads ESP32 toolchain (~hundreds of MB)"] +fn eh_frame_strip_drops_firmware_at_least_150kb() { + // Use two separate tempdirs so .fbuild/build/... paths don't collide. + let preserve_tmp = tempfile::TempDir::new().unwrap(); + let strip_tmp = tempfile::TempDir::new().unwrap(); + let preserve_dir = preserve_tmp.path().to_path_buf(); + let strip_dir = strip_tmp.path().to_path_buf(); + + write_blink_project(&preserve_dir); + write_blink_project(&strip_dir); + + let orchestrator = fbuild_build::esp32::orchestrator::Esp32Orchestrator; + + // --- Build 1: preserve eh_frame --- + // SAFETY: tests in the same process may share env vars; we restore them + // after each build. The test runner doesn't parallelise this binary's + // test cases at the env-var level (cargo serializes the env touches in + // this single-test crate), but this is still best-effort hygiene. + std::env::set_var("FBUILD_KEEP_EH_FRAME", "1"); + std::env::remove_var("FBUILD_STRIP_EH_FRAME"); + let preserve_params = make_params(&preserve_dir); + let preserve_result = orchestrator + .build(&preserve_params) + .expect("preserve build should succeed"); + assert!( + preserve_result.success, + "preserve build should report success" + ); + let preserve_elf = preserve_result + .elf_path + .clone() + .expect("preserve build should produce ELF path"); + let preserve_firmware_bin = preserve_dir.join(".fbuild/build/esp32dev/release/firmware.bin"); + std::env::remove_var("FBUILD_KEEP_EH_FRAME"); + + // --- Build 2: strip eh_frame --- + std::env::set_var("FBUILD_STRIP_EH_FRAME", "1"); + std::env::remove_var("FBUILD_KEEP_EH_FRAME"); + let strip_params = make_params(&strip_dir); + let strip_result = orchestrator + .build(&strip_params) + .expect("strip build should succeed"); + assert!(strip_result.success, "strip build should report success"); + let strip_elf = strip_result + .elf_path + .clone() + .expect("strip build should produce ELF path"); + let strip_firmware_bin = strip_dir.join(".fbuild/build/esp32dev/release/firmware.bin"); + std::env::remove_var("FBUILD_STRIP_EH_FRAME"); + + // --- firmware.bin delta --- + let preserve_bin = fs::metadata(&preserve_firmware_bin) + .expect("preserve firmware.bin should exist") + .len(); + let strip_bin = fs::metadata(&strip_firmware_bin) + .expect("strip firmware.bin should exist") + .len(); + let delta = preserve_bin.saturating_sub(strip_bin); + assert!( + delta >= 150 * 1024, + "expected >=150 KB firmware.bin savings, got {} B (preserve={}, strip={})", + delta, + preserve_bin, + strip_bin, + ); + + // --- ELF .eh_frame section delta --- + use object::{Object, ObjectSection}; + let preserve_elf_bytes = fs::read(&preserve_elf).expect("read preserve ELF"); + let strip_elf_bytes = fs::read(&strip_elf).expect("read strip ELF"); + let preserve_obj = object::File::parse(&*preserve_elf_bytes).expect("parse preserve ELF"); + let strip_obj = object::File::parse(&*strip_elf_bytes).expect("parse strip ELF"); + let preserve_eh = preserve_obj + .section_by_name(".eh_frame") + .map_or(0, |s| s.size()); + let strip_eh = strip_obj + .section_by_name(".eh_frame") + .map_or(0, |s| s.size()); + let eh_delta = preserve_eh.saturating_sub(strip_eh); + assert!( + eh_delta >= 50 * 1024, + ".eh_frame section delta must be >=50 KB (got {} B; preserve={} strip={})", + eh_delta, + preserve_eh, + strip_eh, + ); +} diff --git a/crates/fbuild-config/src/lib.rs b/crates/fbuild-config/src/lib.rs index 9e6c8e97..1e02d01c 100644 --- a/crates/fbuild-config/src/lib.rs +++ b/crates/fbuild-config/src/lib.rs @@ -10,6 +10,7 @@ pub mod board; pub mod ini_parser; pub mod mcu; pub mod pio_env; +pub mod sdkconfig; pub use board::{BoardConfig, DebugToolMeta, Esp32QemuPsramConfig}; pub use ini_parser::PlatformIOConfig; diff --git a/crates/fbuild-config/src/sdkconfig.rs b/crates/fbuild-config/src/sdkconfig.rs new file mode 100644 index 00000000..a82fe1d6 --- /dev/null +++ b/crates/fbuild-config/src/sdkconfig.rs @@ -0,0 +1,258 @@ +//! ESP-IDF `sdkconfig` / `sdkconfig.defaults` parser surfacing the keys +//! fbuild's policy code reads. Not a general-purpose sdkconfig parser — +//! we only need a fixed list of boolean keys. +//! +//! See FastLED/fbuild#243 (ESP32 eh_frame strip decision needs these). + +use std::path::Path; + +/// Summary of sdkconfig keys consumed by build-time policies. +/// +/// Defaults match ESP-IDF Arduino's defaults: panic backtrace is ON, +/// gdbstub is OFF, ocdaware is OFF, optimization debug is OFF. So +/// `SdkConfigSummary::default() == arduino_default()` and represents the +/// safe assumption for a project that hasn't customized its sdkconfig. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub struct SdkConfigSummary { + /// `CONFIG_ESP_SYSTEM_PANIC_PRINT_BACKTRACE=y` + pub panic_print_backtrace: bool, + /// `CONFIG_ESP_SYSTEM_PANIC_GDBSTUB=y` + pub panic_gdbstub: bool, + /// `CONFIG_ESP_DEBUG_OCDAWARE=y` + pub debug_ocdaware: bool, + /// `CONFIG_COMPILER_OPTIMIZATION_DEBUG=y` + pub optimization_debug: bool, +} + +impl Default for SdkConfigSummary { + fn default() -> Self { + Self::arduino_default() + } +} + +impl SdkConfigSummary { + /// ESP-IDF Arduino defaults (`panic_print_backtrace = true`, others false). + pub fn arduino_default() -> Self { + Self { + panic_print_backtrace: true, + panic_gdbstub: false, + debug_ocdaware: false, + optimization_debug: false, + } + } + + /// Parse a single sdkconfig file's contents. Recognized line forms: + /// - `CONFIG_FOO=y` → true + /// - `CONFIG_FOO=n` → false (rare; usually `# CONFIG_FOO is not set`) + /// - `# CONFIG_FOO is not set` → false + /// - other forms (`CONFIG_FOO=123`, `CONFIG_FOO="bar"`, blank lines, + /// comments) → ignored + /// + /// Keys not present default to the same values as `arduino_default()`. + pub fn parse(content: &str) -> Self { + let mut summary = Self::arduino_default(); + for line in content.lines() { + let line = line.trim(); + if line.is_empty() { + continue; + } + // Handle "# CONFIG_FOO is not set" form + if let Some(rest) = line.strip_prefix("# ") { + if let Some(key) = rest.strip_suffix(" is not set") { + Self::apply_bool(&mut summary, key, false); + } + continue; + } + if line.starts_with('#') { + continue; + } + // KEY=VALUE form + let Some((key, value)) = line.split_once('=') else { + continue; + }; + let value = value.trim(); + // Only the y/n forms matter to us. + let bool_val = match value { + "y" => Some(true), + "n" => Some(false), + _ => None, + }; + if let Some(v) = bool_val { + Self::apply_bool(&mut summary, key, v); + } + } + summary + } + + fn apply_bool(s: &mut Self, key: &str, value: bool) { + match key { + "CONFIG_ESP_SYSTEM_PANIC_PRINT_BACKTRACE" => s.panic_print_backtrace = value, + "CONFIG_ESP_SYSTEM_PANIC_GDBSTUB" => s.panic_gdbstub = value, + "CONFIG_ESP_DEBUG_OCDAWARE" => s.debug_ocdaware = value, + "CONFIG_COMPILER_OPTIMIZATION_DEBUG" => s.optimization_debug = value, + _ => {} + } + } + + /// Read sdkconfig / sdkconfig.defaults from the given project dir. + /// Precedence: `sdkconfig` (active config) wins over `sdkconfig.defaults`. + /// If neither file exists, returns `arduino_default()`. + pub fn from_project_dir(project_dir: &Path) -> Self { + let active = project_dir.join("sdkconfig"); + if active.is_file() { + if let Ok(s) = std::fs::read_to_string(&active) { + return Self::parse(&s); + } + } + let defaults = project_dir.join("sdkconfig.defaults"); + if defaults.is_file() { + if let Ok(s) = std::fs::read_to_string(&defaults) { + return Self::parse(&s); + } + } + Self::arduino_default() + } +} + +#[cfg(test)] +mod tests { + use super::*; + use std::fs; + use tempfile::TempDir; + + #[test] + fn arduino_default_values() { + let d = SdkConfigSummary::arduino_default(); + assert!(d.panic_print_backtrace); + assert!(!d.panic_gdbstub); + assert!(!d.debug_ocdaware); + assert!(!d.optimization_debug); + } + + #[test] + fn parse_empty_returns_default() { + assert_eq!( + SdkConfigSummary::parse(""), + SdkConfigSummary::arduino_default() + ); + } + + #[test] + fn parse_panic_print_backtrace_y() { + let s = SdkConfigSummary::parse("CONFIG_ESP_SYSTEM_PANIC_PRINT_BACKTRACE=y\n"); + assert!(s.panic_print_backtrace); + } + + #[test] + fn parse_panic_print_backtrace_not_set() { + let s = SdkConfigSummary::parse("# CONFIG_ESP_SYSTEM_PANIC_PRINT_BACKTRACE is not set\n"); + assert!(!s.panic_print_backtrace); + } + + #[test] + fn parse_panic_gdbstub_y() { + let s = SdkConfigSummary::parse("CONFIG_ESP_SYSTEM_PANIC_GDBSTUB=y\n"); + assert!(s.panic_gdbstub); + } + + #[test] + fn parse_debug_ocdaware_y() { + let s = SdkConfigSummary::parse("CONFIG_ESP_DEBUG_OCDAWARE=y\n"); + assert!(s.debug_ocdaware); + } + + #[test] + fn parse_optimization_debug_y() { + let s = SdkConfigSummary::parse("CONFIG_COMPILER_OPTIMIZATION_DEBUG=y\n"); + assert!(s.optimization_debug); + } + + #[test] + fn parse_mixed_only_flips_specified_key() { + // Start from arduino default; enable only gdbstub. + let s = SdkConfigSummary::parse("CONFIG_ESP_SYSTEM_PANIC_GDBSTUB=y\n"); + let baseline = SdkConfigSummary::arduino_default(); + assert_eq!(s.panic_print_backtrace, baseline.panic_print_backtrace); + assert!(s.panic_gdbstub); + assert_eq!(s.debug_ocdaware, baseline.debug_ocdaware); + assert_eq!(s.optimization_debug, baseline.optimization_debug); + } + + #[test] + fn parse_ignores_unknown_and_non_bool_forms() { + let content = "\ +# This is a comment +# Another comment + +CONFIG_FOO=123 +CONFIG_BAR=\"hello\" +CONFIG_UNKNOWN_KEY=y +not_a_kv_line +"; + let s = SdkConfigSummary::parse(content); + assert_eq!(s, SdkConfigSummary::arduino_default()); + } + + #[test] + fn parse_multiple_keys() { + let content = "\ +CONFIG_ESP_SYSTEM_PANIC_PRINT_BACKTRACE=y +CONFIG_ESP_SYSTEM_PANIC_GDBSTUB=y +CONFIG_ESP_DEBUG_OCDAWARE=y +CONFIG_COMPILER_OPTIMIZATION_DEBUG=y +"; + let s = SdkConfigSummary::parse(content); + assert!(s.panic_print_backtrace); + assert!(s.panic_gdbstub); + assert!(s.debug_ocdaware); + assert!(s.optimization_debug); + } + + #[test] + fn from_project_dir_empty_returns_default() { + let tmp = TempDir::new().unwrap(); + let s = SdkConfigSummary::from_project_dir(tmp.path()); + assert_eq!(s, SdkConfigSummary::arduino_default()); + } + + #[test] + fn from_project_dir_defaults_only() { + let tmp = TempDir::new().unwrap(); + fs::write( + tmp.path().join("sdkconfig.defaults"), + "CONFIG_ESP_SYSTEM_PANIC_GDBSTUB=y\n", + ) + .unwrap(); + let s = SdkConfigSummary::from_project_dir(tmp.path()); + assert!(s.panic_gdbstub); + } + + #[test] + fn from_project_dir_active_wins_over_defaults() { + let tmp = TempDir::new().unwrap(); + // defaults says gdbstub=y + fs::write( + tmp.path().join("sdkconfig.defaults"), + "CONFIG_ESP_SYSTEM_PANIC_GDBSTUB=y\n", + ) + .unwrap(); + // active sdkconfig overrides: ocdaware=y, gdbstub not set + fs::write( + tmp.path().join("sdkconfig"), + "CONFIG_ESP_DEBUG_OCDAWARE=y\n", + ) + .unwrap(); + let s = SdkConfigSummary::from_project_dir(tmp.path()); + assert!(s.debug_ocdaware); + // gdbstub must NOT be set from defaults (sdkconfig wins entirely) + assert!(!s.panic_gdbstub); + } + + #[test] + fn default_equals_arduino_default() { + assert_eq!( + SdkConfigSummary::default(), + SdkConfigSummary::arduino_default() + ); + } +}