From 53440fe27e2f7150a86742fbd9f628b450270ba8 Mon Sep 17 00:00:00 2001 From: Fabian Lippold Date: Fri, 4 Sep 2026 05:29:04 +0200 Subject: [PATCH 1/2] Scope one-shot diffs to the requested symbol Use a symbol-specific core diff path when a positional symbol is provided. This avoids processing unrelated symbols and sections, preventing malformed data symbols from aborting output and substantially reducing work for large binaries while preserving object-wide indexes. --- objdiff-cli/src/cmd/diff.rs | 13 ++++- objdiff-core/src/diff/mod.rs | 94 ++++++++++++++++++++++++++++++ objdiff-core/tests/arch_x86.rs | 101 +++++++++++++++++++++++++++++++++ 3 files changed, 206 insertions(+), 2 deletions(-) diff --git a/objdiff-cli/src/cmd/diff.rs b/objdiff-cli/src/cmd/diff.rs index 96fb454f..15d43d43 100644 --- a/objdiff-cli/src/cmd/diff.rs +++ b/objdiff-cli/src/cmd/diff.rs @@ -212,8 +212,17 @@ fn run_oneshot( .with_context(|| format!("Loading {p}")) }) .transpose()?; - let result = - diff::diff_objs(target.as_ref(), base.as_ref(), None, &diff_config, &mapping_config)?; + let result = if let Some(symbol_name) = args.symbol.as_deref() { + diff::diff_objs_for_symbol( + target.as_ref(), + base.as_ref(), + symbol_name, + &diff_config, + &mapping_config, + )? + } else { + diff::diff_objs(target.as_ref(), base.as_ref(), None, &diff_config, &mapping_config)? + }; let left = target.as_ref().zip(result.left.as_ref()); let right = base.as_ref().zip(result.right.as_ref()); let diff_result = DiffResult::new(left, right, &diff_config)?; diff --git a/objdiff-core/src/diff/mod.rs b/objdiff-core/src/diff/mod.rs index f82242df..ff49c2c6 100644 --- a/objdiff-core/src/diff/mod.rs +++ b/objdiff-core/src/diff/mod.rs @@ -197,6 +197,100 @@ pub struct DiffObjsResult { pub prev: Option, } +fn diffable_symbol_by_name(obj: &Object, name: &str) -> Option { + obj.symbols.iter().position(|symbol| { + symbol.name == name + && (symbol.section.is_some() || symbol.flags.contains(SymbolFlag::Common)) + }) +} + +/// Diff a single named symbol while preserving the object-wide symbol and section indexes. +/// +/// This is intended for consumers that only need one symbol's detailed diff. The returned +/// [`ObjectDiff`] values still contain placeholder entries for every symbol and section so that +/// indexes in relocations and `target_symbol` remain valid, but no unrelated symbols or sections +/// are diffed. +pub fn diff_objs_for_symbol( + left: Option<&Object>, + right: Option<&Object>, + symbol_name: &str, + diff_config: &DiffObjConfig, + mapping_config: &MappingConfig, +) -> Result { + let left_symbol_idx = left.and_then(|obj| diffable_symbol_by_name(obj, symbol_name)); + let right_symbol_idx = match (left, right, left_symbol_idx) { + (Some(left_obj), Some(right_obj), Some(left_idx)) => mapping_config + .mappings + .get(symbol_name) + .and_then(|right_name| diffable_symbol_by_name(right_obj, right_name)) + .or_else(|| find_symbol(Some(right_obj), left_obj, left_idx, None, false)), + (_, Some(right_obj), _) => diffable_symbol_by_name(right_obj, symbol_name), + _ => None, + }; + + if left_symbol_idx.is_none() && right_symbol_idx.is_none() { + return Err(anyhow!("Symbol not found: {symbol_name}")); + } + + let left_kind = + left_symbol_idx.map(|idx| symbol_section_kind(left.unwrap(), &left.unwrap().symbols[idx])); + let right_kind = right_symbol_idx + .map(|idx| symbol_section_kind(right.unwrap(), &right.unwrap().symbols[idx])); + if let (Some(left_kind), Some(right_kind)) = (left_kind, right_kind) + && left_kind != right_kind + { + return Err(anyhow!( + "Symbol section kind mismatch: {symbol_name} ({left_kind:?} vs {right_kind:?})" + )); + } + let section_kind = left_kind.or(right_kind).unwrap_or(SectionKind::Unknown); + if section_kind == SectionKind::Unknown { + return Err(anyhow!("Symbol has no diffable section: {symbol_name}")); + } + + let mut left_diff = left.map(ObjectDiff::new_from_obj); + let mut right_diff = right.map(ObjectDiff::new_from_obj); + match (left_symbol_idx, right_symbol_idx) { + (Some(left_idx), Some(right_idx)) => { + let (left_symbol_diff, right_symbol_diff) = match section_kind { + SectionKind::Code => { + diff_code(left.unwrap(), right.unwrap(), left_idx, right_idx, diff_config) + } + SectionKind::Data => { + diff_data_symbol(left.unwrap(), right.unwrap(), left_idx, right_idx) + } + SectionKind::Bss | SectionKind::Common => { + diff_bss_symbol(left.unwrap(), right.unwrap(), left_idx, right_idx) + } + SectionKind::Unknown => unreachable!(), + }?; + left_diff.as_mut().unwrap().symbols[left_idx] = left_symbol_diff; + right_diff.as_mut().unwrap().symbols[right_idx] = right_symbol_diff; + } + (Some(left_idx), None) => { + let symbol_diff = match section_kind { + SectionKind::Code => no_diff_code(left.unwrap(), left_idx, diff_config), + SectionKind::Data => no_diff_data_symbol(left.unwrap(), left_idx), + SectionKind::Bss | SectionKind::Common => Ok(SymbolDiff::default()), + SectionKind::Unknown => unreachable!(), + }?; + left_diff.as_mut().unwrap().symbols[left_idx] = symbol_diff; + } + (None, Some(right_idx)) => { + let symbol_diff = match section_kind { + SectionKind::Code => no_diff_code(right.unwrap(), right_idx, diff_config), + SectionKind::Data => no_diff_data_symbol(right.unwrap(), right_idx), + SectionKind::Bss | SectionKind::Common => Ok(SymbolDiff::default()), + SectionKind::Unknown => unreachable!(), + }?; + right_diff.as_mut().unwrap().symbols[right_idx] = symbol_diff; + } + (None, None) => unreachable!(), + } + + Ok(DiffObjsResult { left: left_diff, right: right_diff, prev: None }) +} + pub fn diff_objs( left: Option<&Object>, right: Option<&Object>, diff --git a/objdiff-core/tests/arch_x86.rs b/objdiff-core/tests/arch_x86.rs index e5eb6e80..75097605 100644 --- a/objdiff-core/tests/arch_x86.rs +++ b/objdiff-core/tests/arch_x86.rs @@ -20,6 +20,107 @@ fn read_x86() { insta::assert_snapshot!(output); } +#[test] +#[cfg(feature = "x86")] +fn diff_single_x86_symbol() { + let diff_config = diff::DiffObjConfig::default(); + let obj = obj::read::parse( + include_object!("data/x86/staticdebug.obj"), + &diff_config, + diff::DiffSide::Target, + ) + .unwrap(); + let symbol_name = "?PrintThing@@YAXXZ"; + let symbol_idx = obj.symbol_by_name(symbol_name).unwrap(); + + let result = diff::diff_objs_for_symbol( + Some(&obj), + Some(&obj), + symbol_name, + &diff_config, + &diff::MappingConfig::default(), + ) + .unwrap(); + let left = result.left.unwrap(); + let right = result.right.unwrap(); + + assert_eq!(left.symbols.len(), obj.symbols.len()); + assert_eq!(right.symbols.len(), obj.symbols.len()); + assert_eq!(left.symbols[symbol_idx].target_symbol, Some(symbol_idx)); + assert_eq!(right.symbols[symbol_idx].target_symbol, Some(symbol_idx)); + assert!(!left.symbols[symbol_idx].instruction_rows.is_empty()); + assert!(!right.symbols[symbol_idx].instruction_rows.is_empty()); + assert!(left.sections.iter().all(|section| section.data_diff.is_empty())); + assert!(right.sections.iter().all(|section| section.data_diff.is_empty())); + assert!( + left.symbols + .iter() + .enumerate() + .filter(|(idx, _)| *idx != symbol_idx) + .all(|(_, symbol)| symbol.instruction_rows.is_empty()) + ); + assert!( + right + .symbols + .iter() + .enumerate() + .filter(|(idx, _)| *idx != symbol_idx) + .all(|(_, symbol)| symbol.instruction_rows.is_empty()) + ); +} + +#[test] +#[cfg(feature = "x86")] +fn diff_single_symbol_falls_back_from_missing_mapping() { + let diff_config = diff::DiffObjConfig::default(); + let obj = obj::read::parse( + include_object!("data/x86/staticdebug.obj"), + &diff_config, + diff::DiffSide::Target, + ) + .unwrap(); + let symbol_name = "?PrintThing@@YAXXZ"; + let symbol_idx = obj.symbol_by_name(symbol_name).unwrap(); + let mut mapping_config = diff::MappingConfig::default(); + mapping_config.mappings.insert(symbol_name.into(), "missing".into()); + + let result = diff::diff_objs_for_symbol( + Some(&obj), + Some(&obj), + symbol_name, + &diff_config, + &mapping_config, + ) + .unwrap(); + + assert_eq!(result.left.unwrap().symbols[symbol_idx].target_symbol, Some(symbol_idx)); + assert_eq!(result.right.unwrap().symbols[symbol_idx].target_symbol, Some(symbol_idx)); +} + +#[test] +fn diff_single_common_symbol() { + let common_symbol = obj::Symbol { + name: "common".into(), + size: 4, + flags: obj::SymbolFlag::Common.into(), + ..Default::default() + }; + let left = obj::Object { symbols: vec![common_symbol.clone()], ..Default::default() }; + let right = obj::Object { symbols: vec![common_symbol], ..Default::default() }; + + let result = diff::diff_objs_for_symbol( + Some(&left), + Some(&right), + "common", + &diff::DiffObjConfig::default(), + &diff::MappingConfig::default(), + ) + .unwrap(); + + assert_eq!(result.left.unwrap().symbols[0].target_symbol, Some(0)); + assert_eq!(result.right.unwrap().symbols[0].target_symbol, Some(0)); +} + #[test] #[cfg(feature = "x86")] fn read_x86_combine_sections() { From 30dc39c03abc3bdd6df4ff57fcb4b322db39372d Mon Sep 17 00:00:00 2001 From: Fabian Lippold Date: Fri, 4 Sep 2026 06:24:03 +0200 Subject: [PATCH 2/2] Fix reports for linked ELF binaries Preserve linked section addresses, avoid invalid section merging, and add a summary-only diff path for report generation. Support direct target/base report inputs while retaining project mode as the default. --- objdiff-cli/src/cmd/report.rs | 102 +++++++++++----- objdiff-core/src/diff/data.rs | 66 +++++++++- objdiff-core/src/diff/mod.rs | 212 ++++++++++++++++++++++++++++----- objdiff-core/src/obj/read.rs | 70 ++++++++++- objdiff-core/tests/arch_x86.rs | 41 +++++++ 5 files changed, 428 insertions(+), 63 deletions(-) diff --git a/objdiff-cli/src/cmd/report.rs b/objdiff-cli/src/cmd/report.rs index d4ce7cb4..53a6cdef 100644 --- a/objdiff-cli/src/cmd/report.rs +++ b/objdiff-cli/src/cmd/report.rs @@ -7,7 +7,9 @@ use objdiff_core::{ ChangeItem, ChangeItemInfo, ChangeUnit, Changes, ChangesInput, Measures, REPORT_VERSION, Report, ReportCategory, ReportItem, ReportItemMetadata, ReportUnit, ReportUnitMetadata, }, - config::{ProjectObject, ProjectOptions, apply_project_options, path::platform_path}, + config::{ + ProjectConfig, ProjectObject, ProjectOptions, apply_project_options, path::platform_path, + }, diff, obj::{self, SectionKind, SymbolFlag, SymbolKind}, }; @@ -37,9 +39,15 @@ pub enum SubCommand { } #[derive(FromArgs, PartialEq, Debug)] -/// Generate a progress report for a project. +/// Generate a progress report for a project or a pair of object files. #[argp(subcommand, name = "generate")] pub struct GenerateArgs { + #[argp(option, short = '1', from_str_fn(platform_path))] + /// Target object file + target: Option, + #[argp(option, short = '2', from_str_fn(platform_path))] + /// Base object file + base: Option, #[argp(option, short = 'p', from_str_fn(platform_path))] /// Project directory project: Option, @@ -92,34 +100,62 @@ fn generate(args: GenerateArgs) -> Result<()> { }; let output_format = OutputFormat::from_option(args.format.as_deref())?; - let project_dir = args.project.as_deref().unwrap_or_else(|| Utf8PlatformPath::new(".")); - info!("Loading project {}", project_dir); + let direct_input = args.target.is_some() || args.base.is_some(); + if direct_input && args.project.is_some() { + bail!("--project cannot be combined with --target or --base"); + } - let project = match objdiff_core::config::try_project_config(project_dir.as_ref()) { - Some((Ok(config), _)) => config, - Some((Err(err), _)) => bail!("Failed to load project configuration: {}", err), - None => bail!("No project configuration found"), + let project_dir = args.project.as_deref().unwrap_or_else(|| Utf8PlatformPath::new(".")); + let project = if direct_input { + info!("Loading input objects"); + ProjectConfig::default() + } else { + info!("Loading project {}", project_dir); + match objdiff_core::config::try_project_config(project_dir.as_ref()) { + Some((Ok(config), _)) => config, + Some((Err(err), _)) => bail!("Failed to load project configuration: {}", err), + None => bail!("No project configuration found"), + } }; let target_obj_dir = project.target_dir.as_ref().map(|p| project_dir.join(p.with_platform_encoding())); let base_obj_dir = project.base_dir.as_ref().map(|p| project_dir.join(p.with_platform_encoding())); let project_units = project.units.as_deref().unwrap_or_default(); - let objects = project_units - .iter() - .enumerate() - .map(|(idx, o)| { - ( - ObjectConfig::new( - o, - project_dir, - target_obj_dir.as_deref(), - base_obj_dir.as_deref(), - ), - idx, - ) - }) - .collect::>(); + let objects = if direct_input { + let name = args + .target + .as_deref() + .or(args.base.as_deref()) + .and_then(Utf8PlatformPath::file_name) + .unwrap_or("input") + .to_string(); + vec![( + ObjectConfig { + name, + target_path: args.target.clone(), + base_path: args.base.clone(), + ..Default::default() + }, + 0, + )] + } else { + project_units + .iter() + .enumerate() + .map(|(idx, o)| { + ( + ObjectConfig::new( + o, + project_dir, + target_obj_dir.as_deref(), + base_obj_dir.as_deref(), + ), + idx, + ) + }) + .collect::>() + }; info!( "Generating report for {} units (using {} threads)", objects.len(), @@ -215,7 +251,7 @@ fn report_object( selecting_left: None, selecting_right: None, }; - let target = object + let mut target = object .target_path .as_ref() .map(|p| { @@ -223,7 +259,7 @@ fn report_object( .with_context(|| format!("Failed to open {p}")) }) .transpose()?; - let base = object + let mut base = object .base_path .as_ref() .map(|p| { @@ -231,8 +267,20 @@ fn report_object( .with_context(|| format!("Failed to open {p}")) }) .transpose()?; - let result = - diff::diff_objs(target.as_ref(), base.as_ref(), None, diff_config, &mapping_config)?; + for obj in target.iter_mut().chain(base.iter_mut()) { + for symbol in &mut obj.symbols { + if symbol.kind == SymbolKind::Section { + symbol.flags |= SymbolFlag::Ignored; + } + } + } + let result = diff::diff_objs_summary( + target.as_ref(), + base.as_ref(), + None, + diff_config, + &mapping_config, + )?; let metadata = ReportUnitMetadata { complete: object.metadata.complete, diff --git a/objdiff-core/src/diff/data.rs b/objdiff-core/src/diff/data.rs index 032cbf99..e7af5c5b 100644 --- a/objdiff-core/src/diff/data.rs +++ b/objdiff-core/src/diff/data.rs @@ -312,6 +312,66 @@ pub fn diff_data_section( Ok((left_section_diff, right_section_diff)) } +/// Calculate a data section's match percentage without constructing a byte-level edit script. +/// +/// Reports only consume the percentage, so comparing fixed-size blocks avoids pathological Myers +/// diff behavior on large linked sections while still accounting for inserted and deleted data. +pub fn diff_data_section_summary( + left_obj: &Object, + right_obj: &Object, + left_diff: &ObjectDiff, + right_diff: &ObjectDiff, + left_section_idx: usize, + right_section_idx: usize, +) -> Result<(SectionDiff, SectionDiff)> { + const BLOCK_SIZE: usize = 16; + + let left_section = &left_obj.sections[left_section_idx]; + let right_section = &right_obj.sections[right_section_idx]; + let left_max = symbols_matching_section(&left_obj.symbols, left_section_idx) + .filter_map(|(_, s)| s.address.checked_sub(left_section.address).map(|a| a + s.size)) + .max() + .unwrap_or(0) + .min(left_section.size); + let right_max = symbols_matching_section(&right_obj.symbols, right_section_idx) + .filter_map(|(_, s)| s.address.checked_sub(right_section.address).map(|a| a + s.size)) + .max() + .unwrap_or(0) + .min(right_section.size); + let left_data = &left_section.data[..left_max as usize]; + let right_data = &right_section.data[..right_max as usize]; + let left_blocks = left_data.chunks(BLOCK_SIZE).collect::>(); + let right_blocks = right_data.chunks(BLOCK_SIZE).collect::>(); + let ops = capture_diff_slices(Algorithm::Patience, &left_blocks, &right_blocks); + let bytes_match_percent = diff_ratio(&ops, left_blocks.len(), right_blocks.len()) * 100.0; + + let all_left_relocs_match = diff_data_relocs_for_range( + left_obj, + right_obj, + left_section_idx, + right_section_idx, + 0..left_max as usize, + 0..right_max as usize, + ) + .iter() + .all(|(kind, left, _)| left.is_none() || *kind == DataDiffKind::None); + + let (mut left_section_diff, right_section_diff) = diff_generic_section( + left_obj, + right_obj, + left_diff, + right_diff, + left_section_idx, + right_section_idx, + )?; + if all_left_relocs_match + && left_section_diff.match_percent.unwrap_or(-1.0) < bytes_match_percent + { + left_section_diff.match_percent = Some(bytes_match_percent); + } + Ok((left_section_diff, right_section_diff)) +} + pub fn no_diff_data_symbol(obj: &Object, symbol_index: usize) -> Result { let symbol = &obj.symbols[symbol_index]; let section_idx = symbol.section.ok_or_else(|| anyhow!("Data symbol section not found"))?; @@ -320,7 +380,7 @@ pub fn no_diff_data_symbol(obj: &Object, symbol_index: usize) -> Result section.size { return Err(anyhow!( @@ -383,11 +443,11 @@ pub fn diff_data_symbol( let left_start = left_symbol .address .checked_sub(left_section.address) - .ok_or_else(|| anyhow!("Symbol address out of section bounds"))?; + .ok_or_else(|| anyhow!("Symbol {} address out of section bounds", left_symbol.name))?; let right_start = right_symbol .address .checked_sub(right_section.address) - .ok_or_else(|| anyhow!("Symbol address out of section bounds"))?; + .ok_or_else(|| anyhow!("Symbol {} address out of section bounds", right_symbol.name))?; let left_end = left_start + left_symbol.size; if left_end > left_section.size { return Err(anyhow!( diff --git a/objdiff-core/src/diff/mod.rs b/objdiff-core/src/diff/mod.rs index ff49c2c6..1acfde1d 100644 --- a/objdiff-core/src/diff/mod.rs +++ b/objdiff-core/src/diff/mod.rs @@ -12,8 +12,9 @@ use crate::{ diff::{ code::{diff_code, no_diff_code}, data::{ - diff_bss_section, diff_bss_symbol, diff_data_section, diff_data_symbol, - diff_generic_section, no_diff_bss_section, no_diff_data_section, no_diff_data_symbol, + diff_bss_section, diff_bss_symbol, diff_data_section, diff_data_section_summary, + diff_data_symbol, diff_generic_section, no_diff_bss_section, no_diff_data_section, + no_diff_data_symbol, }, }, obj::{ @@ -297,6 +298,28 @@ pub fn diff_objs( prev: Option<&Object>, diff_config: &DiffObjConfig, mapping_config: &MappingConfig, +) -> Result { + diff_objs_impl(left, right, prev, diff_config, mapping_config, false) +} + +/// Diff objects for a progress report without retaining detailed instruction or data edits. +pub fn diff_objs_summary( + left: Option<&Object>, + right: Option<&Object>, + prev: Option<&Object>, + diff_config: &DiffObjConfig, + mapping_config: &MappingConfig, +) -> Result { + diff_objs_impl(left, right, prev, diff_config, mapping_config, true) +} + +fn diff_objs_impl( + left: Option<&Object>, + right: Option<&Object>, + prev: Option<&Object>, + diff_config: &DiffObjConfig, + mapping_config: &MappingConfig, + summary: bool, ) -> Result { let symbol_matches = matching_symbols(left, right, prev, mapping_config)?; let section_matches = matching_sections(left, right)?; @@ -316,13 +339,17 @@ pub fn diff_objs( let (right_obj, right_out) = right.as_mut().unwrap(); match section_kind { SectionKind::Code => { - let (left_diff, right_diff) = diff_code( + let (mut left_diff, mut right_diff) = diff_code( left_obj, right_obj, left_symbol_ref, right_symbol_ref, diff_config, )?; + if summary { + left_diff.instruction_rows.clear(); + right_diff.instruction_rows.clear(); + } left_out.symbols[left_symbol_ref] = left_diff; right_out.symbols[right_symbol_ref] = right_diff; @@ -339,12 +366,16 @@ pub fn diff_objs( } } SectionKind::Data => { - let (left_diff, right_diff) = diff_data_symbol( + let (mut left_diff, mut right_diff) = diff_data_symbol( left_obj, right_obj, left_symbol_ref, right_symbol_ref, )?; + if summary { + left_diff.data_rows.clear(); + right_diff.data_rows.clear(); + } left_out.symbols[left_symbol_ref] = left_diff; right_out.symbols[right_symbol_ref] = right_diff; } @@ -365,12 +396,16 @@ pub fn diff_objs( let (left_obj, left_out) = left.as_mut().unwrap(); match section_kind { SectionKind::Code => { - left_out.symbols[left_symbol_ref] = - no_diff_code(left_obj, left_symbol_ref, diff_config)?; + if !summary { + left_out.symbols[left_symbol_ref] = + no_diff_code(left_obj, left_symbol_ref, diff_config)?; + } } SectionKind::Data => { - left_out.symbols[left_symbol_ref] = - no_diff_data_symbol(left_obj, left_symbol_ref)?; + if !summary { + left_out.symbols[left_symbol_ref] = + no_diff_data_symbol(left_obj, left_symbol_ref)?; + } } SectionKind::Bss | SectionKind::Common => { // Nothing needs to be done @@ -382,12 +417,16 @@ pub fn diff_objs( let (right_obj, right_out) = right.as_mut().unwrap(); match section_kind { SectionKind::Code => { - right_out.symbols[right_symbol_ref] = - no_diff_code(right_obj, right_symbol_ref, diff_config)?; + if !summary { + right_out.symbols[right_symbol_ref] = + no_diff_code(right_obj, right_symbol_ref, diff_config)?; + } } SectionKind::Data => { - right_out.symbols[right_symbol_ref] = - no_diff_data_symbol(right_obj, right_symbol_ref)?; + if !summary { + right_out.symbols[right_symbol_ref] = + no_diff_data_symbol(right_obj, right_symbol_ref)?; + } } SectionKind::Bss | SectionKind::Common => { // Nothing needs to be done @@ -424,14 +463,25 @@ pub fn diff_objs( right_out.sections[right_section_idx] = right_diff; } SectionKind::Data => { - let (left_diff, right_diff) = diff_data_section( - left_obj, - right_obj, - left_out, - right_out, - left_section_idx, - right_section_idx, - )?; + let (left_diff, right_diff) = if summary { + diff_data_section_summary( + left_obj, + right_obj, + left_out, + right_out, + left_section_idx, + right_section_idx, + ) + } else { + diff_data_section( + left_obj, + right_obj, + left_out, + right_out, + left_section_idx, + right_section_idx, + ) + }?; left_out.sections[left_section_idx] = left_diff; right_out.sections[right_section_idx] = right_diff; } @@ -455,8 +505,15 @@ pub fn diff_objs( match section_kind { SectionKind::Code => {} SectionKind::Data => { - left_out.sections[left_section_idx] = - no_diff_data_section(left_obj, left_section_idx)?; + left_out.sections[left_section_idx] = if summary { + SectionDiff { + match_percent: Some(0.0), + data_diff: vec![], + reloc_diff: vec![], + } + } else { + no_diff_data_section(left_obj, left_section_idx)? + }; } SectionKind::Bss | SectionKind::Common => { left_out.sections[left_section_idx] = no_diff_bss_section()?; @@ -469,8 +526,15 @@ pub fn diff_objs( match section_kind { SectionKind::Code => {} SectionKind::Data => { - right_out.sections[right_section_idx] = - no_diff_data_section(right_obj, right_section_idx)?; + right_out.sections[right_section_idx] = if summary { + SectionDiff { + match_percent: Some(0.0), + data_diff: vec![], + reloc_diff: vec![], + } + } else { + no_diff_data_section(right_obj, right_section_idx)? + }; } SectionKind::Bss | SectionKind::Common => { right_out.sections[right_section_idx] = no_diff_bss_section()?; @@ -509,7 +573,8 @@ pub fn diff_objs( } } - if let Some((left_obj, left_out)) = left.as_mut() + if !summary + && let Some((left_obj, left_out)) = left.as_mut() && let Some((right_obj, right_out)) = right.as_mut() { let mut done_section_names = BTreeSet::new(); @@ -579,15 +644,18 @@ fn diff_order_for_section_name( let right_paired_symbols: Vec<_> = symbols_matching_section_name(right_obj, section_name) .filter(|(sym_idx, _)| right_paired_symbol_idxs.contains(sym_idx)) .collect(); + let right_order_by_symbol: BTreeMap<_, _> = right_paired_symbols + .iter() + .enumerate() + .map(|(order_idx, (symbol_idx, _))| (*symbol_idx, order_idx)) + .collect(); let mut expected_right_order_idx = 0; for (left_order_idx, (left_symbol_idx, _left_symbol)) in left_paired_symbols.iter().enumerate() { let right_symbol_idx = left_sym_idx_to_right_sym_idx.get(left_symbol_idx).unwrap(); - let right_order_idx = right_paired_symbols - .iter() - .position(|(sym_idx, _)| sym_idx == right_symbol_idx) - .ok_or_else(|| { + let right_order_idx = + right_order_by_symbol.get(right_symbol_idx).copied().ok_or_else(|| { anyhow!("Failed to find right side symbol for paired left side symbol") })?; if right_order_idx == left_order_idx { @@ -808,6 +876,63 @@ fn apply_symbol_mappings( Ok(()) } +struct SymbolLookup { + by_name: BTreeMap>, + by_normalized_name: BTreeMap>, +} + +impl SymbolLookup { + fn new(obj: &Object) -> Self { + let mut by_name = BTreeMap::>::new(); + let mut by_normalized_name = BTreeMap::>::new(); + for (symbol_idx, symbol) in obj.symbols.iter().enumerate() { + by_name.entry(symbol.name.clone()).or_default().push(symbol_idx); + if let Some(name) = &symbol.normalized_name { + by_normalized_name.entry(name.clone()).or_default().push(symbol_idx); + } + } + Self { by_name, by_normalized_name } + } +} + +fn find_symbol_with_lookup( + obj: Option<&Object>, + lookup: Option<&SymbolLookup>, + in_obj: &Object, + in_symbol_idx: usize, + used: Option<&BTreeSet>, + fuzzy_literals: bool, +) -> Option { + let in_symbol = &in_obj.symbols[in_symbol_idx]; + let (section_name, section_kind) = symbol_section(in_obj, in_symbol)?; + if in_symbol.flags.contains(SymbolFlag::CompilerGenerated) + && matches!(section_kind, SectionKind::Code | SectionKind::Data | SectionKind::Bss) + { + return find_symbol(obj, in_obj, in_symbol_idx, used, fuzzy_literals); + } + + let (Some(obj), Some(lookup)) = (obj, lookup) else { return None }; + let by_name = lookup.by_name.get(&in_symbol.name).into_iter().flatten(); + let by_normalized_name = in_symbol + .normalized_name + .as_ref() + .and_then(|name| lookup.by_normalized_name.get(name)) + .into_iter() + .flatten(); + by_name + .chain(by_normalized_name) + .copied() + .filter(|symbol_idx| !used.is_some_and(|used| used.contains(symbol_idx))) + .filter(|&symbol_idx| { + let symbol = &obj.symbols[symbol_idx]; + !symbol.flags.contains(SymbolFlag::Ignored) + && symbol_name_matches(in_symbol, symbol) + && symbol_section_kind(obj, symbol) == section_kind + && symbol_section(obj, symbol).is_some_and(|(name, _)| name == section_name) + }) + .min() +} + /// Find matching symbols between each object. fn matching_symbols( left: Option<&Object>, @@ -818,6 +943,8 @@ fn matching_symbols( let mut matches = Vec::new(); let mut left_used = BTreeSet::new(); let mut right_used = BTreeSet::new(); + let right_lookup = right.map(SymbolLookup::new); + let prev_lookup = prev.map(SymbolLookup::new); if let Some(left) = left { if let Some(right) = right { apply_symbol_mappings( @@ -845,8 +972,22 @@ fn matching_symbols( } let symbol_match = SymbolMatch { left: Some(symbol_idx), - right: find_symbol(right, left, symbol_idx, Some(&right_used), fuzzy_literals), - prev: find_symbol(prev, left, symbol_idx, None, fuzzy_literals), + right: find_symbol_with_lookup( + right, + right_lookup.as_ref(), + left, + symbol_idx, + Some(&right_used), + fuzzy_literals, + ), + prev: find_symbol_with_lookup( + prev, + prev_lookup.as_ref(), + left, + symbol_idx, + None, + fuzzy_literals, + ), section_kind, }; matches.push(symbol_match); @@ -875,7 +1016,14 @@ fn matching_symbols( let symbol_match = SymbolMatch { left: None, right: Some(symbol_idx), - prev: find_symbol(prev, right, symbol_idx, None, fuzzy_literals), + prev: find_symbol_with_lookup( + prev, + prev_lookup.as_ref(), + right, + symbol_idx, + None, + fuzzy_literals, + ), section_kind, }; matches.push(symbol_match); diff --git a/objdiff-core/src/obj/read.rs b/objdiff-core/src/obj/read.rs index 49ec2c3d..911a315e 100644 --- a/objdiff-core/src/obj/read.rs +++ b/objdiff-core/src/obj/read.rs @@ -292,13 +292,14 @@ fn add_section_symbols(sections: &[Section], symbols: &mut Vec) { }) .map(|s| s.address + s.size) .max() + .and_then(|end| end.checked_sub(section.address)) .unwrap_or(section.size); symbols.push(Symbol { name, demangled_name: None, normalized_name: None, - address: 0, + address: section.address, size, kind: SymbolKind::Section, section: Some(section_idx), @@ -960,11 +961,17 @@ fn combine_sections( } if config.combine_data_sections { for (combined_name, mut section_indices) in data_sections { + if section_indices.iter().any(|&i| sections[i].address != 0) { + continue; + } do_combine_sections(sections, symbols, &mut section_indices, combined_name)?; } } if config.combine_text_sections { for (combined_name, mut section_indices) in text_sections { + if section_indices.iter().any(|&i| sections[i].address != 0) { + continue; + } do_combine_sections(sections, symbols, &mut section_indices, combined_name)?; } } @@ -1312,4 +1319,65 @@ mod test { assert_eq!(sections[1].data.0, (1..=12).collect::>()); insta::assert_debug_snapshot!((sections, symbols)); } + + #[test] + fn test_do_not_combine_linked_sections() { + let mut sections = vec![ + Section { + id: ".got-0".to_string(), + name: ".got".to_string(), + address: 0x1000, + size: 4, + kind: SectionKind::Data, + data: SectionData(vec![1, 2, 3, 4]), + ..Default::default() + }, + Section { + id: ".got.plt-0".to_string(), + name: ".got.plt".to_string(), + address: 0x1004, + size: 4, + kind: SectionKind::Data, + data: SectionData(vec![5, 6, 7, 8]), + ..Default::default() + }, + ]; + let original_sections = sections.clone(); + let mut symbols = vec![]; + let config = DiffObjConfig { combine_data_sections: true, ..Default::default() }; + + combine_sections(&mut sections, &mut symbols, &config).unwrap(); + + assert_eq!(sections[0].id, original_sections[0].id); + assert_eq!(sections[0].data.0, original_sections[0].data.0); + assert_eq!(sections[1].id, original_sections[1].id); + assert_eq!(sections[1].data.0, original_sections[1].data.0); + } + + #[test] + fn test_add_linked_section_symbol() { + let sections = vec![Section { + id: ".rodata-0".to_string(), + name: ".rodata".to_string(), + address: 0x1000, + size: 0x100, + kind: SectionKind::Data, + ..Default::default() + }]; + let mut symbols = vec![Symbol { + name: "data".to_string(), + address: 0x1020, + size: 0x10, + kind: SymbolKind::Object, + section: Some(0), + ..Default::default() + }]; + + add_section_symbols(§ions, &mut symbols); + + let section_symbol = symbols.last().unwrap(); + assert_eq!(section_symbol.name, "[.rodata-0]"); + assert_eq!(section_symbol.address, 0x1000); + assert_eq!(section_symbol.size, 0x30); + } } diff --git a/objdiff-core/tests/arch_x86.rs b/objdiff-core/tests/arch_x86.rs index 75097605..955b2e09 100644 --- a/objdiff-core/tests/arch_x86.rs +++ b/objdiff-core/tests/arch_x86.rs @@ -121,6 +121,47 @@ fn diff_single_common_symbol() { assert_eq!(result.right.unwrap().symbols[0].target_symbol, Some(0)); } +#[test] +#[cfg(feature = "x86")] +fn diff_x86_summary_omits_details() { + let diff_config = diff::DiffObjConfig::default(); + let left = obj::read::parse( + include_object!("data/x86/staticdebug.obj"), + &diff_config, + diff::DiffSide::Target, + ) + .unwrap(); + let right = obj::read::parse( + include_object!("data/x86/staticdebug.obj"), + &diff_config, + diff::DiffSide::Base, + ) + .unwrap(); + + let result = diff::diff_objs_summary( + Some(&left), + Some(&right), + None, + &diff_config, + &diff::MappingConfig::default(), + ) + .unwrap(); + let left_diff = result.left.unwrap(); + let right_diff = result.right.unwrap(); + + assert!( + left_diff + .symbols + .iter() + .filter(|symbol| symbol.target_symbol.is_some()) + .all(|symbol| symbol.match_percent == Some(100.0)) + ); + assert!(left_diff.symbols.iter().all(|symbol| symbol.instruction_rows.is_empty())); + assert!(right_diff.symbols.iter().all(|symbol| symbol.instruction_rows.is_empty())); + assert!(left_diff.sections.iter().all(|section| section.data_diff.is_empty())); + assert!(right_diff.sections.iter().all(|section| section.data_diff.is_empty())); +} + #[test] #[cfg(feature = "x86")] fn read_x86_combine_sections() {