diff --git a/objdiff-core/src/diff/mod.rs b/objdiff-core/src/diff/mod.rs index e0aa91b7..95da2712 100644 --- a/objdiff-core/src/diff/mod.rs +++ b/objdiff-core/src/diff/mod.rs @@ -4,9 +4,9 @@ use alloc::{ vec, vec::Vec, }; -use core::{num::NonZeroU32, ops::Range}; +use core::{cmp::Ordering, num::NonZeroU32, ops::Range}; -use anyhow::Result; +use anyhow::{Result, anyhow}; use crate::{ diff::{ @@ -17,7 +17,7 @@ use crate::{ symbol_name_matches, }, }, - obj::{InstructionRef, Object, Relocation, SectionKind, Symbol, SymbolFlag}, + obj::{InstructionRef, Object, Relocation, SectionKind, Symbol, SymbolFlag, SymbolKind}, }; pub mod code; @@ -47,6 +47,7 @@ pub struct SymbolDiff { pub diff_score: Option<(u64, u64)>, pub instruction_rows: Vec, pub data_rows: Vec, + pub order: Option, } #[derive(Debug, Clone, Default)] @@ -207,8 +208,8 @@ pub fn diff_objs( let mut right = right.map(|p| (p, ObjectDiff::new_from_obj(p))); let mut prev = prev.map(|p| (p, ObjectDiff::new_from_obj(p))); - for symbol_match in symbol_matches { - match symbol_match { + for symbol_match in &symbol_matches { + match *symbol_match { SymbolMatch { left: Some(left_symbol_ref), right: Some(right_symbol_ref), @@ -412,6 +413,26 @@ pub fn diff_objs( } } + if 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(); + for left_section in left_obj.sections.iter() { + if done_section_names.contains(&left_section.name) { + continue; + } + done_section_names.insert(&left_section.name); + diff_order_for_section_name( + left_obj, + right_obj, + left_out, + right_out, + &left_section.name, + &symbol_matches, + )?; + } + } + Ok(DiffObjsResult { left: left.map(|(_, o)| o), right: right.map(|(_, o)| o), @@ -419,6 +440,83 @@ pub fn diff_objs( }) } +fn symbols_matching_section_name<'obj>( + obj: &'obj Object, + section_name: &str, +) -> impl Iterator { + obj.symbols.iter().enumerate().filter(move |(_, s)| { + let curr_section_name = symbol_section(obj, s).map(|(n, _)| n); + curr_section_name == Some(section_name) + && s.kind != SymbolKind::Section + && s.size > 0 + && !s.flags.contains(SymbolFlag::Hidden) + && !s.flags.contains(SymbolFlag::Ignored) + }) +} + +fn diff_order_for_section_name( + left_obj: &Object, + right_obj: &Object, + left_diff: &mut ObjectDiff, + right_diff: &mut ObjectDiff, + section_name: &str, + symbol_matches: &Vec, +) -> Result<()> { + let mut left_paired_symbol_idxs = BTreeSet::new(); + let mut right_paired_symbol_idxs = BTreeSet::new(); + let mut left_sym_idx_to_right_sym_idx = BTreeMap::new(); + for symbol_match in symbol_matches { + let Some(left_symbol_idx) = symbol_match.left else { + continue; + }; + let Some(right_symbol_idx) = symbol_match.right else { + continue; + }; + left_paired_symbol_idxs.insert(left_symbol_idx); + right_paired_symbol_idxs.insert(right_symbol_idx); + left_sym_idx_to_right_sym_idx.insert(left_symbol_idx, right_symbol_idx); + } + + let left_paired_symbols: Vec<_> = symbols_matching_section_name(left_obj, section_name) + .filter(|(sym_idx, _)| left_paired_symbol_idxs.contains(sym_idx)) + .collect(); + 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 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(|| { + anyhow!("Failed to find right side symbol for paired left side symbol") + })?; + if right_order_idx == left_order_idx { + // In the correct spot. + left_diff.symbols[*left_symbol_idx].order = Some(Ordering::Equal); + right_diff.symbols[*right_symbol_idx].order = Some(Ordering::Equal); + expected_right_order_idx = left_order_idx + 1 + } else if right_order_idx == expected_right_order_idx { + // In the wrong spot, but correct relative to the symbol before it. + // Don't show this as a diff to reduce noise. + left_diff.symbols[*left_symbol_idx].order = Some(Ordering::Equal); + right_diff.symbols[*right_symbol_idx].order = Some(Ordering::Equal); + } else { + // In the wrong spot. + left_diff.symbols[*left_symbol_idx].order = + Some(expected_right_order_idx.cmp(&right_order_idx)); + right_diff.symbols[*right_symbol_idx].order = + Some(right_order_idx.cmp(&expected_right_order_idx)); + expected_right_order_idx = right_order_idx; + } + expected_right_order_idx += 1; + } + Ok(()) +} + #[derive(Clone, Copy)] enum MappingSymbol<'a> { Left(&'a str), diff --git a/objdiff-core/tests/snapshots/arch_ppc__diff_ppc-2.snap b/objdiff-core/tests/snapshots/arch_ppc__diff_ppc-2.snap index 65f6082b..d43a37af 100644 --- a/objdiff-core/tests/snapshots/arch_ppc__diff_ppc-2.snap +++ b/objdiff-core/tests/snapshots/arch_ppc__diff_ppc-2.snap @@ -2646,6 +2646,9 @@ expression: "(target_symbol_diff, base_symbol_diff)" }, ], data_rows: [], + order: Some( + Equal, + ), }, SymbolDiff { target_symbol: Some( @@ -5290,5 +5293,8 @@ expression: "(target_symbol_diff, base_symbol_diff)" }, ], data_rows: [], + order: Some( + Equal, + ), }, ) diff --git a/objdiff-gui/src/views/diff.rs b/objdiff-gui/src/views/diff.rs index 76da8aa8..ac44a956 100644 --- a/objdiff-gui/src/views/diff.rs +++ b/objdiff-gui/src/views/diff.rs @@ -300,6 +300,49 @@ pub fn diff_view_ui( ret = Some(DiffViewAction::SetSearch(search)); } + if let Some((_, left_diff)) = left_ctx.obj { + let wrong_order_symbols = left_diff + .symbols + .iter() + .filter(|s| s.order.is_some_and(|o| o != Ordering::Equal)) + .count(); + let mut wrong_order_symbol_idx_to_select = left_diff + .symbols + .iter() + .position(|s| s.order.is_some_and(|o| o != Ordering::Equal)); + if let Some(left_highlight) = state.symbol_state.highlighted_symbol.0 { + let next_wrong_order_symbol_idx = left_diff + .symbols + .iter() + .enumerate() + .skip(left_highlight + 1) + .find(|(_, s)| s.order.is_some_and(|o| o != Ordering::Equal)) + .map(|(i, _)| i); + if next_wrong_order_symbol_idx.is_some() { + wrong_order_symbol_idx_to_select = next_wrong_order_symbol_idx; + } + } + if ui + .add_enabled( + wrong_order_symbol_idx_to_select.is_some(), + egui::Button::new(format!( + "Wrong order: {}/{}", + wrong_order_symbols, + left_diff.symbols.len() + )), + ) + .clicked() + && let Some(left_sym_idx) = wrong_order_symbol_idx_to_select + { + let target_symbol = left_diff.symbols[left_sym_idx].target_symbol; + ret = Some(DiffViewAction::SetSymbolHighlight( + Some(left_sym_idx), + target_symbol, + true, + )); + } + } + ui.with_layout(Layout::right_to_left(egui::Align::TOP), |ui| { if ui.small_button("⏷").on_hover_text_at_pointer("Expand all").clicked() { open_sections.0 = Some(true); @@ -308,7 +351,7 @@ pub fn diff_view_ui( { open_sections.0 = Some(false); } - }) + }); }); } diff --git a/objdiff-gui/src/views/symbol_diff.rs b/objdiff-gui/src/views/symbol_diff.rs index 82046643..f6128a4e 100644 --- a/objdiff-gui/src/views/symbol_diff.rs +++ b/objdiff-gui/src/views/symbol_diff.rs @@ -1,4 +1,4 @@ -use std::mem::take; +use std::{cmp::Ordering, mem::take}; use egui::{ CollapsingHeader, Color32, Id, OpenUrl, ScrollArea, Ui, Widget, style::ScrollAnimation, @@ -605,14 +605,29 @@ fn symbol_ui( write_text(name, appearance.highlight_color, &mut job, appearance.code_font.clone()); if diff_config.show_symbol_sizes == ShowSymbolSizes::Decimal { write_text( - &format!(" (size={})", symbol.size), + &format!(" (size:{})", symbol.size), appearance.text_color, &mut job, appearance.code_font.clone(), ); } else if diff_config.show_symbol_sizes == ShowSymbolSizes::Hex { write_text( - &format!(" (size={:x})", symbol.size), + &format!(" (size:{:x})", symbol.size), + appearance.text_color, + &mut job, + appearance.code_font.clone(), + ); + } + if let Some(order) = symbol_diff.order + && order != Ordering::Equal + { + let order_char = match order { + Ordering::Less => "⏷", + Ordering::Equal => unreachable!(), + Ordering::Greater => "⏶", + }; + write_text( + &format!(" {order_char}"), appearance.text_color, &mut job, appearance.code_font.clone(),