From 98447b7956f87dec5a5c9a94d565b3e77ec79f22 Mon Sep 17 00:00:00 2001 From: zackees Date: Sat, 30 May 2026 11:29:39 -0700 Subject: [PATCH] feat(debug): print link command on -v + always emit firmware.map (fixes #305) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two small debuggability patches per the issue: A. Linker invocations now eprintln! the full command line when -v is passed. tracing::info! is kept for log integration but also goes to stderr so plain CLI users see it. B. Every per-platform linker now adds -Wl,-Map=firmware.map to the link command. Lands next to firmware.elf in output_dir. Cost is essentially zero (~50-500 KB plain text) and pays for itself the first time anyone asks "why is this symbol in my ELF?" (See FastLED/fbuild#304 for the recent investigation that took 45 min for want of a map.) WASM and ESP32 paths intentionally skipped — different artifact shapes. Closes #305. --- crates/fbuild-build/src/avr/avr_linker.rs | 80 ++++++++++++++++--- crates/fbuild-build/src/ch32v/ch32v_linker.rs | 5 ++ .../src/esp8266/esp8266_linker.rs | 5 ++ .../src/generic_arm/arm_linker.rs | 5 ++ crates/fbuild-build/src/nrf52/nrf52_linker.rs | 5 ++ .../src/renesas/renesas_linker.rs | 5 ++ crates/fbuild-build/src/sam/sam_linker.rs | 5 ++ .../fbuild-build/src/silabs/silabs_linker.rs | 5 ++ .../fbuild-build/src/teensy/teensy_linker.rs | 5 ++ 9 files changed, 110 insertions(+), 10 deletions(-) diff --git a/crates/fbuild-build/src/avr/avr_linker.rs b/crates/fbuild-build/src/avr/avr_linker.rs index 3d37bbe8..f5f09768 100644 --- a/crates/fbuild-build/src/avr/avr_linker.rs +++ b/crates/fbuild-build/src/avr/avr_linker.rs @@ -54,21 +54,19 @@ impl AvrLinker { } } -impl Linker for AvrLinker { - fn archive(&self, objects: &[PathBuf], output: &Path) -> Result<()> { - crate::linker::LinkerBase::archive(&self.ar_path, objects, output, "avr-ar") - } - - fn link( +impl AvrLinker { + /// Build the argv that will be passed to `avr-gcc` for the link step. + /// + /// Factored out so it can be unit-tested without invoking the toolchain + /// (see #305 — assert that `-Wl,-Map=` is present). + fn build_link_args( &self, objects: &[PathBuf], archives: &[PathBuf], output_dir: &Path, + elf_path: &Path, extra: &LinkExtraArgs, - ) -> Result { - std::fs::create_dir_all(output_dir)?; - let elf_path = output_dir.join("firmware.elf"); - + ) -> Vec { let mut args: Vec = vec![ self.gcc_path.to_string_lossy().to_string(), format!("-mmcu={}", self.mcu), @@ -85,6 +83,10 @@ impl Linker for AvrLinker { args.extend(["-o".to_string(), elf_path.to_string_lossy().to_string()]); + // Always emit a linker map next to firmware.elf for debugging (#305). + let map_path = output_dir.join("firmware.map"); + args.push(format!("-Wl,-Map={}", map_path.to_string_lossy())); + // Sketch objects first for obj in objects { args.push(obj.to_string_lossy().to_string()); @@ -102,7 +104,29 @@ impl Linker for AvrLinker { args.extend(extra.libs.iter().cloned()); args.push("-Wl,--end-group".to_string()); + args + } +} + +impl Linker for AvrLinker { + fn archive(&self, objects: &[PathBuf], output: &Path) -> Result<()> { + crate::linker::LinkerBase::archive(&self.ar_path, objects, output, "avr-ar") + } + + fn link( + &self, + objects: &[PathBuf], + archives: &[PathBuf], + output_dir: &Path, + extra: &LinkExtraArgs, + ) -> Result { + std::fs::create_dir_all(output_dir)?; + let elf_path = output_dir.join("firmware.elf"); + + let args = self.build_link_args(objects, archives, output_dir, &elf_path, extra); + if self.verbose { + eprintln!("link: {}", args.join(" ")); tracing::info!("link: {}", args.join(" ")); } @@ -168,4 +192,40 @@ mod tests { assert_eq!(linker.max_flash, Some(32256)); assert_eq!(linker.max_ram, Some(2048)); } + + /// #305: every per-platform linker must emit a `firmware.map` next to + /// `firmware.elf`. Assert the generated argv contains a `-Wl,-Map=` token. + #[test] + fn test_avr_link_args_contain_map_flag() { + let linker = AvrLinker::new( + PathBuf::from("/bin/avr-gcc"), + PathBuf::from("/bin/avr-ar"), + PathBuf::from("/bin/avr-objcopy"), + PathBuf::from("/bin/avr-size"), + "atmega328p", + get_avr_config().unwrap(), + BuildProfile::Release, + Some(32256), + Some(2048), + false, + ); + + let tmp = tempfile::TempDir::new().unwrap(); + let output_dir = tmp.path(); + let elf_path = output_dir.join("firmware.elf"); + let extra = LinkExtraArgs::default(); + let args = linker.build_link_args(&[], &[], output_dir, &elf_path, &extra); + + let map_flag = args + .iter() + .find(|a| a.starts_with("-Wl,-Map=")) + .expect("link args must contain -Wl,-Map= for firmware.map emission"); + let expected_map = output_dir.join("firmware.map"); + assert!( + map_flag.contains(&*expected_map.to_string_lossy()), + "expected map flag to reference {}, got {}", + expected_map.display(), + map_flag + ); + } } diff --git a/crates/fbuild-build/src/ch32v/ch32v_linker.rs b/crates/fbuild-build/src/ch32v/ch32v_linker.rs index 55850031..0a07c4bb 100644 --- a/crates/fbuild-build/src/ch32v/ch32v_linker.rs +++ b/crates/fbuild-build/src/ch32v/ch32v_linker.rs @@ -86,6 +86,10 @@ impl Linker for Ch32vLinker { elf_path.to_string_lossy().to_string(), ]); + // Always emit a linker map next to firmware.elf for debugging (#305). + let map_path = output_dir.join("firmware.map"); + args.push(format!("-Wl,-Map={}", map_path.to_string_lossy())); + // Sketch objects first for obj in objects { args.push(obj.to_string_lossy().to_string()); @@ -101,6 +105,7 @@ impl Linker for Ch32vLinker { args.extend(extra.libs.iter().cloned()); if self.verbose { + eprintln!("link: {}", args.join(" ")); tracing::info!("link: {}", args.join(" ")); } diff --git a/crates/fbuild-build/src/esp8266/esp8266_linker.rs b/crates/fbuild-build/src/esp8266/esp8266_linker.rs index ce665b47..220f1c3e 100644 --- a/crates/fbuild-build/src/esp8266/esp8266_linker.rs +++ b/crates/fbuild-build/src/esp8266/esp8266_linker.rs @@ -199,6 +199,10 @@ impl Linker for Esp8266Linker { args.extend(["-o".to_string(), elf_path.to_string_lossy().to_string()]); + // Always emit a linker map next to firmware.elf for debugging (#305). + let map_path = output_dir.join("firmware.map"); + args.push(format!("-Wl,-Map={}", map_path.to_string_lossy())); + // Sketch objects first for obj in objects { args.push(obj.to_string_lossy().to_string()); @@ -216,6 +220,7 @@ impl Linker for Esp8266Linker { args.push("-Wl,--end-group".to_string()); if self.verbose { + eprintln!("link: {}", args.join(" ")); tracing::info!("link: {}", args.join(" ")); } diff --git a/crates/fbuild-build/src/generic_arm/arm_linker.rs b/crates/fbuild-build/src/generic_arm/arm_linker.rs index a409e861..a99e1d94 100644 --- a/crates/fbuild-build/src/generic_arm/arm_linker.rs +++ b/crates/fbuild-build/src/generic_arm/arm_linker.rs @@ -86,6 +86,10 @@ impl Linker for ArmLinker { elf_path.to_string_lossy().to_string(), ]); + // Always emit a linker map next to firmware.elf for debugging (#305). + let map_path = output_dir.join("firmware.map"); + args.push(format!("-Wl,-Map={}", map_path.to_string_lossy())); + // Sketch objects first for obj in objects { args.push(obj.to_string_lossy().to_string()); @@ -101,6 +105,7 @@ impl Linker for ArmLinker { args.extend(extra.libs.iter().cloned()); if self.verbose { + eprintln!("link: {}", args.join(" ")); tracing::info!("link: {}", args.join(" ")); } diff --git a/crates/fbuild-build/src/nrf52/nrf52_linker.rs b/crates/fbuild-build/src/nrf52/nrf52_linker.rs index b0d40372..abdc4740 100644 --- a/crates/fbuild-build/src/nrf52/nrf52_linker.rs +++ b/crates/fbuild-build/src/nrf52/nrf52_linker.rs @@ -94,6 +94,10 @@ impl Linker for Nrf52Linker { elf_path.to_string_lossy().to_string(), ]); + // Always emit a linker map next to firmware.elf for debugging (#305). + let map_path = output_dir.join("firmware.map"); + args.push(format!("-Wl,-Map={}", map_path.to_string_lossy())); + // Sketch objects first for obj in objects { args.push(obj.to_string_lossy().to_string()); @@ -109,6 +113,7 @@ impl Linker for Nrf52Linker { args.extend(extra.libs.iter().cloned()); if self.verbose { + eprintln!("link: {}", args.join(" ")); tracing::info!("link: {}", args.join(" ")); } diff --git a/crates/fbuild-build/src/renesas/renesas_linker.rs b/crates/fbuild-build/src/renesas/renesas_linker.rs index 0206124a..2f7b5dbd 100644 --- a/crates/fbuild-build/src/renesas/renesas_linker.rs +++ b/crates/fbuild-build/src/renesas/renesas_linker.rs @@ -99,6 +99,10 @@ impl Linker for RenesasLinker { elf_path.to_string_lossy().to_string(), ]); + // Always emit a linker map next to firmware.elf for debugging (#305). + let map_path = output_dir.join("firmware.map"); + args.push(format!("-Wl,-Map={}", map_path.to_string_lossy())); + // Sketch objects first for obj in objects { args.push(obj.to_string_lossy().to_string()); @@ -114,6 +118,7 @@ impl Linker for RenesasLinker { args.extend(extra.libs.iter().cloned()); if self.verbose { + eprintln!("link: {}", args.join(" ")); tracing::info!("link: {}", args.join(" ")); } diff --git a/crates/fbuild-build/src/sam/sam_linker.rs b/crates/fbuild-build/src/sam/sam_linker.rs index 4e9ee332..6e7858c6 100644 --- a/crates/fbuild-build/src/sam/sam_linker.rs +++ b/crates/fbuild-build/src/sam/sam_linker.rs @@ -100,6 +100,10 @@ impl Linker for SamLinker { elf_path.to_string_lossy().to_string(), ]); + // Always emit a linker map next to firmware.elf for debugging (#305). + let map_path = output_dir.join("firmware.map"); + args.push(format!("-Wl,-Map={}", map_path.to_string_lossy())); + // Sketch objects first for obj in objects { args.push(obj.to_string_lossy().to_string()); @@ -130,6 +134,7 @@ impl Linker for SamLinker { args.extend(extra.libs.iter().cloned()); if self.verbose { + eprintln!("link: {}", args.join(" ")); tracing::info!("link: {}", args.join(" ")); } diff --git a/crates/fbuild-build/src/silabs/silabs_linker.rs b/crates/fbuild-build/src/silabs/silabs_linker.rs index 04590f65..f1d81dd1 100644 --- a/crates/fbuild-build/src/silabs/silabs_linker.rs +++ b/crates/fbuild-build/src/silabs/silabs_linker.rs @@ -92,6 +92,10 @@ impl Linker for SilabsLinker { elf_path.to_string_lossy().to_string(), ]); + // Always emit a linker map next to firmware.elf for debugging (#305). + let map_path = output_dir.join("firmware.map"); + args.push(format!("-Wl,-Map={}", map_path.to_string_lossy())); + // Sketch objects first for obj in objects { args.push(obj.to_string_lossy().to_string()); @@ -117,6 +121,7 @@ impl Linker for SilabsLinker { args.push("-Wl,--end-group".to_string()); if self.verbose { + eprintln!("link: {}", args.join(" ")); tracing::info!("link: {}", args.join(" ")); } diff --git a/crates/fbuild-build/src/teensy/teensy_linker.rs b/crates/fbuild-build/src/teensy/teensy_linker.rs index 08f857ed..653afc7e 100644 --- a/crates/fbuild-build/src/teensy/teensy_linker.rs +++ b/crates/fbuild-build/src/teensy/teensy_linker.rs @@ -83,6 +83,10 @@ impl Linker for TeensyLinker { args.extend(self.linker_scripts.to_args()); args.extend(["-o".to_string(), elf_path.to_string_lossy().to_string()]); + // Always emit a linker map next to firmware.elf for debugging (#305). + let map_path = output_dir.join("firmware.map"); + args.push(format!("-Wl,-Map={}", map_path.to_string_lossy())); + // Sketch objects first for obj in objects { args.push(obj.to_string_lossy().to_string()); @@ -98,6 +102,7 @@ impl Linker for TeensyLinker { args.extend(extra.libs.iter().cloned()); if self.verbose { + eprintln!("link: {}", args.join(" ")); tracing::info!("link: {}", args.join(" ")); }