From 33b0fd764b5ba4364afafd688677a2d7fb09c6e9 Mon Sep 17 00:00:00 2001 From: zackees Date: Thu, 6 Aug 2026 20:50:50 -0700 Subject: [PATCH 1/4] fix(arm): run the link in the build dir for every ARM linker (#1269) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit #1268 fixed the teensy linker running in the fbuild daemon's inherited cwd — normally the user's project root — which let arm-none-eabi-gcc's collect2 / lto-wrapper drop scratch files there. The user-visible symptom was a stray 0-byte file literally named `-r` appearing in the repo root after every clean build (FastLED/FastLED#3867). The same bug was still latent in every other ARM linker. Apply the fix to generic_arm, nrf52, renesas, sam and silabs, and lift `link_cwd_for` out of teensy_linker.rs into the shared `fbuild-build-engine::linker` module so it lives in one place. The helper now takes the caller's cwd-sensitive path arguments and returns `None` — keeping the historical inherited-cwd behaviour — if the output dir or any of them is relative, because moving the cwd would also relocate a relative `-o`, `-T`, `-L` or object path. Each linker passes exactly the paths it actually hands the linker (objects, archives, linker script, library search dirs, precompiled archives, path-shaped extra libs). It deliberately does not scan the raw argv: linker flag lists legitimately contain bare non-path operands such as the symbol name in `-u app_main`. `stm32/orchestrator/arduino_mbed.rs` keeps its inherited cwd, now with a comment recording why: it is a `g++ -E -P` linker-script preprocess, not a link, so no collect2 / lto-wrapper is spawned and nothing writes cwd-relative scratch. Its `None` env is correct for the same reason — `link_env_for_build` only redirects GCC's LTO temp dir. Closes #1269 (part 1). Co-Authored-By: Claude --- .../src/generic_arm/arm_linker.rs | 20 +++- .../src/nrf52/nrf52_linker.rs | 18 ++- .../src/renesas/renesas_linker.rs | 19 +++- crates/fbuild-build-arm/src/sam/sam_linker.rs | 26 ++++- .../src/silabs/silabs_linker.rs | 18 ++- .../src/stm32/orchestrator/arduino_mbed.rs | 9 ++ .../src/teensy/teensy_linker.rs | 63 +++++------ crates/fbuild-build-engine/src/linker.rs | 103 ++++++++++++++++++ 8 files changed, 230 insertions(+), 46 deletions(-) diff --git a/crates/fbuild-build-arm/src/generic_arm/arm_linker.rs b/crates/fbuild-build-arm/src/generic_arm/arm_linker.rs index 61e68eff9..7164a6064 100644 --- a/crates/fbuild-build-arm/src/generic_arm/arm_linker.rs +++ b/crates/fbuild-build-arm/src/generic_arm/arm_linker.rs @@ -9,7 +9,7 @@ use fbuild_core::subprocess::run_command; use fbuild_core::{BuildProfile, Result, SizeInfo}; use super::mcu_config::ArmMcuConfig; -use crate::linker::{LinkExtraArgs, Linker}; +use crate::linker::{link_cwd_for, LinkExtraArgs, Linker}; /// Generic ARM linker using arm-none-eabi-gcc (link driver), ar, objcopy, size. pub struct ArmLinker { @@ -151,6 +151,20 @@ impl Linker for ArmLinker { tracing::debug!(target: "fbuild_build::linker::generic_arm", "link: {}", args.join(" ")); } + // Run the link in the firmware's own output directory rather than + // inheriting the daemon's cwd (typically the user's project root): + // arm-none-eabi-gcc hands off to collect2 / lto-wrapper, which write + // scratch files relative to the process cwd. See FastLED/fbuild#1269 + // (and #1268, which fixed the same bug for teensy). + let link_cwd = link_cwd_for( + output_dir, + objects + .iter() + .chain(archives.iter()) + .chain(self.lib_search_dirs.iter()) + .chain(std::iter::once(&self.linker_script_path)), + ); + // GCC LTO temp dir for MSYS-safe paths — see FastLED/fbuild#261. let lto_env = fbuild_core::subprocess::link_env_for_build(output_dir)?; let env_slice: Vec<(&str, &str)> = lto_env @@ -183,14 +197,14 @@ impl Linker for ArmLinker { let rsp_arg = format!("@{}", rsp_path.display()); run_command( &[args[0].as_str(), &rsp_arg], - None, + link_cwd, Some(&env_slice), link_timeout, ) .await? } else { let args_ref: Vec<&str> = args.iter().map(|s| s.as_str()).collect(); - run_command(&args_ref, None, Some(&env_slice), link_timeout).await? + run_command(&args_ref, link_cwd, Some(&env_slice), link_timeout).await? }; if !result.success() { diff --git a/crates/fbuild-build-arm/src/nrf52/nrf52_linker.rs b/crates/fbuild-build-arm/src/nrf52/nrf52_linker.rs index 42602ca5a..94618d78b 100644 --- a/crates/fbuild-build-arm/src/nrf52/nrf52_linker.rs +++ b/crates/fbuild-build-arm/src/nrf52/nrf52_linker.rs @@ -9,7 +9,7 @@ use fbuild_core::subprocess::run_command; use fbuild_core::{BuildProfile, Result, SizeInfo}; use super::mcu_config::Nrf52McuConfig; -use crate::linker::{LinkExtraArgs, Linker}; +use crate::linker::{link_cwd_for, LinkExtraArgs, Linker}; /// NRF52-specific linker using arm-none-eabi-gcc (link driver), ar, objcopy, size. pub struct Nrf52Linker { @@ -124,11 +124,25 @@ impl Linker for Nrf52Linker { .map(|(k, v)| (k.as_str(), v.as_str())) .collect(); + // Run the link in the firmware's own output directory rather than + // inheriting the daemon's cwd (typically the user's project root): + // arm-none-eabi-gcc hands off to collect2 / lto-wrapper, which write + // scratch files relative to the process cwd. See FastLED/fbuild#1269 + // (and #1268, which fixed the same bug for teensy). + let link_cwd = link_cwd_for( + output_dir, + objects + .iter() + .chain(archives.iter()) + .chain(self.linker_search_dirs.iter()) + .chain(std::iter::once(&self.linker_script_path)), + ); + let args_ref: Vec<&str> = args.iter().map(|s| s.as_str()).collect(); // FastLED/fbuild#809: bound the link step at 3 min. let result = run_command( &args_ref, - None, + link_cwd, Some(&env_slice), Some(std::time::Duration::from_secs(180)), ) diff --git a/crates/fbuild-build-arm/src/renesas/renesas_linker.rs b/crates/fbuild-build-arm/src/renesas/renesas_linker.rs index e7b72f66f..7a2e1c2c9 100644 --- a/crates/fbuild-build-arm/src/renesas/renesas_linker.rs +++ b/crates/fbuild-build-arm/src/renesas/renesas_linker.rs @@ -9,7 +9,7 @@ use fbuild_core::subprocess::run_command; use fbuild_core::{BuildProfile, Result, SizeInfo}; use super::mcu_config::RenesasMcuConfig; -use crate::linker::{LinkExtraArgs, Linker}; +use crate::linker::{link_cwd_for, LinkExtraArgs, Linker}; /// Renesas-specific linker using arm-none-eabi-gcc (link driver), ar, objcopy, size. pub struct RenesasLinker { @@ -129,11 +129,26 @@ impl Linker for RenesasLinker { .map(|(k, v)| (k.as_str(), v.as_str())) .collect(); + // Run the link in the firmware's own output directory rather than + // inheriting the daemon's cwd (typically the user's project root): + // arm-none-eabi-gcc hands off to collect2 / lto-wrapper, which write + // scratch files relative to the process cwd. See FastLED/fbuild#1269 + // (and #1268, which fixed the same bug for teensy). The `-L` variant + // dir and the `libfsp.a` path are both derived from + // `linker_script_path`, so checking it covers them too. + let link_cwd = link_cwd_for( + output_dir, + objects + .iter() + .chain(archives.iter()) + .chain(std::iter::once(&self.linker_script_path)), + ); + let args_ref: Vec<&str> = args.iter().map(|s| s.as_str()).collect(); // FastLED/fbuild#809: bound the link step at 3 min. let result = run_command( &args_ref, - None, + link_cwd, Some(&env_slice), Some(std::time::Duration::from_secs(180)), ) diff --git a/crates/fbuild-build-arm/src/sam/sam_linker.rs b/crates/fbuild-build-arm/src/sam/sam_linker.rs index d9fb9b395..049c7df24 100644 --- a/crates/fbuild-build-arm/src/sam/sam_linker.rs +++ b/crates/fbuild-build-arm/src/sam/sam_linker.rs @@ -9,7 +9,7 @@ use fbuild_core::subprocess::run_command; use fbuild_core::{BuildProfile, Result, SizeInfo}; use super::mcu_config::SamMcuConfig; -use crate::linker::{LinkExtraArgs, Linker}; +use crate::linker::{link_cwd_for, LinkExtraArgs, Linker}; /// SAM-specific linker using arm-none-eabi-gcc (link driver), ar, objcopy, size. pub struct SamLinker { @@ -145,11 +145,33 @@ impl Linker for SamLinker { .map(|(k, v)| (k.as_str(), v.as_str())) .collect(); + // Run the link in the firmware's own output directory rather than + // inheriting the daemon's cwd (typically the user's project root): + // arm-none-eabi-gcc hands off to collect2 / lto-wrapper, which write + // scratch files relative to the process cwd. See FastLED/fbuild#1269 + // (and #1268, which fixed the same bug for teensy). + let mut cwd_sensitive: Vec<&Path> = vec![&self.linker_script_path]; + cwd_sensitive.extend(objects.iter().map(|p| p.as_path())); + cwd_sensitive.extend(archives.iter().map(|p| p.as_path())); + cwd_sensitive.extend(self.extra_lib_dirs.iter().map(|p| p.as_path())); + // `extra_libs` entries that name a file rather than a `-l` are + // passed through verbatim, so they are cwd-sensitive too. + cwd_sensitive.extend( + self.extra_libs + .iter() + .filter(|lib| { + !lib.starts_with("-l") + && (lib.contains(std::path::MAIN_SEPARATOR) || lib.contains('/')) + }) + .map(Path::new), + ); + let link_cwd = link_cwd_for(output_dir, &cwd_sensitive); + let args_ref: Vec<&str> = args.iter().map(|s| s.as_str()).collect(); // FastLED/fbuild#809: bound the link step at 3 min. let result = run_command( &args_ref, - None, + link_cwd, Some(&env_slice), Some(std::time::Duration::from_secs(180)), ) diff --git a/crates/fbuild-build-arm/src/silabs/silabs_linker.rs b/crates/fbuild-build-arm/src/silabs/silabs_linker.rs index d77fe6e91..57ebe4894 100644 --- a/crates/fbuild-build-arm/src/silabs/silabs_linker.rs +++ b/crates/fbuild-build-arm/src/silabs/silabs_linker.rs @@ -9,7 +9,7 @@ use fbuild_core::subprocess::run_command; use fbuild_core::{BuildProfile, Result, SizeInfo}; use super::mcu_config::SilabsMcuConfig; -use crate::linker::{LinkExtraArgs, Linker}; +use crate::linker::{link_cwd_for, LinkExtraArgs, Linker}; /// Silicon Labs-specific linker using arm-none-eabi-gcc (link driver), ar, objcopy, size. pub struct SilabsLinker { @@ -132,11 +132,25 @@ impl Linker for SilabsLinker { .map(|(k, v)| (k.as_str(), v.as_str())) .collect(); + // Run the link in the firmware's own output directory rather than + // inheriting the daemon's cwd (typically the user's project root): + // arm-none-eabi-gcc hands off to collect2 / lto-wrapper, which write + // scratch files relative to the process cwd. See FastLED/fbuild#1269 + // (and #1268, which fixed the same bug for teensy). + let mut cwd_sensitive: Vec<&Path> = vec![&self.linker_script_path]; + cwd_sensitive.extend(objects.iter().map(|p| p.as_path())); + cwd_sensitive.extend(archives.iter().map(|p| p.as_path())); + cwd_sensitive.extend(self.precompiled_libs.iter().map(|p| p.as_path())); + if let Some(gsdk) = &self.precompiled_gsdk { + cwd_sensitive.push(gsdk); + } + let link_cwd = link_cwd_for(output_dir, &cwd_sensitive); + let args_ref: Vec<&str> = args.iter().map(|s| s.as_str()).collect(); // FastLED/fbuild#809: bound the link step at 3 min. let result = run_command( &args_ref, - None, + link_cwd, Some(&env_slice), Some(std::time::Duration::from_secs(180)), ) diff --git a/crates/fbuild-build-arm/src/stm32/orchestrator/arduino_mbed.rs b/crates/fbuild-build-arm/src/stm32/orchestrator/arduino_mbed.rs index 79acf6800..7d384a2c4 100644 --- a/crates/fbuild-build-arm/src/stm32/orchestrator/arduino_mbed.rs +++ b/crates/fbuild-build-arm/src/stm32/orchestrator/arduino_mbed.rs @@ -286,6 +286,15 @@ async fn preprocess_linker_script( let args_ref: Vec<&str> = args.iter().map(|s| s.as_str()).collect(); // FastLED/fbuild#809: linker-script preprocessing is a trivial // g++ -E invocation; bound to 30s. + // + // Deliberately keeps the inherited cwd (FastLED/fbuild#1269): this is not + // a link. `-E -P` stops after preprocessing, so no `collect2` / + // `lto-wrapper` is spawned and nothing writes cwd-relative scratch files — + // the stray `-r` class of junk cannot originate here. Both the input + // (`variant_dir`) and the output (`build_dir`) are already explicit paths, + // so the cwd does not affect where anything lands. The `None` env is + // likewise correct: `link_env_for_build` only redirects GCC's *LTO* temp + // dir, which a preprocess-only run never uses. let result = fbuild_core::subprocess::run_command( &args_ref, None, diff --git a/crates/fbuild-build-arm/src/teensy/teensy_linker.rs b/crates/fbuild-build-arm/src/teensy/teensy_linker.rs index 1e2ae0c6d..b2c4b3f64 100644 --- a/crates/fbuild-build-arm/src/teensy/teensy_linker.rs +++ b/crates/fbuild-build-arm/src/teensy/teensy_linker.rs @@ -9,7 +9,7 @@ use fbuild_core::subprocess::run_command; use fbuild_core::{BuildProfile, Result, SizeInfo}; use super::mcu_config::TeensyMcuConfig; -use crate::linker::{LinkExtraArgs, Linker, LinkerScripts}; +use crate::linker::{link_cwd_for, LinkExtraArgs, Linker, LinkerScripts}; /// Teensy-specific linker using arm-none-eabi-gcc (link driver), ar, objcopy, size. pub struct TeensyLinker { @@ -120,21 +120,6 @@ impl TeensyLinker { } } -/// Working directory to run the teensy link in. -/// -/// Returns the output directory when it is absolute, so collect2 / -/// lto-wrapper scratch files land in the gitignored build tree instead of -/// the daemon's inherited cwd (the user's project root). Returns `None` for -/// a relative `output_dir`, where changing cwd would also relocate the -/// relative `-o` path — preserving the historical behaviour for that case. -fn link_cwd_for(output_dir: &Path) -> Option<&Path> { - if output_dir.is_absolute() { - Some(output_dir) - } else { - None - } -} - #[async_trait::async_trait] impl Linker for TeensyLinker { async fn archive(&self, objects: &[PathBuf], output: &Path) -> Result<()> { @@ -168,12 +153,19 @@ impl Linker for TeensyLinker { // in the build dir). This completes FastLED/fbuild#261, which // redirected only the env-based LTO temp dir and left cwd inherited. // - // Safe because every link argument is absolute: `-o` is - // `output_dir.join(...)`, objects/archives arrive absolute, and - // `-T