From 4dfc28fc68dddeb334f11692792b23e6cde2bb49 Mon Sep 17 00:00:00 2001 From: Luke Street Date: Mon, 18 Mar 2024 22:56:13 -0600 Subject: [PATCH] Diff cleanup & fixes --- objdiff-cli/src/cmd/diff.rs | 32 ++++++++++------------- objdiff-core/src/diff/code.rs | 13 +++++----- objdiff-core/src/diff/data.rs | 7 +++--- objdiff-core/src/diff/mod.rs | 23 +++++------------ objdiff-core/src/obj/mod.rs | 14 +++++++++++ objdiff-gui/src/views/function_diff.rs | 35 ++++++++++++-------------- 6 files changed, 59 insertions(+), 65 deletions(-) diff --git a/objdiff-cli/src/cmd/diff.rs b/objdiff-cli/src/cmd/diff.rs index d3260ea..2ff1af1 100644 --- a/objdiff-cli/src/cmd/diff.rs +++ b/objdiff-cli/src/cmd/diff.rs @@ -18,10 +18,10 @@ use objdiff_core::{ diff, diff::{ display::{display_diff, DiffText, HighlightKind}, - DiffObjsResult, ObjDiff, ObjInsDiffKind, ObjSymbolDiff, SymbolRef, + DiffObjsResult, ObjDiff, ObjInsDiffKind, ObjSymbolDiff, }, obj, - obj::{ObjInfo, ObjSectionKind, ObjSymbol}, + obj::{ObjInfo, ObjSectionKind, ObjSymbol, SymbolRef}, }; use ratatui::{ prelude::*, @@ -221,20 +221,14 @@ pub fn run(args: Args) -> Result<()> { Ok(()) } +#[inline] fn get_symbol(obj: Option<&ObjInfo>, sym: Option) -> Option<&ObjSymbol> { - if let (Some(obj), Some(sym)) = (obj, sym) { - obj.sections.get(sym.section_idx).and_then(|s| s.symbols.get(sym.symbol_idx)) - } else { - None - } + Some(obj?.section_symbol(sym?).1) } -fn get_diff_symbol(obj: Option<&ObjDiff>, sym: Option) -> Option<&ObjSymbolDiff> { - if let (Some(obj), Some(sym)) = (obj, sym) { - Some(obj.symbol_diff(sym)) - } else { - None - } +#[inline] +fn get_symbol_diff(obj: Option<&ObjDiff>, sym: Option) -> Option<&ObjSymbolDiff> { + Some(obj?.symbol_diff(sym?)) } fn find_function(obj: &ObjInfo, name: &str) -> Option { @@ -336,7 +330,7 @@ impl FunctionDiffUi { f.render_widget(line_l, header_chunks[0]); let mut line_r = Line::default(); - if let Some(percent) = get_diff_symbol(self.diff_result.right.as_ref(), self.right_sym) + if let Some(percent) = get_symbol_diff(self.diff_result.right.as_ref(), self.right_sym) .and_then(|s| s.match_percent) { line_r.spans.push(Span::styled( @@ -360,7 +354,7 @@ impl FunctionDiffUi { let mut max_width = 0; if let (Some(symbol), Some(symbol_diff)) = ( get_symbol(self.left_obj.as_ref(), self.left_sym), - get_diff_symbol(self.diff_result.left.as_ref(), self.left_sym), + get_symbol_diff(self.diff_result.left.as_ref(), self.left_sym), ) { let mut text = Text::default(); let rect = content_chunks[0].inner(&Margin::new(0, 1)); @@ -382,7 +376,7 @@ impl FunctionDiffUi { let mut margin_text = None; if let (Some(symbol), Some(symbol_diff)) = ( get_symbol(self.right_obj.as_ref(), self.right_sym), - get_diff_symbol(self.diff_result.right.as_ref(), self.right_sym), + get_symbol_diff(self.diff_result.right.as_ref(), self.right_sym), ) { let mut text = Text::default(); let rect = content_chunks[2].inner(&Margin::new(0, 1)); @@ -410,7 +404,7 @@ impl FunctionDiffUi { if self.three_way { if let (Some(symbol), Some(symbol_diff)) = ( get_symbol(self.prev_obj.as_ref(), self.prev_sym), - get_diff_symbol(self.diff_result.prev.as_ref(), self.prev_sym), + get_symbol_diff(self.diff_result.prev.as_ref(), self.prev_sym), ) { let mut text = Text::default(); let rect = content_chunks[4].inner(&Margin::new(0, 1)); @@ -838,8 +832,8 @@ impl FunctionDiffUi { let right_sym = base.as_ref().and_then(|o| find_function(o, &self.symbol_name)); let prev_sym = prev.as_ref().and_then(|o| find_function(o, &self.symbol_name)); self.num_rows = match ( - get_diff_symbol(result.left.as_ref(), left_sym), - get_diff_symbol(result.right.as_ref(), right_sym), + get_symbol_diff(result.left.as_ref(), left_sym), + get_symbol_diff(result.right.as_ref(), right_sym), ) { (Some(l), Some(r)) => l.instructions.len().max(r.instructions.len()), (Some(l), None) => l.instructions.len(), diff --git a/objdiff-core/src/diff/code.rs b/objdiff-core/src/diff/code.rs index 832027a..36baf27 100644 --- a/objdiff-core/src/diff/code.rs +++ b/objdiff-core/src/diff/code.rs @@ -10,10 +10,10 @@ use similar::{capture_diff_slices_deadline, Algorithm}; use crate::{ arch::ProcessCodeResult, diff::{ - get_symbol, DiffObjConfig, ObjInsArgDiff, ObjInsBranchFrom, ObjInsBranchTo, ObjInsDiff, - ObjInsDiffKind, ObjSymbolDiff, SymbolRef, + DiffObjConfig, ObjInsArgDiff, ObjInsBranchFrom, ObjInsBranchTo, ObjInsDiff, ObjInsDiffKind, + ObjSymbolDiff, }, - obj::{ObjInfo, ObjInsArg, ObjReloc, ObjSymbol, ObjSymbolFlags}, + obj::{ObjInfo, ObjInsArg, ObjReloc, ObjSymbol, ObjSymbolFlags, SymbolRef}, }; pub fn no_diff_code( @@ -21,8 +21,7 @@ pub fn no_diff_code( symbol_ref: SymbolRef, config: &DiffObjConfig, ) -> Result { - let section = &obj.sections[symbol_ref.section_idx]; - let symbol = §ion.symbols[symbol_ref.symbol_idx]; + let (section, symbol) = obj.section_symbol(symbol_ref); let code = §ion.data [symbol.section_address as usize..(symbol.section_address + symbol.size) as usize]; let out = obj.arch.process_code( @@ -48,8 +47,8 @@ pub fn diff_code( right_symbol_ref: SymbolRef, config: &DiffObjConfig, ) -> Result<(ObjSymbolDiff, ObjSymbolDiff)> { - let (left_section, left_symbol) = get_symbol(left_obj, left_symbol_ref); - let (right_section, right_symbol) = get_symbol(right_obj, right_symbol_ref); + let (left_section, left_symbol) = left_obj.section_symbol(left_symbol_ref); + let (right_section, right_symbol) = right_obj.section_symbol(right_symbol_ref); let left_code = &left_section.data[left_symbol.section_address as usize ..(left_symbol.section_address + left_symbol.size) as usize]; let right_code = &right_section.data[right_symbol.section_address as usize diff --git a/objdiff-core/src/diff/data.rs b/objdiff-core/src/diff/data.rs index 27e6b47..7fa9d16 100644 --- a/objdiff-core/src/diff/data.rs +++ b/objdiff-core/src/diff/data.rs @@ -7,9 +7,10 @@ use anyhow::Result; use similar::{capture_diff_slices_deadline, Algorithm}; use crate::{ - diff::{get_symbol, ObjDataDiff, ObjDataDiffKind, ObjSectionDiff, ObjSymbolDiff, SymbolRef}, + diff::{ ObjDataDiff, ObjDataDiffKind, ObjSectionDiff, ObjSymbolDiff}, obj::{ObjInfo, ObjSection}, }; +use crate::obj::SymbolRef; pub fn diff_bss_symbol( left_obj: &ObjInfo, @@ -17,8 +18,8 @@ pub fn diff_bss_symbol( left_symbol_ref: SymbolRef, right_symbol_ref: SymbolRef, ) -> Result<(ObjSymbolDiff, ObjSymbolDiff)> { - let (_, left_symbol) = get_symbol(left_obj, left_symbol_ref); - let (_, right_symbol) = get_symbol(right_obj, right_symbol_ref); + let (_, left_symbol) = left_obj.section_symbol(left_symbol_ref); + let (_, right_symbol) = right_obj.section_symbol(right_symbol_ref); let percent = if left_symbol.size == right_symbol.size { 100.0 } else { 50.0 }; Ok(( ObjSymbolDiff { diff --git a/objdiff-core/src/diff/mod.rs b/objdiff-core/src/diff/mod.rs index 421b0cf..243265c 100644 --- a/objdiff-core/src/diff/mod.rs +++ b/objdiff-core/src/diff/mod.rs @@ -2,6 +2,8 @@ mod code; mod data; pub mod display; +use std::collections::HashSet; + use anyhow::Result; use crate::{ @@ -9,7 +11,7 @@ use crate::{ code::{diff_code, no_diff_code}, data::{diff_bss_symbol, diff_data, no_diff_bss_symbol}, }, - obj::{ObjInfo, ObjIns, ObjSection, ObjSectionKind, ObjSymbol}, + obj::{ObjInfo, ObjIns, ObjSectionKind, SymbolRef}, }; #[derive(Debug, Copy, Clone, Default, Eq, PartialEq, serde::Deserialize, serde::Serialize)] @@ -313,19 +315,6 @@ pub fn diff_objs( }) } -#[derive(Debug, Copy, Clone, PartialEq, Eq, PartialOrd, Ord)] -pub struct SymbolRef { - pub section_idx: usize, - pub symbol_idx: usize, -} - -#[inline] -pub fn get_symbol(obj: &ObjInfo, ref_: SymbolRef) -> (&ObjSection, &ObjSymbol) { - let section = &obj.sections[ref_.section_idx]; - let symbol = §ion.symbols[ref_.symbol_idx]; - (section, symbol) -} - #[derive(Copy, Clone, Eq, PartialEq)] struct SymbolMatch { left: Option, @@ -348,7 +337,7 @@ fn matching_symbols( prev: Option<&ObjInfo>, ) -> Result> { let mut matches = Vec::new(); - let mut right_used = Vec::new(); + let mut right_used = HashSet::new(); if let Some(left) = left { for (section_idx, section) in left.sections.iter().enumerate() { for (symbol_idx, symbol) in section.symbols.iter().enumerate() { @@ -360,7 +349,7 @@ fn matching_symbols( }; matches.push(symbol_match); if let Some(right) = symbol_match.right { - right_used.push(right); + right_used.insert(right); } } } @@ -369,7 +358,7 @@ fn matching_symbols( for (section_idx, section) in right.sections.iter().enumerate() { for (symbol_idx, symbol) in section.symbols.iter().enumerate() { let symbol_ref = SymbolRef { section_idx, symbol_idx }; - if right_used.binary_search(&symbol_ref).is_ok() { + if right_used.contains(&symbol_ref) { continue; } matches.push(SymbolMatch { diff --git a/objdiff-core/src/obj/mod.rs b/objdiff-core/src/obj/mod.rs index 986774b..6264eeb 100644 --- a/objdiff-core/src/obj/mod.rs +++ b/objdiff-core/src/obj/mod.rs @@ -139,3 +139,17 @@ pub struct ObjReloc { pub target: ObjSymbol, pub target_section: Option, } + +#[derive(Debug, Copy, Clone, PartialEq, Eq, PartialOrd, Ord, Hash)] +pub struct SymbolRef { + pub section_idx: usize, + pub symbol_idx: usize, +} + +impl ObjInfo { + pub fn section_symbol(&self, symbol_ref: SymbolRef) -> (&ObjSection, &ObjSymbol) { + let section = &self.sections[symbol_ref.section_idx]; + let symbol = §ion.symbols[symbol_ref.symbol_idx]; + (section, symbol) + } +} diff --git a/objdiff-gui/src/views/function_diff.rs b/objdiff-gui/src/views/function_diff.rs index 0fab463..2f9bcc4 100644 --- a/objdiff-gui/src/views/function_diff.rs +++ b/objdiff-gui/src/views/function_diff.rs @@ -6,9 +6,9 @@ use objdiff_core::{ arch::ObjArch, diff::{ display::{display_diff, DiffText, HighlightKind}, - get_symbol, ObjDiff, ObjInsDiff, ObjInsDiffKind, SymbolRef, + ObjDiff, ObjInsDiff, ObjInsDiffKind, }, - obj::{ObjInfo, ObjIns, ObjInsArg, ObjInsArgValue, ObjSection, ObjSymbol}, + obj::{ObjInfo, ObjIns, ObjInsArg, ObjInsArgValue, ObjSection, ObjSymbol, SymbolRef}, }; use time::format_description; @@ -249,9 +249,8 @@ fn asm_col_ui( appearance: &Appearance, ins_view_state: &mut FunctionViewState, ) { - let (section, symbol) = get_symbol(&obj.0, symbol_ref); - let ins_diff = &obj.1.sections[symbol_ref.section_idx].symbols[symbol_ref.symbol_idx] - .instructions[row.index()]; + let (section, symbol) = obj.0.section_symbol(symbol_ref); + let ins_diff = &obj.1.symbol_diff(symbol_ref).instructions[row.index()]; let (_, response) = row.col(|ui| { asm_row_ui(ui, ins_diff, symbol, appearance, ins_view_state); }); @@ -279,20 +278,18 @@ fn asm_table_ui( let left_symbol = left_obj.and_then(|(obj, _)| find_symbol(obj, selected_symbol)); let right_symbol = right_obj.and_then(|(obj, _)| find_symbol(obj, selected_symbol)); let instructions_len = match (left_symbol, right_symbol) { - (Some(left_symbol_ref), Some(_)) => left_obj.unwrap().1.sections - [left_symbol_ref.section_idx] - .symbols[left_symbol_ref.symbol_idx] - .instructions - .len(), - (Some(left_symbol_ref), None) => left_obj.unwrap().1.sections[left_symbol_ref.section_idx] - .symbols[left_symbol_ref.symbol_idx] - .instructions - .len(), - (None, Some(right_symbol_ref)) => right_obj.unwrap().1.sections - [right_symbol_ref.section_idx] - .symbols[right_symbol_ref.symbol_idx] - .instructions - .len(), + (Some(left_symbol_ref), Some(right_symbol_ref)) => { + let left_len = left_obj.unwrap().1.symbol_diff(left_symbol_ref).instructions.len(); + let right_len = right_obj.unwrap().1.symbol_diff(right_symbol_ref).instructions.len(); + debug_assert_eq!(left_len, right_len); + left_len + } + (Some(left_symbol_ref), None) => { + left_obj.unwrap().1.symbol_diff(left_symbol_ref).instructions.len() + } + (None, Some(right_symbol_ref)) => { + right_obj.unwrap().1.symbol_diff(right_symbol_ref).instructions.len() + } (None, None) => return None, }; table.body(|body| {