diff --git a/Cargo.lock b/Cargo.lock index 36b7f2fc..f738e0b2 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -988,7 +988,7 @@ checksum = "37909eebbb50d72f9059c3b6d82c0463f2ff062c9e95845c43a6c9c0355411be" [[package]] name = "fbuild-bench-fastled-examples" -version = "2.5.37" +version = "2.5.38" dependencies = [ "fbuild-core", "fbuild-library-select", @@ -1002,7 +1002,7 @@ dependencies = [ [[package]] name = "fbuild-build" -version = "2.5.37" +version = "2.5.38" dependencies = [ "async-trait", "blake3", @@ -1039,7 +1039,7 @@ dependencies = [ [[package]] name = "fbuild-build-arm" -version = "2.5.37" +version = "2.5.38" dependencies = [ "async-trait", "blake3", @@ -1073,7 +1073,7 @@ dependencies = [ [[package]] name = "fbuild-build-engine" -version = "2.5.37" +version = "2.5.38" dependencies = [ "async-trait", "blake3", @@ -1107,7 +1107,7 @@ dependencies = [ [[package]] name = "fbuild-build-esp" -version = "2.5.37" +version = "2.5.38" dependencies = [ "async-trait", "blake3", @@ -1142,7 +1142,7 @@ dependencies = [ [[package]] name = "fbuild-build-mcu" -version = "2.5.37" +version = "2.5.38" dependencies = [ "async-trait", "blake3", @@ -1176,7 +1176,7 @@ dependencies = [ [[package]] name = "fbuild-cli" -version = "2.5.37" +version = "2.5.38" dependencies = [ "blake3", "clap", @@ -1208,7 +1208,7 @@ dependencies = [ [[package]] name = "fbuild-config" -version = "2.5.37" +version = "2.5.38" dependencies = [ "fbuild-core", "fbuild-paths", @@ -1223,7 +1223,7 @@ dependencies = [ [[package]] name = "fbuild-core" -version = "2.5.37" +version = "2.5.38" dependencies = [ "async-trait", "fs2", @@ -1252,7 +1252,7 @@ dependencies = [ [[package]] name = "fbuild-daemon" -version = "2.5.37" +version = "2.5.38" dependencies = [ "async-trait", "axum 0.7.9", @@ -1291,7 +1291,7 @@ dependencies = [ [[package]] name = "fbuild-deploy" -version = "2.5.37" +version = "2.5.38" dependencies = [ "async-trait", "espflash", @@ -1319,7 +1319,7 @@ dependencies = [ [[package]] name = "fbuild-header-scan" -version = "2.5.37" +version = "2.5.38" dependencies = [ "criterion", "fbuild-paths", @@ -1330,7 +1330,7 @@ dependencies = [ [[package]] name = "fbuild-library" -version = "2.5.37" +version = "2.5.38" dependencies = [ "async-trait", "axum 0.7.9", @@ -1362,7 +1362,7 @@ dependencies = [ [[package]] name = "fbuild-library-select" -version = "2.5.37" +version = "2.5.38" dependencies = [ "bincode", "blake3", @@ -1383,7 +1383,7 @@ dependencies = [ [[package]] name = "fbuild-packages" -version = "2.5.37" +version = "2.5.38" dependencies = [ "async-trait", "axum 0.7.9", @@ -1415,7 +1415,7 @@ dependencies = [ [[package]] name = "fbuild-packages-fetch" -version = "2.5.37" +version = "2.5.38" dependencies = [ "async-trait", "axum 0.7.9", @@ -1444,7 +1444,7 @@ dependencies = [ [[package]] name = "fbuild-paths" -version = "2.5.37" +version = "2.5.38" dependencies = [ "blake3", "fbuild-core", @@ -1456,7 +1456,7 @@ dependencies = [ [[package]] name = "fbuild-python" -version = "2.5.37" +version = "2.5.38" dependencies = [ "base64", "fbuild-core", @@ -1480,7 +1480,7 @@ dependencies = [ [[package]] name = "fbuild-serial" -version = "2.5.37" +version = "2.5.38" dependencies = [ "async-trait", "base64", @@ -1503,7 +1503,7 @@ dependencies = [ [[package]] name = "fbuild-test-support" -version = "2.5.37" +version = "2.5.38" dependencies = [ "fbuild-config", "fbuild-core", @@ -1523,7 +1523,7 @@ dependencies = [ [[package]] name = "fbuild-toolchain" -version = "2.5.37" +version = "2.5.38" dependencies = [ "async-trait", "axum 0.7.9", diff --git a/Cargo.toml b/Cargo.toml index 6e6dd725..660f545e 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -93,7 +93,7 @@ targets = [ ] [workspace.package] -version = "2.5.37" +version = "2.5.38" edition = "2021" rust-version = "1.95.0" license = "AGPL-3.0-only" diff --git a/crates/fbuild-build-engine/src/symbol_analyzer/README.md b/crates/fbuild-build-engine/src/symbol_analyzer/README.md index 8c1a05ff..7b3b56fc 100644 --- a/crates/fbuild-build-engine/src/symbol_analyzer/README.md +++ b/crates/fbuild-build-engine/src/symbol_analyzer/README.md @@ -18,3 +18,9 @@ gate: `SidecarOptions`, `write_sidecar_dot_files`, plus the internal graph-section helpers. - `tests.rs` — the unit tests for everything in the module. + +`elf_references.rs` reads absolute method pointers from allocated Itanium +vtables, including AVR word addresses and ARM Thumb pointers. +`reference_analysis.rs` attributes disassembly by address and name, classifies +weak objects from their ELF sections, and records confirmed entry roots and +unexplained retention without guessing linker `KEEP` rules. diff --git a/crates/fbuild-build-engine/src/symbol_analyzer/elf_references.rs b/crates/fbuild-build-engine/src/symbol_analyzer/elf_references.rs new file mode 100644 index 00000000..5e410c08 --- /dev/null +++ b/crates/fbuild-build-engine/src/symbol_analyzer/elf_references.rs @@ -0,0 +1,498 @@ +//! Verified static function pointers in allocated Itanium C++ vtables. +//! This deliberately does not scan arbitrary data for address-shaped integers. +use std::collections::BTreeMap; +use std::path::Path; + +use fbuild_core::{FbuildError, Result}; +use object::{Object, ObjectSection, ObjectSymbol}; + +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct StaticDataReference { + pub source_name: String, + pub source_address: u64, + pub target_name: String, + pub target_address: u64, + pub offset: u64, +} + +pub fn read_vtable_references(elf_path: &Path) -> Result> { + let bytes = std::fs::read(elf_path).map_err(|e| { + FbuildError::BuildFailed(format!("could not read ELF {}: {e}", elf_path.display())) + })?; + references_from_bytes(&bytes) +} + +fn references_from_bytes(bytes: &[u8]) -> Result> { + let file = object::File::parse(bytes) + .map_err(|e| FbuildError::BuildFailed(format!("ELF static reference parse failed: {e}")))?; + if file.format() != object::BinaryFormat::Elf { + return Err(FbuildError::BuildFailed( + "static references require ELF".into(), + )); + } + if file.kind() == object::ObjectKind::Dynamic { + return Err(FbuildError::BuildFailed( + "Static references for ET_DYN/PIE/shared ELF require dynamic relocations, which are not supported.".into(), + )); + } + if file.kind() != object::ObjectKind::Executable { + return Err(FbuildError::BuildFailed( + "Static references require a final ET_EXEC executable ELF, not a relocatable object." + .into(), + )); + } + // AVR uses 16-bit ABI pointers even though its ELF container is ELF32. + let width = if file.architecture() == object::Architecture::Avr { + 2 + } else if file.is_64() { + 8 + } else { + 4 + }; + let normalize = |address: u64| { + if file.architecture() == object::Architecture::Arm { + address & !1 + } else { + address + } + }; + let functions = function_symbols(&file, normalize)?; + let mut edges = Vec::new(); + for symbol in file.symbols() { + let Ok(name) = symbol.name() else { continue }; + if !name.starts_with("_ZTV") || !symbol.is_definition() || symbol.size() == 0 { + continue; + } + let Some(index) = symbol.section_index() else { + continue; + }; + let section = file + .section_by_index(index) + .map_err(|e| FbuildError::BuildFailed(e.to_string()))?; + if !matches!(section.flags(), object::SectionFlags::Elf { sh_flags } if sh_flags & u64::from(object::elf::SHF_ALLOC) != 0) + { + continue; + } + let Some(start) = symbol.address().checked_sub(section.address()) else { + continue; + }; + let data = section + .data() + .map_err(|e| FbuildError::BuildFailed(e.to_string()))?; + let Some(end) = start.checked_add(symbol.size()) else { + continue; + }; + let (Ok(start), Ok(end)) = (usize::try_from(start), usize::try_from(end)) else { + continue; + }; + let Some(table) = data.get(start..end) else { + continue; + }; + // Itanium ABI: the first slots are offset-to-top and RTTI, not methods. + for (slot, word) in table.chunks_exact(width).enumerate().skip(2) { + let pointer = read_pointer(word, file.is_little_endian()); + if pointer == 0 { + continue; + } + // AVR program pointers address words; ELF function symbols address bytes. + let address = if file.architecture() == object::Architecture::Avr { + let Some(address) = pointer.checked_mul(2) else { + continue; + }; + address + } else { + normalize(pointer) + }; + if let Some(targets) = functions.get(&address) { + for target in targets { + edges.push(StaticDataReference { + source_name: name.to_owned(), + source_address: symbol.address(), + target_name: target.clone(), + target_address: address, + offset: (slot * width) as u64, + }); + } + } + } + } + edges.sort_by(|a, b| { + (&a.source_name, a.source_address, a.offset, &a.target_name).cmp(&( + &b.source_name, + b.source_address, + b.offset, + &b.target_name, + )) + }); + edges.dedup(); + Ok(edges) +} + +fn function_symbols( + file: &object::File<'_>, + normalize: impl Fn(u64) -> u64, +) -> Result>> { + let mut functions: BTreeMap> = BTreeMap::new(); + for symbol in file.symbols() { + if symbol.is_definition() && symbol.kind() == object::SymbolKind::Text { + let Some(index) = symbol.section_index() else { + continue; + }; + let section = file + .section_by_index(index) + .map_err(|e| FbuildError::BuildFailed(e.to_string()))?; + let executable = u64::from(object::elf::SHF_ALLOC | object::elf::SHF_EXECINSTR); + if !matches!(section.flags(), object::SectionFlags::Elf { sh_flags } if sh_flags & executable == executable) + { + continue; + } + if let Ok(name) = symbol.name() { + functions + .entry(normalize(symbol.address())) + .or_default() + .push(name.to_owned()); + } + } + } + Ok(functions) +} + +fn read_pointer(word: &[u8], little: bool) -> u64 { + if little { + word.iter().rev().fold(0, |n, b| (n << 8) | u64::from(*b)) + } else { + word.iter().fold(0, |n, b| (n << 8) | u64::from(*b)) + } +} + +#[cfg(test)] +mod tests { + use super::*; + use object::write::{Object as WriteObject, Symbol, SymbolSection}; + use object::{ + Architecture, BinaryFormat, Endianness, SectionKind, SymbolFlags, SymbolKind, SymbolScope, + }; + + fn fixture(architecture: Architecture, endian: Endianness, width: usize) -> Vec { + let mut elf = WriteObject::new(BinaryFormat::Elf, architecture, endian); + let text = elf.add_section(Vec::new(), b".text".to_vec(), SectionKind::Text); + elf.append_section_data(text, &[0; 32], 4); + let data = elf.add_section(Vec::new(), b".rodata".to_vec(), SectionKind::ReadOnlyData); + let mut pointers = Vec::new(); + let address = if architecture == Architecture::Avr { + 8_u64 + } else if architecture == Architecture::Arm { + 17_u64 + } else { + 16_u64 + }; + let encoded = if endian == Endianness::Little { + address.to_le_bytes() + } else { + address.to_be_bytes() + }; + // Even address-shaped header words must not be interpreted as methods. + for _ in 0..3 { + pointers.extend_from_slice(if endian == Endianness::Little { + &encoded[..width] + } else { + &encoded[8 - width..] + }); + } + elf.append_section_data(data, &pointers, width as u64); + for (name, value, size, kind, section) in [ + ("method", 16, 8, SymbolKind::Text, text), + ("method_alias", 16, 8, SymbolKind::Text, text), + ("_ZTV1A", 0, pointers.len() as u64, SymbolKind::Data, data), + ( + "ordinary_scalar", + 0, + pointers.len() as u64, + SymbolKind::Data, + data, + ), + ] { + elf.add_symbol(Symbol { + name: name.as_bytes().to_vec(), + value, + size, + kind, + scope: SymbolScope::Linkage, + weak: false, + section: SymbolSection::Section(section), + flags: SymbolFlags::None, + }); + } + let mut bytes = elf.write().unwrap(); + // Raw final-image fixture: writer emits ET_REL, so mark this explicit + // address fixture ET_EXEC. It has no unresolved relocations. + bytes[16..18].copy_from_slice(if endian == Endianness::Little { + &[2, 0] + } else { + &[0, 2] + }); + bytes + } + + #[test] + fn vtable_edges_are_typed_and_preserve_aliases_across_width_and_endian() { + for (architecture, endian, width) in [ + (Architecture::I386, Endianness::Little, 4), + (Architecture::X86_64, Endianness::Little, 8), + (Architecture::PowerPc, Endianness::Big, 4), + (Architecture::PowerPc64, Endianness::Big, 8), + (Architecture::Arm, Endianness::Little, 4), + (Architecture::Avr, Endianness::Little, 2), + ] { + let edges = references_from_bytes(&fixture(architecture, endian, width)).unwrap(); + assert_eq!(edges.len(), 2, "{architecture:?}"); + assert!(edges.iter().all(|edge| edge.source_name == "_ZTV1A" + && edge.target_address == 16 + && edge.offset == 2 * width as u64)); + } + } + + #[test] + fn relocatable_vtable_bytes_are_not_claimed_as_final_image_references() { + let mut bytes = fixture(Architecture::I386, Endianness::Little, 4); + bytes[16..18].copy_from_slice(&[1, 0]); + assert!(references_from_bytes(&bytes).is_err()); + } + + #[test] + fn dynamic_vtable_bytes_require_relocation_support() { + let mut bytes = fixture(Architecture::X86_64, Endianness::Little, 8); + bytes[16..18].copy_from_slice(&[3, 0]); + let error = references_from_bytes(&bytes).unwrap_err().to_string(); + assert!(error.contains("dynamic relocations")); + } + + #[test] + fn malformed_elf_is_not_silently_reported_as_an_empty_graph() { + assert!(references_from_bytes(b"not an ELF").is_err()); + } +} + +#[cfg(test)] +mod linked_tests { + use super::*; + + #[test] + #[ignore = "requires native C++ compiler c++ to link a real vtable fixture"] + fn linked_cpp_vtable_retains_method_through_static_pointer() { + let dir = tempfile::tempdir().unwrap(); + let source = dir.path().join("vtable.cpp"); + let binary = dir.path().join("vtable.elf"); + std::fs::write( + &source, + r#" + struct A { virtual int value(); }; + int A::value() { return 42; } + A instance; + int main() { return instance.value(); } + "#, + ) + .unwrap(); + let output = fbuild_core::subprocess::run_command_blocking( + &[ + "c++", + "-fno-pie", + "-no-pie", + "-fno-rtti", + "-o", + binary.to_str().unwrap(), + source.to_str().unwrap(), + ], + Some(dir.path()), + None, + Some(std::time::Duration::from_secs(30)), + ) + .unwrap(); + assert!(output.success(), "{}", output.stderr); + let edges = read_vtable_references(&binary).unwrap(); + assert!(edges.iter().any(|edge| edge.source_name == "_ZTV1A" + && edge.target_name == "_ZN1A5valueEv" + && edge.offset == 16)); + } +} + +#[cfg(test)] +mod integration_tests { + use super::*; + use fbuild_core::symbol_analysis::{AnalysisStatus, ReferenceKind}; + + #[tokio::test] + #[ignore = "requires native C++ compiler c++, nm, c++filt and objdump"] + async fn dynamic_pie_reports_static_analysis_error_instead_of_verified_edges() { + let dir = tempfile::tempdir().unwrap(); + let source = dir.path().join("pie.cpp"); + let binary = dir.path().join("pie.elf"); + std::fs::write(&source, "struct A { virtual int value(); }; int A::value() { return 42; } A instance; int main() { return instance.value(); }").unwrap(); + let output = fbuild_core::subprocess::run_command_blocking( + &[ + "c++", + "-fPIE", + "-pie", + "-fno-rtti", + "-o", + binary.to_str().unwrap(), + source.to_str().unwrap(), + ], + Some(dir.path()), + None, + Some(std::time::Duration::from_secs(30)), + ) + .unwrap(); + assert!(output.success(), "{}", output.stderr); + let bytes = std::fs::read(&binary).unwrap(); + let elf = object::File::parse(bytes.as_slice()).unwrap(); + assert_eq!(elf.kind(), object::ObjectKind::Dynamic); + assert!(elf.dynamic_relocations().is_some()); + let report = super::super::analyze_elf(super::super::AnalyzeConfig { + elf_path: &binary, + map_path: None, + nm_path: Path::new("nm"), + cppfilt_path: Some(Path::new("c++filt")), + objdump_path: Some(Path::new("objdump")), + }) + .await + .unwrap(); + assert_eq!( + report.reference_analysis.disassembly.status, + AnalysisStatus::Analyzed + ); + assert_eq!( + report.reference_analysis.static_data.status, + AnalysisStatus::Error + ); + assert!( + report + .reference_analysis + .static_data + .reason + .as_deref() + .unwrap() + .contains("dynamic relocations") + ); + assert!( + !report + .reference_analysis + .edges + .iter() + .any(|edge| edge.kind == ReferenceKind::StaticData) + ); + } + + #[tokio::test] + #[ignore = "requires native C++ compiler c++, nm, c++filt and objdump"] + async fn linked_weak_vtable_has_verified_incoming_edge_in_final_report() { + let dir = tempfile::tempdir().unwrap(); + let source = dir.path().join("virtual.cpp"); + let binary = dir.path().join("virtual.elf"); + std::fs::write( + &source, + r#" + struct A { virtual int value(); }; + int A::value() { return 42; } + A instance; + int main() { return instance.value(); } + "#, + ) + .unwrap(); + let output = fbuild_core::subprocess::run_command_blocking( + &[ + "c++", + "-fno-pie", + "-no-pie", + "-fno-rtti", + "-o", + binary.to_str().unwrap(), + source.to_str().unwrap(), + ], + Some(dir.path()), + None, + Some(std::time::Duration::from_secs(30)), + ) + .unwrap(); + assert!(output.success(), "{}", output.stderr); + let report = super::super::analyze_elf(super::super::AnalyzeConfig { + elf_path: &binary, + map_path: None, + nm_path: Path::new("nm"), + cppfilt_path: Some(Path::new("c++filt")), + objdump_path: Some(Path::new("objdump")), + }) + .await + .unwrap(); + let vtable = report + .symbols + .iter() + .find(|symbol| symbol.mangled == "_ZTV1A") + .expect("allocated weak vtable must be included"); + let method = report + .symbols + .iter() + .find(|symbol| symbol.mangled == "_ZN1A5valueEv") + .unwrap(); + assert!(vtable.size > 0); + let analysis = &report.reference_analysis; + assert_eq!(analysis.disassembly.status, AnalysisStatus::Analyzed); + assert_eq!(analysis.static_data.status, AnalysisStatus::Analyzed); + assert!( + analysis + .edges + .iter() + .any(|edge| edge.kind == ReferenceKind::StaticData + && edge.source.name == vtable.mangled + && edge.source.address == vtable.address + && edge.target.name == method.mangled + && edge.target.address == method.address + && edge.offset == Some(16)) + ); + assert!(analysis.roots.iter().any(|root| root.kind == "entry_point")); + assert!( + !analysis + .unexplained + .iter() + .any(|symbol| symbol.name == method.mangled && symbol.address == method.address) + ); + assert_partial_analysis(&binary, &report, &method.mangled, dir.path()).await; + } + + async fn assert_partial_analysis( + binary: &Path, + report: &fbuild_core::symbol_analysis::FineGrainedSymbolMap, + method: &str, + dir: &Path, + ) { + let missing = dir.join("missing-objdump"); + for (tool, expected) in [ + (None, AnalysisStatus::Unavailable), + (Some(missing.as_path()), AnalysisStatus::Error), + ] { + let partial = super::super::analyze_elf(super::super::AnalyzeConfig { + elf_path: binary, + map_path: None, + nm_path: Path::new("nm"), + cppfilt_path: Some(Path::new("c++filt")), + objdump_path: tool, + }) + .await + .unwrap(); + assert_eq!(partial.reference_analysis.disassembly.status, expected); + assert!(partial.reference_analysis.disassembly.reason.is_some()); + assert_eq!( + partial.reference_analysis.static_data.status, + AnalysisStatus::Analyzed + ); + assert_eq!(partial.total_flash, report.total_flash); + assert_eq!(partial.total_ram, report.total_ram); + assert_eq!(partial.image_flash, report.image_flash); + assert!( + partial.reference_analysis.edges.iter().any(|edge| edge.kind + == ReferenceKind::StaticData + && edge.target.name == method) + ); + } + } +} diff --git a/crates/fbuild-build-engine/src/symbol_analyzer/markdown.rs b/crates/fbuild-build-engine/src/symbol_analyzer/markdown.rs index bebae6e2..7ab98e95 100644 --- a/crates/fbuild-build-engine/src/symbol_analyzer/markdown.rs +++ b/crates/fbuild-build-engine/src/symbol_analyzer/markdown.rs @@ -6,6 +6,7 @@ //! `SidecarOptions`) is re-exported from the parent module for //! back-compat. +use std::fmt::Write as _; use std::path::Path; use fbuild_core::symbol_analysis::graph::{Direction, rank_callees_dual, rank_callers_dual}; @@ -193,13 +194,11 @@ fn emit_backref_graph_section( let _ = writeln!( out, "## Top {limit} symbol graphs\n\n\ - For each symbol below: a bidirectional `dot` block (callers on \ - the back-edge side, callees on the forward-edge side), plus a \ - dual-ranked \"Top callees\" sub-table. The forward edges come \ - from per-symbol `references_to` (objdump-derived), so the AI \ - can tell what `ClocklessIdf5` actually calls vs. what its \ - sibling symbols call. See fbuild #463 (backref walker) + \ - #471 (forward edges)." + For each symbol below: a bidirectional `dot` block with incoming \ + and outgoing references. Typed analysis distinguishes instruction \ + references, static pointers and fragment ownership using exact \ + addresses. Legacy reports retain their caller/callee tables. \ + An instruction reference does not necessarily represent a call." ); let _ = writeln!(out); let index = TuIndex::build(map); @@ -228,19 +227,27 @@ fn emit_backref_graph_section( let _ = writeln!(out, "- **Referenced by**: {} TUs", s.referenced_by.len()); let _ = writeln!( out, - "- **References (calls)**: {} symbols", + "- **Instruction references**: {} symbols", s.references_to.len() ); let _ = writeln!(out); - emit_dual_callers_subtable(out, map, s); - emit_dual_callees_subtable(out, map, s); + if map.reference_analysis.disassembly.status + == fbuild_core::symbol_analysis::AnalysisStatus::Analyzed + || map.reference_analysis.static_data.status + == fbuild_core::symbol_analysis::AnalysisStatus::Analyzed + { + emit_typed_references(out, &index, s); + } else { + emit_dual_callers_subtable(out, map, s); + emit_dual_callees_subtable(out, map, s); + } - let graph = BackrefGraph::build_with_index(map, &index, &s.mangled, &bidir_cfg); + let graph = BackrefGraph::build_for_symbol_with_index(map, &index, s, &bidir_cfg); let _ = writeln!(out, "
"); let _ = writeln!( out, - "Bidirectional graph (callers ← root → callees, Graphviz)" + "Bidirectional reference graph (incoming ← root → outgoing, Graphviz)" ); let _ = writeln!(out); let _ = writeln!(out, "```dot"); @@ -456,7 +463,7 @@ pub fn write_sidecar_dot_files( let rank = i + 1; let stem = sanitize_filename(&s.demangled); let path = graphs_dir.join(format!("{rank:04}_{stem}.dot")); - let graph = BackrefGraph::build_with_index(map, &index, &s.mangled, &options.config); + let graph = BackrefGraph::build_for_symbol_with_index(map, &index, s, &options.config); let dot = graph.to_dot(); if let Err(e) = std::fs::write(&path, dot) { tracing::warn!( @@ -503,3 +510,59 @@ fn format_referenced_by( // Pipe-escape so the joined string doesn't break MD table cells. parts.join(", ").replace('|', "\\|") } + +fn emit_typed_references( + out: &mut String, + index: &fbuild_core::symbol_analysis::TuIndex<'_>, + symbol: &fbuild_core::symbol_analysis::FineGrainedSymbol, +) { + let identity = fbuild_core::symbol_analysis::SymbolIdentity::from(symbol); + for (title, edges, incoming) in [ + ("Incoming references", index.incoming(&identity), true), + ("Outgoing references", index.outgoing(&identity), false), + ] { + if edges.is_empty() { + continue; + } + let _ = writeln!( + out, + "\n#### {title}\n\n| Symbol | Address | Bytes | Evidence |\n|---|---:|---:|---|" + ); + let mut ranked = edges.to_vec(); + ranked.sort_by_key(|edge| { + std::cmp::Reverse( + index + .symbol(if incoming { &edge.source } else { &edge.target }) + .map_or(0, |s| s.size), + ) + }); + for edge in ranked.iter().take(3) { + let endpoint = if incoming { &edge.source } else { &edge.target }; + let resolved = index.symbol(endpoint); + let label = resolved + .map_or(endpoint.name.as_str(), |s| s.demangled.as_str()) + .replace('|', "\\|"); + let kind = match edge.kind { + fbuild_core::symbol_analysis::ReferenceKind::Disassembly => "instruction reference", + fbuild_core::symbol_analysis::ReferenceKind::StaticData => "static pointer", + fbuild_core::symbol_analysis::ReferenceKind::FragmentOwner => "fragment owner", + }; + let offset = edge + .offset + .map_or(String::new(), |offset| format!(" + 0x{offset:x}")); + let _ = writeln!( + out, + "| `{label}` | 0x{:x} | {} | {kind}{offset} |", + endpoint.address, + resolved.map_or(0, |s| s.size) + ); + } + if edges.len() > 3 { + let _ = writeln!( + out, + "\n{} additional references in the JSON report.\n", + edges.len() - 3 + ); + } + } +} diff --git a/crates/fbuild-build-engine/src/symbol_analyzer/mod.rs b/crates/fbuild-build-engine/src/symbol_analyzer/mod.rs index fb8f4dda..671c780e 100644 --- a/crates/fbuild-build-engine/src/symbol_analyzer/mod.rs +++ b/crates/fbuild-build-engine/src/symbol_analyzer/mod.rs @@ -20,7 +20,9 @@ use fbuild_core::symbol_analysis::{ }; use fbuild_core::{FbuildError, Result}; +pub mod elf_references; pub mod markdown; +mod reference_analysis; #[cfg(test)] mod tests; @@ -454,32 +456,50 @@ pub async fn analyze_elf(cfg: AnalyzeConfig<'_>) -> Result ), } - // #471: per-symbol forward edges from `objdump -d`. When the - // analyzer was wired with an objdump path (typically from - // build_info.json::objdump_path), run it once on the linked ELF - // and pull `` annotations out of the disassembly. The - // resulting per-symbol callee map populates each row's - // `references_to` field, which the bidirectional graph + the - // dual-ranked callees sub-table consume. Failures are non-fatal - // — we'd rather ship a report without forward edges than fail - // the whole symbol-analysis post-link step. + use fbuild_core::symbol_analysis::{AnalysisPass, AnalysisStatus}; + map.reference_analysis.object_references = match &map_text { + Some((_, Ok(text))) if text.contains("Cross Reference Table") => AnalysisPass { + status: AnalysisStatus::Analyzed, + tool: None, + reason: None, + }, + Some((_, Ok(_))) => AnalysisPass { + reason: Some("Linker map has no Cross Reference Table.".into()), + ..Default::default() + }, + Some((_, Err(e))) => AnalysisPass { + status: AnalysisStatus::Error, + reason: Some(e.to_string()), + ..Default::default() + }, + None => AnalysisPass { + reason: Some("No linker map supplied.".into()), + ..Default::default() + }, + }; + let arm = match reference_analysis::probe_elf(&mut map, cfg.elf_path) { + Ok(arm) => arm, + Err(e) => { + map.reference_analysis.limitations.push(e.to_string()); + false + } + }; + map.reference_analysis.disassembly.tool = + cfg.objdump_path.map(|p| p.to_string_lossy().into_owned()); if let Some(objdump_path) = cfg.objdump_path { - match run_objdump_and_attribute(objdump_path, cfg.elf_path, &mut map).await { - Ok(edge_count) => { - tracing::info!( - "objdump: extracted {edge_count} forward edges from {}", - cfg.elf_path.display() - ); - } + match run_objdump_and_attribute(objdump_path, cfg.elf_path, &mut map, arm).await { + Ok(_) => map.reference_analysis.disassembly.status = AnalysisStatus::Analyzed, Err(e) => { - tracing::warn!( - "objdump forward-edge extraction failed for {} ({e}); \ - references_to will be empty", - cfg.elf_path.display() - ); + map.reference_analysis.disassembly.status = AnalysisStatus::Error; + map.reference_analysis.disassembly.reason = Some(e.to_string()); + tracing::warn!("objdump reference extraction failed: {e}"); } } + } else { + map.reference_analysis.disassembly.reason = Some("No objdump tool available.".into()); } + reference_analysis::static_edges(&mut map, cfg.elf_path, arm); + reference_analysis::finalize(&mut map); Ok(map) } @@ -492,9 +512,9 @@ async fn run_objdump_and_attribute( objdump_path: &Path, elf_path: &Path, map: &mut FineGrainedSymbolMap, + arm: bool, ) -> Result { use fbuild_core::subprocess::run_command; - use fbuild_core::symbol_analysis::callgraph::{invert, parse_disasm}; let objdump_s = objdump_path.to_string_lossy().to_string(); let elf_s = elf_path.to_string_lossy().to_string(); @@ -515,31 +535,24 @@ async fn run_objdump_and_attribute( ))); } - let edges = parse_disasm(&result.stdout); - // #478: invert once so both per-symbol directions come from the - // same disassembly pass. `called_by[X]` = every symbol whose - // forward edge list contains X — the per-symbol-precision view - // that complements the TU-level `referenced_by` (cref-derived). - let backward = invert(&edges); - let mut total = 0usize; - for sym in &mut map.symbols { - if let Some(callees) = edges.get(&sym.mangled) { - sym.references_to = callees.clone(); - total += callees.len(); - } else if let Some(callees) = edges.get(&sym.demangled) { - // Some toolchains demangle in-place when emitting the - // disassembly, so the function header uses the demangled - // name. Match against either. - sym.references_to = callees.clone(); - total += callees.len(); - } - if let Some(callers) = backward.get(&sym.mangled) { - sym.called_by = callers.clone(); - } else if let Some(callers) = backward.get(&sym.demangled) { - sym.called_by = callers.clone(); - } + if map + .symbols + .iter() + .any(|s| matches!(s.sym_type, 'T' | 't' | 'W' | 'w')) + && !result + .stdout + .lines() + .any(|line| line.contains(" <") && line.ends_with(">:")) + { + return Err(FbuildError::BuildFailed( + "objdump emitted no function headers for a report containing executable symbols".into(), + )); } - Ok(total) + Ok(reference_analysis::attribute_disassembly( + map, + &result.stdout, + arm, + )) } /// Format a fine-grained per-symbol map as a human-readable text report diff --git a/crates/fbuild-build-engine/src/symbol_analyzer/reference_analysis.rs b/crates/fbuild-build-engine/src/symbol_analyzer/reference_analysis.rs new file mode 100644 index 00000000..922d8bd1 --- /dev/null +++ b/crates/fbuild-build-engine/src/symbol_analyzer/reference_analysis.rs @@ -0,0 +1,401 @@ +//! Address-qualified reference attribution; names alone cannot identify fragments. +use fbuild_core::symbol_analysis::*; +use fbuild_core::{FbuildError, MemoryRegion, Result}; +use object::{Object, ObjectSection, ObjectSymbol}; +use std::collections::BTreeSet; +use std::path::Path; + +fn normalized(address: u64, arm: bool) -> u64 { + if arm { address & !1 } else { address } +} + +fn identity(map: &FineGrainedSymbolMap, name: &str, address: u64, arm: bool) -> SymbolIdentity { + map.symbols + .iter() + .find(|s| s.source == "nm" && s.mangled == name && normalized(s.address, arm) == address) + .map(SymbolIdentity::from) + .unwrap_or_else(|| SymbolIdentity { + name: name.into(), + address, + source: "external".into(), + }) +} + +pub(super) fn attribute_disassembly( + map: &mut FineGrainedSymbolMap, + text: &str, + arm: bool, +) -> usize { + let index: std::collections::BTreeMap<_, _> = map + .symbols + .iter() + .filter(|s| s.source == "nm") + .map(|s| { + ( + (s.mangled.clone(), normalized(s.address, arm)), + SymbolIdentity::from(s), + ) + }) + .collect(); + let resolve = |name: &str, address: u64| { + index + .get(&(name.to_string(), address)) + .cloned() + .unwrap_or_else(|| SymbolIdentity { + name: name.into(), + address, + source: "external".into(), + }) + }; + let mut current: Option = None; + let mut seen = BTreeSet::new(); + for line in text.lines() { + let trimmed = line.trim(); + if trimmed.starts_with("Disassembly of section") { + current = None; + continue; + } + if let Some(header) = trimmed.strip_suffix(":").and_then(|h| h.split_once(" <")) { + if let (Ok(address), Some(name)) = ( + u64::from_str_radix(header.0.trim_start_matches("0x"), 16), + header.1.strip_suffix('>'), + ) { + current = Some(resolve(name, normalized(address, arm))); + continue; + } + } + let Some(source) = current.as_ref() else { + continue; + }; + if source.source == "external" { + continue; + } + let Some((_, instruction)) = trimmed.split_once(':') else { + continue; + }; + let Some(close) = instruction.rfind('>') else { + continue; + }; + let Some(open) = instruction[..close].rfind('<') else { + continue; + }; + let name = &instruction[open + 1..close]; + if name.contains("+0x") + || name.contains("-0x") + || name.ends_with("@plt") + || name.starts_with('$') + { + continue; + } + let Some(address_text) = instruction[..open].split_whitespace().last() else { + continue; + }; + let Ok(address) = u64::from_str_radix(address_text.trim_start_matches("0x"), 16) else { + continue; + }; + let target = resolve(name, normalized(address, arm)); + if source == &target || !seen.insert((source.clone(), target.clone())) { + continue; + } + map.reference_analysis.edges.push(ReferenceEdge { + source: source.clone(), + target, + kind: ReferenceKind::Disassembly, + offset: None, + }); + } + populate_legacy_lists(map); + seen.len() +} + +fn populate_legacy_lists(map: &mut FineGrainedSymbolMap) { + // Legacy name lists retain instruction references only. Static pointer + // owners remain separately typed so they are never advertised as callers. + let mut forward = std::collections::BTreeMap::>::new(); + let mut backward = std::collections::BTreeMap::>::new(); + for edge in &map.reference_analysis.edges { + if edge.kind != ReferenceKind::Disassembly { + continue; + } + forward + .entry(edge.source.clone()) + .or_default() + .insert(edge.target.name.clone()); + backward + .entry(edge.target.clone()) + .or_default() + .insert(edge.source.name.clone()); + } + for symbol in &mut map.symbols { + let id = SymbolIdentity::from(&*symbol); + symbol.references_to = forward + .remove(&id) + .unwrap_or_default() + .into_iter() + .collect(); + symbol.called_by = backward + .remove(&id) + .unwrap_or_default() + .into_iter() + .collect(); + } +} + +pub(super) fn probe_elf(map: &mut FineGrainedSymbolMap, path: &Path) -> Result { + let bytes = std::fs::read(path).map_err(FbuildError::Io)?; + let file = object::File::parse(bytes.as_slice()) + .map_err(|e| FbuildError::BuildFailed(format!("reference ELF probe: {e}")))?; + if !matches!( + file.kind(), + object::ObjectKind::Executable | object::ObjectKind::Dynamic + ) { + return Err(FbuildError::BuildFailed( + "Reference roots require a final executable/shared ELF, not a relocatable object." + .into(), + )); + } + let arm = file.architecture() == object::Architecture::Arm; + // nm's weak-object V/v does not encode its storage region. Use the + // allocated ELF section, not a Flash guess or a mangled-name heuristic. + let mut weak = std::collections::BTreeMap::new(); + for symbol in file.symbols() { + let (Ok(name), Some(section_index)) = (symbol.name(), symbol.section_index()) else { + continue; + }; + let section = file + .section_by_index(section_index) + .map_err(|e| FbuildError::BuildFailed(e.to_string()))?; + if !matches!(section.flags(), object::SectionFlags::Elf {sh_flags} if sh_flags & u64::from(object::elf::SHF_ALLOC) != 0) + { + continue; + } + let region = if section.kind() == object::SectionKind::UninitializedData + || matches!(section.flags(), object::SectionFlags::Elf {sh_flags} if sh_flags & u64::from(object::elf::SHF_WRITE) != 0) + { + MemoryRegion::Ram + } else { + MemoryRegion::Flash + }; + weak.insert( + (name.to_string(), symbol.address()), + (region, section.name().unwrap_or("").to_string()), + ); + } + map.symbols.retain_mut(|symbol| { + if !matches!(symbol.sym_type, 'V' | 'v') { + return true; + } + let Some((region, section)) = weak.get(&(symbol.mangled.clone(), symbol.address)) else { + return false; + }; + symbol.region = *region; + symbol.output_section = Some(section.clone()); + true + }); + map.total_flash = map + .symbols + .iter() + .filter(|s| s.region == MemoryRegion::Flash) + .map(|s| s.size) + .sum(); + map.total_ram = map + .symbols + .iter() + .filter(|s| s.region == MemoryRegion::Ram) + .map(|s| s.size) + .sum(); + let entry = normalized(file.entry(), arm); + for symbol in file.symbols() { + if symbol.is_definition() + && symbol.kind() == object::SymbolKind::Text + && normalized(symbol.address(), arm) == entry + { + if let Ok(name) = symbol.name() { + map.reference_analysis.roots.push(RetentionRoot { + symbol: identity(map, name, entry, arm), + kind: "entry_point".into(), + }); + } + } + } + Ok(arm) +} + +pub(super) fn static_edges(map: &mut FineGrainedSymbolMap, path: &Path, arm: bool) { + match super::elf_references::read_vtable_references(path) { + Ok(edges) => { + map.reference_analysis.static_data.status = AnalysisStatus::Analyzed; + for edge in edges { + let source = identity(map, &edge.source_name, edge.source_address, arm); + let target = identity(map, &edge.target_name, edge.target_address, arm); + map.reference_analysis.edges.push(ReferenceEdge { + source, + target, + kind: ReferenceKind::StaticData, + offset: Some(edge.offset), + }); + } + } + Err(e) => { + map.reference_analysis.static_data.status = AnalysisStatus::Error; + map.reference_analysis.static_data.reason = Some(e.to_string()); + } + } +} + +pub(super) fn finalize(map: &mut FineGrainedSymbolMap) { + // Map-derived pools carry a compiler-provided owning function name. + // Keep ownership separate from a machine instruction or pointer edge. + let mut owners = std::collections::BTreeMap::<_, Vec<&FineGrainedSymbol>>::new(); + for owner in &map.symbols { + if owner.source != "nm" || !matches!(owner.sym_type, 'T' | 't' | 'W' | 'w') { + continue; + } + if let Some(object) = owner.object.as_deref() { + owners + .entry((owner.mangled.as_str(), owner.archive.as_deref(), object)) + .or_default() + .push(owner); + } + } + for fragment in &map.symbols { + if fragment.source != "map-derived" { + continue; + } + let Some(object) = fragment.object.as_deref() else { + continue; + }; + let Some(candidates) = owners.get(&( + fragment.mangled.as_str(), + fragment.archive.as_deref(), + object, + )) else { + continue; + }; + if let [owner] = candidates.as_slice() { + map.reference_analysis.edges.push(ReferenceEdge { + source: (*owner).into(), + target: fragment.into(), + kind: ReferenceKind::FragmentOwner, + offset: None, + }); + } + } + let incoming: BTreeSet<_> = map + .reference_analysis + .edges + .iter() + .map(|e| e.target.clone()) + .collect(); + let roots: BTreeSet<_> = map + .reference_analysis + .roots + .iter() + .map(|r| r.symbol.clone()) + .collect(); + map.reference_analysis.unexplained = map + .symbols + .iter() + .filter(|s| { + s.size > 0 + && s.referenced_by.is_empty() + && !incoming.contains(&SymbolIdentity::from(*s)) + && !roots.contains(&SymbolIdentity::from(*s)) + }) + .map(SymbolIdentity::from) + .collect(); + let unresolved: BTreeSet<_> = map + .reference_analysis + .edges + .iter() + .flat_map(|e| [&e.source, &e.target]) + .filter(|id| id.source == "external") + .map(|id| (id.name.clone(), id.address)) + .collect(); + map.reference_analysis.unresolved = unresolved + .into_iter() + .map(|(name, address)| UnresolvedReference { name, address }) + .collect(); +} + +#[cfg(test)] +mod tests { + use super::*; + #[test] + fn disassembly_edges_do_not_leak_into_same_name_rodata_fragments() { + let mut map = build_fine_grained_map( + "x".into(), + None, + vec![ + (0x100, 16, 'T', "caller".into()), + (0x200, 16, 'T', "target".into()), + (0x300, 8, 'r', "target".into()), + ], + vec!["caller".into(), "target".into(), "target".into()], + vec![], + ); + map.symbols[2].source = "map-derived".into(); + assert_eq!( + attribute_disassembly( + &mut map, + "00000100 :\n 100: call 200 \n", + false + ), + 1 + ); + assert_eq!(map.symbols[1].called_by, vec!["caller"]); + assert!(map.symbols[2].called_by.is_empty()); + assert_eq!(map.reference_analysis.edges[0].target.address, 0x200); + } + #[test] + fn missing_local_or_rom_symbols_remain_addressed_external_targets() { + let mut map = build_fine_grained_map( + "x".into(), + None, + vec![(0x100, 16, 'T', "caller".into())], + vec!["caller".into()], + vec![], + ); + assert_eq!( + attribute_disassembly( + &mut map, + "00000100 :\n 100: call 400 \n", + false + ), + 1 + ); + let edge = &map.reference_analysis.edges[0]; + assert_eq!(edge.target.source, "external"); + assert_eq!(edge.target.address, 0x400); + } + #[test] + fn same_name_local_fragment_owners_require_matching_object_provenance() { + let mut map = build_fine_grained_map( + "x".into(), + None, + vec![ + (0x100, 16, 'T', "local_fn".into()), + (0x200, 16, 'T', "local_fn".into()), + (0x300, 8, 'r', "local_fn".into()), + (0x400, 8, 'r', "local_fn".into()), + ], + vec!["local_fn".into(); 4], + vec![], + ); + for (i, symbol) in map.symbols.iter_mut().enumerate() { + symbol.object = Some(if i % 2 == 0 { "first.o" } else { "second.o" }.into()); + if i >= 2 { + symbol.source = "map-derived".into(); + } + } + finalize(&mut map); + assert_eq!(map.reference_analysis.edges.len(), 2); + assert!( + map.reference_analysis + .edges + .iter() + .all(|e| (e.source.address == 0x100 && e.target.address == 0x300) + || (e.source.address == 0x200 && e.target.address == 0x400)) + ); + } +} diff --git a/crates/fbuild-build-engine/src/symbol_analyzer/tests.rs b/crates/fbuild-build-engine/src/symbol_analyzer/tests.rs index 7cce0717..aedd3516 100644 --- a/crates/fbuild-build-engine/src/symbol_analyzer/tests.rs +++ b/crates/fbuild-build-engine/src/symbol_analyzer/tests.rs @@ -101,6 +101,7 @@ fn discover_elf_returns_none_when_nothing_found() { fn format_markdown_report_emits_tables() { use fbuild_core::symbol_analysis::{FineGrainedSymbol, FineGrainedSymbolMap, SectionBytes}; let map = FineGrainedSymbolMap { + reference_analysis: Default::default(), elf_path: "fw.elf".into(), map_path: Some("fw.map".into()), total_flash: 100, @@ -158,6 +159,7 @@ fn format_markdown_report_escapes_pipes_in_symbol_names() { use fbuild_core::symbol_analysis::{FineGrainedSymbol, FineGrainedSymbolMap, SectionBytes}; // operator|| is a real C++ name shape that demangles with pipes. let map = FineGrainedSymbolMap { + reference_analysis: Default::default(), elf_path: "fw.elf".into(), map_path: None, total_flash: 10, @@ -193,6 +195,7 @@ fn format_markdown_report_renders_referenced_by_column() { FineGrainedSymbol, FineGrainedSymbolMap, SectionBytes, SymbolReference, }; let map = FineGrainedSymbolMap { + reference_analysis: Default::default(), elf_path: "fw.elf".into(), map_path: None, total_flash: 11309, @@ -259,6 +262,7 @@ fn markdown_report_with_graphs_embeds_dot_blocks_for_top_symbols() { FineGrainedSymbol, FineGrainedSymbolMap, GraphConfig, SectionBytes, SymbolReference, }; let map = FineGrainedSymbolMap { + reference_analysis: Default::default(), elf_path: "fw.elf".into(), map_path: None, total_flash: 11_309, @@ -302,7 +306,7 @@ fn markdown_report_with_graphs_embeds_dot_blocks_for_top_symbols() { ); assert!( md.contains("
") - && md.contains("Bidirectional graph (callers ← root → callees,"), + && md.contains("Bidirectional reference graph (incoming ← root → outgoing,"), "missing details summary in:\n{md}" ); assert!( @@ -404,6 +408,7 @@ fn markdown_report_emits_dual_ranked_callees_subtable() { let mut all = vec![root]; all.extend(callees); let map = FineGrainedSymbolMap { + reference_analysis: Default::default(), elf_path: "test.elf".into(), map_path: None, total_flash: 12_860, @@ -447,6 +452,7 @@ fn markdown_report_emits_dual_ranked_callees_subtable() { fn markdown_report_legacy_path_skips_graph_blocks() { use fbuild_core::symbol_analysis::{FineGrainedSymbol, FineGrainedSymbolMap, SectionBytes}; let map = FineGrainedSymbolMap { + reference_analysis: Default::default(), elf_path: "fw.elf".into(), map_path: None, total_flash: 10, @@ -481,6 +487,7 @@ fn sidecar_dot_files_written_for_symbols_above_min_bytes() { }; let tmp = tempfile::tempdir().unwrap(); let map = FineGrainedSymbolMap { + reference_analysis: Default::default(), elf_path: "fw.elf".into(), map_path: None, total_flash: 1_200, @@ -553,6 +560,7 @@ fn sidecar_disabled_writes_nothing() { }; let tmp = tempfile::tempdir().unwrap(); let map = FineGrainedSymbolMap { + reference_analysis: Default::default(), elf_path: "fw.elf".into(), map_path: None, total_flash: 1_000, @@ -594,6 +602,7 @@ fn sidecar_disabled_writes_nothing() { fn format_markdown_report_referenced_by_empty_renders_dash() { use fbuild_core::symbol_analysis::{FineGrainedSymbol, FineGrainedSymbolMap, SectionBytes}; let map = FineGrainedSymbolMap { + reference_analysis: Default::default(), elf_path: "fw.elf".into(), map_path: None, total_flash: 10, diff --git a/crates/fbuild-cli/src/cli/bloat_lookup.rs b/crates/fbuild-cli/src/cli/bloat_lookup.rs index caf21bf9..75a5a6ef 100644 --- a/crates/fbuild-cli/src/cli/bloat_lookup.rs +++ b/crates/fbuild-cli/src/cli/bloat_lookup.rs @@ -58,7 +58,8 @@ pub async fn run_bloat_lookup( nm.as_deref(), cppfilt.as_deref(), build_info.as_deref(), - )?; + ) + .await?; let map_path_owned = map .map(PathBuf::from) diff --git a/crates/fbuild-cli/src/cli/graph_cmd.rs b/crates/fbuild-cli/src/cli/graph_cmd.rs index b0a01553..43675a19 100644 --- a/crates/fbuild-cli/src/cli/graph_cmd.rs +++ b/crates/fbuild-cli/src/cli/graph_cmd.rs @@ -57,7 +57,8 @@ pub async fn run_bloat_graph( nm.as_deref(), cppfilt.as_deref(), build_info.as_deref(), - )?; + ) + .await?; let map_path_owned = map .map(PathBuf::from) @@ -79,7 +80,13 @@ pub async fn run_bloat_graph( &collapse_archive, &exclude_archive, )?; - let graph = BackrefGraph::build(&report, &symbol, &graph_config); + let selected = select_symbol(&report.symbols, &symbol)?; + let graph = if let Some(root) = selected { + let index = fbuild_core::symbol_analysis::TuIndex::build(&report); + BackrefGraph::build_for_symbol_with_index(&report, &index, root, &graph_config) + } else { + BackrefGraph::build(&report, &symbol, &graph_config) + }; let dot = graph.to_dot(); match output { @@ -102,6 +109,42 @@ pub async fn run_bloat_graph( Ok(()) } +fn select_symbol<'a>( + symbols: &'a [fbuild_core::symbol_analysis::FineGrainedSymbol], + symbol: &str, +) -> Result> { + let (query, address) = symbol + .rsplit_once("@0x") + .and_then(|(name, addr)| { + u64::from_str_radix(addr, 16) + .ok() + .map(|addr| (name, Some(addr))) + }) + .unwrap_or((symbol, None)); + let mut matches: Vec<_> = symbols + .iter() + .filter(|s| { + (s.mangled == query || s.demangled == query) && address.is_none_or(|a| s.address == a) + }) + .collect(); + if address.is_none() && matches.iter().any(|s| s.source == "nm") { + matches.retain(|s| s.source == "nm"); + } + matches.sort_by_key(|s| (s.address, s.source.as_str(), s.mangled.as_str())); + matches.dedup_by_key(|s| (s.address, s.source.as_str(), s.mangled.as_str())); + if matches.len() > 1 { + return Err(FbuildError::BuildFailed(format!( + "Ambiguous symbol {query}; select name@0xADDRESS. Matches: {}", + matches + .iter() + .map(|s| format!("0x{:x} ({})", s.address, s.source)) + .collect::>() + .join(", ") + ))); + } + Ok(matches.first().copied()) +} + /// Parse the user-facing flag strings into a fully-populated /// [`GraphConfig`]. Public-ish helper because the report-embed path /// in `symbols_cmd.rs` parses the same flag shapes for the @@ -189,3 +232,60 @@ mod tests { assert_eq!(c.fan_out, 1); } } + +#[cfg(test)] +mod identity_tests { + use super::select_symbol; + use fbuild_core::MemoryRegion; + use fbuild_core::symbol_analysis::FineGrainedSymbol; + fn fixture(address: u64, source: &str) -> FineGrainedSymbol { + FineGrainedSymbol { + mangled: "function".into(), + demangled: "function()".into(), + address, + size: 8, + sym_type: 'T', + region: MemoryRegion::Flash, + archive: None, + object: Some("main.o".into()), + output_section: None, + source: source.into(), + referenced_by: vec![], + references_to: vec![], + called_by: vec![], + } + } + #[test] + fn exact_selector_keeps_fragment_identity_and_rejects_ambiguous_functions() { + let symbols = vec![fixture(100, "map"), fixture(200, "nm")]; + assert_eq!( + select_symbol(&symbols, "function") + .unwrap() + .unwrap() + .address, + 200 + ); + assert_eq!( + select_symbol(&symbols, "function@0x64") + .unwrap() + .unwrap() + .source, + "map" + ); + assert!(select_symbol(&symbols, "missing").unwrap().is_none()); + let symbols = vec![fixture(200, "nm"), fixture(300, "nm")]; + assert!( + select_symbol(&symbols, "function") + .unwrap_err() + .to_string() + .contains("Ambiguous symbol") + ); + assert_eq!( + select_symbol(&symbols, "function()@0x12c") + .unwrap() + .unwrap() + .address, + 300 + ); + } +} diff --git a/crates/fbuild-cli/src/cli/symbols_cmd.rs b/crates/fbuild-cli/src/cli/symbols_cmd.rs index d10a1670..59ed8f7d 100644 --- a/crates/fbuild-cli/src/cli/symbols_cmd.rs +++ b/crates/fbuild-cli/src/cli/symbols_cmd.rs @@ -8,7 +8,7 @@ //! Toolchain resolution (see #428): //! 1. `--nm` / `--cppfilt` CLI flags (user wins). //! 2. `--build-info ` if provided — `nm_path` / `cppfilt_path` -//! read from that file. +//! or PlatformIO aliases read from that file. //! 3. Auto-discovery: walk up from the ELF directory looking for //! `build_info.json` or `build_info_.json`. //! 4. PATH-based lookup of `nm`, with `c++filt` derived by stem. @@ -16,13 +16,15 @@ use std::path::{Path, PathBuf}; -use fbuild_build::build_info::{find_build_info_near, load_build_info}; +use std::collections::BTreeMap; + use fbuild_build::symbol_analyzer::{ AnalyzeConfig, MarkdownGraphOptions, SidecarOptions, analyze_elf, default_map_path, derive_cppfilt_path, discover_elf_in_project, format_markdown_report, format_markdown_report_with_graphs, format_text_report, write_sidecar_dot_files, }; use fbuild_core::{FbuildError, Result}; +use serde::Deserialize; use crate::output; @@ -62,7 +64,8 @@ pub async fn run_symbols( nm.as_deref(), cppfilt.as_deref(), build_info.as_deref(), - )?; + ) + .await?; let nm_path = tool_paths.nm; let cppfilt_path = tool_paths.cppfilt; if !nm_path.exists() { @@ -243,8 +246,8 @@ struct ToolPaths { /// (`objdump_path`) or by deriving it from the nm path using the /// GCC cross-tool naming convention. `None` when neither path /// can be found — the analyzer falls back to an empty - /// `references_to` (forward graphs are unavailable; backref - /// graphs are unaffected). + /// `references_to` (instruction references are unavailable; map + /// object references are unaffected). objdump: Option, } @@ -253,38 +256,36 @@ impl ToolPaths { /// documented in the module header. `build_info_arg` is the /// explicit `--build-info` path; when absent, walk up from /// `elf_path`. - fn resolve( + async fn resolve( elf_path: &Path, nm: Option<&str>, cppfilt: Option<&str>, build_info_arg: Option<&str>, ) -> Result { - // Try the build_info source (explicit flag wins over auto-discovery). - let build_info_path = build_info_arg - .map(PathBuf::from) - .or_else(|| elf_path.parent().and_then(find_build_info_near)); - + let build_info_path = match build_info_arg { + Some(path) => Some(PathBuf::from(path)), + None if nm.is_some() => None, + None => discover_tool_metadata(elf_path)?, + }; let (bi_nm, bi_cppfilt, bi_objdump) = match build_info_path { - Some(path) => match load_build_info(&path) { - Ok((_env, info)) => { - tracing::info!("symbols: read toolchain paths from {}", path.display()); - ( - option_path(&info.nm_path), - option_path(&info.cppfilt_path), - option_path(&info.objdump_path), - ) + Some(path) => { + let info = read_tool_metadata(&path, elf_path).await?; + if nm.is_none() && info.tool("nm", &info.nm_path).is_none() { + return Err(metadata_error( + &path, + "selected environment has no nm_path or aliases.nm; pass --nm", + )); } - Err(e) => { - tracing::warn!( - "symbols: ignoring {}: {} (falling back to PATH)", - path.display(), - e - ); - (None, None, None) - } - }, + tracing::info!("symbols: read toolchain paths from {}", path.display()); + ( + info.tool("nm", &info.nm_path), + info.tool("c++filt", &info.cppfilt_path), + info.tool("objdump", &info.objdump_path), + ) + } None => (None, None, None), }; + let explicit_nm = nm.is_some(); let nm = match nm { Some(p) => PathBuf::from(p), @@ -296,7 +297,7 @@ impl ToolPaths { let cppfilt = match cppfilt { Some(p) => Some(PathBuf::from(p)), - None => bi_cppfilt.or_else(|| { + None => (if explicit_nm { None } else { bi_cppfilt }).or_else(|| { let derived = derive_cppfilt_path(&nm); if derived.exists() { Some(derived) @@ -311,7 +312,11 @@ impl ToolPaths { // Same prefix-replacement strategy `derive_cppfilt_path` // uses; inlined here to keep symbol_analyzer's public surface // minimal — objdump derivation isn't useful outside this CLI. - let objdump = bi_objdump.or_else(|| derive_sibling_tool(&nm, "objdump")); + let objdump = if explicit_nm { + derive_sibling_tool(&nm, "objdump") + } else { + bi_objdump.or_else(|| derive_sibling_tool(&nm, "objdump")) + }; Ok(Self { nm, @@ -360,13 +365,13 @@ fn derive_sibling_tool(nm_path: &Path, target: &str) -> Option { /// the objdump used to populate per-symbol forward refs /// (`references_to`). Callers that don't care about forward graphs /// can discard the third field. -pub fn resolve_tool_paths_public( +pub async fn resolve_tool_paths_public( elf_path: &Path, nm: Option<&str>, cppfilt: Option<&str>, build_info_arg: Option<&str>, ) -> Result<(PathBuf, Option, Option)> { - let resolved = ToolPaths::resolve(elf_path, nm, cppfilt, build_info_arg)?; + let resolved = ToolPaths::resolve(elf_path, nm, cppfilt, build_info_arg).await?; if !resolved.nm.exists() { return Err(FbuildError::BuildFailed(format!( "nm not found at {}\n\ @@ -377,18 +382,149 @@ pub fn resolve_tool_paths_public( Ok((resolved.nm, resolved.cppfilt, resolved.objdump)) } -/// Treat an empty BuildInfo path field (the schema's "missing" -/// sentinel) as `None`. `BuildInfo`'s `*_path` fields became -/// `NormalizedPath` in #437 Phase 2, so emptiness is checked on the -/// underlying `OsStr` rather than on a `String`. -fn option_path(p: &fbuild_core::path::NormalizedPath) -> Option { - if p.as_path().as_os_str().is_empty() { - None - } else { - Some(p.as_path().to_path_buf()) +/// Project only the tool metadata: PlatformIO aliases do not carry all native +/// BuildInfo fields, and requiring compiler/build flags silently lost graphs. +#[derive(Default, Deserialize)] +struct SymbolToolMetadata { + #[serde(default)] + prog_path: String, + #[serde(default)] + nm_path: String, + #[serde(default)] + cppfilt_path: String, + #[serde(default)] + objdump_path: String, + #[serde(default)] + aliases: BTreeMap, +} + +impl SymbolToolMetadata { + fn tool(&self, alias: &str, direct: &str) -> Option { + let value = if direct.is_empty() { + self.aliases.get(alias).map(String::as_str)? + } else { + direct + }; + (!value.is_empty()).then(|| PathBuf::from(value)) } } +fn elf_environment(elf: &Path) -> Option<&str> { + // Build profiles may add release/debug beneath the environment directory. + elf.parent()?.ancestors().find_map(|directory| { + (directory.parent()?.file_name()? == "build") + .then(|| directory.file_name()?.to_str()) + .flatten() + }) +} + +fn metadata_error(path: &Path, reason: impl std::fmt::Display) -> FbuildError { + FbuildError::BuildFailed(format!( + "symbols: invalid tool metadata {}: {reason}", + path.display() + )) +} + +async fn read_tool_metadata(path: &Path, elf: &Path) -> Result { + let bytes = fbuild_core::fs::read(path) + .await + .map_err(|e| metadata_error(path, e))?; + let mut envs: BTreeMap = + serde_json::from_slice(&bytes).map_err(|e| metadata_error(path, e))?; + if envs.is_empty() { + return Err(metadata_error(path, "no environments")); + } + let mut matches = Vec::new(); + let canonical_elf = fbuild_core::path::canonicalize_existing(elf).await.ok(); + for (name, info) in &envs { + if info.prog_path.is_empty() { + continue; + } + let program = Path::new(&info.prog_path); + let candidate = if program.is_absolute() { + program.to_path_buf() + } else { + path.parent().unwrap_or(Path::new(".")).join(program) + }; + let same_lexical_path = fbuild_core::path::NormalizedPath::new(program) + == fbuild_core::path::NormalizedPath::new(elf); + let same_existing_path = match &canonical_elf { + Some(elf_identity) => fbuild_core::path::canonicalize_existing(candidate) + .await + .ok() + .is_some_and(|identity| &identity == elf_identity), + None => false, + }; + if same_lexical_path || same_existing_path { + matches.push(name.clone()); + } + } + let selected = match matches.as_slice() { + [name] => name.clone(), + [] => match elf_environment(elf).filter(|name| envs.contains_key(*name)) { + Some(name) => name.to_string(), + None if envs.len() == 1 && elf_environment(elf).is_none() => envs + .keys() + .next() + .cloned() + .ok_or_else(|| metadata_error(path, "no environments"))?, + None => { + return Err(metadata_error( + path, + "no unambiguous environment matches the ELF; use matching prog_path or build//firmware.elf", + )); + } + }, + _ => return Err(metadata_error(path, "multiple environments match the ELF")), + }; + envs.remove(&selected) + .ok_or_else(|| metadata_error(path, "selected environment is missing")) +} + +fn discover_tool_metadata(elf: &Path) -> Result> { + let mut cursor = elf.parent(); + while let Some(dir) = cursor { + if let Some(env) = elf_environment(elf) { + let matching = dir.join(format!("build_info_{env}.json")); + if matching.is_file() { + return Ok(Some(matching)); + } + } + let generic = dir.join("build_info.json"); + if generic.is_file() { + return Ok(Some(generic)); + } + let mut candidates = Vec::new(); + if let Ok(entries) = std::fs::read_dir(dir) { + for entry in entries.flatten() { + let path = entry.path(); + if path.is_file() + && path + .file_name() + .and_then(|n| n.to_str()) + .is_some_and(|name| { + name.starts_with("build_info_") && name.ends_with(".json") + }) + { + candidates.push(path); + } + } + } + match candidates.len() { + 0 => {} + 1 => return Ok(candidates.pop()), + _ => { + return Err(metadata_error( + dir, + "ambiguous build_info_.json files; pass --build-info", + )); + } + } + cursor = dir.parent(); + } + Ok(None) +} + #[cfg(test)] mod tests { use super::*; @@ -420,8 +556,8 @@ mod tests { /// #428: when `build_info.json` lives near the ELF and carries /// `nm_path`, the symbols CLI must pick it up automatically. - #[test] - fn resolve_reads_nm_from_build_info_auto_discovery() { + #[tokio::test(flavor = "multi_thread")] + async fn resolve_reads_nm_from_build_info_auto_discovery() { let tmp = tempfile::TempDir::new().unwrap(); let project = tmp.path(); let build_dir = fbuild_paths::get_project_fbuild_dir(project) @@ -438,13 +574,13 @@ mod tests { let info = dummy_build_info(&nm_file.to_string_lossy(), ""); emit_build_info(project, "uno", &info).unwrap(); - let tools = ToolPaths::resolve(&elf, None, None, None).unwrap(); + let tools = ToolPaths::resolve(&elf, None, None, None).await.unwrap(); assert_eq!(tools.nm, nm_file); } /// Explicit `--nm` overrides whatever build_info.json says. - #[test] - fn resolve_explicit_nm_wins_over_build_info() { + #[tokio::test(flavor = "multi_thread")] + async fn resolve_explicit_nm_wins_over_build_info() { let tmp = tempfile::TempDir::new().unwrap(); let project = tmp.path(); let elf = project.join("firmware.elf"); @@ -460,14 +596,16 @@ mod tests { let cli_nm = project.join("from-cli"); std::fs::write(&cli_nm, b"x").unwrap(); - let tools = ToolPaths::resolve(&elf, Some(cli_nm.to_str().unwrap()), None, None).unwrap(); + let tools = ToolPaths::resolve(&elf, Some(cli_nm.to_str().unwrap()), None, None) + .await + .unwrap(); assert_eq!(tools.nm, cli_nm); } /// `--build-info ` is honoured even when the ELF isn't under /// the project containing build_info.json. - #[test] - fn resolve_explicit_build_info_path_is_honoured() { + #[tokio::test(flavor = "multi_thread")] + async fn resolve_explicit_build_info_path_is_honoured() { let tmp = tempfile::TempDir::new().unwrap(); let elf = tmp.path().join("firmware.elf"); std::fs::write(&elf, b"\x7fELF").unwrap(); @@ -480,7 +618,192 @@ mod tests { emit_build_info(&bi_dir, "uno", &info).unwrap(); let bi_path = bi_dir.join("build_info.json"); - let tools = ToolPaths::resolve(&elf, None, None, Some(bi_path.to_str().unwrap())).unwrap(); + let tools = ToolPaths::resolve(&elf, None, None, Some(bi_path.to_str().unwrap())) + .await + .unwrap(); assert_eq!(tools.nm, nm_file); } + #[tokio::test] + async fn resolve_partial_aliases_and_matching_environment() { + let tmp = tempfile::TempDir::new().unwrap(); + let elf = tmp.path().join(".pio/build/uno/firmware.elf"); + let metadata = tmp.path().join("build_info.json"); + std::fs::write(&metadata, serde_json::to_vec(&serde_json::json!({ + "uno": {"aliases": {"nm": "/target/avr-nm", "c++filt": "/target/avr-c++filt", "objdump": "/target/avr-objdump"}}, + "esp": {"aliases": {"nm": "/target/xtensa-nm"}} + })).unwrap()).unwrap(); + let tools = ToolPaths::resolve(&elf, None, None, Some(metadata.to_str().unwrap())) + .await + .unwrap(); + assert_eq!(tools.nm, PathBuf::from("/target/avr-nm")); + assert_eq!(tools.objdump, Some(PathBuf::from("/target/avr-objdump"))); + assert_eq!(tools.cppfilt, Some(PathBuf::from("/target/avr-c++filt"))); + std::fs::write(&metadata, br#"{"esp":{"aliases":{"nm":"xtensa-nm"}}}"#).unwrap(); + assert!( + ToolPaths::resolve(&elf, None, None, Some(metadata.to_str().unwrap())) + .await + .is_err() + ); + } + + #[tokio::test] + async fn resolve_nested_release_metadata_environment() { + let tmp = tempfile::TempDir::new().unwrap(); + let elf = tmp + .path() + .join(fbuild_paths::FBUILD_DIR_NAME) + .join(fbuild_paths::BUILD_DIR_NAME) + .join("uno/release/firmware.elf"); + let metadata = tmp.path().join("build_info.json"); + std::fs::write( + &metadata, + br#"{"esp":{"aliases":{"nm":"xtensa-nm"}},"uno":{"aliases":{"nm":"avr-nm"}}}"#, + ) + .unwrap(); + let tools = ToolPaths::resolve(&elf, None, None, Some(metadata.to_str().unwrap())) + .await + .unwrap(); + assert_eq!(tools.nm, PathBuf::from("avr-nm")); + } + + #[tokio::test] + async fn resolve_metadata_selects_matching_program_path() { + let tmp = tempfile::TempDir::new().unwrap(); + let elf = tmp.path().join("firmware.elf"); + let metadata = tmp.path().join("build_info.json"); + std::fs::write( + &metadata, + serde_json::to_vec(&serde_json::json!({ + "uno": {"prog_path": elf, "aliases": {"nm": "avr-nm"}}, + "esp": {"prog_path": "other.elf", "aliases": {"nm": "xtensa-nm"}} + })) + .unwrap(), + ) + .unwrap(); + let tools = ToolPaths::resolve(&elf, None, None, Some(metadata.to_str().unwrap())) + .await + .unwrap(); + assert_eq!(tools.nm, PathBuf::from("avr-nm")); + std::fs::write( + &metadata, + serde_json::to_vec(&serde_json::json!({ + "uno": {"prog_path": elf, "aliases": {"nm": "avr-nm"}}, + "esp": {"prog_path": elf, "aliases": {"nm": "xtensa-nm"}} + })) + .unwrap(), + ) + .unwrap(); + assert!( + ToolPaths::resolve(&elf, None, None, Some(metadata.to_str().unwrap())) + .await + .is_err() + ); + } + + #[tokio::test] + async fn resolve_metadata_matches_symlink_program_path() { + let tmp = tempfile::TempDir::new().unwrap(); + let elf = tmp.path().join("firmware.elf"); + std::fs::write(&elf, b"fixture").unwrap(); + let alias = tmp.path().join("firmware-alias.elf"); + if let Err(error) = fbuild_core::platform::fs::symlink_file(&elf, &alias) { + if fbuild_core::platform::host::is_windows() + && error.kind() == std::io::ErrorKind::PermissionDenied + { + // Hosts without symlink privileges cannot construct this fixture. + return; + } + panic!("create file symlink: {error}"); + } + let metadata = tmp.path().join("build_info.json"); + std::fs::write( + &metadata, + serde_json::to_vec(&serde_json::json!({ + "uno": {"prog_path": alias, "aliases": {"nm": "avr-nm"}}, + "esp": {"prog_path": "other.elf", "aliases": {"nm": "xtensa-nm"}} + })) + .unwrap(), + ) + .unwrap(); + let tools = ToolPaths::resolve(&elf, None, None, Some(metadata.to_str().unwrap())) + .await + .unwrap(); + assert_eq!(tools.nm, PathBuf::from("avr-nm")); + } + + #[tokio::test] + async fn resolve_rejects_ambiguous_or_invalid_explicit_metadata() { + let tmp = tempfile::TempDir::new().unwrap(); + let elf = tmp.path().join("firmware.elf"); + let metadata = tmp.path().join("build_info.json"); + std::fs::write( + &metadata, + br#"{"uno":{"aliases":{"nm":"avr-nm"}},"esp":{"aliases":{"nm":"xtensa-nm"}}}"#, + ) + .unwrap(); + assert!( + ToolPaths::resolve(&elf, None, None, Some(metadata.to_str().unwrap())) + .await + .is_err() + ); + std::fs::write(&metadata, b"invalid json").unwrap(); + assert!( + ToolPaths::resolve(&elf, None, None, Some(metadata.to_str().unwrap())) + .await + .is_err() + ); + std::fs::write(&metadata, br#"{"uno":{"aliases":{}}}"#).unwrap(); + assert!( + ToolPaths::resolve(&elf, None, None, Some(metadata.to_str().unwrap())) + .await + .is_err() + ); + } + + #[tokio::test] + async fn resolve_explicit_nm_selects_its_sibling_tools() { + let tmp = tempfile::TempDir::new().unwrap(); + let elf = tmp.path().join("firmware.elf"); + let metadata = tmp.path().join("build_info.json"); + std::fs::write(&metadata, br#"{"uno":{"aliases":{"nm":"wrong-nm","objdump":"wrong-objdump","c++filt":"wrong-c++filt"}}}"#).unwrap(); + let nm = tmp.path().join("avr-nm"); + let objdump = tmp.path().join("avr-objdump"); + let cppfilt = tmp.path().join("avr-c++filt"); + for path in [&nm, &objdump, &cppfilt] { + std::fs::write(path, b"x").unwrap(); + } + let tools = ToolPaths::resolve( + &elf, + Some(nm.to_str().unwrap()), + None, + Some(metadata.to_str().unwrap()), + ) + .await + .unwrap(); + assert_eq!(tools.objdump, Some(objdump)); + assert_eq!(tools.cppfilt, Some(cppfilt)); + } + #[tokio::test] + async fn resolve_auto_discovery_selects_matching_env_file() { + let tmp = tempfile::TempDir::new().unwrap(); + let elf = tmp.path().join(".pio/build/uno/firmware.elf"); + std::fs::create_dir_all(elf.parent().unwrap()).unwrap(); + std::fs::write( + tmp.path().join("build_info_esp.json"), + br#"{"esp":{"aliases":{"nm":"wrong-nm"}}}"#, + ) + .unwrap(); + std::fs::write( + tmp.path().join("build_info.json"), + br#"{"esp":{"aliases":{"nm":"wrong-generic-nm"}}}"#, + ) + .unwrap(); + std::fs::write( + tmp.path().join("build_info_uno.json"), + br#"{"uno":{"aliases":{"nm":"avr-nm"}}}"#, + ) + .unwrap(); + let tools = ToolPaths::resolve(&elf, None, None, None).await.unwrap(); + assert_eq!(tools.nm, PathBuf::from("avr-nm")); + } } diff --git a/crates/fbuild-core/src/symbol_analysis/README.md b/crates/fbuild-core/src/symbol_analysis/README.md index 5e79bb17..983fd125 100644 --- a/crates/fbuild-core/src/symbol_analysis/README.md +++ b/crates/fbuild-core/src/symbol_analysis/README.md @@ -20,3 +20,8 @@ Intentionally has no ELF-parsing dep; ELF I/O lives in `fbuild_build::symbol_ana - `cref.rs` — `Cross Reference Table` parser. - `graph.rs` — back-reference graph walker + `.dot` renderer. - `tests.rs` — unit tests. + +`references.rs` defines the versioned, address-qualified evidence contract: +analysis availability, typed disassembly/static-pointer/fragment-owner edges, +confirmed entry roots, unresolved targets and unexplained retention. Empty +reference lists are never a guarantee of unused code. diff --git a/crates/fbuild-core/src/symbol_analysis/graph/README.md b/crates/fbuild-core/src/symbol_analysis/graph/README.md index 2e235f72..d5358eef 100644 --- a/crates/fbuild-core/src/symbol_analysis/graph/README.md +++ b/crates/fbuild-core/src/symbol_analysis/graph/README.md @@ -17,3 +17,9 @@ while preserving the public API previously exposed as keep the original import path. - **`tests.rs`** — `#[cfg(test)]` suite covering the full walker + serialization path; only loaded by `mod.rs` under `#[cfg(test)]`. + +`typed.rs` traverses versioned final-image edges using full symbol identities. +The report index is reused for adjacency and endpoint lookups. Typed graphs +label instruction, static-pointer and fragment-owner evidence distinctly and +honor direction, depth, archive filtering/collapse and fan-out controls. Legacy +reports retain their historical walker. diff --git a/crates/fbuild-core/src/symbol_analysis/graph/mod.rs b/crates/fbuild-core/src/symbol_analysis/graph/mod.rs index 3a893c3b..6fde47d9 100644 --- a/crates/fbuild-core/src/symbol_analysis/graph/mod.rs +++ b/crates/fbuild-core/src/symbol_analysis/graph/mod.rs @@ -37,6 +37,7 @@ use std::collections::{BTreeMap, BTreeSet, VecDeque}; use super::{FineGrainedSymbolMap, SymbolReference}; +mod typed; mod walker; pub use walker::{CalleeRanked, CallerRanked, rank_callees_dual, rank_callers_dual}; use walker::{ @@ -151,6 +152,8 @@ pub enum NodeKind { /// `size_hint` is the sum of symbol sizes attributed to this TU /// in the report, used both for fan-out ranking and node sizing. TranslationUnit { size_hint: Option }, + /// An address-qualified reference endpoint; does not imply a runtime call. + ReferenceSymbol { size: u64 }, /// A callee — a symbol the root (or one of its callees) calls. /// Distinct from `TranslationUnit` because forward edges are /// per-symbol, not per-TU. `size` is the callee's own flash @@ -192,6 +195,10 @@ pub enum EdgeDirection { #[default] Backward, Forward, + InstructionReference, + StaticPointer, + FragmentOwner, + ObjectReference, } /// Directed edge. For `Backward` edges `from` referenced `to`; for @@ -237,6 +244,9 @@ pub struct BackrefGraph { pub struct TuIndex<'a> { /// `(archive, object) -> Vec<&FineGrainedSymbol>`. `archive: None` /// keeps bare-object TUs separate (`main.cpp.o` has no archive). + by_identity: BTreeMap, + incoming: BTreeMap>, + outgoing: BTreeMap>, by_tu: BTreeMap<(Option, String), Vec<&'a super::FineGrainedSymbol>>, } @@ -253,7 +263,33 @@ impl<'a> TuIndex<'a> { .or_default() .push(s); } - Self { by_tu } + let by_identity = map + .symbols + .iter() + .map(|s| (super::SymbolIdentity::from(s), s)) + .collect(); + let mut incoming = BTreeMap::<_, Vec<_>>::new(); + let mut outgoing = BTreeMap::<_, Vec<_>>::new(); + for edge in &map.reference_analysis.edges { + incoming.entry(edge.target.clone()).or_default().push(edge); + outgoing.entry(edge.source.clone()).or_default().push(edge); + } + Self { + by_tu, + by_identity, + incoming, + outgoing, + } + } + + pub fn symbol(&self, identity: &super::SymbolIdentity) -> Option<&'a super::FineGrainedSymbol> { + self.by_identity.get(identity).copied() + } + pub fn incoming(&self, identity: &super::SymbolIdentity) -> &[&'a super::ReferenceEdge] { + self.incoming.get(identity).map_or(&[], Vec::as_slice) + } + pub fn outgoing(&self, identity: &super::SymbolIdentity) -> &[&'a super::ReferenceEdge] { + self.outgoing.get(identity).map_or(&[], Vec::as_slice) } /// All symbols defined in a TU. @@ -294,6 +330,20 @@ impl BackrefGraph { Self::build_with_index(map, &index, target_mangled, config) } + /// Build from the exact selected row, preserving fragment/address identity. + pub fn build_for_symbol_with_index( + map: &FineGrainedSymbolMap, + index: &TuIndex<'_>, + root: &super::FineGrainedSymbol, + config: &GraphConfig, + ) -> Self { + if typed::available(map) { + typed::build(index, root, config) + } else { + Self::build_with_index(map, index, &root.mangled, config) + } + } + /// Same as [`Self::build`] but reuses a pre-built index — useful when /// emitting graphs for every top-N symbol (the per-symbol index /// rebuild would be O(N²)). @@ -308,7 +358,13 @@ impl BackrefGraph { let root = map .symbols .iter() - .find(|s| s.mangled == target_mangled || s.demangled == target_mangled); + .filter(|s| s.mangled == target_mangled || s.demangled == target_mangled) + .min_by_key(|s| s.source != "nm"); + if typed::available(map) { + if let Some(root) = root { + return typed::build(index, root, config); + } + } let Some(root) = root else { // Unknown symbol: emit a single isolated node so the // caller still gets a renderable .dot, not a parse error. @@ -633,6 +689,21 @@ impl BackrefGraph { EdgeDirection::Backward => { out.push_str(&format!(" \"{}\" -> \"{}\";\n", e.from, e.to)); } + EdgeDirection::InstructionReference + | EdgeDirection::StaticPointer + | EdgeDirection::FragmentOwner + | EdgeDirection::ObjectReference => { + let label = match e.direction { + EdgeDirection::InstructionReference => "instruction reference", + EdgeDirection::StaticPointer => "static pointer", + EdgeDirection::ObjectReference => "object reference", + _ => "fragment owner", + }; + out.push_str(&format!( + " \"{}\" -> \"{}\" [label=\"{label}\"];\n", + e.from, e.to + )); + } EdgeDirection::Forward => { out.push_str(&format!( " \"{}\" -> \"{}\" [style=dashed, color=\"#0066cc\", fontcolor=\"#0066cc\", label=\"calls\"];\n", @@ -677,6 +748,7 @@ fn node_width(n: &GraphNode) -> Option { } => *b, NodeKind::Callee { size, .. } => *size, NodeKind::Caller { size, .. } => *size, + NodeKind::ReferenceSymbol { size } => *size, _ => return None, }; if bytes == 0 { diff --git a/crates/fbuild-core/src/symbol_analysis/graph/tests.rs b/crates/fbuild-core/src/symbol_analysis/graph/tests.rs index 3008c5b5..be4b9918 100644 --- a/crates/fbuild-core/src/symbol_analysis/graph/tests.rs +++ b/crates/fbuild-core/src/symbol_analysis/graph/tests.rs @@ -6,7 +6,7 @@ use crate::symbol_analysis::{ FineGrainedSymbol, FineGrainedSymbolMap, SectionBytes, SymbolReference, }; -fn sym( +pub(super) fn sym( mangled: &str, demangled: &str, size: u64, @@ -31,15 +31,16 @@ fn sym( } } -fn refr(archive: Option<&str>, object: &str) -> SymbolReference { +pub(super) fn refr(archive: Option<&str>, object: &str) -> SymbolReference { SymbolReference { archive: archive.map(|s| s.to_string()), object: object.to_string(), } } -fn map(symbols: Vec) -> FineGrainedSymbolMap { +pub(super) fn map(symbols: Vec) -> FineGrainedSymbolMap { FineGrainedSymbolMap { + reference_analysis: Default::default(), elf_path: "test.elf".to_string(), map_path: None, total_flash: symbols.iter().map(|s| s.size).sum(), @@ -838,3 +839,120 @@ fn rank_callers_dual_sorts_each_axis_independently() { // `tiny` was in neither bucket, so it counts as "other". assert_eq!(other, 1); } + +#[test] +fn typed_graph_prefers_real_symbol_and_shows_static_owner() { + use crate::symbol_analysis::{AnalysisStatus, ReferenceEdge, ReferenceKind}; + let mut fragment = sym("method", "method", 33_611, None, "method.o", vec![]); + fragment.address = 0x300; + fragment.source = "map-derived".into(); + let mut method = sym("method", "method", 619, None, "method.o", vec![]); + method.address = 0x100; + let mut vtable = sym("_ZTVTest", "vtable", 76, None, "method.o", vec![]); + vtable.address = 0x200; + let mut report = map(vec![fragment, method, vtable]); + report.reference_analysis.static_data.status = AnalysisStatus::Analyzed; + report.reference_analysis.edges.push(ReferenceEdge { + source: (&report.symbols[2]).into(), + target: (&report.symbols[1]).into(), + kind: ReferenceKind::StaticData, + offset: Some(72), + }); + let graph = BackrefGraph::build(&report, "method", &GraphConfig::default()); + assert!(matches!( + graph.nodes[0].kind, + NodeKind::RootSymbol { size: 619, .. } + )); + assert!(graph.to_dot().contains("static pointer")); + assert!(graph.nodes.iter().any(|n| n.label.contains("vtable"))); +} + +#[test] +fn typed_graph_exact_roots_and_controls_preserve_identity() { + use crate::symbol_analysis::{AnalysisStatus, ReferenceEdge, ReferenceKind}; + let mut root = sym("method", "method", 619, Some("app.a"), "method.o", vec![]); + root.address = 0x100; + let mut first = sym( + "same_name", + "same_name", + 76, + Some("other.a"), + "first.o", + vec![], + ); + first.address = 0x200; + let mut second = sym( + "same_name", + "same_name", + 24, + Some("other.a"), + "second.o", + vec![], + ); + second.address = 0x300; + let mut fragment = sym("method", "method", 33611, Some("app.a"), "method.o", vec![]); + fragment.address = 0x400; + fragment.source = "map-derived".into(); + let mut report = map(vec![root, first, second, fragment]); + report.reference_analysis.static_data.status = AnalysisStatus::Analyzed; + for (source, target) in [(1, 0), (2, 1)] { + report.reference_analysis.edges.push(ReferenceEdge { + source: (&report.symbols[source]).into(), + target: (&report.symbols[target]).into(), + kind: ReferenceKind::StaticData, + offset: Some(8), + }); + } + let index = TuIndex::build(&report); + let config = GraphConfig { + depth: GraphDepth::Fixed(2), + collapse_archives: vec![], + ..Default::default() + }; + let graph = + BackrefGraph::build_for_symbol_with_index(&report, &index, &report.symbols[0], &config); + assert_eq!(graph.nodes.len(), 3); + assert_ne!(graph.nodes[1].id, graph.nodes[2].id); + let exact = + BackrefGraph::build_for_symbol_with_index(&report, &index, &report.symbols[3], &config); + assert!(matches!( + exact.nodes[0].kind, + NodeKind::RootSymbol { size: 33611, .. } + )); + let adaptive = GraphConfig { + depth: GraphDepth::Adaptive, + ..config.clone() + }; + assert_eq!( + BackrefGraph::build_for_symbol_with_index(&report, &index, &report.symbols[0], &adaptive) + .nodes + .len(), + 2 + ); + let excluded = GraphConfig { + exclude_archives: vec!["other.a".into()], + ..config.clone() + }; + assert_eq!( + BackrefGraph::build_for_symbol_with_index(&report, &index, &report.symbols[0], &excluded) + .nodes + .len(), + 1 + ); + let collapsed = GraphConfig { + collapse_archives: vec!["other.a".into()], + ..config.clone() + }; + let graph = + BackrefGraph::build_for_symbol_with_index(&report, &index, &report.symbols[0], &collapsed); + assert!(matches!(graph.nodes[1].kind, NodeKind::Collapsed { .. })); + let capped = GraphConfig { + fan_out: 0, + ..config + }; + assert!( + BackrefGraph::build_for_symbol_with_index(&report, &index, &report.symbols[0], &capped) + .to_dot() + .contains("more references") + ); +} diff --git a/crates/fbuild-core/src/symbol_analysis/graph/typed.rs b/crates/fbuild-core/src/symbol_analysis/graph/typed.rs new file mode 100644 index 00000000..372f6a57 --- /dev/null +++ b/crates/fbuild-core/src/symbol_analysis/graph/typed.rs @@ -0,0 +1,548 @@ +//! Traversal of address-qualified final-image evidence, without name-only joins. +use super::*; +use crate::symbol_analysis::{ + AnalysisStatus, FineGrainedSymbol, ReferenceEdge, ReferenceKind, SymbolIdentity, +}; + +pub(super) fn available(map: &FineGrainedSymbolMap) -> bool { + map.reference_analysis.disassembly.status == AnalysisStatus::Analyzed + || map.reference_analysis.static_data.status == AnalysisStatus::Analyzed +} + +fn node_id(identity: &SymbolIdentity) -> String { + let encoded: String = identity + .name + .as_bytes() + .iter() + .map(|b| format!("{b:02x}")) + .collect(); + format!( + "sym__{encoded}__{:x}__{}", + identity.address, + sanitize_id(&identity.source) + ) +} + +fn edge_direction(kind: &ReferenceKind) -> EdgeDirection { + match kind { + ReferenceKind::Disassembly => EdgeDirection::InstructionReference, + ReferenceKind::StaticData => EdgeDirection::StaticPointer, + ReferenceKind::FragmentOwner => EdgeDirection::FragmentOwner, + } +} + +struct Candidate<'a> { + identity: SymbolIdentity, + symbol: Option<&'a FineGrainedSymbol>, + edges: Vec<&'a ReferenceEdge>, +} +impl Candidate<'_> { + fn size(&self) -> u64 { + self.symbol.map_or(0, |s| s.size) + } + fn archive(&self) -> Option<&str> { + self.symbol.and_then(|s| s.archive.as_deref()) + } +} + +fn candidates<'a>( + index: &TuIndex<'a>, + identity: &SymbolIdentity, + incoming: bool, + config: &GraphConfig, +) -> Vec> { + let adjacency = if incoming { + &index.incoming + } else { + &index.outgoing + }; + let mut grouped = BTreeMap::>::new(); + for edge in adjacency.get(identity).into_iter().flatten() { + let target = if incoming { &edge.source } else { &edge.target }; + grouped.entry(target.clone()).or_default().push(*edge); + } + let mut candidates: Vec<_> = grouped + .into_iter() + .map(|(identity, edges)| { + let symbol = index.by_identity.get(&identity).copied(); + Candidate { + identity, + symbol, + edges, + } + }) + .filter(|c| { + !c.archive() + .is_some_and(|a| config.exclude_archives.iter().any(|x| x == a)) + }) + .collect(); + candidates.sort_by(|a, b| { + b.size() + .cmp(&a.size()) + .then_with(|| a.identity.cmp(&b.identity)) + }); + candidates +} + +struct Walker<'a, 'b> { + index: &'b TuIndex<'a>, + config: &'b GraphConfig, + root_archive: Option, + graph: BackrefGraph, + nodes: BTreeSet, + expanded: BTreeSet<(SymbolIdentity, bool)>, + queue: VecDeque<(SymbolIdentity, bool, u32)>, +} +impl<'a, 'b> Walker<'a, 'b> { + fn expand(&mut self, identity: SymbolIdentity, incoming: bool, depth: u32) { + let limit = match self.config.depth { + GraphDepth::Fixed(n) => n.min(self.config.max_depth), + GraphDepth::Adaptive => self.config.max_depth, + }; + if depth >= limit || !self.expanded.insert((identity.clone(), incoming)) { + return; + } + let current = node_id(&identity); + let candidates = candidates(self.index, &identity, incoming, self.config); + let mut collapsed = BTreeMap::>>::new(); + let mut remaining = Vec::new(); + for c in &candidates { + if let Some(archive) = c + .archive() + .filter(|a| self.config.collapse_archives.iter().any(|x| x == a)) + { + collapsed.entry(archive.to_string()).or_default().push(c); + continue; + } + remaining.push(c); + } + for c in remaining.iter().take(self.config.fan_out) { + let id = node_id(&c.identity); + if self.nodes.insert(id.clone()) { + self.graph.nodes.push(GraphNode { + id: id.clone(), + label: format!( + "{}\n{} B\n0x{:x} · {}", + c.symbol + .map_or(c.identity.name.as_str(), |s| s.demangled.as_str()), + c.size(), + c.identity.address, + c.identity.source + ), + archive: c.symbol.and_then(|s| s.archive.clone()), + object: c.symbol.and_then(|s| s.object.clone()), + kind: NodeKind::ReferenceSymbol { size: c.size() }, + depth: depth + 1, + }); + } + self.connect(¤t, &id, incoming, &c.edges); + let crosses_archive = + self.root_archive.is_some() && c.archive() != self.root_archive.as_deref(); + if !matches!(self.config.depth, GraphDepth::Adaptive) || !crosses_archive { + self.queue + .push_back((c.identity.clone(), incoming, depth + 1)); + } + } + for (archive, members) in collapsed { + let id = format!( + "typed_collapse__{current}__{}__{incoming}", + sanitize_id(&archive) + ); + if self.nodes.insert(id.clone()) { + self.graph.nodes.push(GraphNode { + id: id.clone(), + label: format!("{archive}\n{} references", members.len()), + archive: Some(archive.clone()), + object: None, + kind: NodeKind::Collapsed { + archive, + count: members.len(), + }, + depth: depth + 1, + }); + } + for c in members { + self.connect(¤t, &id, incoming, &c.edges); + } + } + let overflow = remaining.len().saturating_sub(self.config.fan_out); + if overflow > 0 { + let id = format!("typed_overflow__{current}__{incoming}"); + self.graph.nodes.push(GraphNode { + id: id.clone(), + label: format!("(… and {overflow} more references)"), + archive: None, + object: None, + kind: NodeKind::Collapsed { + archive: "(overflow)".into(), + count: overflow, + }, + depth: depth + 1, + }); + self.graph.edges.push(if incoming { + GraphEdge::backward(id, current) + } else { + // Overflow may mix static pointers and instruction references; + // an unqualified arrow must never claim runtime calls. + GraphEdge::backward(current, id) + }); + } + } + fn connect(&mut self, current: &str, other: &str, incoming: bool, evidence: &[&ReferenceEdge]) { + for e in evidence { + let (from, to) = if incoming { + (other, current) + } else { + (current, other) + }; + let edge = GraphEdge { + from: from.into(), + to: to.into(), + direction: edge_direction(&e.kind), + }; + if !self.graph.edges.contains(&edge) { + self.graph.edges.push(edge); + } + } + } +} + +pub(super) fn build( + index: &TuIndex<'_>, + root: &FineGrainedSymbol, + config: &GraphConfig, +) -> BackrefGraph { + let identity = SymbolIdentity::from(root); + let id = node_id(&identity); + let root_node = GraphNode { + id: id.clone(), + label: format!( + "{}\n{} B\n0x{:x} · {}", + root.demangled, root.size, root.address, root.source + ), + archive: root.archive.clone(), + object: root.object.clone(), + kind: NodeKind::RootSymbol { + demangled: root.demangled.clone(), + size: root.size, + }, + depth: 0, + }; + let mut walker = Walker { + index, + config, + root_archive: root.archive.clone(), + graph: BackrefGraph { + root_id: id.clone(), + nodes: vec![root_node], + edges: vec![], + }, + nodes: BTreeSet::from([id]), + expanded: BTreeSet::new(), + queue: VecDeque::new(), + }; + if matches!( + config.direction, + Direction::Backward | Direction::Bidirectional + ) { + walker.queue.push_back((identity.clone(), true, 0)); + } + if matches!( + config.direction, + Direction::Forward | Direction::Bidirectional + ) { + walker.queue.push_back((identity, false, 0)); + } + while let Some((identity, incoming, depth)) = walker.queue.pop_front() { + walker.expand(identity, incoming, depth); + } + add_objects(&mut walker.graph, index, root, config); + walker.graph +} + +fn add_objects( + graph: &mut BackrefGraph, + index: &TuIndex<'_>, + root: &FineGrainedSymbol, + config: &GraphConfig, +) { + let limit = match config.depth { + GraphDepth::Fixed(n) => n.min(config.max_depth), + GraphDepth::Adaptive => config.max_depth, + }; + if config.direction == Direction::Forward || limit == 0 { + return; + } + let mut nodes = BTreeSet::new(); + let mut queue = VecDeque::from([(root.referenced_by.clone(), graph.root_id.clone(), 1)]); + while let Some((references, target, depth)) = queue.pop_front() { + for reference in rank_and_cap_referencers(&references, index, config, &root.archive, depth) + { + let (node, tu) = object_node(reference, index, &target, depth); + let id = node.id.clone(); + let fresh = nodes.insert(id.clone()); + if fresh { + graph.nodes.push(node); + } + let edge = GraphEdge { + from: id.clone(), + to: target.clone(), + direction: EdgeDirection::ObjectReference, + }; + if edge.from != edge.to && !graph.edges.contains(&edge) { + graph.edges.push(edge); + } + let Some(tu) = tu else { continue }; + let crosses_archive = root.archive.is_some() && tu.archive != root.archive; + if !fresh + || depth >= limit + || (matches!(config.depth, GraphDepth::Adaptive) && crosses_archive) + { + continue; + } + let mut seen = BTreeSet::new(); + let mut parents = Vec::new(); + for symbol in index.symbols_in(&tu) { + for parent in &symbol.referenced_by { + let key = (parent.archive.clone(), parent.object.clone()); + if key != (tu.archive.clone(), tu.object.clone()) && seen.insert(key) { + parents.push(parent.clone()); + } + } + } + queue.push_back((parents, id, depth + 1)); + } + } +} + +fn object_node( + reference: CappedReferencer, + index: &TuIndex<'_>, + target: &str, + depth: u32, +) -> (GraphNode, Option) { + let (identity, label, archive, object, kind, tu) = match reference { + CappedReferencer::Tu(tu) => { + let size = index.bytes_in(&tu); + ( + format!("{:?}/{}", tu.archive, tu.object), + format!("{}\nobject reference\n{size} B in TU", tu.object), + tu.archive.clone(), + Some(tu.object.clone()), + NodeKind::TranslationUnit { + size_hint: Some(size), + }, + Some(tu), + ) + } + CappedReferencer::CollapsedArchive { archive, count } => ( + format!("collapse/{target}/{archive}"), + format!("{archive}\n{count} object references"), + Some(archive.clone()), + None, + NodeKind::Collapsed { archive, count }, + None, + ), + CappedReferencer::FanOutOverflow { count } => ( + format!("overflow/{target}/{depth}"), + format!("(… and {count} more object references)"), + None, + None, + NodeKind::Collapsed { + archive: "(overflow)".into(), + count, + }, + None, + ), + }; + let id = node_id(&SymbolIdentity { + name: identity, + address: 0, + source: "object".into(), + }); + ( + GraphNode { + id, + label, + archive, + object, + kind, + depth, + }, + tu, + ) +} + +#[cfg(test)] +mod tests { + use super::super::tests::{map, refr, sym}; + use super::*; + + fn typed_chain( + mut symbols: Vec, + edges: &[(usize, usize)], + ) -> FineGrainedSymbolMap { + use crate::symbol_analysis::{AnalysisStatus, ReferenceEdge, ReferenceKind}; + for (i, symbol) in symbols.iter_mut().enumerate() { + symbol.address += i as u64 * 0x100; + } + let mut report = map(symbols); + report.reference_analysis.static_data.status = AnalysisStatus::Analyzed; + for &(source, target) in edges { + report.reference_analysis.edges.push(ReferenceEdge { + source: (&report.symbols[source]).into(), + target: (&report.symbols[target]).into(), + kind: ReferenceKind::StaticData, + offset: Some(8), + }); + } + report + } + + #[test] + fn typed_forward_overflow_never_claims_runtime_calls() { + let report = typed_chain( + vec![ + sym("root", "root", 100, None, "root.o", vec![]), + sym("target", "target", 10, None, "target.o", vec![]), + ], + &[(0, 1)], + ); + let config = GraphConfig { + direction: Direction::Forward, + fan_out: 0, + ..Default::default() + }; + let dot = BackrefGraph::build(&report, "root", &config).to_dot(); + assert!(dot.contains("more references")); + assert!(!dot.contains("label=\"calls\"")); + } + + #[test] + fn typed_collapse_precedes_fanout_and_counts_the_entire_archive() { + let mut symbols = vec![ + sym("root", "root", 100, Some("app.a"), "root.o", vec![]), + sym( + "application", + "application", + 1, + Some("app.a"), + "app.o", + vec![], + ), + ]; + for i in 0..6 { + symbols.push(sym( + &format!("libc{i}"), + &format!("libc{i}"), + 1000, + Some("libc.a"), + &format!("libc{i}.o"), + vec![], + )); + } + let edges: Vec<_> = (1..symbols.len()).map(|i| (i, 0)).collect(); + let report = typed_chain(symbols, &edges); + let config = GraphConfig { + depth: GraphDepth::Fixed(1), + fan_out: 1, + ..Default::default() + }; + let graph = BackrefGraph::build(&report, "root", &config); + assert!( + graph + .nodes + .iter() + .any(|n| n.label.starts_with("application\n")) + ); + assert!(graph.nodes.iter().any( + |n| matches!(&n.kind, NodeKind::Collapsed { archive, count: 6 } if archive == "libc.a") + )); + assert!(!graph.to_dot().contains("more references")); + } + + #[test] + fn typed_adaptive_bare_root_can_expand_archived_endpoints() { + let report = typed_chain( + vec![ + sym("root", "root", 100, None, "root.o", vec![]), + sym("first", "first", 10, Some("driver.a"), "first.o", vec![]), + sym("second", "second", 10, Some("driver.a"), "second.o", vec![]), + ], + &[(1, 0), (2, 1)], + ); + let graph = BackrefGraph::build(&report, "root", &GraphConfig::default()); + assert_eq!(graph.nodes.len(), 3); + } + + #[test] + fn typed_object_references_preserve_transitive_depth_cycles_and_controls() { + let report = typed_chain( + vec![ + sym( + "root", + "root", + 100, + Some("app.a"), + "root.o", + vec![refr(Some("app.a"), "first.o")], + ), + sym( + "first", + "first", + 20, + Some("app.a"), + "first.o", + vec![refr(Some("app.a"), "second.o")], + ), + sym( + "second", + "second", + 30, + Some("app.a"), + "second.o", + vec![refr(Some("app.a"), "first.o")], + ), + ], + &[], + ); + let config = GraphConfig { + depth: GraphDepth::Fixed(3), + collapse_archives: vec![], + ..Default::default() + }; + let graph = BackrefGraph::build(&report, "root", &config); + assert_eq!(graph.nodes.len(), 3); + assert!( + graph + .nodes + .iter() + .any(|n| n.object.as_deref() == Some("second.o") && n.depth == 2) + ); + assert_eq!(graph.edges.len(), 3); + let shallow = GraphConfig { + depth: GraphDepth::Fixed(1), + ..config.clone() + }; + assert_eq!( + BackrefGraph::build(&report, "root", &shallow).nodes.len(), + 2 + ); + let excluded = GraphConfig { + exclude_archives: vec!["app.a".into()], + ..config.clone() + }; + assert_eq!( + BackrefGraph::build(&report, "root", &excluded).nodes.len(), + 1 + ); + let forward = GraphConfig { + direction: Direction::Forward, + ..config + }; + assert_eq!( + BackrefGraph::build(&report, "root", &forward).nodes.len(), + 1 + ); + } +} diff --git a/crates/fbuild-core/src/symbol_analysis/mod.rs b/crates/fbuild-core/src/symbol_analysis/mod.rs index ab71ebfa..3903dc30 100644 --- a/crates/fbuild-core/src/symbol_analysis/mod.rs +++ b/crates/fbuild-core/src/symbol_analysis/mod.rs @@ -23,6 +23,8 @@ pub mod callgraph; pub mod cref; pub mod graph; pub mod markers; +pub mod references; +pub use references::*; use std::collections::BTreeMap; @@ -131,6 +133,9 @@ pub struct SectionBytes { /// The complete per-symbol view of a single binary. #[derive(Debug, Clone, Serialize, Deserialize)] pub struct FineGrainedSymbolMap { + /// Versioned, address-qualified final-image reference evidence (#1661). + #[serde(default)] + pub reference_analysis: ReferenceAnalysis, pub elf_path: String, pub map_path: Option, pub total_flash: u64, @@ -611,6 +616,7 @@ pub fn region_from_output_section(name: &str) -> Option { pub fn classify_region(sym_type: char) -> Option { match sym_type { 'T' | 't' | 'R' | 'r' | 'W' | 'w' => Some(MemoryRegion::Flash), + 'V' | 'v' => Some(MemoryRegion::Flash), 'D' | 'd' | 'B' | 'b' => Some(MemoryRegion::Ram), _ => None, } @@ -770,14 +776,21 @@ pub fn build_fine_grained_map_with_synth( let mut total_flash = 0u64; let mut total_ram = 0u64; for ((addr, size, sym_type, mangled), demangled) in nm_rows.into_iter().zip(demangled) { - let Some(region) = classify_region(sym_type) else { + let attribution = index.lookup(addr); + let region = if matches!(sym_type, 'V' | 'v') { + attribution + .and_then(|r| region_from_output_section(&r.output_section)) + .or_else(|| classify_region(sym_type)) + } else { + classify_region(sym_type) + }; + let Some(region) = region else { continue; }; match region { MemoryRegion::Flash => total_flash += size, MemoryRegion::Ram => total_ram += size, } - let attribution = index.lookup(addr); let referenced_by = cref.get(&mangled).cloned().unwrap_or_default(); symbols.push(FineGrainedSymbol { mangled, @@ -841,6 +854,7 @@ pub fn build_fine_grained_map_with_synth( }); } FineGrainedSymbolMap { + reference_analysis: ReferenceAnalysis::default(), elf_path, map_path, total_flash, diff --git a/crates/fbuild-core/src/symbol_analysis/references.rs b/crates/fbuild-core/src/symbol_analysis/references.rs new file mode 100644 index 00000000..1a53537f --- /dev/null +++ b/crates/fbuild-core/src/symbol_analysis/references.rs @@ -0,0 +1,99 @@ +//! Versioned reference evidence for the final linked image. Missing edges are +//! not evidence of dead code: indirect dispatch and linker roots are partial. +use serde::{Deserialize, Serialize}; + +#[derive(Debug, Clone, PartialEq, Eq, PartialOrd, Ord, Serialize, Deserialize)] +pub struct SymbolIdentity { + pub name: String, + pub address: u64, + pub source: String, +} + +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +#[serde(rename_all = "snake_case")] +pub enum ReferenceKind { + Disassembly, + StaticData, + FragmentOwner, +} + +#[derive(Debug, Clone, Serialize, Deserialize)] +pub struct ReferenceEdge { + pub source: SymbolIdentity, + pub target: SymbolIdentity, + pub kind: ReferenceKind, + pub offset: Option, +} + +#[derive(Debug, Clone, Default, PartialEq, Eq, Serialize, Deserialize)] +#[serde(rename_all = "snake_case")] +pub enum AnalysisStatus { + Analyzed, + #[default] + Unavailable, + Error, +} + +#[derive(Debug, Clone, Default, Serialize, Deserialize)] +pub struct AnalysisPass { + pub status: AnalysisStatus, + pub tool: Option, + pub reason: Option, +} + +#[derive(Debug, Clone, Serialize, Deserialize)] +pub struct RetentionRoot { + pub symbol: SymbolIdentity, + pub kind: String, +} + +#[derive(Debug, Clone, Serialize, Deserialize)] +pub struct UnresolvedReference { + pub name: String, + pub address: u64, +} + +#[derive(Debug, Clone, Serialize, Deserialize)] +pub struct ReferenceAnalysis { + pub schema: u32, + pub disassembly: AnalysisPass, + pub static_data: AnalysisPass, + pub object_references: AnalysisPass, + pub edges: Vec, + pub roots: Vec, + pub unexplained: Vec, + pub unresolved: Vec, + pub limitations: Vec, +} + +impl Default for ReferenceAnalysis { + fn default() -> Self { + Self { + schema: 1, + disassembly: AnalysisPass::default(), + static_data: AnalysisPass::default(), + object_references: AnalysisPass::default(), + edges: Vec::new(), + roots: Vec::new(), + unexplained: Vec::new(), + unresolved: Vec::new(), + limitations: vec![ + "Runtime indirect calls are not reconstructed.".into(), + "Static pointer extraction covers absolute Itanium vtables; relative vtables and function descriptors are unsupported.".into(), + "KEEP and platform-specific linker roots require additional linker evidence." + .into(), + "Disassembly annotations are references, not necessarily function calls; interior-offset annotations are not reconstructed.".into(), + ], + } + } +} + +impl From<&super::FineGrainedSymbol> for SymbolIdentity { + fn from(symbol: &super::FineGrainedSymbol) -> Self { + Self { + name: symbol.mangled.clone(), + address: symbol.address, + source: symbol.source.clone(), + } + } +} diff --git a/crates/fbuild-core/src/symbol_analysis/tests.rs b/crates/fbuild-core/src/symbol_analysis/tests.rs index 1783bc1e..83cc0482 100644 --- a/crates/fbuild-core/src/symbol_analysis/tests.rs +++ b/crates/fbuild-core/src/symbol_analysis/tests.rs @@ -507,6 +507,7 @@ fn retain_loaded_symbols_drops_boundary_markers() { sample_symbol(0x00026100, u64::MAX, MemoryRegion::Flash, "overflow"), ]; let mut map = FineGrainedSymbolMap { + reference_analysis: Default::default(), elf_path: "fixture.elf".into(), map_path: None, total_flash: 0, @@ -689,6 +690,7 @@ fn retain_loaded_symbols_no_op_when_regions_empty() { // Defensive: if the caller couldn't probe PT_LOAD (corrupt ELF, // non-ELF input), leave the map untouched rather than empty it. let mut map = FineGrainedSymbolMap { + reference_analysis: Default::default(), elf_path: "fixture.elf".into(), map_path: None, total_flash: 0x40, @@ -717,6 +719,7 @@ fn find_symbol_dispatches_correctly() { let mut other = sample_symbol(0x3000, 25, MemoryRegion::Flash, "ns::other()"); other.mangled = "_ZN2ns5otherEv".to_string(); let map = FineGrainedSymbolMap { + reference_analysis: Default::default(), elf_path: "x.elf".into(), map_path: None, total_flash: 175, @@ -894,3 +897,24 @@ fn strip_unsized_symbols_drops_nm_synthesised_sizes() { [(0x40374000u64, "_WindowOverflow4".to_string())].into(); assert_eq!(strip_unsized_symbols(host, &zero_sized), cross); } + +#[test] +fn weak_objects_are_included_and_use_section_region() { + let rows = vec![ + (0x1000, 24, 'V', "_ZTVTest".into()), + (0x2000, 8, 'v', "weak_ram".into()), + ]; + let ranges = parse_linker_map( + "Linker script and memory map\n.rodata 0x1000 0x18\n .rodata._ZTVTest 0x1000 0x18 test.o\n.data 0x2000 0x8\n .data.weak_ram 0x2000 0x8 test.o\n", + ); + let map = build_fine_grained_map( + "test.elf".into(), + None, + rows, + vec!["vtable".into(), "weak_ram".into()], + ranges, + ); + assert_eq!(map.symbols.len(), 2); + assert_eq!(map.symbols[0].region, MemoryRegion::Flash); + assert_eq!(map.symbols[1].region, MemoryRegion::Ram); +} diff --git a/docs/symbols.md b/docs/symbols.md index 36fe7ce9..7099cc2f 100644 --- a/docs/symbols.md +++ b/docs/symbols.md @@ -35,7 +35,7 @@ downstream tools. These are looked up in this order: 1. **`--nm ` / `--cppfilt ` flags** (highest precedence; the user's explicit override always wins). 2. **`--build-info `** — load `nm_path` / `cppfilt_path` from - that file. + that file, accepting native fields or partial PlatformIO `aliases`. 3. **Auto-discovery** — walk up from the ELF's directory looking for `build_info.json` or `build_info_.json`. Both fbuild and PlatformIO write one next to `platformio.ini`. @@ -69,16 +69,22 @@ block) next to `platformio.ini`. `fbuild symbols .pio/build/esp32s3/firmware.elf walks up from `.pio/build/esp32s3/` to the project root, finds it, and reads the toolchain paths from there. No `--nm` needed. -## When auto-discovery falls back to PATH +## Metadata selection and PATH fallback -- The ELF isn't under a project that contains a `build_info.json` - (e.g. you copied just the ELF into `/tmp`). -- The `build_info.json` is older than the schema this fbuild expects - (`nm_path` field missing). +Multi-environment metadata selects the environment whose `prog_path` matches +this ELF, or the ELF's `build//` directory (including nested build profiles). +Automatic discovery prefers the matching `build_info_.json`, then +`build_info.json`. Ambiguous +metadata files or environments and malformed metadata are explicit errors; +fbuild does not silently substitute host tools. -In both cases the analyzer falls back to bare `nm` on `PATH`, which -works only for host ELFs. For cross-toolchain ELFs, pass `--nm` or -`--build-info` explicitly. +An explicit `--nm` also selects its sibling `c++filt` and `objdump`, rather +than mixing that toolchain with tools from build metadata. `--cppfilt` still +has highest precedence. + +When no metadata exists, the analyzer falls back to bare `nm` on `PATH`, +which works only for host ELFs. For a copied cross-toolchain ELF, pass +`--nm` or `--build-info` explicitly. ## Schema: the `aliases` block @@ -193,3 +199,49 @@ sanctioned answer is "use `fbuild bloat .` and skip the awk". `referenced_by` cref back-references on every row. - PR [#424] / PR [#427] — the fine-grained analyzer and map-derived rodata attribution this CLI drives. + +## Final-image reference evidence (schema 1) + +Reports include `reference_analysis` alongside the legacy name lists. Each +analysis pass (`disassembly`, `static_data`, `object_references`) records +`analyzed`, `unavailable`, or `error`, its tool when applicable, and a reason. +An analyzed pass means the supported extraction ran; it does not claim a +complete runtime call graph. + +Edges identify each endpoint by `(name, address, source)` so a function and its +map-derived literal/string pools cannot be confused. Edge kinds are: + +- `disassembly`: an address-qualified symbol annotation in the final ELF's + disassembly, which may be a code or data reference, not necessarily a call. +- `static_data`: an absolute function pointer in an allocated Itanium vtable; + `offset` is the pointer slot's byte offset from the owning vtable. +- `fragment_owner`: ownership recorded by the compiler's map input-section + name. This is attribution evidence, not proof of a runtime access. + +Allocated `V`/`v` weak objects are included and classified using their ELF +sections. AVR's two-byte word-addressed function pointers and ARM's Thumb bit +are handled explicitly. Relative vtables and function-descriptor ABIs are not +reconstructed. Arbitrary integers in data are not guessed to be pointers. +Static extraction reports an error for position-independent/shared ELFs until +their dynamic relocations are supported, rather than claiming an empty analysis. + +`roots` records confirmed ELF entry points, including unsized entries omitted +from the byte tables. `unresolved` retains external/ROM/local targets with their +addresses; `unexplained` retains positive-size rows without a recorded incoming +edge, object reference, or confirmed root. Empty lists do not prove unused code: +indirect dispatch, generic callback tables, `KEEP` directives and additional +platform linker roots may remain unexplained. `limitations` states that scope +in the transport so consumers can show it alongside the graph. + +Tool discovery accepts the diagnostic paths in native build metadata and the +partial per-environment `aliases` metadata used by external builders. It +selects the ELF's matching environment and rejects ambiguous or invalid +metadata instead of silently substituting host tools. Explicit `--nm` selects +its sibling tools before metadata from a different toolchain. + +Graphviz graphs and per-row sidecars consume the same typed identities and +edge kinds. Static pointers and fragment ownership are labeled separately from +instruction references. Sidecars root at the exact selected row, so data pools +sharing a function's name remain distinct. For ambiguous local names, the CLI +accepts `--symbol 'name@0xADDRESS'`; it rejects an ambiguous name instead of +choosing an arbitrary function. diff --git a/pyproject.toml b/pyproject.toml index 5789b19a..bd0e0e5f 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -1,6 +1,6 @@ [project] name = "fbuild" -version = "2.5.37" +version = "2.5.38" description = "PlatformIO-compatible embedded build tool (Rust implementation)" readme = "README.md" license = "AGPL-3.0-only"