fix(acceptance): #223 stm32 SPI + teensyLC LTO-symbol probes

crates/fbuild-build/tests/stm32_acceptance.rs

@@ -16,7 +16,15 @@
 //! 1. The build succeeds.
 //! 2. `compile_commands.json` references at least one source file under
 //!    the SPI library (substring `SPI`).
-//! 3. The ELF contains a symbol whose mangled name contains `SPIClass`.
+//! 3. The ELF contains evidence that `Arduino_Core_STM32/libraries/SPI/`
+//!    was compiled and linked into the firmware (#202, #223). The probe
+//!    accepts either a mangled `SPIClass*` C++ symbol (visible without LTO)
+//!    or a `PinMap_SPI_*` array from the library's `utility/spi_com.c`
+//!    (an LTO-stable global whose address is referenced by the SPI
+//!    peripheral pin tables). The Release profile uses
+//!    `-flto -Os -fno-rtti`, which inlines `SPIClass::begin()` and friends
+//!    into their callers and strips their independent symbols — see #223
+//!    for the diagnostic walk-through.
 
 use std::path::{Path, PathBuf};
 
@@ -84,11 +92,28 @@ fn stm32f103c8_blink_with_spi_auto_discovers_library_205_ac4() {
         .as_ref()
         .expect("stm32 build must produce ELF");
     let probe = ElfProbe::open(elf).expect("ELF parses");
+    // WHY two-shot: the Release profile's `-flto -Os -fno-rtti` inlines
+    // `SPIClass::begin()` (and the other SPI methods called from setup())
+    // into their callers and discards the independent mangled symbols. So
+    // `SPIClass` substring is reliable in non-LTO builds (Quick) but not
+    // in LTO builds (Release). `PinMap_SPI_MOSI` is a `const` global array
+    // declared in `Arduino_Core_STM32/libraries/SPI/src/utility/spi_com.c`
+    // whose address is taken by the SPI peripheral pin tables — it survives
+    // both LTO and `--gc-sections`. Either signal proves the SPI library
+    // was discovered, compiled, and linked. See #223 for the trace.
+    let has_spiclass = probe
+        .has_symbol_containing("SPIClass")
+        .expect("symbol query");
+    let has_pinmap = probe
+        .has_symbol_containing("PinMap_SPI_")
+        .expect("symbol query");
     assert!(
-        probe
-            .has_symbol_containing("SPIClass")
-            .expect("symbol query"),
-        "AC#4: SPIClass symbol must be present in ELF — closes #202"
+        has_spiclass || has_pinmap,
+        "AC#4: SPI library must be present in ELF — closes #202; saw \
+         neither a mangled `SPIClass*` symbol nor a `PinMap_SPI_*` global \
+         (probed both because the Release profile's LTO can inline the \
+         former). If only one form is missing, the library is auto-\
+         discovered correctly but the probe needs a third candidate."
     );
 
     let compdb = locate_compile_commands(&build_dir, "stm32f103c8")

crates/fbuild-build/tests/teensylc_acceptance.rs

@@ -75,30 +75,53 @@ fn teensylc_blink_meets_205_acceptance_criteria() {
         );
     }
     // WHY: setup/loop are extern "C" via Arduino.h's prototype, so
-    // ideally appear unmangled. But Teensyduino's main calls them via
-    // the framework's main.cpp and toolchain LTO can leave only the
-    // mangled C++ symbols (`_Z5setupv` / `_Z4loopv`) when the .ino is
-    // compiled as C++ without the extern "C" prototype reaching the
-    // definition. Accept either form — the contract is "the user's
-    // setup/loop landed in the firmware", not "they kept their C
-    // linkage". The earlier `has_symbol_containing` was rejected in
-    // PR #209 review for matching `Stream::setupXxx`-style false
-    // positives; the explicit-mangled fallback below is targeted and
-    // doesn't share that problem.
+    // ideally appear unmangled. But under the orchestrator's Release
+    // profile (-flto + -Os) Teensyduino's main.cpp and the .ino are
+    // visible in the same LTO unit, so the linker inlines the tiny
+    // setup()/loop() bodies into main() and discards both the
+    // unmangled and the mangled (`_Z5setupv` / `_Z4loopv`) symbols
+    // via --gc-sections. Same root-cause family as #223. Accept any
+    // of three signals — the contract is "the user's setup/loop
+    // landed in the firmware":
+    //   1. unmangled `setup` / `loop` symbol survived (no LTO inline)
+    //   2. mangled `_Z5setupv` / `_Z4loopv` survived (LTO disabled)
+    //   3. the sketch's unique `Serial.println` literal is present
+    //      in the firmware bytes — proves the .ino's println() chain
+    //      was linked. Strings in .rodata survive --gc-sections
+    //      because their address is taken by the println call.
+    // The earlier `has_symbol_containing` was rejected in PR #209
+    // review for matching `Stream::setupXxx`-style false positives;
+    // exact-name and byte-needle probes don't share that problem.
+    let elf_bytes = std::fs::read(elf).expect("read ELF for byte probe");
+    // Marker chosen from tests/platform/teensylc/src/main.ino — must
+    // stay in sync with the sketch's first println literal.
+    const SKETCH_MARKER: &[u8] = b"Teensy LC Test - LED Blink";
+    let sketch_bytes_present = elf_bytes
+        .windows(SKETCH_MARKER.len())
+        .any(|w| w == SKETCH_MARKER);
     for (required, mangled) in [("setup", "_Z5setupv"), ("loop", "_Z4loopv")] {
         let unmangled_present = probe.has_symbol(required).expect("symbol query");
         let mangled_present = probe.has_symbol(mangled).expect("symbol query");
         assert!(
-            unmangled_present || mangled_present,
+            unmangled_present || mangled_present || sketch_bytes_present,
             "A-11: required symbol '{required}' missing from ELF \
-             (also looked for mangled '{mangled}')"
+             (also looked for mangled '{mangled}' and the sketch's \
+             '{}' literal in firmware bytes)",
+            std::str::from_utf8(SKETCH_MARKER).unwrap()
         );
     }
 
     // ── compile_commands.json probes (AC#1, A-20..A-22) ─────────────────
-    let compdb_path = locate_compile_commands(build_dir.path(), "teensylc")
-        .expect("compile_commands.json should land in build dir");
-    let db = CompileDb::from_path(&compdb_path).expect("parse compile_commands.json");
+    // WHY use result.compile_database_path: the pipeline ignores
+    // params.build_dir and roots its build cache at
+    // <project_dir>/.fbuild/build/<env>/<profile>/, so a tempdir-based
+    // walk from build_dir.path() never finds the file. The orchestrator
+    // already reports the effective location in BuildResult — trust it.
+    let compdb_path = result
+        .compile_database_path
+        .as_ref()
+        .expect("teensy build must report compile_commands.json path");
+    let db = CompileDb::from_path(compdb_path).expect("parse compile_commands.json");
     assert!(
         db.tu_count() <= 250,
         "AC#1: TU count must be <= 250; got {} entries",
@@ -122,26 +145,3 @@ fn repo_fixture(name: &str) -> PathBuf {
         .join("tests/platform")
         .join(name)
 }
-
-fn locate_compile_commands(build_dir: &std::path::Path, env: &str) -> Option<PathBuf> {
-    // Per fbuild's layout the file lives at one of:
-    //   <build_dir>/<env>/compile_commands.json
-    //   <build_dir>/compile_commands.json
-    // Search both, prefer the per-env path.
-    let candidates = [
-        build_dir.join(env).join("compile_commands.json"),
-        build_dir.join("compile_commands.json"),
-    ];
-    for c in candidates {
-        if c.exists() {
-            return Some(c);
-        }
-    }
-    // Fallback: walk the build_dir for any compile_commands.json.
-    for entry in walkdir::WalkDir::new(build_dir).into_iter().flatten() {
-        if entry.file_name() == "compile_commands.json" {
-            return Some(entry.into_path());
-        }
-    }
-    None
-}