From 414eb75ec3031222a485166de75eab26210bf652 Mon Sep 17 00:00:00 2001 From: zackees Date: Wed, 3 Jun 2026 19:34:07 -0700 Subject: [PATCH] fix(nrf52): scope -Wno-array-bounds to Adafruit BSP vendor sources only The Adafruit nRF52 BSP's nrfx HAL (nordic/nrfx/hal/nrf_clock.h:800, inlined into dcd_nrf5x.c:919 hfclk_running) casts a nrf_clock_hfclk_t* (1-element array) to uint32_t*, which GCC's array-bounds analysis correctly flags as partly-OOB. It is a benign upstream strict-aliasing pattern, but the warning surfaces ~340 times per build across all 4 nRF52 boards (adafruit_feather_nrf52840_sense, adafruit_xiaoblesense, nrf52840_dk, nrf52_xiaoblesense). This change adds the same per-source warning-scoping pattern PR #399 introduced for CH32V/OpenWCH: - Nrf52Compiler::with_framework_root attaches the resolved Adafruit nRF52 BSP install root. - compile_one demotes `-Wno-array-bounds` for sources rooted under that install path only. FastLED + user sketch code still sees the full `-Warray-bounds`. - The orchestrator passes framework_dir into the compiler so the scope is always populated. Tests: - test_array_bounds_suppression_is_not_global asserts -Wno-array-bounds does NOT appear in the workspace-wide C or C++ flag set - test_is_framework_source_detection covers vendor-vs-sketch routing - test_framework_suppression_flags_are_narrow pins the suppression list to exactly one flag so future additions are deliberate 21/21 nrf52 unit tests pass; clippy --workspace -D warnings clean. Closes #407. --- .../fbuild-build/src/nrf52/nrf52_compiler.rs | 94 ++++++++++++++++++- crates/fbuild-build/src/nrf52/orchestrator.rs | 6 +- 2 files changed, 98 insertions(+), 2 deletions(-) diff --git a/crates/fbuild-build/src/nrf52/nrf52_compiler.rs b/crates/fbuild-build/src/nrf52/nrf52_compiler.rs index d1fc8d51..846f72ff 100644 --- a/crates/fbuild-build/src/nrf52/nrf52_compiler.rs +++ b/crates/fbuild-build/src/nrf52/nrf52_compiler.rs @@ -21,6 +21,28 @@ pub struct Nrf52Compiler { temp_dir: PathBuf, /// PlatformIO `build_unflags`. See FastLED/fbuild#37. build_unflags: Vec, + /// Optional framework root used to scope third-party warning + /// suppressions to vendor sources only. Set by the orchestrator after + /// `Esp32Framework::ensure_installed`. See FastLED/fbuild#407. + framework_root: Option, +} + +/// Per-source warning demotions scoped to Adafruit nRF52 BSP / NRFX HAL +/// vendor sources only. `nordic/nrfx/hal/nrf_clock.h:800` casts a +/// `nrf_clock_hfclk_t*` (1-element array) to `uint32_t*`, which GCC's +/// array-bounds analysis correctly flags but is a benign upstream strict- +/// aliasing pattern. Same shape for `dcd_nrf5x.c:919`. See +/// FastLED/fbuild#407. +fn framework_suppression_flags() -> &'static [&'static str] { + &["-Wno-array-bounds"] +} + +/// `true` when `source` lives under the Adafruit nRF52 BSP install root. +fn is_framework_source(source: &Path, framework_root: Option<&Path>) -> bool { + let Some(root) = framework_root else { + return false; + }; + source.starts_with(root) } impl Nrf52Compiler { @@ -50,6 +72,7 @@ impl Nrf52Compiler { profile, temp_dir: fbuild_core::response_file::windows_temp_dir(), build_unflags: Vec::new(), + framework_root: None, } } @@ -59,6 +82,14 @@ impl Nrf52Compiler { self } + /// Attach the Adafruit nRF52 BSP install root so per-source warning + /// suppressions can be scoped to vendor sources only. See + /// FastLED/fbuild#407. + pub fn with_framework_root(mut self, root: PathBuf) -> Self { + self.framework_root = Some(root); + self + } + /// Build the common ARM Cortex-M4F compiler flags. fn common_flags(&self) -> Vec { let mut flags = Vec::new(); @@ -84,12 +115,32 @@ impl Compiler for Nrf52Compiler { flags: &[String], extra_flags: &[String], ) -> Result { + // Demote `-Warray-bounds` for Adafruit nRF52 BSP / NRFX HAL sources + // (e.g. `nordic/nrfx/hal/nrf_clock.h:800`) only. FastLED + user + // sketch code still sees the full `-Warray-bounds`. See + // FastLED/fbuild#407. + let suppressed_extra: Vec; + let effective_extra: &[String] = + if is_framework_source(source, self.framework_root.as_deref()) { + suppressed_extra = extra_flags + .iter() + .cloned() + .chain( + framework_suppression_flags() + .iter() + .map(|s| (*s).to_string()), + ) + .collect(); + &suppressed_extra + } else { + extra_flags + }; crate::compiler::compile_source( compiler_path, source, output, flags, - extra_flags, + effective_extra, &self.temp_dir, "nrf52", self.base.verbose, @@ -196,4 +247,45 @@ mod tests { assert!(flags.contains(&"-fno-rtti".to_string())); assert!(flags.contains(&"-fno-threadsafe-statics".to_string())); } + + /// FastLED/fbuild#407: `-Wno-array-bounds` must NOT be in the + /// workspace-wide flag set. It belongs to the per-framework-source + /// scope only. + #[test] + fn test_array_bounds_suppression_is_not_global() { + let compiler = test_compiler(); + let c_flags = compiler.c_flags(); + let cpp_flags = compiler.cpp_flags(); + for f in &c_flags { + assert_ne!( + f, "-Wno-array-bounds", + "-Wno-array-bounds must not be in the workspace-wide C flag set" + ); + } + for f in &cpp_flags { + assert_ne!( + f, "-Wno-array-bounds", + "-Wno-array-bounds must not be in the workspace-wide C++ flag set" + ); + } + } + + /// FastLED/fbuild#407: framework-source detection. + #[test] + fn test_is_framework_source_detection() { + let root = PathBuf::from("/cache/nrf52/framework-arduinoadafruitnrf52"); + let vendor = root.join("cores/nRF5/nordic/nrfx/hal/nrf_clock.h"); + let sketch = PathBuf::from("/proj/src/main.cpp"); + assert!(is_framework_source(&vendor, Some(&root))); + assert!(!is_framework_source(&sketch, Some(&root))); + assert!(!is_framework_source(&vendor, None)); + } + + /// FastLED/fbuild#407: the suppression list is exactly + /// `-Wno-array-bounds` — broader suppressions should be intentional. + #[test] + fn test_framework_suppression_flags_are_narrow() { + let flags = framework_suppression_flags(); + assert_eq!(flags, &["-Wno-array-bounds"]); + } } diff --git a/crates/fbuild-build/src/nrf52/orchestrator.rs b/crates/fbuild-build/src/nrf52/orchestrator.rs index 75449187..4d84e786 100644 --- a/crates/fbuild-build/src/nrf52/orchestrator.rs +++ b/crates/fbuild-build/src/nrf52/orchestrator.rs @@ -299,7 +299,11 @@ impl BuildOrchestrator for Nrf52Orchestrator { params.profile, params.verbose, ) - .with_build_unflags(ctx.build_unflags.clone()); + .with_build_unflags(ctx.build_unflags.clone()) + // Scope `-Wno-array-bounds` to Adafruit nRF52 BSP sources only — + // FastLED + user sketch code still sees the full diagnostic. See + // FastLED/fbuild#407. + .with_framework_root(framework_dir.clone()); // 7. Create linker (reuse the alias-resolved linker_script_path // computed up front so the fingerprint hash and the actual linker