From 44f813155951e518de76a45901482e7e223da11d Mon Sep 17 00:00:00 2001 From: LagoLunatic Date: Mon, 20 Jul 2026 18:55:36 -0400 Subject: [PATCH 1/5] Indicate when symbols are in the wrong order --- objdiff-core/src/diff/mod.rs | 95 ++++++++++++++++++- .../tests/snapshots/arch_ppc__diff_ppc-2.snap | 6 ++ objdiff-gui/src/views/diff.rs | 31 ++++++ objdiff-gui/src/views/symbol_diff.rs | 21 +++- 4 files changed, 145 insertions(+), 8 deletions(-) diff --git a/objdiff-core/src/diff/mod.rs b/objdiff-core/src/diff/mod.rs index 2e641250..51e98ca0 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,70 @@ 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_symbol_idx, _left_symbol) in left_paired_symbols.iter() { + 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 symbol"))?; + 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)); + if right_order_idx == expected_right_order_idx { + expected_right_order_idx += 1; + } else { + expected_right_order_idx = right_order_idx + 1; + } + } + Ok(()) +} + /// Score entry for a candidate symbol when searching for similar functions. #[derive(Debug, Clone)] pub struct SimilarSymbol { 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 084f7a03..68b25bb4 100644 --- a/objdiff-gui/src/views/diff.rs +++ b/objdiff-gui/src/views/diff.rs @@ -375,6 +375,37 @@ pub fn diff_view_ui( { open_sections.0 = Some(false); } + + 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 first_wrong_order_symbol = left_diff + .symbols + .iter() + .position(|s| s.order.is_some_and(|o| o != Ordering::Equal)); + if ui + .add_enabled( + first_wrong_order_symbol.is_some(), + egui::Button::new(format!( + "Wrong order: {}/{}", + wrong_order_symbols, + left_diff.symbols.len() + )), + ) + .clicked() + && let Some(left_sym_idx) = first_wrong_order_symbol + { + let target_symbol = left_diff.symbols[left_sym_idx].target_symbol; + ret = Some(DiffViewAction::SetSymbolHighlight( + Some(left_sym_idx), + target_symbol, + true, + )); + } + } }) }); } diff --git a/objdiff-gui/src/views/symbol_diff.rs b/objdiff-gui/src/views/symbol_diff.rs index 3363bb27..f3da58e5 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, @@ -734,14 +734,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(), From fb54004ae5bcf4a2f4b12947b2437549b9e2cd34 Mon Sep 17 00:00:00 2001 From: LagoLunatic Date: Mon, 20 Jul 2026 20:25:27 -0400 Subject: [PATCH 2/5] Wrong order button: Cycle through all wrong symbols --- objdiff-gui/src/views/diff.rs | 19 ++++++++++++++++--- 1 file changed, 16 insertions(+), 3 deletions(-) diff --git a/objdiff-gui/src/views/diff.rs b/objdiff-gui/src/views/diff.rs index 68b25bb4..d6b37186 100644 --- a/objdiff-gui/src/views/diff.rs +++ b/objdiff-gui/src/views/diff.rs @@ -382,13 +382,26 @@ pub fn diff_view_ui( .iter() .filter(|s| s.order.is_some_and(|o| o != Ordering::Equal)) .count(); - let first_wrong_order_symbol = left_diff + 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) + .filter(|(_, s)| s.order.is_some_and(|o| o != Ordering::Equal)) + .next() + .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( - first_wrong_order_symbol.is_some(), + wrong_order_symbol_idx_to_select.is_some(), egui::Button::new(format!( "Wrong order: {}/{}", wrong_order_symbols, @@ -396,7 +409,7 @@ pub fn diff_view_ui( )), ) .clicked() - && let Some(left_sym_idx) = first_wrong_order_symbol + && 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( From 47b4b8a2a1aef00b73502e75105afafbb3af129c Mon Sep 17 00:00:00 2001 From: LagoLunatic Date: Mon, 20 Jul 2026 20:46:58 -0400 Subject: [PATCH 3/5] Improve order diff algo to further reduce noise --- objdiff-core/src/diff/mod.rs | 31 ++++++++++++++++++++++--------- 1 file changed, 22 insertions(+), 9 deletions(-) diff --git a/objdiff-core/src/diff/mod.rs b/objdiff-core/src/diff/mod.rs index 51e98ca0..b39d7491 100644 --- a/objdiff-core/src/diff/mod.rs +++ b/objdiff-core/src/diff/mod.rs @@ -485,21 +485,34 @@ fn diff_order_for_section_name( .collect(); let mut expected_right_order_idx = 0; - for (left_symbol_idx, _left_symbol) in left_paired_symbols.iter() { + 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 symbol"))?; - 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)); - if right_order_idx == expected_right_order_idx { - expected_right_order_idx += 1; + .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 { - expected_right_order_idx = right_order_idx + 1; + // 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(()) } From 6cf37d14a694562a1c22a1bf2090db04959f0690 Mon Sep 17 00:00:00 2001 From: LagoLunatic Date: Mon, 20 Jul 2026 20:47:28 -0400 Subject: [PATCH 4/5] Don't right-justify "Wrong order" button --- objdiff-gui/src/views/diff.rs | 90 +++++++++++++++++------------------ 1 file changed, 45 insertions(+), 45 deletions(-) diff --git a/objdiff-gui/src/views/diff.rs b/objdiff-gui/src/views/diff.rs index d6b37186..8968461b 100644 --- a/objdiff-gui/src/views/diff.rs +++ b/objdiff-gui/src/views/diff.rs @@ -367,6 +367,50 @@ 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) + .filter(|(_, s)| s.order.is_some_and(|o| o != Ordering::Equal)) + .next() + .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); @@ -375,51 +419,7 @@ pub fn diff_view_ui( { open_sections.0 = Some(false); } - - 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) - .filter(|(_, s)| s.order.is_some_and(|o| o != Ordering::Equal)) - .next() - .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, - )); - } - } - }) + }); }); } From f790559399293c604993620fbe9098c689fc39dd Mon Sep 17 00:00:00 2001 From: LagoLunatic Date: Mon, 20 Jul 2026 21:15:38 -0400 Subject: [PATCH 5/5] Clippy --- objdiff-gui/src/views/diff.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/objdiff-gui/src/views/diff.rs b/objdiff-gui/src/views/diff.rs index 8968461b..173abf29 100644 --- a/objdiff-gui/src/views/diff.rs +++ b/objdiff-gui/src/views/diff.rs @@ -383,8 +383,7 @@ pub fn diff_view_ui( .iter() .enumerate() .skip(left_highlight + 1) - .filter(|(_, s)| s.order.is_some_and(|o| o != Ordering::Equal)) - .next() + .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;