diff --git a/objdiff-core/src/bindings/diff.rs b/objdiff-core/src/bindings/diff.rs index 3a90b11..e856b92 100644 --- a/objdiff-core/src/bindings/diff.rs +++ b/objdiff-core/src/bindings/diff.rs @@ -42,6 +42,7 @@ impl SectionDiff { let section = &obj.sections[section_index]; let symbols = section_diff.symbols.iter().map(|d| SymbolDiff::new(obj, d)).collect(); let data = section_diff.data_diff.iter().map(|d| DataDiff::new(obj, d)).collect(); + // TODO: section_diff.reloc_diff Self { name: section.name.to_string(), kind: SectionKind::from(section.kind) as i32, diff --git a/objdiff-core/src/diff/data.rs b/objdiff-core/src/diff/data.rs index 35d6e76..e293319 100644 --- a/objdiff-core/src/diff/data.rs +++ b/objdiff-core/src/diff/data.rs @@ -8,7 +8,7 @@ use similar::{capture_diff_slices_deadline, get_diff_ratio, Algorithm}; use super::code::section_name_eq; use crate::{ - diff::{ObjDataDiff, ObjDataDiffKind, ObjSectionDiff, ObjSymbolDiff}, + diff::{ObjDataDiff, ObjDataDiffKind, ObjDataRelocDiff, ObjSectionDiff, ObjSymbolDiff}, obj::{ObjInfo, ObjReloc, ObjSection, ObjSymbolFlags, SymbolRef}, }; @@ -103,7 +103,7 @@ fn diff_data_relocs_for_range( let right_offset = r.address as usize - right_range.start; right_offset == left_offset }) else { - diffs.push((ObjDataDiffKind::Replace, Some(left_reloc.clone()), None)); + diffs.push((ObjDataDiffKind::Delete, Some(left_reloc.clone()), None)); continue; }; if reloc_eq(left_obj, right_obj, left_reloc, right_reloc) { @@ -132,7 +132,7 @@ fn diff_data_relocs_for_range( let left_offset = r.address as usize - left_range.start; left_offset == right_offset }) else { - diffs.push((ObjDataDiffKind::Replace, None, Some(right_reloc.clone()))); + diffs.push((ObjDataDiffKind::Insert, None, Some(right_reloc.clone()))); continue; }; // No need to check the cases for relocations being deleted or matching again. @@ -176,94 +176,6 @@ pub fn diff_data_section( ObjDataDiffKind::Replace } }; - if kind == ObjDataDiffKind::None { - let mut found_any_relocs = false; - let mut left_curr_addr = left_range.start; - let mut right_curr_addr = right_range.start; - for (diff_kind, left_reloc, right_reloc) in diff_data_relocs_for_range( - left_obj, - right_obj, - left, - right, - left_range.clone(), - right_range.clone(), - ) { - found_any_relocs = true; - - if let Some(left_reloc) = left_reloc { - let left_reloc_addr = left_reloc.address as usize; - if left_reloc_addr > left_curr_addr { - let len = left_reloc_addr - left_curr_addr; - let left_data = &left.data[left_curr_addr..left_reloc_addr]; - left_diff.push(ObjDataDiff { - data: left_data[..min(len, left_data.len())].to_vec(), - kind: ObjDataDiffKind::None, - len, - ..Default::default() - }); - } - let reloc_diff_len = left_obj.arch.get_reloc_byte_size(left_reloc.flags); - let left_data = &left.data[left_reloc_addr..left_reloc_addr + reloc_diff_len]; - left_diff.push(ObjDataDiff { - data: left_data[..min(reloc_diff_len, left_data.len())].to_vec(), - kind: diff_kind, - len: reloc_diff_len, - reloc: Some(left_reloc.clone()), - ..Default::default() - }); - left_curr_addr = left_reloc_addr + reloc_diff_len; - } - - if let Some(right_reloc) = right_reloc { - let right_reloc_addr = right_reloc.address as usize; - if right_reloc_addr > right_curr_addr { - let len = right_reloc_addr - right_curr_addr; - let right_data = &right.data[right_curr_addr..right_reloc_addr]; - right_diff.push(ObjDataDiff { - data: right_data[..min(len, right_data.len())].to_vec(), - kind: ObjDataDiffKind::None, - len, - ..Default::default() - }); - } - let reloc_diff_len = right_obj.arch.get_reloc_byte_size(right_reloc.flags); - let right_data = - &right.data[right_reloc_addr..right_reloc_addr + reloc_diff_len]; - right_diff.push(ObjDataDiff { - data: right_data[..min(reloc_diff_len, right_data.len())].to_vec(), - kind: diff_kind, - len: reloc_diff_len, - reloc: Some(right_reloc.clone()), - ..Default::default() - }); - right_curr_addr = right_reloc_addr + reloc_diff_len; - } - } - - if found_any_relocs { - if left_curr_addr < left_range.end - 1 { - let len = left_range.end - left_curr_addr; - let left_data = &left.data[left_curr_addr..left_range.end]; - left_diff.push(ObjDataDiff { - data: left_data[..min(len, left_data.len())].to_vec(), - kind: ObjDataDiffKind::None, - len, - ..Default::default() - }); - } - if right_curr_addr < right_range.end - 1 { - let len = right_range.end - right_curr_addr; - let right_data = &right.data[right_curr_addr..right_range.end]; - right_diff.push(ObjDataDiff { - data: right_data[..min(len, right_data.len())].to_vec(), - kind: ObjDataDiffKind::None, - len, - ..Default::default() - }); - } - continue; - } - } let left_data = &left.data[left_range]; let right_data = &right.data[right_range]; left_diff.push(ObjDataDiff { @@ -315,12 +227,35 @@ pub fn diff_data_section( } } + let mut left_reloc_diffs = Vec::new(); + let mut right_reloc_diffs = Vec::new(); + for (diff_kind, left_reloc, right_reloc) in diff_data_relocs_for_range( + left_obj, + right_obj, + left, + right, + 0..left_max as usize, + 0..right_max as usize, + ) { + if let Some(left_reloc) = left_reloc { + let len = left_obj.arch.get_reloc_byte_size(left_reloc.flags); + let range = left_reloc.address as usize..left_reloc.address as usize + len; + left_reloc_diffs.push(ObjDataRelocDiff { reloc: left_reloc, kind: diff_kind, range }); + } + if let Some(right_reloc) = right_reloc { + let len = right_obj.arch.get_reloc_byte_size(right_reloc.flags); + let range = right_reloc.address as usize..right_reloc.address as usize + len; + right_reloc_diffs.push(ObjDataRelocDiff { reloc: right_reloc, kind: diff_kind, range }); + } + } + let (mut left_section_diff, mut right_section_diff) = diff_generic_section(left, right, left_section_diff, right_section_diff)?; - let all_left_relocs_match = - left_diff.iter().all(|d| d.kind == ObjDataDiffKind::None || d.reloc.is_none()); + let all_left_relocs_match = left_reloc_diffs.iter().all(|d| d.kind == ObjDataDiffKind::None); left_section_diff.data_diff = left_diff; right_section_diff.data_diff = right_diff; + left_section_diff.reloc_diff = left_reloc_diffs; + right_section_diff.reloc_diff = right_reloc_diffs; if all_left_relocs_match { // Use the highest match percent between two options: // - Left symbols matching right symbols by name @@ -430,8 +365,18 @@ pub fn diff_generic_section( / 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) }, + ObjSectionDiff { + symbols: vec![], + data_diff: vec![], + reloc_diff: vec![], + match_percent: Some(match_percent), + }, + ObjSectionDiff { + symbols: vec![], + data_diff: vec![], + reloc_diff: vec![], + match_percent: Some(match_percent), + }, )) } @@ -456,7 +401,17 @@ pub fn diff_bss_section( } Ok(( - ObjSectionDiff { symbols: vec![], data_diff: vec![], match_percent: Some(match_percent) }, - ObjSectionDiff { symbols: vec![], data_diff: vec![], match_percent: Some(match_percent) }, + ObjSectionDiff { + symbols: vec![], + data_diff: vec![], + reloc_diff: vec![], + match_percent: Some(match_percent), + }, + ObjSectionDiff { + symbols: vec![], + data_diff: vec![], + reloc_diff: vec![], + match_percent: Some(match_percent), + }, )) } diff --git a/objdiff-core/src/diff/mod.rs b/objdiff-core/src/diff/mod.rs index 9157585..49611e5 100644 --- a/objdiff-core/src/diff/mod.rs +++ b/objdiff-core/src/diff/mod.rs @@ -1,4 +1,4 @@ -use std::collections::HashSet; +use std::{collections::HashSet, ops::Range}; use anyhow::Result; @@ -36,6 +36,7 @@ impl DiffObjConfig { pub struct ObjSectionDiff { pub symbols: Vec, pub data_diff: Vec, + pub reloc_diff: Vec, pub match_percent: Option, } @@ -43,6 +44,7 @@ impl ObjSectionDiff { fn merge(&mut self, other: ObjSectionDiff) { // symbols ignored self.data_diff = other.data_diff; + self.reloc_diff = other.reloc_diff; self.match_percent = other.match_percent; } } @@ -87,7 +89,13 @@ pub struct ObjDataDiff { pub kind: ObjDataDiffKind, pub len: usize, pub symbol: String, - pub reloc: Option, +} + +#[derive(Debug, Clone)] +pub struct ObjDataRelocDiff { + pub reloc: ObjReloc, + pub kind: ObjDataDiffKind, + pub range: Range, } #[derive(Debug, Copy, Clone, Eq, PartialEq, Default)] @@ -156,8 +164,8 @@ impl ObjDiff { kind: ObjDataDiffKind::None, len: section.data.len(), symbol: section.name.clone(), - ..Default::default() }], + reloc_diff: vec![], match_percent: None, }); } diff --git a/objdiff-core/src/obj/mod.rs b/objdiff-core/src/obj/mod.rs index 1ef6d0f..a060ad6 100644 --- a/objdiff-core/src/obj/mod.rs +++ b/objdiff-core/src/obj/mod.rs @@ -28,7 +28,7 @@ flags! { HasExtra, } } -#[derive(Debug, Copy, Clone, Default)] +#[derive(Debug, Copy, Clone, Default, PartialEq)] pub struct ObjSymbolFlagSet(pub FlagSet); #[derive(Debug, Clone)] @@ -132,7 +132,7 @@ pub enum ObjSymbolKind { Section, } -#[derive(Debug, Clone)] +#[derive(Debug, Clone, PartialEq)] pub struct ObjSymbol { pub name: String, pub demangled_name: Option, @@ -161,7 +161,7 @@ pub struct ObjInfo { pub split_meta: Option, } -#[derive(Debug, Clone)] +#[derive(Debug, Clone, PartialEq)] pub struct ObjReloc { pub flags: RelocationFlags, pub address: u64, diff --git a/objdiff-gui/src/views/data_diff.rs b/objdiff-gui/src/views/data_diff.rs index d83eeab..d9ee844 100644 --- a/objdiff-gui/src/views/data_diff.rs +++ b/objdiff-gui/src/views/data_diff.rs @@ -6,7 +6,7 @@ use std::{ use egui::{text::LayoutJob, Id, Label, RichText, Sense, Widget}; use objdiff_core::{ - diff::{ObjDataDiff, ObjDataDiffKind, ObjDiff}, + diff::{ObjDataDiff, ObjDataDiffKind, ObjDataRelocDiff, ObjDiff}, obj::ObjInfo, }; use time::format_description; @@ -30,27 +30,31 @@ fn find_section(obj: &ObjInfo, section_name: &str) -> Option { fn data_row_hover_ui( ui: &mut egui::Ui, obj: &ObjInfo, - diffs: &[ObjDataDiff], + diffs: &[(ObjDataDiff, Vec)], appearance: &Appearance, ) { ui.scope(|ui| { ui.style_mut().override_text_style = Some(egui::TextStyle::Monospace); ui.style_mut().wrap_mode = Some(egui::TextWrapMode::Extend); - for diff in diffs { - let Some(reloc) = &diff.reloc else { + let reloc_diffs = diffs.iter().flat_map(|(_, reloc_diffs)| reloc_diffs); + let mut prev_reloc = None; + for reloc_diff in reloc_diffs { + let reloc = &reloc_diff.reloc; + if prev_reloc == Some(reloc) { + // Avoid showing consecutive duplicate relocations. + // We do this because a single relocation can span across multiple diffs if the + // bytes in the relocation changed (e.g. first byte is added, second is unchanged). continue; - }; + } + prev_reloc = Some(reloc); - // Use a slightly different font color for nonmatching relocations so they stand out. - let color = match diff.kind { - ObjDataDiffKind::None => appearance.highlight_color, - _ => appearance.replace_color, - }; + let color = get_color_for_diff_kind(reloc_diff.kind, appearance); // TODO: Most of this code is copy-pasted from ins_hover_ui. // Try to separate this out into a shared function. ui.label(format!("Relocation type: {}", obj.arch.display_reloc(reloc.flags))); + ui.label(format!("Relocation address: {:x}", reloc.address)); let addend_str = match reloc.addend.cmp(&0i64) { Ordering::Greater => format!("+{:x}", reloc.addend), Ordering::Less => format!("-{:x}", -reloc.addend), @@ -76,15 +80,22 @@ fn data_row_hover_ui( }); } -fn data_row_context_menu(ui: &mut egui::Ui, diffs: &[ObjDataDiff]) { +fn data_row_context_menu(ui: &mut egui::Ui, diffs: &[(ObjDataDiff, Vec)]) { ui.scope(|ui| { ui.style_mut().override_text_style = Some(egui::TextStyle::Monospace); ui.style_mut().wrap_mode = Some(egui::TextWrapMode::Extend); - for diff in diffs { - let Some(reloc) = &diff.reloc else { + let reloc_diffs = diffs.iter().flat_map(|(_, reloc_diffs)| reloc_diffs); + let mut prev_reloc = None; + for reloc_diff in reloc_diffs { + let reloc = &reloc_diff.reloc; + if prev_reloc == Some(reloc) { + // Avoid showing consecutive duplicate relocations. + // We do this because a single relocation can span across multiple diffs if the + // bytes in the relocation changed (e.g. first byte is added, second is unchanged). continue; - }; + } + prev_reloc = Some(reloc); // TODO: This code is copy-pasted from ins_context_menu. // Try to separate this out into a shared function. @@ -102,14 +113,25 @@ fn data_row_context_menu(ui: &mut egui::Ui, diffs: &[ObjDataDiff]) { }); } +fn get_color_for_diff_kind(diff_kind: ObjDataDiffKind, appearance: &Appearance) -> egui::Color32 { + match diff_kind { + ObjDataDiffKind::None => appearance.text_color, + ObjDataDiffKind::Replace => appearance.replace_color, + ObjDataDiffKind::Delete => appearance.delete_color, + ObjDataDiffKind::Insert => appearance.insert_color, + } +} + fn data_row_ui( ui: &mut egui::Ui, obj: Option<&ObjInfo>, address: usize, - diffs: &[ObjDataDiff], + diffs: &[(ObjDataDiff, Vec)], appearance: &Appearance, ) { - if diffs.iter().any(|d| d.kind != ObjDataDiffKind::None) { + if diffs.iter().any(|(dd, rds)| { + dd.kind != ObjDataDiffKind::None || rds.iter().any(|rd| rd.kind != ObjDataDiffKind::None) + }) { ui.painter().rect_filled(ui.available_rect_before_wrap(), 0.0, ui.visuals().faint_bg_color); } let mut job = LayoutJob::default(); @@ -119,29 +141,34 @@ fn data_row_ui( &mut job, appearance.code_font.clone(), ); + // The offset shown on the side of the GUI, shifted by insertions/deletions. let mut cur_addr = 0usize; - for diff in diffs { - let base_color = match diff.kind { - ObjDataDiffKind::None => appearance.text_color, - ObjDataDiffKind::Replace => appearance.replace_color, - ObjDataDiffKind::Delete => appearance.delete_color, - ObjDataDiffKind::Insert => appearance.insert_color, - }; + // The offset into the actual bytes of the section on this side, ignoring differences. + let mut cur_addr_actual = address; + for (diff, reloc_diffs) in diffs { + let base_color = get_color_for_diff_kind(diff.kind, appearance); if diff.data.is_empty() { let mut str = " ".repeat(diff.len); str.push_str(" ".repeat(diff.len / 8).as_str()); write_text(str.as_str(), base_color, &mut job, appearance.code_font.clone()); cur_addr += diff.len; } else { - let mut text = String::new(); for byte in &diff.data { - text.push_str(format!("{byte:02x} ").as_str()); + let mut byte_color = base_color; + if let Some(reloc_diff) = reloc_diffs.iter().find(|reloc_diff| { + reloc_diff.kind != ObjDataDiffKind::None + && reloc_diff.range.contains(&cur_addr_actual) + }) { + byte_color = get_color_for_diff_kind(reloc_diff.kind, appearance); + } + let byte_text = format!("{byte:02x} "); + write_text(byte_text.as_str(), byte_color, &mut job, appearance.code_font.clone()); cur_addr += 1; + cur_addr_actual += 1; if cur_addr % 8 == 0 { - text.push(' '); + write_text(" ", base_color, &mut job, appearance.code_font.clone()); } } - write_text(text.as_str(), base_color, &mut job, appearance.code_font.clone()); } } if cur_addr < BYTES_PER_ROW { @@ -152,13 +179,8 @@ fn data_row_ui( write_text(str.as_str(), appearance.text_color, &mut job, appearance.code_font.clone()); } write_text(" ", appearance.text_color, &mut job, appearance.code_font.clone()); - for diff in diffs { - let base_color = match diff.kind { - ObjDataDiffKind::None => appearance.text_color, - ObjDataDiffKind::Replace => appearance.replace_color, - ObjDataDiffKind::Delete => appearance.delete_color, - ObjDataDiffKind::Insert => appearance.insert_color, - }; + for (diff, _) in diffs { + let base_color = get_color_for_diff_kind(diff.kind, appearance); if diff.data.is_empty() { write_text( " ".repeat(diff.len).as_str(), @@ -188,17 +210,24 @@ fn data_row_ui( } } -fn split_diffs(diffs: &[ObjDataDiff]) -> Vec> { - let mut split_diffs = Vec::>::new(); - let mut row_diffs = Vec::::new(); +fn split_diffs( + diffs: &[ObjDataDiff], + reloc_diffs: &[ObjDataRelocDiff], +) -> Vec)>> { + let mut split_diffs = Vec::)>>::new(); + let mut row_diffs = Vec::<(ObjDataDiff, Vec)>::new(); + // The offset shown on the side of the GUI, shifted by insertions/deletions. let mut cur_addr = 0usize; + // The offset into the actual bytes of the section on this side, ignoring differences. + let mut cur_addr_actual = 0usize; for diff in diffs { let mut cur_len = 0usize; while cur_len < diff.len { let remaining_len = diff.len - cur_len; let mut remaining_in_row = BYTES_PER_ROW - (cur_addr % BYTES_PER_ROW); let len = min(remaining_len, remaining_in_row); - row_diffs.push(ObjDataDiff { + + let data_diff = ObjDataDiff { data: if diff.data.is_empty() { Vec::new() } else { @@ -207,8 +236,27 @@ fn split_diffs(diffs: &[ObjDataDiff]) -> Vec> { kind: diff.kind, len, symbol: String::new(), // TODO - reloc: diff.reloc.clone(), - }); + }; + let row_reloc_diffs: Vec = if diff.data.is_empty() { + Vec::new() + } else { + let diff_range = cur_addr_actual + cur_len..cur_addr_actual + cur_len + len; + reloc_diffs + .iter() + .filter_map(|reloc_diff| { + if reloc_diff.range.start < diff_range.end + && diff_range.start < reloc_diff.range.end + { + Some(reloc_diff.clone()) + } else { + None + } + }) + .collect() + }; + let row_diff = (data_diff, row_reloc_diffs); + + row_diffs.push(row_diff); remaining_in_row -= len; cur_len += len; cur_addr += len; @@ -216,6 +264,7 @@ fn split_diffs(diffs: &[ObjDataDiff]) -> Vec> { split_diffs.push(take(&mut row_diffs)); } } + cur_addr_actual += diff.data.len(); } if !row_diffs.is_empty() { split_diffs.push(take(&mut row_diffs)); @@ -267,8 +316,10 @@ fn data_table_ui( } let total_rows = (total_bytes - 1) / BYTES_PER_ROW + 1; - let left_diffs = left_section.map(|(_, section)| split_diffs(§ion.data_diff)); - let right_diffs = right_section.map(|(_, section)| split_diffs(§ion.data_diff)); + let left_diffs = + left_section.map(|(_, section)| split_diffs(§ion.data_diff, §ion.reloc_diff)); + let right_diffs = + right_section.map(|(_, section)| split_diffs(§ion.data_diff, §ion.reloc_diff)); hotkeys::check_scroll_hotkeys(ui, true);