From e1ccee1e73d1828f95a53a139e5e14038df58365 Mon Sep 17 00:00:00 2001 From: Luke Street Date: Sun, 11 Aug 2024 13:53:52 -0600 Subject: [PATCH] Improve data section diffing more This re-implements the older algorithm used for data and BSS section match percentages. We perform both and choose the highest match percent between the two options. Resolves #84, #85 --- objdiff-core/src/diff/data.rs | 40 +++++++++++++++++++++++++++++ objdiff-core/src/diff/mod.rs | 47 +++++++++++++++++++++++++---------- objdiff-core/src/obj/read.rs | 2 +- 3 files changed, 75 insertions(+), 14 deletions(-) diff --git a/objdiff-core/src/diff/data.rs b/objdiff-core/src/diff/data.rs index c1cf7c8..262edf9 100644 --- a/objdiff-core/src/diff/data.rs +++ b/objdiff-core/src/diff/data.rs @@ -54,6 +54,7 @@ pub fn diff_data_section( let right_data = &right.data[..right_max as usize]; let ops = capture_diff_slices_deadline(Algorithm::Patience, left_data, right_data, Some(deadline)); + let match_percent = get_diff_ratio(&ops, left_data.len(), right_data.len()) * 100.0; let mut left_diff = Vec::::new(); let mut right_diff = Vec::::new(); @@ -127,6 +128,13 @@ pub fn diff_data_section( diff_generic_section(left, right, left_section_diff, right_section_diff)?; left_section_diff.data_diff = left_diff; right_section_diff.data_diff = right_diff; + // Use the highest match percent between two options: + // - Left symbols matching right symbols by name + // - Diff of the data itself + if left_section_diff.match_percent.unwrap_or(-1.0) < match_percent { + left_section_diff.match_percent = Some(match_percent); + right_section_diff.match_percent = Some(match_percent); + } Ok((left_section_diff, right_section_diff)) } @@ -191,3 +199,35 @@ pub fn diff_generic_section( ObjSectionDiff { symbols: vec![], data_diff: vec![], match_percent: Some(match_percent) }, )) } + +/// Compare the addresses and sizes of each symbol in the BSS sections. +pub fn diff_bss_section( + left: &ObjSection, + right: &ObjSection, + left_diff: &ObjSectionDiff, + right_diff: &ObjSectionDiff, +) -> Result<(ObjSectionDiff, ObjSectionDiff)> { + let deadline = Instant::now() + Duration::from_secs(5); + let left_sizes = left.symbols.iter().map(|s| (s.section_address, s.size)).collect::>(); + let right_sizes = right.symbols.iter().map(|s| (s.section_address, s.size)).collect::>(); + let ops = capture_diff_slices_deadline( + Algorithm::Patience, + &left_sizes, + &right_sizes, + Some(deadline), + ); + let mut match_percent = get_diff_ratio(&ops, left_sizes.len(), right_sizes.len()) * 100.0; + + // Use the highest match percent between two options: + // - Left symbols matching right symbols by name + // - Diff of the addresses and sizes of each symbol + let (generic_diff, _) = diff_generic_section(left, right, left_diff, right_diff)?; + if generic_diff.match_percent.unwrap_or(-1.0) > match_percent { + match_percent = generic_diff.match_percent.unwrap(); + } + + Ok(( + ObjSectionDiff { symbols: vec![], data_diff: vec![], match_percent: Some(match_percent) }, + ObjSectionDiff { symbols: vec![], data_diff: vec![], match_percent: Some(match_percent) }, + )) +} diff --git a/objdiff-core/src/diff/mod.rs b/objdiff-core/src/diff/mod.rs index 8e9d9f2..c755a2c 100644 --- a/objdiff-core/src/diff/mod.rs +++ b/objdiff-core/src/diff/mod.rs @@ -6,8 +6,8 @@ use crate::{ diff::{ code::{diff_code, no_diff_code, process_code_symbol}, data::{ - diff_bss_symbol, diff_data_section, diff_data_symbol, diff_generic_section, - no_diff_symbol, + diff_bss_section, diff_bss_symbol, diff_data_section, diff_data_symbol, + diff_generic_section, no_diff_symbol, }, }, obj::{ObjInfo, ObjIns, ObjSection, ObjSectionKind, ObjSymbol, SymbolRef}, @@ -483,7 +483,7 @@ pub fn diff_objs( let left_section = &left_obj.sections[left_section_idx]; let right_section = &right_obj.sections[right_section_idx]; match section_kind { - ObjSectionKind::Code | ObjSectionKind::Bss => { + ObjSectionKind::Code => { let left_section_diff = left_out.section_diff(left_section_idx); let right_section_diff = right_out.section_diff(right_section_idx); let (left_diff, right_diff) = diff_generic_section( @@ -507,6 +507,18 @@ pub fn diff_objs( left_out.section_diff_mut(left_section_idx).merge(left_diff); right_out.section_diff_mut(right_section_idx).merge(right_diff); } + ObjSectionKind::Bss => { + let left_section_diff = left_out.section_diff(left_section_idx); + let right_section_diff = right_out.section_diff(right_section_idx); + let (left_diff, right_diff) = diff_bss_section( + left_section, + right_section, + left_section_diff, + right_section_diff, + )?; + left_out.section_diff_mut(left_section_idx).merge(left_diff); + right_out.section_diff_mut(right_section_idx).merge(right_diff); + } } } } @@ -546,8 +558,8 @@ fn matching_symbols( for (symbol_idx, symbol) in section.symbols.iter().enumerate() { let symbol_match = SymbolMatch { left: Some(SymbolRef { section_idx, symbol_idx }), - right: find_symbol(right, symbol, section), - prev: find_symbol(prev, symbol, section), + right: find_symbol(right, symbol, section, Some(&right_used)), + prev: find_symbol(prev, symbol, section, None), section_kind: section.kind, }; matches.push(symbol_match); @@ -579,7 +591,7 @@ fn matching_symbols( matches.push(SymbolMatch { left: None, right: Some(symbol_ref), - prev: find_symbol(prev, symbol, section), + prev: find_symbol(prev, symbol, section, None), section_kind: section.kind, }); } @@ -604,6 +616,7 @@ fn find_symbol( obj: Option<&ObjInfo>, in_symbol: &ObjSymbol, in_section: &ObjSection, + used: Option<&HashSet>, ) -> Option { let obj = obj?; // Try to find an exact name match @@ -641,13 +654,21 @@ fn find_symbol( if section.kind != in_section.kind { continue; } - if let Some(symbol_idx) = section.symbols.iter().position(|symbol| { - if let Some((p, s)) = symbol.name.split_once('$') { - prefix == p && s.chars().all(char::is_numeric) - } else { - false - } - }) { + if let Some((symbol_idx, _)) = + section.symbols.iter().enumerate().find(|&(symbol_idx, symbol)| { + if used + .map(|u| u.contains(&SymbolRef { section_idx, symbol_idx })) + .unwrap_or(false) + { + return false; + } + if let Some((p, s)) = symbol.name.split_once('$') { + prefix == p && s.chars().all(char::is_numeric) + } else { + false + } + }) + { return Some(SymbolRef { section_idx, symbol_idx }); } } diff --git a/objdiff-core/src/obj/read.rs b/objdiff-core/src/obj/read.rs index 750e1d9..6f103fe 100644 --- a/objdiff-core/src/obj/read.rs +++ b/objdiff-core/src/obj/read.rs @@ -149,7 +149,7 @@ fn symbols_by_section( } } } - result.sort_by_key(|v| v.address); + result.sort_by(|a, b| a.address.cmp(&b.address).then(a.size.cmp(&b.size))); let mut iter = result.iter_mut().peekable(); while let Some(symbol) = iter.next() { if symbol.size == 0 {