Skip to content

Commit 3e6efb7

Browse files
LagoLunaticLagoLunatic
authored
Check relocation addends when diffing functions (#158)
* Check relocation addends when diffing functions * Also highlight addend when reloc differs
1 parent 674c942 commit 3e6efb7

File tree

5 files changed

+53
-34
lines changed

5 files changed

+53
-34
lines changed

‎objdiff-cli/src/views/function_diff.rs‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,5 @@
1+
use std::cmp::Ordering;
2+
13
use anyhow::{bail, Result};
24
use crossterm::event::{Event, KeyCode, KeyEventKind, KeyModifiers, MouseButton, MouseEventKind};
35
use objdiff_core::{
@@ -563,6 +565,18 @@ impl FunctionDiffUi {
563565
base_color = Color::White;
564566
}
565567
}
568+
DiffText::Addend(addend, diff) => {
569+
label_text = match addend.cmp(&0i64) {
570+
Ordering::Greater => format!("+{:#x}", addend),
571+
Ordering::Less => format!("-{:#x}", -addend),
572+
_ => "".to_string(),
573+
};
574+
if let Some(diff) = diff {
575+
base_color = COLOR_ROTATION[diff.idx % COLOR_ROTATION.len()]
576+
} else {
577+
base_color = Color::White;
578+
}
579+
}
566580
DiffText::Spacing(n) => {
567581
line.spans.push(Span::raw(" ".repeat(n)));
568582
sx += n as u16;

‎objdiff-core/src/diff/code.rs‎

Lines changed: 18 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -192,8 +192,18 @@ fn resolve_branches(vec: &mut [ObjInsDiff]) {
192192
}
193193
}
194194

195-
fn address_eq(left: &ObjReloc, right: &ObjReloc) -> bool {
196-
left.target.address as i64 + left.addend == right.target.address as i64 + right.addend
195+
pub fn address_eq(left: &ObjReloc, right: &ObjReloc) -> bool {
196+
if right.target.size == 0 && left.target.size != 0 {
197+
// The base relocation is against a pool but the target relocation isn't.
198+
// This can happen in rare cases where the compiler will generate a pool+addend relocation
199+
// in the base's data, but the one detected in the target is direct with no addend.
200+
// Just check that the final address is the same so these count as a match.
201+
left.target.address as i64 + left.addend == right.target.address as i64 + right.addend
202+
} else {
203+
// But otherwise, if the compiler isn't using a pool, we're more strict and check that the
204+
// target symbol address and relocation addend both match exactly.
205+
left.target.address == right.target.address && left.addend == right.addend
206+
}
197207
}
198208

199209
pub fn section_name_eq(
@@ -235,13 +245,14 @@ fn reloc_eq(
235245
return true;
236246
}
237247

238-
let symbol_name_matches = left.target.name == right.target.name;
248+
let symbol_name_addend_matches =
249+
left.target.name == right.target.name && left.addend == right.addend;
239250
match (&left.target.orig_section_index, &right.target.orig_section_index) {
240251
(Some(sl), Some(sr)) => {
241-
// Match if section and name or address match
252+
// Match if section and name+addend or address match
242253
section_name_eq(left_obj, right_obj, *sl, *sr)
243254
&& (config.function_reloc_diffs == FunctionRelocDiffs::DataValue
244-
|| symbol_name_matches
255+
|| symbol_name_addend_matches
245256
|| address_eq(left, right))
246257
&& (config.function_reloc_diffs == FunctionRelocDiffs::NameAddress
247258
|| left.target.kind != ObjSymbolKind::Object
@@ -251,9 +262,9 @@ fn reloc_eq(
251262
(Some(_), None) => false,
252263
(None, Some(_)) => {
253264
// Match if possibly stripped weak symbol
254-
symbol_name_matches && right.target.flags.0.contains(ObjSymbolFlags::Weak)
265+
symbol_name_addend_matches && right.target.flags.0.contains(ObjSymbolFlags::Weak)
255266
}
256-
(None, None) => symbol_name_matches,
267+
(None, None) => symbol_name_addend_matches,
257268
}
258269
}
259270

‎objdiff-core/src/diff/data.rs‎

Lines changed: 6 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ use std::{
66
use anyhow::{anyhow, Result};
77
use similar::{capture_diff_slices_deadline, get_diff_ratio, Algorithm};
88

9-
use super::code::section_name_eq;
9+
use super::code::{address_eq, section_name_eq};
1010
use crate::{
1111
diff::{ObjDataDiff, ObjDataDiffKind, ObjDataRelocDiff, ObjSectionDiff, ObjSymbolDiff},
1212
obj::{ObjInfo, ObjReloc, ObjSection, ObjSymbolFlags, SymbolRef},
@@ -41,39 +41,25 @@ pub fn no_diff_symbol(_obj: &ObjInfo, symbol_ref: SymbolRef) -> ObjSymbolDiff {
4141
ObjSymbolDiff { symbol_ref, target_symbol: None, instructions: vec![], match_percent: None }
4242
}
4343

44-
fn address_eq(left: &ObjReloc, right: &ObjReloc) -> bool {
45-
if right.target.size == 0 && left.target.size != 0 {
46-
// The base relocation is against a pool but the target relocation isn't.
47-
// This can happen in rare cases where the compiler will generate a pool+addend relocation
48-
// in the base, but the one detected in the target is direct with no addend.
49-
// Just check that the final address is the same so these count as a match.
50-
left.target.address as i64 + left.addend == right.target.address as i64 + right.addend
51-
} else {
52-
// But otherwise, if the compiler isn't using a pool, we're more strict and check that the
53-
// target symbol address and relocation addend both match exactly.
54-
left.target.address == right.target.address && left.addend == right.addend
55-
}
56-
}
57-
5844
fn reloc_eq(left_obj: &ObjInfo, right_obj: &ObjInfo, left: &ObjReloc, right: &ObjReloc) -> bool {
5945
if left.flags != right.flags {
6046
return false;
6147
}
6248

63-
let symbol_name_matches = left.target.name == right.target.name;
49+
let symbol_name_addend_matches =
50+
left.target.name == right.target.name && left.addend == right.addend;
6451
match (&left.target.orig_section_index, &right.target.orig_section_index) {
6552
(Some(sl), Some(sr)) => {
6653
// Match if section and name+addend or address match
6754
section_name_eq(left_obj, right_obj, *sl, *sr)
68-
&& ((symbol_name_matches && left.addend == right.addend) || address_eq(left, right))
55+
&& (symbol_name_addend_matches || address_eq(left, right))
6956
}
7057
(Some(_), None) => false,
7158
(None, Some(_)) => {
7259
// Match if possibly stripped weak symbol
73-
(symbol_name_matches && left.addend == right.addend)
74-
&& right.target.flags.0.contains(ObjSymbolFlags::Weak)
60+
symbol_name_addend_matches && right.target.flags.0.contains(ObjSymbolFlags::Weak)
7561
}
76-
(None, None) => symbol_name_matches,
62+
(None, None) => symbol_name_addend_matches,
7763
}
7864
}
7965

‎objdiff-core/src/diff/display.rs‎

Lines changed: 3 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,3 @@
1-
use std::cmp::Ordering;
2-
31
use crate::{
42
diff::{ObjInsArgDiff, ObjInsDiff},
53
obj::{ObjInsArg, ObjInsArgValue, ObjReloc, ObjSymbol},
@@ -23,6 +21,8 @@ pub enum DiffText<'a> {
2321
BranchDest(u64, Option<&'a ObjInsArgDiff>),
2422
/// Symbol name
2523
Symbol(&'a ObjSymbol, Option<&'a ObjInsArgDiff>),
24+
/// Relocation addend
25+
Addend(i64, Option<&'a ObjInsArgDiff>),
2626
/// Number of spaces
2727
Spacing(usize),
2828
/// End of line
@@ -99,11 +99,7 @@ fn display_reloc_name<E>(
9999
diff: Option<&ObjInsArgDiff>,
100100
) -> Result<(), E> {
101101
cb(DiffText::Symbol(&reloc.target, diff))?;
102-
match reloc.addend.cmp(&0i64) {
103-
Ordering::Greater => cb(DiffText::Basic(&format!("+{:#x}", reloc.addend))),
104-
Ordering::Less => cb(DiffText::Basic(&format!("-{:#x}", -reloc.addend))),
105-
_ => Ok(()),
106-
}
102+
cb(DiffText::Addend(reloc.addend, diff))
107103
}
108104

109105
impl PartialEq<DiffText<'_>> for HighlightKind {

‎objdiff-gui/src/views/function_diff.rs‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -324,6 +324,18 @@ fn diff_text_ui(
324324
base_color = appearance.emphasized_text_color;
325325
}
326326
}
327+
DiffText::Addend(addend, diff) => {
328+
label_text = match addend.cmp(&0i64) {
329+
Ordering::Greater => format!("+{:#x}", addend),
330+
Ordering::Less => format!("-{:#x}", -addend),
331+
_ => "".to_string(),
332+
};
333+
if let Some(diff) = diff {
334+
base_color = appearance.diff_colors[diff.idx % appearance.diff_colors.len()]
335+
} else {
336+
base_color = appearance.emphasized_text_color;
337+
}
338+
}
327339
DiffText::Spacing(n) => {
328340
ui.add_space(n as f32 * space_width);
329341
return ret;

0 commit comments

Comments
 (0)