From 22a24f37f54935a9624fd4e64544ab2db64756a0 Mon Sep 17 00:00:00 2001 From: Luke Street Date: Mon, 20 May 2024 23:52:59 -0600 Subject: [PATCH 01/11] Diff data symbols & improve symbol match logic --- objdiff-core/src/arch/mips.rs | 3 +- objdiff-core/src/arch/ppc.rs | 3 +- objdiff-core/src/arch/x86.rs | 1 + objdiff-core/src/diff/code.rs | 4 +- objdiff-core/src/diff/data.rs | 55 +++++++++-- objdiff-core/src/diff/mod.rs | 127 +++++++++++++++++++------ objdiff-core/src/obj/mod.rs | 10 +- objdiff-gui/src/views/function_diff.rs | 1 + objdiff-gui/src/views/symbol_diff.rs | 36 +++++-- 9 files changed, 190 insertions(+), 50 deletions(-) diff --git a/objdiff-core/src/arch/mips.rs b/objdiff-core/src/arch/mips.rs index ba4ca80..f21738b 100644 --- a/objdiff-core/src/arch/mips.rs +++ b/objdiff-core/src/arch/mips.rs @@ -1,6 +1,6 @@ use std::borrow::Cow; -use anyhow::{bail, Result}; +use anyhow::{anyhow, bail, Result}; use object::{elf, Endian, Endianness, File, Object, Relocation, RelocationFlags}; use rabbitizer::{config, Abi, InstrCategory, Instruction, OperandType}; @@ -35,6 +35,7 @@ impl ObjArch for ObjArchMips { config: &DiffObjConfig, ) -> Result { let (section, symbol) = obj.section_symbol(symbol_ref); + let section = section.ok_or_else(|| anyhow!("Code symbol section not found"))?; let code = §ion.data [symbol.section_address as usize..(symbol.section_address + symbol.size) as usize]; diff --git a/objdiff-core/src/arch/ppc.rs b/objdiff-core/src/arch/ppc.rs index 485ef6b..c27fbe2 100644 --- a/objdiff-core/src/arch/ppc.rs +++ b/objdiff-core/src/arch/ppc.rs @@ -1,6 +1,6 @@ use std::borrow::Cow; -use anyhow::{bail, Result}; +use anyhow::{anyhow, bail, Result}; use object::{elf, File, Relocation, RelocationFlags}; use ppc750cl::{Argument, InsIter, GPR}; @@ -36,6 +36,7 @@ impl ObjArch for ObjArchPpc { config: &DiffObjConfig, ) -> Result { let (section, symbol) = obj.section_symbol(symbol_ref); + let section = section.ok_or_else(|| anyhow!("Code symbol section not found"))?; let code = §ion.data [symbol.section_address as usize..(symbol.section_address + symbol.size) as usize]; diff --git a/objdiff-core/src/arch/x86.rs b/objdiff-core/src/arch/x86.rs index 89e0345..8bbecd7 100644 --- a/objdiff-core/src/arch/x86.rs +++ b/objdiff-core/src/arch/x86.rs @@ -33,6 +33,7 @@ impl ObjArch for ObjArchX86 { config: &DiffObjConfig, ) -> Result { let (section, symbol) = obj.section_symbol(symbol_ref); + let section = section.ok_or_else(|| anyhow!("Code symbol section not found"))?; let code = §ion.data [symbol.section_address as usize..(symbol.section_address + symbol.size) as usize]; diff --git a/objdiff-core/src/diff/code.rs b/objdiff-core/src/diff/code.rs index f676440..bbd300d 100644 --- a/objdiff-core/src/diff/code.rs +++ b/objdiff-core/src/diff/code.rs @@ -28,7 +28,7 @@ pub fn no_diff_code( diff.push(ObjInsDiff { ins: Some(i), kind: ObjInsDiffKind::None, ..Default::default() }); } resolve_branches(&mut diff); - Ok(ObjSymbolDiff { diff_symbol: None, instructions: diff, match_percent: None }) + Ok(ObjSymbolDiff { symbol_ref, diff_symbol: None, instructions: diff, match_percent: None }) } pub fn diff_code( @@ -66,11 +66,13 @@ pub fn diff_code( Ok(( ObjSymbolDiff { + symbol_ref: left_symbol_ref, diff_symbol: Some(right_symbol_ref), instructions: left_diff, match_percent: Some(percent), }, ObjSymbolDiff { + symbol_ref: right_symbol_ref, diff_symbol: Some(left_symbol_ref), instructions: right_diff, match_percent: Some(percent), diff --git a/objdiff-core/src/diff/data.rs b/objdiff-core/src/diff/data.rs index d0a4c6e..e8882d7 100644 --- a/objdiff-core/src/diff/data.rs +++ b/objdiff-core/src/diff/data.rs @@ -3,8 +3,8 @@ use std::{ time::{Duration, Instant}, }; -use anyhow::Result; -use similar::{capture_diff_slices_deadline, Algorithm}; +use anyhow::{anyhow, Result}; +use similar::{capture_diff_slices_deadline, get_diff_ratio, Algorithm}; use crate::{ diff::{ObjDataDiff, ObjDataDiffKind, ObjSectionDiff, ObjSymbolDiff}, @@ -22,11 +22,13 @@ pub fn diff_bss_symbol( let percent = if left_symbol.size == right_symbol.size { 100.0 } else { 50.0 }; Ok(( ObjSymbolDiff { + symbol_ref: left_symbol_ref, diff_symbol: Some(right_symbol_ref), instructions: vec![], match_percent: Some(percent), }, ObjSymbolDiff { + symbol_ref: right_symbol_ref, diff_symbol: Some(left_symbol_ref), instructions: vec![], match_percent: Some(percent), @@ -34,8 +36,8 @@ pub fn diff_bss_symbol( )) } -pub fn no_diff_bss_symbol(_obj: &ObjInfo, _symbol_ref: SymbolRef) -> ObjSymbolDiff { - ObjSymbolDiff { diff_symbol: None, instructions: vec![], match_percent: Some(0.0) } +pub fn no_diff_symbol(_obj: &ObjInfo, symbol_ref: SymbolRef) -> ObjSymbolDiff { + ObjSymbolDiff { symbol_ref, diff_symbol: None, instructions: vec![], match_percent: None } } pub fn diff_data( @@ -45,6 +47,7 @@ pub fn diff_data( let deadline = Instant::now() + Duration::from_secs(5); 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(); @@ -118,14 +121,50 @@ pub fn diff_data( ObjSectionDiff { symbols: vec![], data_diff: left_diff, - // TODO - match_percent: None, + match_percent: Some(match_percent), }, ObjSectionDiff { symbols: vec![], data_diff: right_diff, - // TODO - match_percent: None, + match_percent: Some(match_percent), + }, + )) +} + +pub fn diff_data_symbol( + left_obj: &ObjInfo, + right_obj: &ObjInfo, + left_symbol_ref: SymbolRef, + right_symbol_ref: SymbolRef, +) -> Result<(ObjSymbolDiff, ObjSymbolDiff)> { + 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_section = left_section.ok_or_else(|| anyhow!("Data symbol section not found"))?; + let right_section = right_section.ok_or_else(|| anyhow!("Data symbol section not found"))?; + + let left_data = &left_section.data[left_symbol.section_address as usize + ..(left_symbol.section_address + left_symbol.size) as usize]; + let right_data = &right_section.data[right_symbol.section_address as usize + ..(right_symbol.section_address + right_symbol.size) as usize]; + + let deadline = Instant::now() + Duration::from_secs(5); + 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; + + Ok(( + ObjSymbolDiff { + symbol_ref: left_symbol_ref, + diff_symbol: Some(right_symbol_ref), + instructions: vec![], + match_percent: Some(match_percent), + }, + ObjSymbolDiff { + symbol_ref: right_symbol_ref, + diff_symbol: Some(left_symbol_ref), + instructions: vec![], + match_percent: Some(match_percent), }, )) } diff --git a/objdiff-core/src/diff/mod.rs b/objdiff-core/src/diff/mod.rs index 399a6d7..de8d3ea 100644 --- a/objdiff-core/src/diff/mod.rs +++ b/objdiff-core/src/diff/mod.rs @@ -1,7 +1,3 @@ -mod code; -mod data; -pub mod display; - use std::collections::HashSet; use anyhow::Result; @@ -9,11 +5,15 @@ use anyhow::Result; use crate::{ diff::{ code::{diff_code, no_diff_code}, - data::{diff_bss_symbol, diff_data, no_diff_bss_symbol}, + data::{diff_bss_symbol, diff_data, diff_data_symbol, no_diff_symbol}, }, - obj::{ObjInfo, ObjIns, ObjSectionKind, SymbolRef}, + obj::{ObjInfo, ObjIns, ObjSection, ObjSectionKind, ObjSymbol, SymbolRef}, }; +mod code; +mod data; +pub mod display; + #[derive(Debug, Copy, Clone, Default, Eq, PartialEq, serde::Deserialize, serde::Serialize)] pub enum X86Formatter { #[default] @@ -62,6 +62,7 @@ impl ObjSectionDiff { #[derive(Debug, Clone, Default)] pub struct ObjSymbolDiff { + pub symbol_ref: SymbolRef, pub diff_symbol: Option, pub instructions: Vec, pub match_percent: Option, @@ -142,10 +143,11 @@ impl ObjDiff { sections: Vec::with_capacity(obj.sections.len()), common: Vec::with_capacity(obj.common.len()), }; - for section in &obj.sections { + for (section_idx, section) in obj.sections.iter().enumerate() { let mut symbols = Vec::with_capacity(section.symbols.len()); - for _ in §ion.symbols { + for (symbol_idx, _) in section.symbols.iter().enumerate() { symbols.push(ObjSymbolDiff { + symbol_ref: SymbolRef { section_idx, symbol_idx }, diff_symbol: None, instructions: vec![], match_percent: None, @@ -162,8 +164,9 @@ impl ObjDiff { match_percent: None, }); } - for _ in &obj.common { + for (symbol_idx, _) in obj.common.iter().enumerate() { result.common.push(ObjSymbolDiff { + symbol_ref: SymbolRef { section_idx: obj.sections.len(), symbol_idx }, diff_symbol: None, instructions: vec![], match_percent: None, @@ -184,12 +187,20 @@ impl ObjDiff { #[inline] pub fn symbol_diff(&self, symbol_ref: SymbolRef) -> &ObjSymbolDiff { - &self.section_diff(symbol_ref.section_idx).symbols[symbol_ref.symbol_idx] + if symbol_ref.section_idx == self.sections.len() { + &self.common[symbol_ref.symbol_idx] + } else { + &self.section_diff(symbol_ref.section_idx).symbols[symbol_ref.symbol_idx] + } } #[inline] pub fn symbol_diff_mut(&mut self, symbol_ref: SymbolRef) -> &mut ObjSymbolDiff { - &mut self.section_diff_mut(symbol_ref.section_idx).symbols[symbol_ref.symbol_idx] + if symbol_ref.section_idx == self.sections.len() { + &mut self.common[symbol_ref.symbol_idx] + } else { + &mut self.section_diff_mut(symbol_ref.section_idx).symbols[symbol_ref.symbol_idx] + } } } @@ -247,7 +258,14 @@ pub fn diff_objs( } } ObjSectionKind::Data => { - // TODO diff data symbol + let (left_diff, right_diff) = diff_data_symbol( + left_obj, + right_obj, + left_symbol_ref, + right_symbol_ref, + )?; + *left_out.symbol_diff_mut(left_symbol_ref) = left_diff; + *right_out.symbol_diff_mut(right_symbol_ref) = right_diff; } ObjSectionKind::Bss => { let (left_diff, right_diff) = diff_bss_symbol( @@ -268,10 +286,9 @@ pub fn diff_objs( *left_out.symbol_diff_mut(left_symbol_ref) = no_diff_code(left_obj, left_symbol_ref, config)?; } - ObjSectionKind::Data => {} - ObjSectionKind::Bss => { + ObjSectionKind::Data | ObjSectionKind::Bss => { *left_out.symbol_diff_mut(left_symbol_ref) = - no_diff_bss_symbol(left_obj, left_symbol_ref); + no_diff_symbol(left_obj, left_symbol_ref); } } } @@ -282,10 +299,9 @@ pub fn diff_objs( *right_out.symbol_diff_mut(right_symbol_ref) = no_diff_code(right_obj, right_symbol_ref, config)?; } - ObjSectionKind::Data => {} - ObjSectionKind::Bss => { + ObjSectionKind::Data | ObjSectionKind::Bss => { *right_out.symbol_diff_mut(right_symbol_ref) = - no_diff_bss_symbol(right_obj, right_symbol_ref); + no_diff_symbol(right_obj, right_symbol_ref); } } } @@ -357,8 +373,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.name, section.kind), - prev: find_symbol(prev, &symbol.name, section.kind), + right: find_symbol(right, symbol, section), + prev: find_symbol(prev, symbol, section), section_kind: section.kind, }; matches.push(symbol_match); @@ -367,6 +383,18 @@ fn matching_symbols( } } } + for (symbol_idx, symbol) in left.common.iter().enumerate() { + let symbol_match = SymbolMatch { + left: Some(SymbolRef { section_idx: left.sections.len(), symbol_idx }), + right: find_common_symbol(right, symbol), + prev: find_common_symbol(prev, symbol), + section_kind: ObjSectionKind::Bss, + }; + matches.push(symbol_match); + if let Some(right) = symbol_match.right { + right_used.insert(right); + } + } } if let Some(right) = right { for (section_idx, section) in right.sections.iter().enumerate() { @@ -378,29 +406,68 @@ fn matching_symbols( matches.push(SymbolMatch { left: None, right: Some(symbol_ref), - prev: find_symbol(prev, &symbol.name, section.kind), + prev: find_symbol(prev, symbol, section), section_kind: section.kind, }); } } + for (symbol_idx, symbol) in right.common.iter().enumerate() { + let symbol_ref = SymbolRef { section_idx: right.sections.len(), symbol_idx }; + if right_used.contains(&symbol_ref) { + continue; + } + matches.push(SymbolMatch { + left: None, + right: Some(symbol_ref), + prev: find_common_symbol(prev, symbol), + section_kind: ObjSectionKind::Bss, + }); + } } Ok(matches) } fn find_symbol( obj: Option<&ObjInfo>, - name: &str, - section_kind: ObjSectionKind, + in_symbol: &ObjSymbol, + in_section: &ObjSection, ) -> Option { - for (section_idx, section) in obj?.sections.iter().enumerate() { - if section.kind != section_kind { + let obj = obj?; + // Try to find an exact name match + for (section_idx, section) in obj.sections.iter().enumerate() { + if section.kind != in_section.kind { continue; } - let symbol_idx = match section.symbols.iter().position(|symbol| symbol.name == name) { - Some(symbol_idx) => symbol_idx, - None => continue, - }; - return Some(SymbolRef { section_idx, symbol_idx }); + if let Some(symbol_idx) = + section.symbols.iter().position(|symbol| symbol.name == in_symbol.name) + { + return Some(SymbolRef { section_idx, symbol_idx }); + } + } + // Match compiler-generated symbols against each other (e.g. @251 -> @60) + // If they are at the same address in the same section + if in_symbol.name.starts_with('@') + && matches!(in_section.kind, ObjSectionKind::Data | ObjSectionKind::Bss) + { + if let Some((section_idx, section)) = + obj.sections.iter().enumerate().find(|(_, s)| s.name == in_section.name) + { + if let Some(symbol_idx) = section.symbols.iter().position(|symbol| { + symbol.address == in_symbol.address && symbol.name.starts_with('@') + }) { + return Some(SymbolRef { section_idx, symbol_idx }); + } + } + } + None +} + +fn find_common_symbol(obj: Option<&ObjInfo>, in_symbol: &ObjSymbol) -> Option { + let obj = obj?; + for (symbol_idx, symbol) in obj.common.iter().enumerate() { + if symbol.name == in_symbol.name { + return Some(SymbolRef { section_idx: obj.sections.len(), symbol_idx }); + } } None } diff --git a/objdiff-core/src/obj/mod.rs b/objdiff-core/src/obj/mod.rs index 02f69b7..f1586c9 100644 --- a/objdiff-core/src/obj/mod.rs +++ b/objdiff-core/src/obj/mod.rs @@ -142,16 +142,20 @@ pub struct ObjReloc { pub target_section: Option, } -#[derive(Debug, Copy, Clone, PartialEq, Eq, PartialOrd, Ord, Hash)] +#[derive(Debug, Copy, Clone, PartialEq, Eq, PartialOrd, Ord, Hash, Default)] pub struct SymbolRef { pub section_idx: usize, pub symbol_idx: usize, } impl ObjInfo { - pub fn section_symbol(&self, symbol_ref: SymbolRef) -> (&ObjSection, &ObjSymbol) { + pub fn section_symbol(&self, symbol_ref: SymbolRef) -> (Option<&ObjSection>, &ObjSymbol) { + if symbol_ref.section_idx == self.sections.len() { + let symbol = &self.common[symbol_ref.symbol_idx]; + return (None, symbol); + } let section = &self.sections[symbol_ref.section_idx]; let symbol = §ion.symbols[symbol_ref.symbol_idx]; - (section, symbol) + (Some(section), symbol) } } diff --git a/objdiff-gui/src/views/function_diff.rs b/objdiff-gui/src/views/function_diff.rs index 8421782..b1d3b88 100644 --- a/objdiff-gui/src/views/function_diff.rs +++ b/objdiff-gui/src/views/function_diff.rs @@ -282,6 +282,7 @@ fn asm_col_ui( ins_view_state: &mut FunctionViewState, ) { let (section, symbol) = obj.0.section_symbol(symbol_ref); + let section = section.unwrap(); let ins_diff = &obj.1.symbol_diff(symbol_ref).instructions[row.index()]; let response_cb = |response: Response| { if let Some(ins) = &ins_diff.ins { diff --git a/objdiff-gui/src/views/symbol_diff.rs b/objdiff-gui/src/views/symbol_diff.rs index 58d0736..8dfb41e 100644 --- a/objdiff-gui/src/views/symbol_diff.rs +++ b/objdiff-gui/src/views/symbol_diff.rs @@ -7,7 +7,7 @@ use egui::{ use egui_extras::{Size, StripBuilder}; use objdiff_core::{ diff::{ObjDiff, ObjSymbolDiff}, - obj::{ObjInfo, ObjSection, ObjSectionKind, ObjSymbol, ObjSymbolFlags}, + obj::{ObjInfo, ObjSection, ObjSectionKind, ObjSymbol, ObjSymbolFlags, SymbolRef}, }; use crate::{ @@ -52,7 +52,7 @@ pub struct DiffViewState { #[derive(Default)] pub struct SymbolViewState { - pub highlighted_symbol: Option, + pub highlighted_symbol: (Option, Option), pub selected_symbol: Option, pub reverse_fn_order: bool, pub disable_reverse_fn_order: bool, @@ -184,6 +184,7 @@ fn symbol_ui( section: Option<&ObjSection>, state: &mut SymbolViewState, appearance: &Appearance, + left: bool, ) -> Option { if symbol.flags.0.contains(ObjSymbolFlags::Hidden) && !state.show_hidden_symbols { return None; @@ -193,8 +194,10 @@ fn symbol_ui( let name: &str = if let Some(demangled) = &symbol.demangled_name { demangled } else { &symbol.name }; let mut selected = false; - if let Some(sym) = &state.highlighted_symbol { - selected = sym == &symbol.name; + if let Some(sym_ref) = + if left { state.highlighted_symbol.0 } else { state.highlighted_symbol.1 } + { + selected = symbol_diff.symbol_ref == sym_ref; } write_text("[", appearance.text_color, &mut job, appearance.code_font.clone()); if symbol.flags.0.contains(ObjSymbolFlags::Common) { @@ -245,7 +248,15 @@ fn symbol_ui( } } } else if response.hovered() { - state.highlighted_symbol = Some(symbol.name.clone()); + state.highlighted_symbol = if let Some(diff_symbol) = symbol_diff.diff_symbol { + if left { + (Some(symbol_diff.symbol_ref), Some(diff_symbol)) + } else { + (Some(diff_symbol), Some(symbol_diff.symbol_ref)) + } + } else { + (None, None) + }; } ret } @@ -267,6 +278,7 @@ fn symbol_list_ui( state: &mut SymbolViewState, lower_search: &str, appearance: &Appearance, + left: bool, ) -> Option { let mut ret = None; ScrollArea::both().auto_shrink([false, false]).show(ui, |ui| { @@ -277,7 +289,15 @@ fn symbol_list_ui( if !obj.0.common.is_empty() { CollapsingHeader::new(".comm").default_open(true).show(ui, |ui| { for (symbol, symbol_diff) in obj.0.common.iter().zip(&obj.1.common) { - ret = ret.or(symbol_ui(ui, symbol, symbol_diff, None, state, appearance)); + ret = ret.or(symbol_ui( + ui, + symbol, + symbol_diff, + None, + state, + appearance, + left, + )); } }); } @@ -301,6 +321,7 @@ fn symbol_list_ui( Some(section), state, appearance, + left, )); } } else { @@ -317,6 +338,7 @@ fn symbol_list_ui( Some(section), state, appearance, + left, )); } } @@ -447,6 +469,7 @@ pub fn symbol_diff_ui(ui: &mut Ui, state: &mut DiffViewState, appearance: &Appea symbol_state, &lower_search, appearance, + true, )); } else { missing_obj_ui(ui, appearance); @@ -466,6 +489,7 @@ pub fn symbol_diff_ui(ui: &mut Ui, state: &mut DiffViewState, appearance: &Appea symbol_state, &lower_search, appearance, + false, )); } else { missing_obj_ui(ui, appearance); From f5b5a612fc5ad3c7157db1eb735972afedc98d46 Mon Sep 17 00:00:00 2001 From: Aetias <144526980+AetiasHax@users.noreply.github.com> Date: Tue, 21 May 2024 17:55:39 +0200 Subject: [PATCH 02/11] Display correct line numbers for multiple .text sections (#63) * Support multiple DWARF sequences * Rework DWARF line info parsing - Properly handles multiple sections in DWARF 1 - line_info moved into ObjSection - DWARF 2 parser no longer errors with no .text section - Both parsers properly skip empty sections * Simplify line_info (no Option) --------- Co-authored-by: Luke Street --- objdiff-core/src/arch/mips.rs | 5 +- objdiff-core/src/arch/ppc.rs | 5 +- objdiff-core/src/arch/x86.rs | 3 +- objdiff-core/src/obj/mod.rs | 4 +- objdiff-core/src/obj/read.rs | 96 ++++++++++++++++++++++++----------- 5 files changed, 71 insertions(+), 42 deletions(-) diff --git a/objdiff-core/src/arch/mips.rs b/objdiff-core/src/arch/mips.rs index f21738b..ea556dc 100644 --- a/objdiff-core/src/arch/mips.rs +++ b/objdiff-core/src/arch/mips.rs @@ -112,10 +112,7 @@ impl ObjArch for ObjArchMips { } } } - let line = obj - .line_info - .as_ref() - .and_then(|map| map.range(..=cur_addr as u64).last().map(|(_, &b)| b)); + let line = section.line_info.range(..=cur_addr as u64).last().map(|(_, &b)| b); insts.push(ObjIns { address: cur_addr as u64, size: 4, diff --git a/objdiff-core/src/arch/ppc.rs b/objdiff-core/src/arch/ppc.rs index c27fbe2..107b9d9 100644 --- a/objdiff-core/src/arch/ppc.rs +++ b/objdiff-core/src/arch/ppc.rs @@ -133,10 +133,7 @@ impl ObjArch for ObjArchPpc { } ops.push(ins.op as u16); - let line = obj - .line_info - .as_ref() - .and_then(|map| map.range(..=cur_addr as u64).last().map(|(_, &b)| b)); + let line = section.line_info.range(..=cur_addr as u64).last().map(|(_, &b)| b); insts.push(ObjIns { address: cur_addr as u64, size: 4, diff --git a/objdiff-core/src/arch/x86.rs b/objdiff-core/src/arch/x86.rs index 8bbecd7..3b7c9a6 100644 --- a/objdiff-core/src/arch/x86.rs +++ b/objdiff-core/src/arch/x86.rs @@ -74,6 +74,7 @@ impl ObjArch for ObjArchX86 { .relocations .iter() .find(|r| r.address >= address && r.address < address + instruction.len() as u64); + let line = section.line_info.range(..=address).last().map(|(_, &b)| b); output.ins = ObjIns { address, size: instruction.len() as u8, @@ -82,7 +83,7 @@ impl ObjArch for ObjArchX86 { args: vec![], reloc: reloc.cloned(), branch_dest: None, - line: obj.line_info.as_ref().and_then(|m| m.get(&address).cloned()), + line, formatted: String::new(), orig: None, }; diff --git a/objdiff-core/src/obj/mod.rs b/objdiff-core/src/obj/mod.rs index f1586c9..6769f1f 100644 --- a/objdiff-core/src/obj/mod.rs +++ b/objdiff-core/src/obj/mod.rs @@ -39,6 +39,8 @@ pub struct ObjSection { pub symbols: Vec, pub relocations: Vec, pub virtual_address: Option, + /// Line number info (.line or .debug_line section) + pub line_info: BTreeMap, } #[derive(Debug, Clone, Eq, PartialEq)] @@ -128,8 +130,6 @@ pub struct ObjInfo { pub sections: Vec, /// Common BSS symbols pub common: Vec, - /// Line number info (.line or .debug_line section) - pub line_info: Option>, /// Split object metadata (.note.split section) pub split_meta: Option, } diff --git a/objdiff-core/src/obj/read.rs b/objdiff-core/src/obj/read.rs index 017d232..8462da8 100644 --- a/objdiff-core/src/obj/read.rs +++ b/objdiff-core/src/obj/read.rs @@ -1,4 +1,4 @@ -use std::{collections::BTreeMap, fs, io::Cursor, path::Path}; +use std::{fs, io::Cursor, path::Path}; use anyhow::{anyhow, bail, ensure, Context, Result}; use byteorder::{BigEndian, ReadBytesExt}; @@ -111,6 +111,7 @@ fn filter_sections(obj_file: &File<'_>, split_meta: Option<&SplitMeta>) -> Resul symbols: Vec::new(), relocations: Vec::new(), virtual_address, + line_info: Default::default(), }); } result.sort_by(|a, b| a.name.cmp(&b.name)); @@ -268,26 +269,40 @@ fn relocations_by_section( Ok(relocations) } -fn line_info(obj_file: &File<'_>) -> Result>> { +fn line_info(obj_file: &File<'_>, sections: &mut [ObjSection]) -> Result<()> { // DWARF 1.1 - let mut map = BTreeMap::new(); if let Some(section) = obj_file.section_by_name(".line") { - if section.size() == 0 { - return Ok(None); - } let data = section.uncompressed_data()?; let mut reader = Cursor::new(data.as_ref()); - let size = reader.read_u32::()?; - let base_address = reader.read_u32::()? as u64; - while reader.position() < size as u64 { - let line_number = reader.read_u32::()? as u64; - let statement_pos = reader.read_u16::()?; - if statement_pos != 0xFFFF { - log::warn!("Unhandled statement pos {}", statement_pos); + let mut text_sections = obj_file.sections().filter(|s| s.kind() == SectionKind::Text); + while reader.position() < data.len() as u64 { + let text_section_index = text_sections + .next() + .ok_or_else(|| anyhow!("Next text section not found for line info"))? + .index() + .0; + let start = reader.position(); + let size = reader.read_u32::()?; + let base_address = reader.read_u32::()? as u64; + let Some(out_section) = + sections.iter_mut().find(|s| s.orig_index == text_section_index) + else { + // Skip line info for sections we filtered out + reader.set_position(start + size as u64); + continue; + }; + let end = start + size as u64; + while reader.position() < end { + let line_number = reader.read_u32::()? as u64; + let statement_pos = reader.read_u16::()?; + if statement_pos != 0xFFFF { + log::warn!("Unhandled statement pos {}", statement_pos); + } + let address_delta = reader.read_u32::()? as u64; + out_section.line_info.insert(base_address + address_delta, line_number); + log::debug!("Line: {:#x} -> {}", base_address + address_delta, line_number); } - let address_delta = reader.read_u32::()? as u64; - map.insert(base_address + address_delta, line_number); } } @@ -308,22 +323,48 @@ fn line_info(obj_file: &File<'_>) -> Result>> { }; let dwarf = dwarf_cow.borrow(|section| gimli::EndianSlice::new(section, endian)); let mut iter = dwarf.units(); - while let Some(header) = iter.next()? { + if let Some(header) = iter.next()? { let unit = dwarf.unit(header)?; if let Some(program) = unit.line_program.clone() { + let mut text_sections = + obj_file.sections().filter(|s| s.kind() == SectionKind::Text); + let section_index = text_sections + .next() + .ok_or_else(|| anyhow!("Next text section not found for line info"))? + .index() + .0; + let mut lines = sections + .iter_mut() + .find(|s| s.orig_index == section_index) + .map(|s| &mut s.line_info); + let mut rows = program.rows(); while let Some((_header, row)) = rows.next_row()? { - if let Some(line) = row.line() { - map.insert(row.address(), line.get()); + if let (Some(line), Some(lines)) = (row.line(), &mut lines) { + lines.insert(row.address(), line.get()); + } + if row.end_sequence() { + // The next row is the start of a new sequence, which means we must + // advance to the next .text section. + let section_index = text_sections + .next() + .ok_or_else(|| anyhow!("Next text section not found for line info"))? + .index() + .0; + lines = sections + .iter_mut() + .find(|s| s.orig_index == section_index) + .map(|s| &mut s.line_info); } } } } + if iter.next()?.is_some() { + log::warn!("Multiple units found in DWARF data, only processing the first"); + } } - if map.is_empty() { - return Ok(None); - } - Ok(Some(map)) + + Ok(()) } pub fn read(obj_path: &Path) -> Result { @@ -342,16 +383,9 @@ pub fn read(obj_path: &Path) -> Result { section.relocations = relocations_by_section(arch.as_ref(), &obj_file, section, split_meta.as_ref())?; } + line_info(&obj_file, &mut sections)?; let common = common_symbols(arch.as_ref(), &obj_file, split_meta.as_ref())?; - Ok(ObjInfo { - arch, - path: obj_path.to_owned(), - timestamp, - sections, - common, - line_info: line_info(&obj_file)?, - split_meta, - }) + Ok(ObjInfo { arch, path: obj_path.to_owned(), timestamp, sections, common, split_meta }) } pub fn has_function(obj_path: &Path, symbol_name: &str) -> Result { From 94f1f07b00e8805dfa8a8bc03c8f65fb85bc2ba7 Mon Sep 17 00:00:00 2001 From: Luke Street Date: Tue, 21 May 2024 09:50:27 -0600 Subject: [PATCH 03/11] Bump to 2.0.0-alpha.1 & fix version checks --- Cargo.lock | 6 +++--- objdiff-cli/Cargo.toml | 2 +- objdiff-core/Cargo.toml | 2 +- objdiff-core/src/config/mod.rs | 9 +++++---- objdiff-gui/Cargo.toml | 2 +- objdiff-gui/src/jobs/check_update.rs | 12 +++++++++--- objdiff-gui/src/jobs/update.rs | 17 ++++++++++------- objdiff-gui/src/update.rs | 8 +++++--- objdiff-gui/src/views/config.rs | 16 ++++++++-------- 9 files changed, 43 insertions(+), 31 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 1970460..94ddee6 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3081,7 +3081,7 @@ dependencies = [ [[package]] name = "objdiff-cli" -version = "0.1.0" +version = "2.0.0-alpha.1" dependencies = [ "anyhow", "argp", @@ -3100,7 +3100,7 @@ dependencies = [ [[package]] name = "objdiff-core" -version = "1.0.0" +version = "2.0.0-alpha.1" dependencies = [ "anyhow", "byteorder", @@ -3127,7 +3127,7 @@ dependencies = [ [[package]] name = "objdiff-gui" -version = "1.0.0" +version = "2.0.0-alpha.1" dependencies = [ "anyhow", "bytes", diff --git a/objdiff-cli/Cargo.toml b/objdiff-cli/Cargo.toml index 54c9c0c..396643d 100644 --- a/objdiff-cli/Cargo.toml +++ b/objdiff-cli/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "objdiff-cli" -version = "0.1.0" +version = "2.0.0-alpha.1" edition = "2021" rust-version = "1.70" authors = ["Luke Street "] diff --git a/objdiff-core/Cargo.toml b/objdiff-core/Cargo.toml index 16a0a94..fd5d820 100644 --- a/objdiff-core/Cargo.toml +++ b/objdiff-core/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "objdiff-core" -version = "1.0.0" +version = "2.0.0-alpha.1" edition = "2021" rust-version = "1.70" authors = ["Luke Street "] diff --git a/objdiff-core/src/config/mod.rs b/objdiff-core/src/config/mod.rs index 9f0f54b..f1fd1fb 100644 --- a/objdiff-core/src/config/mod.rs +++ b/objdiff-core/src/config/mod.rs @@ -143,10 +143,11 @@ fn validate_min_version(config: &ProjectConfig) -> Result<()> { let Some(min_version) = &config.min_version else { return Ok(()) }; let version = semver::Version::parse(env!("CARGO_PKG_VERSION")) .context("Failed to parse package version")?; - match semver::VersionReq::parse(&format!(">={min_version}")) { - Ok(version_req) if version_req.matches(&version) => Ok(()), - Ok(_) => Err(anyhow!("Project requires objdiff version {min_version} or higher")), - Err(e) => Err(anyhow::Error::new(e).context("Failed to parse min_version")), + let min_version = semver::Version::parse(min_version).context("Failed to parse min_version")?; + if version >= min_version { + Ok(()) + } else { + Err(anyhow!("Project requires objdiff version {min_version} or higher")) } } diff --git a/objdiff-gui/Cargo.toml b/objdiff-gui/Cargo.toml index e57a881..63b21a8 100644 --- a/objdiff-gui/Cargo.toml +++ b/objdiff-gui/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "objdiff-gui" -version = "1.0.0" +version = "2.0.0-alpha.1" edition = "2021" rust-version = "1.70" authors = ["Luke Street "] diff --git a/objdiff-gui/src/jobs/check_update.rs b/objdiff-gui/src/jobs/check_update.rs index 89bd288..671cd9a 100644 --- a/objdiff-gui/src/jobs/check_update.rs +++ b/objdiff-gui/src/jobs/check_update.rs @@ -5,13 +5,13 @@ use self_update::{cargo_crate_version, update::Release}; use crate::{ jobs::{start_job, update_status, Job, JobContext, JobResult, JobState}, - update::{build_updater, BIN_NAME}, + update::{build_updater, BIN_NAME_NEW, BIN_NAME_OLD}, }; pub struct CheckUpdateResult { pub update_available: bool, pub latest_release: Release, - pub found_binary: bool, + pub found_binary: Option, } fn run_check_update(context: &JobContext, cancel: Receiver<()>) -> Result> { @@ -20,7 +20,13 @@ fn run_check_update(context: &JobContext, cancel: Receiver<()>) -> Result) -> Result> { +fn run_update( + status: &JobContext, + cancel: Receiver<()>, + bin_name: String, +) -> Result> { update_status(status, "Fetching latest release".to_string(), 0, 3, &cancel)?; let updater = build_updater().context("Failed to create release updater")?; let latest_release = updater.get_latest_release()?; let asset = latest_release .assets .iter() - .find(|a| a.name == BIN_NAME) - .ok_or_else(|| anyhow::Error::msg(formatcp!("No release asset for {}", BIN_NAME)))?; + .find(|a| a.name == bin_name) + .ok_or_else(|| anyhow::Error::msg(format!("No release asset for {bin_name}")))?; update_status(status, "Downloading release".to_string(), 1, 3, &cancel)?; let tmp_dir = tempfile::Builder::new().prefix("update").tempdir_in(current_dir()?)?; @@ -53,8 +56,8 @@ fn run_update(status: &JobContext, cancel: Receiver<()>) -> Result JobState { +pub fn start_update(ctx: &egui::Context, bin_name: String) -> JobState { start_job(ctx, "Update app", Job::Update, move |context, cancel| { - run_update(&context, cancel).map(JobResult::Update) + run_update(&context, cancel, bin_name).map(JobResult::Update) }) } diff --git a/objdiff-gui/src/update.rs b/objdiff-gui/src/update.rs index fcd33c9..b2605f2 100644 --- a/objdiff-gui/src/update.rs +++ b/objdiff-gui/src/update.rs @@ -20,8 +20,9 @@ cfg_if! { } pub const GITHUB_USER: &str = "encounter"; pub const GITHUB_REPO: &str = "objdiff"; -pub const BIN_NAME: &str = - formatcp!("{}-{}-{}{}", GITHUB_REPO, OS, ARCH, std::env::consts::EXE_SUFFIX); +pub const BIN_NAME_NEW: &str = + formatcp!("objdiff-gui-{}-{}{}", OS, ARCH, std::env::consts::EXE_SUFFIX); +pub const BIN_NAME_OLD: &str = formatcp!("objdiff-{}-{}{}", OS, ARCH, std::env::consts::EXE_SUFFIX); pub const RELEASE_URL: &str = formatcp!("https://github.com/{}/{}/releases/latest", GITHUB_USER, GITHUB_REPO); @@ -29,7 +30,8 @@ pub fn build_updater() -> self_update::errors::Result> { self_update::backends::github::Update::configure() .repo_owner(GITHUB_USER) .repo_name(GITHUB_REPO) - .bin_name(BIN_NAME) + // bin_name is required, but unused? + .bin_name(BIN_NAME_NEW) .no_confirm(true) .show_output(false) .current_version(cargo_crate_version!()) diff --git a/objdiff-gui/src/views/config.rs b/objdiff-gui/src/views/config.rs index 1c5c298..b04e1f4 100644 --- a/objdiff-gui/src/views/config.rs +++ b/objdiff-gui/src/views/config.rs @@ -41,7 +41,7 @@ pub struct ConfigViewState { pub check_update_running: bool, pub queue_check_update: bool, pub update_running: bool, - pub queue_update: bool, + pub queue_update: Option, pub build_running: bool, pub queue_build: bool, pub watch_pattern_text: String, @@ -127,9 +127,8 @@ impl ConfigViewState { jobs.push_once(Job::CheckUpdate, || start_check_update(ctx)); } - if self.queue_update { - self.queue_update = false; - jobs.push_once(Job::Update, || start_update(ctx)); + if let Some(bin_name) = self.queue_update.take() { + jobs.push_once(Job::Update, || start_update(ctx, bin_name)); } } } @@ -201,15 +200,16 @@ pub fn config_ui( if result.update_available { ui.colored_label(appearance.insert_color, "Update available"); ui.horizontal(|ui| { - if result.found_binary - && ui + if let Some(bin_name) = &result.found_binary { + if ui .add_enabled(!state.update_running, egui::Button::new("Automatic")) .on_hover_text_at_pointer( "Automatically download and replace the current build", ) .clicked() - { - state.queue_update = true; + { + state.queue_update = Some(bin_name.clone()); + } } if ui .button("Manual") From 31e9c14681a50f717fce5aa9c5937911dbf45c1d Mon Sep 17 00:00:00 2001 From: Aetias Date: Tue, 21 May 2024 18:07:58 +0200 Subject: [PATCH 04/11] Allow `None` section when parsing line info Fixes an error upon ending the last DWARF sequence --- objdiff-core/src/obj/read.rs | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-) diff --git a/objdiff-core/src/obj/read.rs b/objdiff-core/src/obj/read.rs index 8462da8..86f0025 100644 --- a/objdiff-core/src/obj/read.rs +++ b/objdiff-core/src/obj/read.rs @@ -346,15 +346,14 @@ fn line_info(obj_file: &File<'_>, sections: &mut [ObjSection]) -> Result<()> { if row.end_sequence() { // The next row is the start of a new sequence, which means we must // advance to the next .text section. - let section_index = text_sections - .next() - .ok_or_else(|| anyhow!("Next text section not found for line info"))? - .index() - .0; - lines = sections - .iter_mut() - .find(|s| s.orig_index == section_index) - .map(|s| &mut s.line_info); + let section_index = text_sections.next().map(|s| s.index().0); + lines = section_index.map(|index| { + &mut sections + .iter_mut() + .find(|s| s.orig_index == index) + .unwrap() + .line_info + }); } } } From dc0c170db9969d8acaa7891e4789be42a0f6219f Mon Sep 17 00:00:00 2001 From: Luke Street Date: Tue, 21 May 2024 10:12:40 -0600 Subject: [PATCH 05/11] Add .obj to object select filter Resolves #54 --- objdiff-gui/src/views/config.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/objdiff-gui/src/views/config.rs b/objdiff-gui/src/views/config.rs index b04e1f4..81357f9 100644 --- a/objdiff-gui/src/views/config.rs +++ b/objdiff-gui/src/views/config.rs @@ -242,7 +242,7 @@ pub fn config_ui( Box::pin( rfd::AsyncFileDialog::new() .set_directory(&target_dir) - .add_filter("Object file", &["o", "elf"]) + .add_filter("Object file", &["o", "elf", "obj"]) .pick_file(), ) }, From 7148b51fe0c6c2a5b28f5ee0dcf6c79aa04fb130 Mon Sep 17 00:00:00 2001 From: Luke Street Date: Tue, 21 May 2024 10:16:45 -0600 Subject: [PATCH 06/11] x86: Handle IMAGE_REL_I386_REL32 LabelAddress Resolves #57 --- objdiff-core/src/arch/x86.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/objdiff-core/src/arch/x86.rs b/objdiff-core/src/arch/x86.rs index 3b7c9a6..637eb0b 100644 --- a/objdiff-core/src/arch/x86.rs +++ b/objdiff-core/src/arch/x86.rs @@ -265,7 +265,7 @@ impl FormatterOutput for InstructionFormatterOutput { FormatterTextKind::LabelAddress => { if let Some(reloc) = self.ins.reloc.as_ref() { if matches!(reloc.flags, RelocationFlags::Coff { - typ: pe::IMAGE_REL_I386_DIR32 + typ: pe::IMAGE_REL_I386_DIR32 | pe::IMAGE_REL_I386_REL32 }) { self.ins.args.push(ObjInsArg::Reloc); return; From 320efcb8cb3bcf9133108058c1be93350059ee35 Mon Sep 17 00:00:00 2001 From: Robin Avery Date: Tue, 21 May 2024 14:01:10 -0400 Subject: [PATCH 07/11] objdiff-cli report: Support data sections (#49) * objdiff-cli report: Support data sections * Minor fixes for section match % --------- Co-authored-by: Luke Street --- objdiff-cli/src/cmd/report.rs | 247 +++++++++++++++++++++------------- 1 file changed, 157 insertions(+), 90 deletions(-) diff --git a/objdiff-cli/src/cmd/report.rs b/objdiff-cli/src/cmd/report.rs index 50c7929..2b671c9 100644 --- a/objdiff-cli/src/cmd/report.rs +++ b/objdiff-cli/src/cmd/report.rs @@ -42,7 +42,7 @@ pub struct GenerateArgs { /// Output JSON file output: Option, #[argp(switch, short = 'd')] - /// Deduplicate global and weak symbols + /// Deduplicate global and weak symbols (runs single-threaded) deduplicate: bool, } @@ -64,9 +64,12 @@ pub struct ChangesArgs { #[derive(Debug, Clone, Default, serde::Serialize, serde::Deserialize)] struct Report { fuzzy_match_percent: f32, - total_size: u64, - matched_size: u64, - matched_size_percent: f32, + total_code: u64, + matched_code: u64, + matched_code_percent: f32, + total_data: u64, + matched_data: u64, + matched_data_percent: f32, total_functions: u32, matched_functions: u32, matched_functions_percent: f32, @@ -77,8 +80,10 @@ struct Report { struct ReportUnit { name: String, fuzzy_match_percent: f32, - total_size: u64, - matched_size: u64, + total_code: u64, + matched_code: u64, + total_data: u64, + matched_data: u64, total_functions: u32, matched_functions: u32, #[serde(skip_serializing_if = "Option::is_none")] @@ -87,11 +92,12 @@ struct ReportUnit { module_name: Option, #[serde(skip_serializing_if = "Option::is_none")] module_id: Option, - functions: Vec, + sections: Vec, + functions: Vec, } #[derive(Debug, Clone, Default, serde::Serialize, serde::Deserialize)] -struct ReportFunction { +struct ReportItem { name: String, #[serde(skip_serializing_if = "Option::is_none")] demangled_name: Option, @@ -160,21 +166,29 @@ fn generate(args: GenerateArgs) -> Result<()> { report.units = units.into_iter().flatten().collect(); } for unit in &report.units { - report.fuzzy_match_percent += unit.fuzzy_match_percent * unit.total_size as f32; - report.total_size += unit.total_size; - report.matched_size += unit.matched_size; + report.fuzzy_match_percent += unit.fuzzy_match_percent * unit.total_code as f32; + report.total_code += unit.total_code; + report.matched_code += unit.matched_code; + report.total_data += unit.total_data; + report.matched_data += unit.matched_data; report.total_functions += unit.total_functions; report.matched_functions += unit.matched_functions; } - if report.total_size == 0 { + if report.total_code == 0 { report.fuzzy_match_percent = 100.0; } else { - report.fuzzy_match_percent /= report.total_size as f32; + report.fuzzy_match_percent /= report.total_code as f32; } - report.matched_size_percent = if report.total_size == 0 { + + report.matched_code_percent = if report.total_code == 0 { 100.0 } else { - report.matched_size as f32 / report.total_size as f32 * 100.0 + report.matched_code as f32 / report.total_code as f32 * 100.0 + }; + report.matched_data_percent = if report.total_data == 0 { + 100.0 + } else { + report.matched_data as f32 / report.total_data as f32 * 100.0 }; report.matched_functions_percent = if report.total_functions == 0 { 100.0 @@ -216,7 +230,6 @@ fn report_object( } _ => {} } - // println!("Checking {}", object.name()); let target = object .target_path .as_ref() @@ -240,11 +253,37 @@ fn report_object( ..Default::default() }; let obj = target.as_ref().or(base.as_ref()).unwrap(); + let obj_diff = result.left.as_ref().or(result.right.as_ref()).unwrap(); for (section, section_diff) in obj.sections.iter().zip(&obj_diff.sections) { - if section.kind != ObjSectionKind::Code { - continue; + let section_match_percent = section_diff.match_percent.unwrap_or_else(|| { + // Support cases where we don't have a target object, + // assume complete means 100% match + if object.complete == Some(true) { + 100.0 + } else { + 0.0 + } + }); + unit.sections.push(ReportItem { + name: section.name.clone(), + demangled_name: None, + fuzzy_match_percent: section_match_percent, + size: section.size, + address: section.virtual_address, + }); + + match section.kind { + ObjSectionKind::Data | ObjSectionKind::Bss => { + unit.total_data += section.size; + if section_match_percent == 100.0 { + unit.matched_data += section.size; + } + continue; + } + ObjSectionKind::Code => (), } + for (symbol, symbol_diff) in section.symbols.iter().zip(§ion_diff.symbols) { if symbol.size == 0 { continue; @@ -267,11 +306,11 @@ fn report_object( } }); unit.fuzzy_match_percent += match_percent * symbol.size as f32; - unit.total_size += symbol.size; + unit.total_code += symbol.size; if match_percent == 100.0 { - unit.matched_size += symbol.size; + unit.matched_code += symbol.size; } - unit.functions.push(ReportFunction { + unit.functions.push(ReportItem { name: symbol.name.clone(), demangled_name: symbol.demangled_name.clone(), size: symbol.size, @@ -284,10 +323,10 @@ fn report_object( unit.total_functions += 1; } } - if unit.total_size == 0 { + if unit.total_code == 0 { unit.fuzzy_match_percent = 100.0; } else { - unit.fuzzy_match_percent /= unit.total_size as f32; + unit.fuzzy_match_percent /= unit.total_code as f32; } Ok(Some(unit)) } @@ -302,9 +341,12 @@ struct Changes { #[derive(Debug, Clone, Default, PartialEq, serde::Serialize, serde::Deserialize)] struct ChangeInfo { fuzzy_match_percent: f32, - total_size: u64, - matched_size: u64, - matched_size_percent: f32, + total_code: u64, + matched_code: u64, + matched_code_percent: f32, + total_data: u64, + matched_data: u64, + matched_data_percent: f32, total_functions: u32, matched_functions: u32, matched_functions_percent: f32, @@ -314,9 +356,12 @@ impl From<&Report> for ChangeInfo { fn from(report: &Report) -> Self { Self { fuzzy_match_percent: report.fuzzy_match_percent, - total_size: report.total_size, - matched_size: report.matched_size, - matched_size_percent: report.matched_size_percent, + total_code: report.total_code, + matched_code: report.matched_code, + matched_code_percent: report.matched_code_percent, + total_data: report.total_data, + matched_data: report.matched_data, + matched_data_percent: report.matched_data_percent, total_functions: report.total_functions, matched_functions: report.matched_functions, matched_functions_percent: report.matched_functions_percent, @@ -328,12 +373,19 @@ impl From<&ReportUnit> for ChangeInfo { fn from(value: &ReportUnit) -> Self { Self { fuzzy_match_percent: value.fuzzy_match_percent, - total_size: value.total_size, - matched_size: value.matched_size, - matched_size_percent: if value.total_size == 0 { + total_code: value.total_code, + matched_code: value.matched_code, + matched_code_percent: if value.total_code == 0 { 100.0 } else { - value.matched_size as f32 / value.total_size as f32 * 100.0 + value.matched_code as f32 / value.total_code as f32 * 100.0 + }, + total_data: value.total_data, + matched_data: value.matched_data, + matched_data_percent: if value.total_data == 0 { + 100.0 + } else { + value.matched_data as f32 / value.total_data as f32 * 100.0 }, total_functions: value.total_functions, matched_functions: value.matched_functions, @@ -351,24 +403,25 @@ struct ChangeUnit { name: String, from: Option, to: Option, - functions: Vec, + sections: Vec, + functions: Vec, } #[derive(Debug, Clone, Default, serde::Serialize, serde::Deserialize)] -struct ChangeFunction { +struct ChangeItem { name: String, - from: Option, - to: Option, + from: Option, + to: Option, } #[derive(Debug, Clone, Default, PartialEq, serde::Serialize, serde::Deserialize)] -struct ChangeFunctionInfo { +struct ChangeItemInfo { fuzzy_match_percent: f32, size: u64, } -impl From<&ReportFunction> for ChangeFunctionInfo { - fn from(value: &ReportFunction) -> Self { +impl From<&ReportItem> for ChangeItemInfo { + fn from(value: &ReportItem) -> Self { Self { fuzzy_match_percent: value.fuzzy_match_percent, size: value.size } } } @@ -382,54 +435,18 @@ fn changes(args: ChangesArgs) -> Result<()> { units: vec![], }; for prev_unit in &previous.units { - let prev_unit_info = ChangeInfo::from(prev_unit); let curr_unit = current.units.iter().find(|u| u.name == prev_unit.name); + let sections = process_items(prev_unit, curr_unit, |u| &u.sections); + let functions = process_items(prev_unit, curr_unit, |u| &u.functions); + + let prev_unit_info = ChangeInfo::from(prev_unit); let curr_unit_info = curr_unit.map(ChangeInfo::from); - let mut functions = vec![]; - if let Some(curr_unit) = curr_unit { - for prev_func in &prev_unit.functions { - let prev_func_info = ChangeFunctionInfo::from(prev_func); - let curr_func = curr_unit.functions.iter().find(|f| f.name == prev_func.name); - let curr_func_info = curr_func.map(ChangeFunctionInfo::from); - if let Some(curr_func_info) = curr_func_info { - if prev_func_info != curr_func_info { - functions.push(ChangeFunction { - name: prev_func.name.clone(), - from: Some(prev_func_info), - to: Some(curr_func_info), - }); - } - } else { - functions.push(ChangeFunction { - name: prev_func.name.clone(), - from: Some(prev_func_info), - to: None, - }); - } - } - for curr_func in &curr_unit.functions { - if !prev_unit.functions.iter().any(|f| f.name == curr_func.name) { - functions.push(ChangeFunction { - name: curr_func.name.clone(), - from: None, - to: Some(ChangeFunctionInfo::from(curr_func)), - }); - } - } - } else { - for prev_func in &prev_unit.functions { - functions.push(ChangeFunction { - name: prev_func.name.clone(), - from: Some(ChangeFunctionInfo::from(prev_func)), - to: None, - }); - } - } if !functions.is_empty() || !matches!(&curr_unit_info, Some(v) if v == &prev_unit_info) { changes.units.push(ChangeUnit { name: prev_unit.name.clone(), from: Some(prev_unit_info), to: curr_unit_info, + sections, functions, }); } @@ -440,15 +457,8 @@ fn changes(args: ChangesArgs) -> Result<()> { name: curr_unit.name.clone(), from: None, to: Some(ChangeInfo::from(curr_unit)), - functions: curr_unit - .functions - .iter() - .map(|f| ChangeFunction { - name: f.name.clone(), - from: None, - to: Some(ChangeFunctionInfo::from(f)), - }) - .collect(), + sections: process_new_items(&curr_unit.sections), + functions: process_new_items(&curr_unit.functions), }); } } @@ -466,6 +476,63 @@ fn changes(args: ChangesArgs) -> Result<()> { Ok(()) } +fn process_items &Vec>( + prev_unit: &ReportUnit, + curr_unit: Option<&ReportUnit>, + getter: F, +) -> Vec { + let prev_items = getter(prev_unit); + let mut items = vec![]; + if let Some(curr_unit) = curr_unit { + let curr_items = getter(curr_unit); + for prev_func in prev_items { + let prev_func_info = ChangeItemInfo::from(prev_func); + let curr_func = curr_items.iter().find(|f| f.name == prev_func.name); + let curr_func_info = curr_func.map(ChangeItemInfo::from); + if let Some(curr_func_info) = curr_func_info { + if prev_func_info != curr_func_info { + items.push(ChangeItem { + name: prev_func.name.clone(), + from: Some(prev_func_info), + to: Some(curr_func_info), + }); + } + } else { + items.push(ChangeItem { + name: prev_func.name.clone(), + from: Some(prev_func_info), + to: None, + }); + } + } + for curr_func in curr_items { + if !prev_items.iter().any(|f| f.name == curr_func.name) { + items.push(ChangeItem { + name: curr_func.name.clone(), + from: None, + to: Some(ChangeItemInfo::from(curr_func)), + }); + } + } + } else { + for prev_func in prev_items { + items.push(ChangeItem { + name: prev_func.name.clone(), + from: Some(ChangeItemInfo::from(prev_func)), + to: None, + }); + } + } + items +} + +fn process_new_items(items: &[ReportItem]) -> Vec { + items + .iter() + .map(|f| ChangeItem { name: f.name.clone(), from: None, to: Some(ChangeItemInfo::from(f)) }) + .collect() +} + fn read_report(path: &Path) -> Result { serde_json::from_reader(BufReader::new( File::open(path).with_context(|| format!("Failed to open {}", path.display()))?, From e254af5acf07dc4935647f3248a3923a913e3760 Mon Sep 17 00:00:00 2001 From: Luke Street Date: Tue, 21 May 2024 12:02:00 -0600 Subject: [PATCH 08/11] Support bss and text section diffing Display section diff % in symbols view --- objdiff-core/src/diff/data.rs | 53 ++++++++++++++++++++++++++-- objdiff-core/src/diff/mod.rs | 22 +++++++++--- objdiff-gui/src/views/symbol_diff.rs | 27 +++++++++++++- 3 files changed, 94 insertions(+), 8 deletions(-) diff --git a/objdiff-core/src/diff/data.rs b/objdiff-core/src/diff/data.rs index e8882d7..ceffa83 100644 --- a/objdiff-core/src/diff/data.rs +++ b/objdiff-core/src/diff/data.rs @@ -11,6 +11,27 @@ use crate::{ obj::{ObjInfo, ObjSection, SymbolRef}, }; +/// Compare the addresses and sizes of each symbol in the BSS sections. +pub fn diff_bss_section( + left: &ObjSection, + right: &ObjSection, +) -> 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 match_percent = get_diff_ratio(&ops, left_sizes.len(), right_sizes.len()) * 100.0; + Ok(( + ObjSectionDiff { symbols: vec![], data_diff: vec![], match_percent: Some(match_percent) }, + ObjSectionDiff { symbols: vec![], data_diff: vec![], match_percent: Some(match_percent) }, + )) +} + pub fn diff_bss_symbol( left_obj: &ObjInfo, right_obj: &ObjInfo, @@ -40,14 +61,19 @@ pub fn no_diff_symbol(_obj: &ObjInfo, symbol_ref: SymbolRef) -> ObjSymbolDiff { ObjSymbolDiff { symbol_ref, diff_symbol: None, instructions: vec![], match_percent: None } } -pub fn diff_data( +/// Compare the data sections of two object files. +pub fn diff_data_section( left: &ObjSection, right: &ObjSection, ) -> Result<(ObjSectionDiff, ObjSectionDiff)> { let deadline = Instant::now() + Duration::from_secs(5); + let left_max = left.symbols.iter().map(|s| s.section_address + s.size).max().unwrap_or(0); + let right_max = right.symbols.iter().map(|s| s.section_address + s.size).max().unwrap_or(0); + let left_data = &left.data[..left_max as usize]; + 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; + 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(); @@ -168,3 +194,24 @@ pub fn diff_data_symbol( }, )) } + +/// Compare the text sections of two object files. +/// This essentially adds up the match percentage of each symbol in the text section. +pub fn diff_text_section( + left: &ObjSection, + _right: &ObjSection, + left_diff: &ObjSectionDiff, + _right_diff: &ObjSectionDiff, +) -> Result<(ObjSectionDiff, ObjSectionDiff)> { + let match_percent = left + .symbols + .iter() + .zip(left_diff.symbols.iter()) + .map(|(s, d)| d.match_percent.unwrap_or(0.0) * s.size as f32) + .sum::() + / left.size as f32; + 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 de8d3ea..a194d5f 100644 --- a/objdiff-core/src/diff/mod.rs +++ b/objdiff-core/src/diff/mod.rs @@ -5,7 +5,10 @@ use anyhow::Result; use crate::{ diff::{ code::{diff_code, no_diff_code}, - data::{diff_bss_symbol, diff_data, diff_data_symbol, no_diff_symbol}, + data::{ + diff_bss_section, diff_bss_symbol, diff_data_section, diff_data_symbol, + diff_text_section, no_diff_symbol, + }, }, obj::{ObjInfo, ObjIns, ObjSection, ObjSectionKind, ObjSymbol, SymbolRef}, }; @@ -324,15 +327,26 @@ pub fn diff_objs( let right_section = &right_obj.sections[right_section_idx]; match section_kind { ObjSectionKind::Code => { - // TODO? + 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_text_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); } ObjSectionKind::Data => { - let (left_diff, right_diff) = diff_data(left_section, right_section)?; + let (left_diff, right_diff) = diff_data_section(left_section, right_section)?; left_out.section_diff_mut(left_section_idx).merge(left_diff); right_out.section_diff_mut(right_section_idx).merge(right_diff); } ObjSectionKind::Bss => { - // TODO + let (left_diff, right_diff) = diff_bss_section(left_section, right_section)?; + left_out.section_diff_mut(left_section_idx).merge(left_diff); + right_out.section_diff_mut(right_section_idx).merge(right_diff); } } } diff --git a/objdiff-gui/src/views/symbol_diff.rs b/objdiff-gui/src/views/symbol_diff.rs index 8dfb41e..7dadf98 100644 --- a/objdiff-gui/src/views/symbol_diff.rs +++ b/objdiff-gui/src/views/symbol_diff.rs @@ -303,7 +303,32 @@ fn symbol_list_ui( } for (section, section_diff) in obj.0.sections.iter().zip(&obj.1.sections) { - CollapsingHeader::new(format!("{} ({:x})", section.name, section.size)) + let mut header = LayoutJob::simple_singleline( + format!("{} ({:x})", section.name, section.size), + appearance.code_font.clone(), + Color32::PLACEHOLDER, + ); + if let Some(match_percent) = section_diff.match_percent { + write_text( + " (", + Color32::PLACEHOLDER, + &mut header, + appearance.code_font.clone(), + ); + write_text( + &format!("{match_percent:.0}%"), + match_color_for_symbol(match_percent, appearance), + &mut header, + appearance.code_font.clone(), + ); + write_text( + ")", + Color32::PLACEHOLDER, + &mut header, + appearance.code_font.clone(), + ); + } + CollapsingHeader::new(header) .id_source(Id::new(section.name.clone()).with(section.orig_index)) .default_open(true) .show(ui, |ui| { From 9e57a66a05d9b3410db09f539fa3e3fd797df397 Mon Sep 17 00:00:00 2001 From: Luke Street Date: Tue, 21 May 2024 18:06:14 -0600 Subject: [PATCH 09/11] Auto-detect MIPS ABI/category & add config Under Diff Options -> Arch Settings, one can override the ABI/instruction category --- Cargo.lock | 8 ++-- objdiff-cli/Cargo.toml | 2 +- objdiff-cli/src/cmd/diff.rs | 6 ++- objdiff-core/Cargo.toml | 3 +- objdiff-core/src/arch/mips.rs | 59 ++++++++++++++++++++++---- objdiff-core/src/diff/mod.rs | 73 ++++++++++++++++++++++++++++++++- objdiff-gui/Cargo.toml | 3 +- objdiff-gui/src/app.rs | 12 +++--- objdiff-gui/src/views/config.rs | 56 ++++++++++++++++++++----- 9 files changed, 188 insertions(+), 34 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 94ddee6..2fd304a 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3081,7 +3081,7 @@ dependencies = [ [[package]] name = "objdiff-cli" -version = "2.0.0-alpha.1" +version = "2.0.0-alpha.2" dependencies = [ "anyhow", "argp", @@ -3100,7 +3100,7 @@ dependencies = [ [[package]] name = "objdiff-core" -version = "2.0.0-alpha.1" +version = "2.0.0-alpha.2" dependencies = [ "anyhow", "byteorder", @@ -3123,11 +3123,12 @@ dependencies = [ "serde_json", "serde_yaml", "similar", + "strum", ] [[package]] name = "objdiff-gui" -version = "2.0.0-alpha.1" +version = "2.0.0-alpha.2" dependencies = [ "anyhow", "bytes", @@ -3157,6 +3158,7 @@ dependencies = [ "serde", "serde_json", "shell-escape", + "strum", "tempfile", "time", "tracing-subscriber", diff --git a/objdiff-cli/Cargo.toml b/objdiff-cli/Cargo.toml index 396643d..b3f4a05 100644 --- a/objdiff-cli/Cargo.toml +++ b/objdiff-cli/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "objdiff-cli" -version = "2.0.0-alpha.1" +version = "2.0.0-alpha.2" edition = "2021" rust-version = "1.70" authors = ["Luke Street "] diff --git a/objdiff-cli/src/cmd/diff.rs b/objdiff-cli/src/cmd/diff.rs index f0e0a3d..d0992a1 100644 --- a/objdiff-cli/src/cmd/diff.rs +++ b/objdiff-cli/src/cmd/diff.rs @@ -821,8 +821,10 @@ impl FunctionDiffUi { .transpose()?; let config = diff::DiffObjConfig { relax_reloc_diffs: self.relax_reloc_diffs, - space_between_args: true, // TODO - x86_formatter: Default::default(), // TODO + space_between_args: true, // TODO + x86_formatter: Default::default(), // TODO + mips_abi: Default::default(), // TODO + mips_instr_category: Default::default(), // TODO }; let result = diff::diff_objs(&config, target.as_ref(), base.as_ref(), prev.as_ref())?; diff --git a/objdiff-core/Cargo.toml b/objdiff-core/Cargo.toml index fd5d820..7bad507 100644 --- a/objdiff-core/Cargo.toml +++ b/objdiff-core/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "objdiff-core" -version = "2.0.0-alpha.1" +version = "2.0.0-alpha.2" edition = "2021" rust-version = "1.70" authors = ["Luke Street "] @@ -31,6 +31,7 @@ num-traits = "0.2.18" object = { version = "0.35.0", features = ["read_core", "std", "elf", "pe"], default-features = false } serde = { version = "1", features = ["derive"] } similar = { version = "2.5.0", default-features = false } +strum = { version = "0.26.2", features = ["derive"] } # config globset = { version = "0.4.14", features = ["serde1"], optional = true } diff --git a/objdiff-core/src/arch/mips.rs b/objdiff-core/src/arch/mips.rs index ea556dc..406f0ad 100644 --- a/objdiff-core/src/arch/mips.rs +++ b/objdiff-core/src/arch/mips.rs @@ -1,29 +1,56 @@ -use std::borrow::Cow; +use std::{borrow::Cow, sync::Mutex}; use anyhow::{anyhow, bail, Result}; -use object::{elf, Endian, Endianness, File, Object, Relocation, RelocationFlags}; +use object::{elf, Endian, Endianness, File, FileFlags, Object, Relocation, RelocationFlags}; use rabbitizer::{config, Abi, InstrCategory, Instruction, OperandType}; use crate::{ arch::{ObjArch, ProcessCodeResult}, - diff::DiffObjConfig, + diff::{DiffObjConfig, MipsAbi, MipsInstrCategory}, obj::{ObjInfo, ObjIns, ObjInsArg, ObjInsArgValue, ObjReloc, ObjSection, SymbolRef}, }; -fn configure_rabbitizer() { +static RABBITIZER_MUTEX: Mutex<()> = Mutex::new(()); + +fn configure_rabbitizer(abi: Abi) { unsafe { - config::RabbitizerConfig_Cfg.reg_names.fpr_abi_names = Abi::O32; + config::RabbitizerConfig_Cfg.reg_names.fpr_abi_names = abi; } } pub struct ObjArchMips { pub endianness: Endianness, + pub abi: Abi, + pub instr_category: InstrCategory, } +const EF_MIPS_ABI: u32 = 0x0000F000; +const EF_MIPS_MACH: u32 = 0x00FF0000; + +const E_MIPS_MACH_ALLEGREX: u32 = 0x00840000; +const E_MIPS_MACH_5900: u32 = 0x00920000; + impl ObjArchMips { pub fn new(object: &File) -> Result { - configure_rabbitizer(); - Ok(Self { endianness: object.endianness() }) + let mut abi = Abi::NUMERIC; + let mut instr_category = InstrCategory::CPU; + match object.flags() { + FileFlags::None => {} + FileFlags::Elf { e_flags, .. } => { + abi = match e_flags & EF_MIPS_ABI { + elf::EF_MIPS_ABI_O32 => Abi::O32, + elf::EF_MIPS_ABI_EABI32 | elf::EF_MIPS_ABI_EABI64 => Abi::N32, + _ => Abi::NUMERIC, + }; + instr_category = match e_flags & EF_MIPS_MACH { + E_MIPS_MACH_ALLEGREX => InstrCategory::R4000ALLEGREX, + E_MIPS_MACH_5900 => InstrCategory::R5900, + _ => InstrCategory::CPU, + }; + } + _ => bail!("Unsupported MIPS file flags"), + } + Ok(Self { endianness: object.endianness(), abi, instr_category }) } } @@ -39,6 +66,22 @@ impl ObjArch for ObjArchMips { let code = §ion.data [symbol.section_address as usize..(symbol.section_address + symbol.size) as usize]; + let _guard = RABBITIZER_MUTEX.lock().map_err(|e| anyhow!("Failed to lock mutex: {e}"))?; + configure_rabbitizer(match config.mips_abi { + MipsAbi::Auto => self.abi, + MipsAbi::O32 => Abi::O32, + MipsAbi::N32 => Abi::N32, + MipsAbi::N64 => Abi::N64, + }); + let instr_category = match config.mips_instr_category { + MipsInstrCategory::Auto => self.instr_category, + MipsInstrCategory::Cpu => InstrCategory::CPU, + MipsInstrCategory::Rsp => InstrCategory::RSP, + MipsInstrCategory::R3000Gte => InstrCategory::R3000GTE, + MipsInstrCategory::R4000Allegrex => InstrCategory::R4000ALLEGREX, + MipsInstrCategory::R5900 => InstrCategory::R5900, + }; + let start_address = symbol.address; let end_address = symbol.address + symbol.size; let ins_count = code.len() / 4; @@ -48,7 +91,7 @@ impl ObjArch for ObjArchMips { for chunk in code.chunks_exact(4) { let reloc = section.relocations.iter().find(|r| (r.address as u32 & !3) == cur_addr); let code = self.endianness.read_u32_bytes(chunk.try_into()?); - let instruction = Instruction::new(code, cur_addr, InstrCategory::CPU); + let instruction = Instruction::new(code, cur_addr, instr_category); let formatted = instruction.disassemble(None, 0); let op = instruction.unique_id as u16; diff --git a/objdiff-core/src/diff/mod.rs b/objdiff-core/src/diff/mod.rs index a194d5f..3388efc 100644 --- a/objdiff-core/src/diff/mod.rs +++ b/objdiff-core/src/diff/mod.rs @@ -17,15 +17,82 @@ mod code; mod data; pub mod display; -#[derive(Debug, Copy, Clone, Default, Eq, PartialEq, serde::Deserialize, serde::Serialize)] +#[derive( + Debug, + Copy, + Clone, + Default, + Eq, + PartialEq, + serde::Deserialize, + serde::Serialize, + strum::VariantArray, + strum::EnumMessage, +)] pub enum X86Formatter { #[default] + #[strum(message = "Intel (default)")] Intel, + #[strum(message = "AT&T")] Gas, + #[strum(message = "NASM")] Nasm, + #[strum(message = "MASM")] Masm, } +#[derive( + Debug, + Copy, + Clone, + Default, + Eq, + PartialEq, + serde::Deserialize, + serde::Serialize, + strum::VariantArray, + strum::EnumMessage, +)] +pub enum MipsAbi { + #[default] + #[strum(message = "Auto (default)")] + Auto, + #[strum(message = "O32")] + O32, + #[strum(message = "N32")] + N32, + #[strum(message = "N64")] + N64, +} + +#[derive( + Debug, + Copy, + Clone, + Default, + Eq, + PartialEq, + serde::Deserialize, + serde::Serialize, + strum::VariantArray, + strum::EnumMessage, +)] +pub enum MipsInstrCategory { + #[default] + #[strum(message = "Auto (default)")] + Auto, + #[strum(message = "CPU")] + Cpu, + #[strum(message = "RSP (N64)")] + Rsp, + #[strum(message = "R3000 GTE (PS1)")] + R3000Gte, + #[strum(message = "R4000 ALLEGREX (PSP)")] + R4000Allegrex, + #[strum(message = "R5900 EE (PS2)")] + R5900, +} + #[inline] const fn default_true() -> bool { true } @@ -35,7 +102,11 @@ pub struct DiffObjConfig { pub relax_reloc_diffs: bool, #[serde(default = "default_true")] pub space_between_args: bool, + // x86 pub x86_formatter: X86Formatter, + // MIPS + pub mips_abi: MipsAbi, + pub mips_instr_category: MipsInstrCategory, } impl DiffObjConfig { diff --git a/objdiff-gui/Cargo.toml b/objdiff-gui/Cargo.toml index 63b21a8..9c9eaa8 100644 --- a/objdiff-gui/Cargo.toml +++ b/objdiff-gui/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "objdiff-gui" -version = "2.0.0-alpha.1" +version = "2.0.0-alpha.2" edition = "2021" rust-version = "1.70" authors = ["Luke Street "] @@ -46,6 +46,7 @@ ron = "0.8.1" serde = { version = "1", features = ["derive"] } serde_json = "1.0.116" shell-escape = "0.1.5" +strum = { version = "0.26.2", features = ["derive"] } tempfile = "3.10.1" time = { version = "0.3.36", features = ["formatting", "local-offset"] } diff --git a/objdiff-gui/src/app.rs b/objdiff-gui/src/app.rs index bcd125c..5f6a6f9 100644 --- a/objdiff-gui/src/app.rs +++ b/objdiff-gui/src/app.rs @@ -30,7 +30,7 @@ use crate::{ views::{ appearance::{appearance_window, Appearance}, config::{ - config_ui, diff_config_window, project_window, ConfigViewState, CONFIG_DISABLED_TEXT, + arch_config_window, config_ui, project_window, ConfigViewState, CONFIG_DISABLED_TEXT, }, data_diff::data_diff_ui, debug::debug_window, @@ -52,7 +52,7 @@ pub struct ViewState { pub show_appearance_config: bool, pub show_demangle: bool, pub show_project_config: bool, - pub show_diff_config: bool, + pub show_arch_config: bool, pub show_debug: bool, } @@ -414,7 +414,7 @@ impl eframe::App for App { show_appearance_config, show_demangle, show_project_config, - show_diff_config, + show_arch_config, show_debug, } = view_state; @@ -468,8 +468,8 @@ impl eframe::App for App { } }); ui.menu_button("Diff Options", |ui| { - if ui.button("More…").clicked() { - *show_diff_config = !*show_diff_config; + if ui.button("Arch Settings…").clicked() { + *show_arch_config = !*show_arch_config; ui.close_menu(); } let mut config = config.write().unwrap(); @@ -541,7 +541,7 @@ impl eframe::App for App { project_window(ctx, config, show_project_config, config_state, appearance); appearance_window(ctx, show_appearance_config, appearance); demangle_window(ctx, show_demangle, demangle_state, appearance); - diff_config_window(ctx, config, show_diff_config, appearance); + arch_config_window(ctx, config, show_arch_config, appearance); debug_window(ctx, show_debug, frame_history, appearance); self.post_update(ctx); diff --git a/objdiff-gui/src/views/config.rs b/objdiff-gui/src/views/config.rs index 81357f9..99edf75 100644 --- a/objdiff-gui/src/views/config.rs +++ b/objdiff-gui/src/views/config.rs @@ -16,9 +16,10 @@ use egui::{ use globset::Glob; use objdiff_core::{ config::{ProjectObject, DEFAULT_WATCH_PATTERNS}, - diff::X86Formatter, + diff::{MipsAbi, MipsInstrCategory, X86Formatter}, }; use self_update::cargo_crate_version; +use strum::{EnumMessage, VariantArray}; use crate::{ app::{AppConfig, AppConfigRef, ObjectConfig}, @@ -842,29 +843,28 @@ fn split_obj_config_ui( }); } -pub fn diff_config_window( +pub fn arch_config_window( ctx: &egui::Context, config: &AppConfigRef, show: &mut bool, appearance: &Appearance, ) { let mut config_guard = config.write().unwrap(); - egui::Window::new("Diff Config").open(show).show(ctx, |ui| { - diff_config_ui(ui, &mut config_guard, appearance); + egui::Window::new("Arch Settings").open(show).show(ctx, |ui| { + arch_config_ui(ui, &mut config_guard, appearance); }); } -fn diff_config_ui(ui: &mut egui::Ui, config: &mut AppConfig, _appearance: &Appearance) { - egui::ComboBox::new("x86_formatter", "X86 Format") - .selected_text(format!("{:?}", config.diff_obj_config.x86_formatter)) +fn arch_config_ui(ui: &mut egui::Ui, config: &mut AppConfig, _appearance: &Appearance) { + ui.heading("x86"); + egui::ComboBox::new("x86_formatter", "Format") + .selected_text(config.diff_obj_config.x86_formatter.get_message().unwrap()) .show_ui(ui, |ui| { - for &formatter in - &[X86Formatter::Intel, X86Formatter::Gas, X86Formatter::Nasm, X86Formatter::Masm] - { + for &formatter in X86Formatter::VARIANTS { if ui .selectable_label( config.diff_obj_config.x86_formatter == formatter, - format!("{:?}", formatter), + formatter.get_message().unwrap(), ) .clicked() { @@ -873,4 +873,38 @@ fn diff_config_ui(ui: &mut egui::Ui, config: &mut AppConfig, _appearance: &Appea } } }); + ui.separator(); + ui.heading("MIPS"); + egui::ComboBox::new("mips_abi", "ABI") + .selected_text(config.diff_obj_config.mips_abi.get_message().unwrap()) + .show_ui(ui, |ui| { + for &abi in MipsAbi::VARIANTS { + if ui + .selectable_label( + config.diff_obj_config.mips_abi == abi, + abi.get_message().unwrap(), + ) + .clicked() + { + config.diff_obj_config.mips_abi = abi; + config.queue_reload = true; + } + } + }); + egui::ComboBox::new("mips_instr_category", "Instruction Category") + .selected_text(config.diff_obj_config.mips_instr_category.get_message().unwrap()) + .show_ui(ui, |ui| { + for &category in MipsInstrCategory::VARIANTS { + if ui + .selectable_label( + config.diff_obj_config.mips_instr_category == category, + category.get_message().unwrap(), + ) + .clicked() + { + config.diff_obj_config.mips_instr_category = category; + config.queue_reload = true; + } + } + }); } From f30b3cfae2a436b7e265d1a3c5a0d08a1dd4f901 Mon Sep 17 00:00:00 2001 From: Luke Street Date: Tue, 21 May 2024 18:09:46 -0600 Subject: [PATCH 10/11] Default "Space between args" -> true --- objdiff-core/src/diff/mod.rs | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/objdiff-core/src/diff/mod.rs b/objdiff-core/src/diff/mod.rs index 3388efc..fab8350 100644 --- a/objdiff-core/src/diff/mod.rs +++ b/objdiff-core/src/diff/mod.rs @@ -96,7 +96,7 @@ pub enum MipsInstrCategory { #[inline] const fn default_true() -> bool { true } -#[derive(Debug, Clone, Default, Eq, PartialEq, serde::Deserialize, serde::Serialize)] +#[derive(Debug, Clone, Eq, PartialEq, serde::Deserialize, serde::Serialize)] #[serde(default)] pub struct DiffObjConfig { pub relax_reloc_diffs: bool, @@ -109,6 +109,18 @@ pub struct DiffObjConfig { pub mips_instr_category: MipsInstrCategory, } +impl Default for DiffObjConfig { + fn default() -> Self { + Self { + relax_reloc_diffs: false, + space_between_args: true, + x86_formatter: Default::default(), + mips_abi: Default::default(), + mips_instr_category: Default::default(), + } + } +} + impl DiffObjConfig { pub fn separator(&self) -> &'static str { if self.space_between_args { From 0c20a0d9cd48abe2179bdbe99fd56615f8f75f78 Mon Sep 17 00:00:00 2001 From: Luke Street Date: Tue, 21 May 2024 18:12:58 -0600 Subject: [PATCH 11/11] Update README.md --- README.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index 4b3636a..a946780 100644 --- a/README.md +++ b/README.md @@ -14,8 +14,8 @@ Features: - Click to highlight all instances of values and registers. Supports: -- PowerPC 750CL (GameCube & Wii) -- MIPS (Nintendo 64 & PS2) +- PowerPC 750CL (GameCube, Wii) +- MIPS (N64, PS1, PS2, PSP) - x86 (COFF only at the moment) See [Usage](#usage) for more information.