fix(avr): archive framework objects with gcc-ar (fixes #304)

crates/fbuild-build/src/avr/avr_linker.rs

@@ -15,6 +15,7 @@ use crate::linker::{LinkExtraArgs, Linker};
 pub struct AvrLinker {
     gcc_path: PathBuf,
     ar_path: PathBuf,
+    gcc_ar_path: PathBuf,
     objcopy_path: PathBuf,
     size_path: PathBuf,
     mcu: String,
@@ -30,6 +31,7 @@ impl AvrLinker {
     pub fn new(
         gcc_path: PathBuf,
         ar_path: PathBuf,
+        gcc_ar_path: PathBuf,
         objcopy_path: PathBuf,
         size_path: PathBuf,
         mcu: &str,
@@ -42,6 +44,7 @@ impl AvrLinker {
         Self {
             gcc_path,
             ar_path,
+            gcc_ar_path,
             objcopy_path,
             size_path,
             mcu: mcu.to_string(),
@@ -54,11 +57,32 @@ impl AvrLinker {
     }
 }
 
+/// Partition a mixed list of link inputs into (existing archives, raw objects).
+///
+/// Inputs ending in `.a` (case-sensitive — matches the toolchain convention) are
+/// real static archives and should be passed straight to the linker. Everything
+/// else (typically `.o` files coming from the framework compile step) is treated
+/// as a raw object that needs to be wrapped in an archive so the linker pulls in
+/// only members whose strong symbols are referenced.
+///
+/// Lives at module scope so unit tests can exercise the partition logic without
+/// invoking `avr-gcc-ar`.
+pub(crate) fn partition_link_inputs(inputs: &[PathBuf]) -> (Vec<PathBuf>, Vec<PathBuf>) {
+    let (existing_archives, raw_objects): (Vec<PathBuf>, Vec<PathBuf>) = inputs
+        .iter()
+        .cloned()
+        .partition(|p| p.extension().and_then(|e| e.to_str()) == Some("a"));
+    (existing_archives, raw_objects)
+}
+
 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).
+    /// (see #305 — assert that `-Wl,-Map=` is present). `archives` is the
+    /// already-partitioned archive list (real `.a` files + any framework
+    /// archive synthesised by `link()` for #304); this function does not
+    /// touch the filesystem.
     fn build_link_args(
         &self,
         objects: &[PathBuf],
@@ -92,8 +116,11 @@ impl AvrLinker {
             args.push(obj.to_string_lossy().to_string());
         }
 
-        // Core objects passed directly (not archived) for LTO compatibility
-        // Python fbuild does the same: archive breaks LTO symbol visibility
+        // Archives (already partitioned by `link()`): the framework archive
+        // synthesised from raw `.o` framework objects + any real `.a` files
+        // passed in. Archive-member selection drops unreferenced members so
+        // unused-but-not-eliminable ISRs (e.g. Tone.cpp __vector_1/3 on AVR)
+        // no longer get pulled in. See FastLED/fbuild#304.
         for archive in archives {
             args.push(archive.to_string_lossy().to_string());
         }
@@ -123,7 +150,33 @@ impl Linker for AvrLinker {
         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);
+        // Partition: real `.a` archives are pass-through; raw `.o` files get
+        // archived into `libframework.a` so the linker only pulls in members
+        // whose strong symbols are referenced. Without this, unused-but-not-
+        // eliminable ISRs (e.g. Tone.cpp __vector_1/__vector_3 in the AVR
+        // framework) are dragged in via weak-symbol override of
+        // __vector_default, inflating the binary by ~10% on attiny85.
+        // See FastLED/fbuild#304.
+        //
+        // Use gcc-ar (not plain ar) so the LTO bytecode plugin index is
+        // written — preserves -fuse-linker-plugin compatibility, which the
+        // default avr.json profile enables via `-flto -fuse-linker-plugin`.
+        let (existing_archives, raw_objects) = partition_link_inputs(archives);
+        let mut linker_archives: Vec<PathBuf> =
+            Vec::with_capacity(existing_archives.len() + usize::from(!raw_objects.is_empty()));
+        if !raw_objects.is_empty() {
+            let framework_archive = output_dir.join("libframework.a");
+            crate::linker::LinkerBase::archive(
+                &self.gcc_ar_path,
+                &raw_objects,
+                &framework_archive,
+                "avr-gcc-ar",
+            )?;
+            linker_archives.push(framework_archive);
+        }
+        linker_archives.extend(existing_archives);
+
+        let args = self.build_link_args(objects, &linker_archives, output_dir, &elf_path, extra);
 
         if self.verbose {
             eprintln!("link: {}", args.join(" "));
@@ -191,6 +244,7 @@ mod tests {
         let linker = AvrLinker::new(
             PathBuf::from("/bin/avr-gcc"),
             PathBuf::from("/bin/avr-ar"),
+            PathBuf::from("/bin/avr-gcc-ar"),
             PathBuf::from("/bin/avr-objcopy"),
             PathBuf::from("/bin/avr-size"),
             "atmega328p",
@@ -203,6 +257,63 @@ mod tests {
         assert_eq!(linker.mcu, "atmega328p");
         assert_eq!(linker.max_flash, Some(32256));
         assert_eq!(linker.max_ram, Some(2048));
+        assert_eq!(linker.gcc_ar_path, PathBuf::from("/bin/avr-gcc-ar"));
+    }
+
+    #[test]
+    fn link_partitions_archives_and_objects() {
+        // Regression test for FastLED/fbuild#304: link() must split mixed
+        // .a/.o inputs so .o files are archived (and thus subject to
+        // unreferenced-member elimination by the linker) while real .a
+        // files pass through unchanged.
+        let inputs = vec![
+            PathBuf::from("/tmp/Tone.cpp.o"),
+            PathBuf::from("/tmp/libfoo.a"),
+            PathBuf::from("/tmp/main.cpp.o"),
+        ];
+        let (archives, objects) = partition_link_inputs(&inputs);
+        assert_eq!(archives, vec![PathBuf::from("/tmp/libfoo.a")]);
+        assert_eq!(
+            objects,
+            vec![
+                PathBuf::from("/tmp/Tone.cpp.o"),
+                PathBuf::from("/tmp/main.cpp.o"),
+            ]
+        );
+    }
+
+    #[test]
+    fn partition_handles_all_archives() {
+        let inputs = vec![PathBuf::from("/tmp/a.a"), PathBuf::from("/tmp/b.a")];
+        let (archives, objects) = partition_link_inputs(&inputs);
+        assert_eq!(archives.len(), 2);
+        assert!(objects.is_empty());
+    }
+
+    #[test]
+    fn partition_handles_all_objects() {
+        let inputs = vec![PathBuf::from("/tmp/x.o"), PathBuf::from("/tmp/y.o")];
+        let (archives, objects) = partition_link_inputs(&inputs);
+        assert!(archives.is_empty());
+        assert_eq!(objects.len(), 2);
+    }
+
+    #[test]
+    fn partition_handles_empty() {
+        let (archives, objects) = partition_link_inputs(&[]);
+        assert!(archives.is_empty());
+        assert!(objects.is_empty());
+    }
+
+    #[test]
+    fn partition_treats_unknown_extensions_as_objects() {
+        // Defensive: anything that isn't .a (e.g. .lo, .obj, no extension)
+        // is treated as an object and archived. This is safer than letting
+        // an unknown extension fall through to a raw avr-gcc positional arg.
+        let inputs = vec![PathBuf::from("/tmp/weird.lo"), PathBuf::from("/tmp/no_ext")];
+        let (archives, objects) = partition_link_inputs(&inputs);
+        assert!(archives.is_empty());
+        assert_eq!(objects.len(), 2);
     }
 
     /// #305: every per-platform linker must emit a `firmware.map` next to
@@ -212,6 +323,7 @@ mod tests {
         let linker = AvrLinker::new(
             PathBuf::from("/bin/avr-gcc"),
             PathBuf::from("/bin/avr-ar"),
+            PathBuf::from("/bin/avr-gcc-ar"),
             PathBuf::from("/bin/avr-objcopy"),
             PathBuf::from("/bin/avr-size"),
             "atmega328p",

crates/fbuild-build/src/avr/orchestrator.rs

@@ -237,10 +237,13 @@ impl BuildOrchestrator for AvrOrchestrator {
         .with_build_unflags(ctx.build_unflags.clone())
         .with_eh_frame_policy(eh_frame_policy);
 
-        // 7. Create linker
+        // 7. Create linker — pass gcc-ar so framework .o inputs can be
+        // archived into libframework.a with the LTO bytecode plugin index
+        // intact (preserves `-fuse-linker-plugin`). See FastLED/fbuild#304.
         let linker = AvrLinker::new(
             toolchain.get_gcc_path(),
             toolchain.get_ar_path(),
+            toolchain.get_gcc_ar_path(),
             toolchain.get_objcopy_path(),
             toolchain.get_size_path(),
             &ctx.board.mcu,