From 3e6efb7736b0ada6b02a38189fea5db142b358d4 Mon Sep 17 00:00:00 2001 From: LagoLunatic Date: Mon, 10 Feb 2025 00:26:49 -0500 Subject: [PATCH] Check relocation addends when diffing functions (#158) * Check relocation addends when diffing functions * Also highlight addend when reloc differs --- objdiff-cli/src/views/function_diff.rs | 14 ++++++++++++++ objdiff-core/src/diff/code.rs | 25 ++++++++++++++++++------- objdiff-core/src/diff/data.rs | 26 ++++++-------------------- objdiff-core/src/diff/display.rs | 10 +++------- objdiff-gui/src/views/function_diff.rs | 12 ++++++++++++ 5 files changed, 53 insertions(+), 34 deletions(-) diff --git a/objdiff-cli/src/views/function_diff.rs b/objdiff-cli/src/views/function_diff.rs index 44dde3e..a300c08 100644 --- a/objdiff-cli/src/views/function_diff.rs +++ b/objdiff-cli/src/views/function_diff.rs @@ -1,3 +1,5 @@ +use std::cmp::Ordering; + use anyhow::{bail, Result}; use crossterm::event::{Event, KeyCode, KeyEventKind, KeyModifiers, MouseButton, MouseEventKind}; use objdiff_core::{ @@ -563,6 +565,18 @@ impl FunctionDiffUi { base_color = Color::White; } } + DiffText::Addend(addend, diff) => { + label_text = match addend.cmp(&0i64) { + Ordering::Greater => format!("+{:#x}", addend), + Ordering::Less => format!("-{:#x}", -addend), + _ => "".to_string(), + }; + if let Some(diff) = diff { + base_color = COLOR_ROTATION[diff.idx % COLOR_ROTATION.len()] + } else { + base_color = Color::White; + } + } DiffText::Spacing(n) => { line.spans.push(Span::raw(" ".repeat(n))); sx += n as u16; diff --git a/objdiff-core/src/diff/code.rs b/objdiff-core/src/diff/code.rs index 62e6caf..9dc0fbc 100644 --- a/objdiff-core/src/diff/code.rs +++ b/objdiff-core/src/diff/code.rs @@ -192,8 +192,18 @@ fn resolve_branches(vec: &mut [ObjInsDiff]) { } } -fn address_eq(left: &ObjReloc, right: &ObjReloc) -> bool { - left.target.address as i64 + left.addend == right.target.address as i64 + right.addend +pub fn address_eq(left: &ObjReloc, right: &ObjReloc) -> bool { + if right.target.size == 0 && left.target.size != 0 { + // The base relocation is against a pool but the target relocation isn't. + // This can happen in rare cases where the compiler will generate a pool+addend relocation + // in the base's data, but the one detected in the target is direct with no addend. + // Just check that the final address is the same so these count as a match. + left.target.address as i64 + left.addend == right.target.address as i64 + right.addend + } else { + // But otherwise, if the compiler isn't using a pool, we're more strict and check that the + // target symbol address and relocation addend both match exactly. + left.target.address == right.target.address && left.addend == right.addend + } } pub fn section_name_eq( @@ -235,13 +245,14 @@ fn reloc_eq( return true; } - let symbol_name_matches = left.target.name == right.target.name; + let symbol_name_addend_matches = + left.target.name == right.target.name && left.addend == right.addend; match (&left.target.orig_section_index, &right.target.orig_section_index) { (Some(sl), Some(sr)) => { - // Match if section and name or address match + // Match if section and name+addend or address match section_name_eq(left_obj, right_obj, *sl, *sr) && (config.function_reloc_diffs == FunctionRelocDiffs::DataValue - || symbol_name_matches + || symbol_name_addend_matches || address_eq(left, right)) && (config.function_reloc_diffs == FunctionRelocDiffs::NameAddress || left.target.kind != ObjSymbolKind::Object @@ -251,9 +262,9 @@ fn reloc_eq( (Some(_), None) => false, (None, Some(_)) => { // Match if possibly stripped weak symbol - symbol_name_matches && right.target.flags.0.contains(ObjSymbolFlags::Weak) + symbol_name_addend_matches && right.target.flags.0.contains(ObjSymbolFlags::Weak) } - (None, None) => symbol_name_matches, + (None, None) => symbol_name_addend_matches, } } diff --git a/objdiff-core/src/diff/data.rs b/objdiff-core/src/diff/data.rs index e293319..954fc55 100644 --- a/objdiff-core/src/diff/data.rs +++ b/objdiff-core/src/diff/data.rs @@ -6,7 +6,7 @@ use std::{ use anyhow::{anyhow, Result}; use similar::{capture_diff_slices_deadline, get_diff_ratio, Algorithm}; -use super::code::section_name_eq; +use super::code::{address_eq, section_name_eq}; use crate::{ diff::{ObjDataDiff, ObjDataDiffKind, ObjDataRelocDiff, ObjSectionDiff, ObjSymbolDiff}, obj::{ObjInfo, ObjReloc, ObjSection, ObjSymbolFlags, SymbolRef}, @@ -41,39 +41,25 @@ pub fn no_diff_symbol(_obj: &ObjInfo, symbol_ref: SymbolRef) -> ObjSymbolDiff { ObjSymbolDiff { symbol_ref, target_symbol: None, instructions: vec![], match_percent: None } } -fn address_eq(left: &ObjReloc, right: &ObjReloc) -> bool { - if right.target.size == 0 && left.target.size != 0 { - // The base relocation is against a pool but the target relocation isn't. - // This can happen in rare cases where the compiler will generate a pool+addend relocation - // in the base, but the one detected in the target is direct with no addend. - // Just check that the final address is the same so these count as a match. - left.target.address as i64 + left.addend == right.target.address as i64 + right.addend - } else { - // But otherwise, if the compiler isn't using a pool, we're more strict and check that the - // target symbol address and relocation addend both match exactly. - left.target.address == right.target.address && left.addend == right.addend - } -} - fn reloc_eq(left_obj: &ObjInfo, right_obj: &ObjInfo, left: &ObjReloc, right: &ObjReloc) -> bool { if left.flags != right.flags { return false; } - let symbol_name_matches = left.target.name == right.target.name; + let symbol_name_addend_matches = + left.target.name == right.target.name && left.addend == right.addend; match (&left.target.orig_section_index, &right.target.orig_section_index) { (Some(sl), Some(sr)) => { // Match if section and name+addend or address match section_name_eq(left_obj, right_obj, *sl, *sr) - && ((symbol_name_matches && left.addend == right.addend) || address_eq(left, right)) + && (symbol_name_addend_matches || address_eq(left, right)) } (Some(_), None) => false, (None, Some(_)) => { // Match if possibly stripped weak symbol - (symbol_name_matches && left.addend == right.addend) - && right.target.flags.0.contains(ObjSymbolFlags::Weak) + symbol_name_addend_matches && right.target.flags.0.contains(ObjSymbolFlags::Weak) } - (None, None) => symbol_name_matches, + (None, None) => symbol_name_addend_matches, } } diff --git a/objdiff-core/src/diff/display.rs b/objdiff-core/src/diff/display.rs index 45fdca8..29fccdb 100644 --- a/objdiff-core/src/diff/display.rs +++ b/objdiff-core/src/diff/display.rs @@ -1,5 +1,3 @@ -use std::cmp::Ordering; - use crate::{ diff::{ObjInsArgDiff, ObjInsDiff}, obj::{ObjInsArg, ObjInsArgValue, ObjReloc, ObjSymbol}, @@ -23,6 +21,8 @@ pub enum DiffText<'a> { BranchDest(u64, Option<&'a ObjInsArgDiff>), /// Symbol name Symbol(&'a ObjSymbol, Option<&'a ObjInsArgDiff>), + /// Relocation addend + Addend(i64, Option<&'a ObjInsArgDiff>), /// Number of spaces Spacing(usize), /// End of line @@ -99,11 +99,7 @@ fn display_reloc_name( diff: Option<&ObjInsArgDiff>, ) -> Result<(), E> { cb(DiffText::Symbol(&reloc.target, diff))?; - match reloc.addend.cmp(&0i64) { - Ordering::Greater => cb(DiffText::Basic(&format!("+{:#x}", reloc.addend))), - Ordering::Less => cb(DiffText::Basic(&format!("-{:#x}", -reloc.addend))), - _ => Ok(()), - } + cb(DiffText::Addend(reloc.addend, diff)) } impl PartialEq> for HighlightKind { diff --git a/objdiff-gui/src/views/function_diff.rs b/objdiff-gui/src/views/function_diff.rs index b07e22c..3928a75 100644 --- a/objdiff-gui/src/views/function_diff.rs +++ b/objdiff-gui/src/views/function_diff.rs @@ -324,6 +324,18 @@ fn diff_text_ui( base_color = appearance.emphasized_text_color; } } + DiffText::Addend(addend, diff) => { + label_text = match addend.cmp(&0i64) { + Ordering::Greater => format!("+{:#x}", addend), + Ordering::Less => format!("-{:#x}", -addend), + _ => "".to_string(), + }; + if let Some(diff) = diff { + base_color = appearance.diff_colors[diff.idx % appearance.diff_colors.len()] + } else { + base_color = appearance.emphasized_text_color; + } + } DiffText::Spacing(n) => { ui.add_space(n as f32 * space_width); return ret;