Skip to content

Commit a23d855

Browse files
zackeesclaude
andauthored
fix(acceptance): #223 stm32 SPI + teensyLC LTO-symbol probes (#226)
Two pre-existing acceptance gates failed for the same reason: under each orchestrator's Release profile (`-flto -Os` + `--gc-sections`) the test was probing for a symbol the linker had inlined and stripped. Both fixes broaden the probe to also accept an LTO-stable signal that proves the intended code was actually linked. stm32f103c8 SPI auto-discovery (#223): - crates/fbuild-build/tests/stm32_acceptance.rs:99 — accept either the mangled `_ZN8SPIClass*` symbol (non-LTO builds) OR a `PinMap_SPI_*` global from `Arduino_Core_STM32/libraries/SPI/src/utility/spi_com.c` (LTO builds; address-taken `const` array survives `--gc-sections`). Either signal proves the SPI library was auto-discovered, compiled, and linked. Orchestrator wiring is unchanged — already correct since #214 (commit 86bb2d9). teensyLC blink (acceptance-205 baseline failure since 2026-04-25): - crates/fbuild-build/tests/teensylc_acceptance.rs:88 — accept the unmangled `setup`/`loop`, the mangled `_Z5setupv`/`_Z4loopv`, OR the sketch's `Serial.println` literal `"Teensy LC Test - LED Blink"` in the firmware bytes. Strings in `.rodata` survive `--gc-sections` because their address is taken by the println call. - crates/fbuild-build/tests/teensylc_acceptance.rs:114 — use `result.compile_database_path` from the BuildResult instead of walking `params.build_dir` (which the pipeline ignores: it roots its cache at `<project_dir>/.fbuild/build/<env>/<profile>/`). Verified locally on Windows host (toolchain + packages cached): both acceptance tests pass under Release LTO. Closes #223 Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent a04bb2a commit a23d855

2 files changed

Lines changed: 68 additions & 43 deletions

File tree

crates/fbuild-build/tests/stm32_acceptance.rs

Lines changed: 30 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,15 @@
1616
//! 1. The build succeeds.
1717
//! 2. `compile_commands.json` references at least one source file under
1818
//! the SPI library (substring `SPI`).
19-
//! 3. The ELF contains a symbol whose mangled name contains `SPIClass`.
19+
//! 3. The ELF contains evidence that `Arduino_Core_STM32/libraries/SPI/`
20+
//! was compiled and linked into the firmware (#202, #223). The probe
21+
//! accepts either a mangled `SPIClass*` C++ symbol (visible without LTO)
22+
//! or a `PinMap_SPI_*` array from the library's `utility/spi_com.c`
23+
//! (an LTO-stable global whose address is referenced by the SPI
24+
//! peripheral pin tables). The Release profile uses
25+
//! `-flto -Os -fno-rtti`, which inlines `SPIClass::begin()` and friends
26+
//! into their callers and strips their independent symbols — see #223
27+
//! for the diagnostic walk-through.
2028
2129
use std::path::{Path, PathBuf};
2230

@@ -84,11 +92,28 @@ fn stm32f103c8_blink_with_spi_auto_discovers_library_205_ac4() {
8492
.as_ref()
8593
.expect("stm32 build must produce ELF");
8694
let probe = ElfProbe::open(elf).expect("ELF parses");
95+
// WHY two-shot: the Release profile's `-flto -Os -fno-rtti` inlines
96+
// `SPIClass::begin()` (and the other SPI methods called from setup())
97+
// into their callers and discards the independent mangled symbols. So
98+
// `SPIClass` substring is reliable in non-LTO builds (Quick) but not
99+
// in LTO builds (Release). `PinMap_SPI_MOSI` is a `const` global array
100+
// declared in `Arduino_Core_STM32/libraries/SPI/src/utility/spi_com.c`
101+
// whose address is taken by the SPI peripheral pin tables — it survives
102+
// both LTO and `--gc-sections`. Either signal proves the SPI library
103+
// was discovered, compiled, and linked. See #223 for the trace.
104+
let has_spiclass = probe
105+
.has_symbol_containing("SPIClass")
106+
.expect("symbol query");
107+
let has_pinmap = probe
108+
.has_symbol_containing("PinMap_SPI_")
109+
.expect("symbol query");
87110
assert!(
88-
probe
89-
.has_symbol_containing("SPIClass")
90-
.expect("symbol query"),
91-
"AC#4: SPIClass symbol must be present in ELF — closes #202"
111+
has_spiclass || has_pinmap,
112+
"AC#4: SPI library must be present in ELF — closes #202; saw \
113+
neither a mangled `SPIClass*` symbol nor a `PinMap_SPI_*` global \
114+
(probed both because the Release profile's LTO can inline the \
115+
former). If only one form is missing, the library is auto-\
116+
discovered correctly but the probe needs a third candidate."
92117
);
93118

94119
let compdb = locate_compile_commands(&build_dir, "stm32f103c8")

crates/fbuild-build/tests/teensylc_acceptance.rs

Lines changed: 38 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -75,30 +75,53 @@ fn teensylc_blink_meets_205_acceptance_criteria() {
7575
);
7676
}
7777
// WHY: setup/loop are extern "C" via Arduino.h's prototype, so
78-
// ideally appear unmangled. But Teensyduino's main calls them via
79-
// the framework's main.cpp and toolchain LTO can leave only the
80-
// mangled C++ symbols (`_Z5setupv` / `_Z4loopv`) when the .ino is
81-
// compiled as C++ without the extern "C" prototype reaching the
82-
// definition. Accept either form — the contract is "the user's
83-
// setup/loop landed in the firmware", not "they kept their C
84-
// linkage". The earlier `has_symbol_containing` was rejected in
85-
// PR #209 review for matching `Stream::setupXxx`-style false
86-
// positives; the explicit-mangled fallback below is targeted and
87-
// doesn't share that problem.
78+
// ideally appear unmangled. But under the orchestrator's Release
79+
// profile (-flto + -Os) Teensyduino's main.cpp and the .ino are
80+
// visible in the same LTO unit, so the linker inlines the tiny
81+
// setup()/loop() bodies into main() and discards both the
82+
// unmangled and the mangled (`_Z5setupv` / `_Z4loopv`) symbols
83+
// via --gc-sections. Same root-cause family as #223. Accept any
84+
// of three signals — the contract is "the user's setup/loop
85+
// landed in the firmware":
86+
// 1. unmangled `setup` / `loop` symbol survived (no LTO inline)
87+
// 2. mangled `_Z5setupv` / `_Z4loopv` survived (LTO disabled)
88+
// 3. the sketch's unique `Serial.println` literal is present
89+
// in the firmware bytes — proves the .ino's println() chain
90+
// was linked. Strings in .rodata survive --gc-sections
91+
// because their address is taken by the println call.
92+
// The earlier `has_symbol_containing` was rejected in PR #209
93+
// review for matching `Stream::setupXxx`-style false positives;
94+
// exact-name and byte-needle probes don't share that problem.
95+
let elf_bytes = std::fs::read(elf).expect("read ELF for byte probe");
96+
// Marker chosen from tests/platform/teensylc/src/main.ino — must
97+
// stay in sync with the sketch's first println literal.
98+
const SKETCH_MARKER: &[u8] = b"Teensy LC Test - LED Blink";
99+
let sketch_bytes_present = elf_bytes
100+
.windows(SKETCH_MARKER.len())
101+
.any(|w| w == SKETCH_MARKER);
88102
for (required, mangled) in [("setup", "_Z5setupv"), ("loop", "_Z4loopv")] {
89103
let unmangled_present = probe.has_symbol(required).expect("symbol query");
90104
let mangled_present = probe.has_symbol(mangled).expect("symbol query");
91105
assert!(
92-
unmangled_present || mangled_present,
106+
unmangled_present || mangled_present || sketch_bytes_present,
93107
"A-11: required symbol '{required}' missing from ELF \
94-
(also looked for mangled '{mangled}')"
108+
(also looked for mangled '{mangled}' and the sketch's \
109+
'{}' literal in firmware bytes)",
110+
std::str::from_utf8(SKETCH_MARKER).unwrap()
95111
);
96112
}
97113

98114
// ── compile_commands.json probes (AC#1, A-20..A-22) ─────────────────
99-
let compdb_path = locate_compile_commands(build_dir.path(), "teensylc")
100-
.expect("compile_commands.json should land in build dir");
101-
let db = CompileDb::from_path(&compdb_path).expect("parse compile_commands.json");
115+
// WHY use result.compile_database_path: the pipeline ignores
116+
// params.build_dir and roots its build cache at
117+
// <project_dir>/.fbuild/build/<env>/<profile>/, so a tempdir-based
118+
// walk from build_dir.path() never finds the file. The orchestrator
119+
// already reports the effective location in BuildResult — trust it.
120+
let compdb_path = result
121+
.compile_database_path
122+
.as_ref()
123+
.expect("teensy build must report compile_commands.json path");
124+
let db = CompileDb::from_path(compdb_path).expect("parse compile_commands.json");
102125
assert!(
103126
db.tu_count() <= 250,
104127
"AC#1: TU count must be <= 250; got {} entries",
@@ -122,26 +145,3 @@ fn repo_fixture(name: &str) -> PathBuf {
122145
.join("tests/platform")
123146
.join(name)
124147
}
125-
126-
fn locate_compile_commands(build_dir: &std::path::Path, env: &str) -> Option<PathBuf> {
127-
// Per fbuild's layout the file lives at one of:
128-
// <build_dir>/<env>/compile_commands.json
129-
// <build_dir>/compile_commands.json
130-
// Search both, prefer the per-env path.
131-
let candidates = [
132-
build_dir.join(env).join("compile_commands.json"),
133-
build_dir.join("compile_commands.json"),
134-
];
135-
for c in candidates {
136-
if c.exists() {
137-
return Some(c);
138-
}
139-
}
140-
// Fallback: walk the build_dir for any compile_commands.json.
141-
for entry in walkdir::WalkDir::new(build_dir).into_iter().flatten() {
142-
if entry.file_name() == "compile_commands.json" {
143-
return Some(entry.into_path());
144-
}
145-
}
146-
None
147-
}

0 commit comments

Comments
 (0)