From 7c469df0dc6d0538dc5e4d55795d0022263fa7ba Mon Sep 17 00:00:00 2001
From: LagoLunatic <LagoLunatic@users.noreply.github.com>
Date: Sun, 12 Jan 2025 13:40:25 -0500
Subject: [PATCH 1/9] Data view: Show data bytes with differing relocations as
 a diff

---
 objdiff-core/src/diff/data.rs      | 146 ++++++++++++++++++++++++++++-
 objdiff-core/src/diff/mod.rs       |   6 +-
 objdiff-gui/src/views/data_diff.rs |   4 +-
 3 files changed, 151 insertions(+), 5 deletions(-)

diff --git a/objdiff-core/src/diff/data.rs b/objdiff-core/src/diff/data.rs
index 84984398..5aa8d509 100644
--- a/objdiff-core/src/diff/data.rs
+++ b/objdiff-core/src/diff/data.rs
@@ -1,11 +1,14 @@
-use std::cmp::{max, min, Ordering};
+use std::{
+    cmp::{max, min, Ordering},
+    ops::Range,
+};
 
 use anyhow::{anyhow, Result};
 use similar::{capture_diff_slices_deadline, get_diff_ratio, Algorithm};
 
 use crate::{
     diff::{ObjDataDiff, ObjDataDiffKind, ObjSectionDiff, ObjSymbolDiff},
-    obj::{ObjInfo, ObjSection, SymbolRef},
+    obj::{ObjInfo, ObjReloc, ObjSection, SymbolRef},
 };
 
 pub fn diff_bss_symbol(
@@ -37,6 +40,59 @@ pub fn no_diff_symbol(_obj: &ObjInfo, symbol_ref: SymbolRef) -> ObjSymbolDiff {
     ObjSymbolDiff { symbol_ref, target_symbol: None, instructions: vec![], match_percent: None }
 }
 
+fn diff_data_relocs_for_range(
+    left: &ObjSection,
+    right: &ObjSection,
+    left_range: Range<usize>,
+    right_range: Range<usize>,
+) -> Vec<(ObjDataDiffKind, Option<ObjReloc>, Option<ObjReloc>)> {
+    let mut diffs = Vec::new();
+    for left_reloc in left.relocations.iter() {
+        if !left_range.contains(&(left_reloc.address as usize)) {
+            continue;
+        }
+        let left_offset = left_reloc.address as usize - left_range.start;
+        let Some(right_reloc) = right.relocations.iter().find(|r| {
+            if !right_range.contains(&(r.address as usize)) {
+                return false;
+            }
+            let right_offset = r.address as usize - right_range.start;
+            right_offset == left_offset
+        }) else {
+            diffs.push((ObjDataDiffKind::Delete, Some(left_reloc.clone()), None));
+            continue;
+        };
+        if left_reloc.target.name != right_reloc.target.name {
+            diffs.push((
+                ObjDataDiffKind::Replace,
+                Some(left_reloc.clone()),
+                Some(right_reloc.clone()),
+            ));
+            continue;
+        }
+        diffs.push((ObjDataDiffKind::None, Some(left_reloc.clone()), Some(right_reloc.clone())));
+    }
+    for right_reloc in right.relocations.iter() {
+        if !right_range.contains(&(right_reloc.address as usize)) {
+            continue;
+        }
+        let right_offset = right_reloc.address as usize - right_range.start;
+        let Some(_) = left.relocations.iter().find(|r| {
+            if !left_range.contains(&(r.address as usize)) {
+                return false;
+            }
+            let left_offset = r.address as usize - left_range.start;
+            left_offset == right_offset
+        }) else {
+            diffs.push((ObjDataDiffKind::Insert, None, Some(right_reloc.clone())));
+            continue;
+        };
+        // No need to check the cases for ObjDataDiffKind::Replace and ObjDataDiffKind::None again.
+        // They were already handled in the loop over the left relocs.
+    }
+    diffs
+}
+
 /// Compare the data sections of two object files.
 pub fn diff_data_section(
     left: &ObjSection,
@@ -70,6 +126,92 @@ pub fn diff_data_section(
                 ObjDataDiffKind::Replace
             }
         };
+        if kind == ObjDataDiffKind::None {
+            let mut found_any_reloc_diffs = 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, right, left_range.clone(), right_range.clone())
+            {
+                if diff_kind == ObjDataDiffKind::None {
+                    continue;
+                }
+                found_any_reloc_diffs = 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 = 4; // TODO don't hardcode
+                    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 = 4; // TODO don't hardcode
+                    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_reloc_diffs {
+                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 {
diff --git a/objdiff-core/src/diff/mod.rs b/objdiff-core/src/diff/mod.rs
index 405645dc..f2a3511e 100644
--- a/objdiff-core/src/diff/mod.rs
+++ b/objdiff-core/src/diff/mod.rs
@@ -11,7 +11,9 @@ use crate::{
             diff_generic_section, no_diff_symbol,
         },
     },
-    obj::{ObjInfo, ObjIns, ObjSection, ObjSectionKind, ObjSymbol, SymbolRef, SECTION_COMMON},
+    obj::{
+        ObjInfo, ObjIns, ObjReloc, ObjSection, ObjSectionKind, ObjSymbol, SymbolRef, SECTION_COMMON,
+    },
 };
 
 pub mod code;
@@ -85,6 +87,7 @@ pub struct ObjDataDiff {
     pub kind: ObjDataDiffKind,
     pub len: usize,
     pub symbol: String,
+    pub reloc: Option<ObjReloc>,
 }
 
 #[derive(Debug, Copy, Clone, Eq, PartialEq, Default)]
@@ -153,6 +156,7 @@ impl ObjDiff {
                     kind: ObjDataDiffKind::None,
                     len: section.data.len(),
                     symbol: section.name.clone(),
+                    ..Default::default()
                 }],
                 match_percent: None,
             });
diff --git a/objdiff-gui/src/views/data_diff.rs b/objdiff-gui/src/views/data_diff.rs
index 37f2a6ce..1c693046 100644
--- a/objdiff-gui/src/views/data_diff.rs
+++ b/objdiff-gui/src/views/data_diff.rs
@@ -117,8 +117,8 @@ fn split_diffs(diffs: &[ObjDataDiff]) -> Vec<Vec<ObjDataDiff>> {
                 },
                 kind: diff.kind,
                 len,
-                // TODO
-                symbol: String::new(),
+                symbol: String::new(), // TODO
+                reloc: diff.reloc.clone(),
             });
             remaining_in_row -= len;
             cur_len += len;

From 57fcb4b6986575f94fbf7be93c4bd35eeb51e6f4 Mon Sep 17 00:00:00 2001
From: LagoLunatic <LagoLunatic@users.noreply.github.com>
Date: Sun, 12 Jan 2025 13:43:26 -0500
Subject: [PATCH 2/9] Data view: Show differing relocations on hover

---
 objdiff-gui/src/views/data_diff.rs | 81 +++++++++++++++++++++++++++---
 1 file changed, 74 insertions(+), 7 deletions(-)

diff --git a/objdiff-gui/src/views/data_diff.rs b/objdiff-gui/src/views/data_diff.rs
index 1c693046..93778007 100644
--- a/objdiff-gui/src/views/data_diff.rs
+++ b/objdiff-gui/src/views/data_diff.rs
@@ -1,4 +1,8 @@
-use std::{cmp::min, default::Default, mem::take};
+use std::{
+    cmp::{min, Ordering},
+    default::Default,
+    mem::take,
+};
 
 use egui::{text::LayoutJob, Id, Label, RichText, Sense, Widget};
 use objdiff_core::{
@@ -23,7 +27,65 @@ fn find_section(obj: &ObjInfo, section_name: &str) -> Option<usize> {
     obj.sections.iter().position(|section| section.name == section_name)
 }
 
-fn data_row_ui(ui: &mut egui::Ui, address: usize, diffs: &[ObjDataDiff], appearance: &Appearance) {
+fn data_row_hover_ui(
+    ui: &mut egui::Ui,
+    obj: &ObjInfo,
+    diffs: &[ObjDataDiff],
+    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 {
+                continue;
+            };
+
+            // 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)));
+            let addend_str = match reloc.addend.cmp(&0i64) {
+                Ordering::Greater => format!("+{:x}", reloc.addend),
+                Ordering::Less => format!("-{:x}", -reloc.addend),
+                _ => "".to_string(),
+            };
+            ui.colored_label(
+                appearance.highlight_color,
+                format!("Name: {}{}", reloc.target.name, addend_str),
+            );
+            if let Some(orig_section_index) = reloc.target.orig_section_index {
+                if let Some(section) =
+                    obj.sections.iter().find(|s| s.orig_index == orig_section_index)
+                {
+                    ui.colored_label(
+                        appearance.highlight_color,
+                        format!("Section: {}", section.name),
+                    );
+                }
+                ui.colored_label(
+                    appearance.highlight_color,
+                    format!("Address: {:x}{}", reloc.target.address, addend_str),
+                );
+                ui.colored_label(
+                    appearance.highlight_color,
+                    format!("Size: {:x}", reloc.target.size),
+                );
+                if reloc.addend >= 0 && reloc.target.bytes.len() > reloc.addend as usize {}
+            } else {
+                ui.colored_label(appearance.highlight_color, "Extern".to_string());
+            }
+        }
+    });
+}
+
+fn data_row_ui(
+    ui: &mut egui::Ui,
+    obj: Option<&ObjInfo>,
+    address: usize,
+    diffs: &[ObjDataDiff],
+    appearance: &Appearance,
+) {
     if diffs.iter().any(|d| d.kind != ObjDataDiffKind::None) {
         ui.painter().rect_filled(ui.available_rect_before_wrap(), 0.0, ui.visuals().faint_bg_color);
     }
@@ -94,9 +156,12 @@ fn data_row_ui(ui: &mut egui::Ui, address: usize, diffs: &[ObjDataDiff], appeara
             write_text(text.as_str(), base_color, &mut job, appearance.code_font.clone());
         }
     }
-    Label::new(job).sense(Sense::click()).ui(ui);
-    //     .on_hover_ui_at_pointer(|ui| ins_hover_ui(ui, ins))
-    //     .context_menu(|ui| ins_context_menu(ui, ins));
+
+    let response = Label::new(job).sense(Sense::click()).ui(ui);
+    if let Some(obj) = obj {
+        response.on_hover_ui_at_pointer(|ui| data_row_hover_ui(ui, obj, diffs, appearance));
+        // .context_menu(|ui| data_row_context_menu(ui, ins)); // TODO
+    }
 }
 
 fn split_diffs(diffs: &[ObjDataDiff]) -> Vec<Vec<ObjDataDiff>> {
@@ -161,6 +226,8 @@ fn data_table_ui(
     right_ctx: Option<SectionDiffContext<'_>>,
     config: &Appearance,
 ) -> Option<()> {
+    let left_obj = left_ctx.map(|ctx| ctx.obj);
+    let right_obj = right_ctx.map(|ctx| ctx.obj);
     let left_section = left_ctx
         .and_then(|ctx| ctx.section_index.map(|i| (&ctx.obj.sections[i], &ctx.diff.sections[i])));
     let right_section = right_ctx
@@ -187,11 +254,11 @@ fn data_table_ui(
         row.col(|ui| {
             if column == 0 {
                 if let Some(left_diffs) = &left_diffs {
-                    data_row_ui(ui, address, &left_diffs[i], config);
+                    data_row_ui(ui, left_obj, address, &left_diffs[i], config);
                 }
             } else if column == 1 {
                 if let Some(right_diffs) = &right_diffs {
-                    data_row_ui(ui, address, &right_diffs[i], config);
+                    data_row_ui(ui, right_obj, address, &right_diffs[i], config);
                 }
             }
         });

From 0b588b60bacf9c9025b78a17b7bd96e01221e961 Mon Sep 17 00:00:00 2001
From: LagoLunatic <LagoLunatic@users.noreply.github.com>
Date: Sun, 12 Jan 2025 14:57:45 -0500
Subject: [PATCH 3/9] Symbol list view: Adjust symbol/section match %s when
 relocations differ

---
 objdiff-core/src/diff/data.rs | 56 ++++++++++++++++++++++++++++-------
 1 file changed, 45 insertions(+), 11 deletions(-)

diff --git a/objdiff-core/src/diff/data.rs b/objdiff-core/src/diff/data.rs
index 5aa8d509..cb494d68 100644
--- a/objdiff-core/src/diff/data.rs
+++ b/objdiff-core/src/diff/data.rs
@@ -265,14 +265,19 @@ pub fn diff_data_section(
 
     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());
     left_section_diff.data_diff = left_diff;
     right_section_diff.data_diff = right_diff;
-    // Use the highest match percent between two options:
-    // - Left symbols matching right symbols by name
-    // - Diff of the data itself
-    if left_section_diff.match_percent.unwrap_or(-1.0) < match_percent {
-        left_section_diff.match_percent = Some(match_percent);
-        right_section_diff.match_percent = Some(match_percent);
+    if all_left_relocs_match {
+        // Use the highest match percent between two options:
+        // - Left symbols matching right symbols by name
+        // - Diff of the data itself
+        // We only do this when all relocations on the left side match.
+        if left_section_diff.match_percent.unwrap_or(-1.0) < match_percent {
+            left_section_diff.match_percent = Some(match_percent);
+            right_section_diff.match_percent = Some(match_percent);
+        }
     }
     Ok((left_section_diff, right_section_diff))
 }
@@ -289,13 +294,42 @@ pub fn diff_data_symbol(
     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 left_range = left_symbol.section_address as usize
+        ..(left_symbol.section_address + left_symbol.size) as usize;
+    let right_range = right_symbol.section_address as usize
+        ..(right_symbol.section_address + right_symbol.size) as usize;
+    let left_data = &left_section.data[left_range.clone()];
+    let right_data = &right_section.data[right_range.clone()];
+
+    let reloc_diffs =
+        diff_data_relocs_for_range(left_section, right_section, left_range, right_range);
 
     let ops = capture_diff_slices_deadline(Algorithm::Patience, left_data, right_data, None);
-    let match_percent = get_diff_ratio(&ops, left_data.len(), right_data.len()) * 100.0;
+    let bytes_match_ratio = get_diff_ratio(&ops, left_data.len(), right_data.len());
+
+    let mut match_ratio = bytes_match_ratio;
+    if !reloc_diffs.is_empty() {
+        let mut total_reloc_bytes = 0;
+        let mut matching_reloc_bytes = 0;
+        for (diff_kind, _, _) in reloc_diffs {
+            let reloc_diff_len: usize = 4; // TODO don't hardcode
+            total_reloc_bytes += reloc_diff_len;
+            if diff_kind == ObjDataDiffKind::None {
+                matching_reloc_bytes += reloc_diff_len;
+            }
+        }
+        let relocs_match_ratio = matching_reloc_bytes as f32 / total_reloc_bytes as f32;
+        // Adjust the overall match ratio to include relocation differences.
+        // We calculate it so that bytes that contain a relocation are counted twice: once for the
+        // byte's raw value, and once for its relocation.
+        // e.g. An 8 byte symbol that has 8 matching raw bytes and a single 4 byte relocation that
+        // doesn't match would show as 66% (weighted average of 100% and 0%).
+        match_ratio = ((bytes_match_ratio * (left_data.len() as f32))
+            + (relocs_match_ratio * total_reloc_bytes as f32))
+            / (left_data.len() + total_reloc_bytes) as f32;
+    }
+
+    let match_percent = match_ratio * 100.0;
 
     Ok((
         ObjSymbolDiff {

From 5bb4ea725fa95a4ff0710204ba4c9ee3045ba224 Mon Sep 17 00:00:00 2001
From: LagoLunatic <LagoLunatic@users.noreply.github.com>
Date: Sun, 12 Jan 2025 22:59:39 -0500
Subject: [PATCH 4/9] Improve data reloc diffing logic

---
 objdiff-core/src/diff/code.rs |  2 +-
 objdiff-core/src/diff/data.rs | 74 ++++++++++++++++++++++++++++++-----
 objdiff-core/src/diff/mod.rs  |  2 +
 3 files changed, 68 insertions(+), 10 deletions(-)

diff --git a/objdiff-core/src/diff/code.rs b/objdiff-core/src/diff/code.rs
index 4e1d6ced..82248571 100644
--- a/objdiff-core/src/diff/code.rs
+++ b/objdiff-core/src/diff/code.rs
@@ -192,7 +192,7 @@ fn address_eq(left: &ObjReloc, right: &ObjReloc) -> bool {
     left.target.address as i64 + left.addend == right.target.address as i64 + right.addend
 }
 
-fn section_name_eq(
+pub fn section_name_eq(
     left_obj: &ObjInfo,
     right_obj: &ObjInfo,
     left_orig_section_index: usize,
diff --git a/objdiff-core/src/diff/data.rs b/objdiff-core/src/diff/data.rs
index cb494d68..96ca3115 100644
--- a/objdiff-core/src/diff/data.rs
+++ b/objdiff-core/src/diff/data.rs
@@ -6,9 +6,10 @@ use std::{
 use anyhow::{anyhow, Result};
 use similar::{capture_diff_slices_deadline, get_diff_ratio, Algorithm};
 
+use super::code::section_name_eq;
 use crate::{
     diff::{ObjDataDiff, ObjDataDiffKind, ObjSectionDiff, ObjSymbolDiff},
-    obj::{ObjInfo, ObjReloc, ObjSection, SymbolRef},
+    obj::{ObjInfo, ObjReloc, ObjSection, ObjSymbolFlags, SymbolRef},
 };
 
 pub fn diff_bss_symbol(
@@ -40,7 +41,45 @@ 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;
+    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))
+        }
+        (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)
+        }
+        (None, None) => symbol_name_matches,
+    }
+}
+
 fn diff_data_relocs_for_range(
+    left_obj: &ObjInfo,
+    right_obj: &ObjInfo,
     left: &ObjSection,
     right: &ObjSection,
     left_range: Range<usize>,
@@ -62,15 +101,19 @@ fn diff_data_relocs_for_range(
             diffs.push((ObjDataDiffKind::Delete, Some(left_reloc.clone()), None));
             continue;
         };
-        if left_reloc.target.name != right_reloc.target.name {
+        if reloc_eq(left_obj, right_obj, left_reloc, right_reloc) {
+            diffs.push((
+                ObjDataDiffKind::None,
+                Some(left_reloc.clone()),
+                Some(right_reloc.clone()),
+            ));
+        } else {
             diffs.push((
                 ObjDataDiffKind::Replace,
                 Some(left_reloc.clone()),
                 Some(right_reloc.clone()),
             ));
-            continue;
         }
-        diffs.push((ObjDataDiffKind::None, Some(left_reloc.clone()), Some(right_reloc.clone())));
     }
     for right_reloc in right.relocations.iter() {
         if !right_range.contains(&(right_reloc.address as usize)) {
@@ -95,6 +138,8 @@ fn diff_data_relocs_for_range(
 
 /// Compare the data sections of two object files.
 pub fn diff_data_section(
+    left_obj: &ObjInfo,
+    right_obj: &ObjInfo,
     left: &ObjSection,
     right: &ObjSection,
     left_section_diff: &ObjSectionDiff,
@@ -130,9 +175,14 @@ pub fn diff_data_section(
             let mut found_any_reloc_diffs = 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, right, left_range.clone(), right_range.clone())
-            {
+            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(),
+            ) {
                 if diff_kind == ObjDataDiffKind::None {
                     continue;
                 }
@@ -301,8 +351,14 @@ pub fn diff_data_symbol(
     let left_data = &left_section.data[left_range.clone()];
     let right_data = &right_section.data[right_range.clone()];
 
-    let reloc_diffs =
-        diff_data_relocs_for_range(left_section, right_section, left_range, right_range);
+    let reloc_diffs = diff_data_relocs_for_range(
+        left_obj,
+        right_obj,
+        left_section,
+        right_section,
+        left_range,
+        right_range,
+    );
 
     let ops = capture_diff_slices_deadline(Algorithm::Patience, left_data, right_data, None);
     let bytes_match_ratio = get_diff_ratio(&ops, left_data.len(), right_data.len());
diff --git a/objdiff-core/src/diff/mod.rs b/objdiff-core/src/diff/mod.rs
index f2a3511e..91575851 100644
--- a/objdiff-core/src/diff/mod.rs
+++ b/objdiff-core/src/diff/mod.rs
@@ -349,6 +349,8 @@ pub fn diff_objs(
                     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_data_section(
+                        left_obj,
+                        right_obj,
                         left_section,
                         right_section,
                         left_section_diff,

From abd2c6a597b531fcd28649b40d0acba2d09f5bf5 Mon Sep 17 00:00:00 2001
From: LagoLunatic <LagoLunatic@users.noreply.github.com>
Date: Sun, 12 Jan 2025 23:01:07 -0500
Subject: [PATCH 5/9] Don't make reloc diffs cause bytes to show as red or
 green

---
 objdiff-core/src/diff/data.rs | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/objdiff-core/src/diff/data.rs b/objdiff-core/src/diff/data.rs
index 96ca3115..1c42ed53 100644
--- a/objdiff-core/src/diff/data.rs
+++ b/objdiff-core/src/diff/data.rs
@@ -77,6 +77,11 @@ fn reloc_eq(left_obj: &ObjInfo, right_obj: &ObjInfo, left: &ObjReloc, right: &Ob
     }
 }
 
+/// Compares relocations contained with a certain data range.
+/// The ObjDataDiffKind for each diff will either be `None`` (if the relocation matches),
+/// or `Replace` (if a relocation was changed, added, or removed).
+/// `Insert` and `Delete` are not used when a relocation is added or removed to avoid confusing diffs
+/// where it looks like the bytes themselves were changed but actually only the relocations changed.
 fn diff_data_relocs_for_range(
     left_obj: &ObjInfo,
     right_obj: &ObjInfo,
@@ -98,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::Delete, Some(left_reloc.clone()), None));
+            diffs.push((ObjDataDiffKind::Replace, Some(left_reloc.clone()), None));
             continue;
         };
         if reloc_eq(left_obj, right_obj, left_reloc, right_reloc) {
@@ -127,10 +132,10 @@ fn diff_data_relocs_for_range(
             let left_offset = r.address as usize - left_range.start;
             left_offset == right_offset
         }) else {
-            diffs.push((ObjDataDiffKind::Insert, None, Some(right_reloc.clone())));
+            diffs.push((ObjDataDiffKind::Replace, None, Some(right_reloc.clone())));
             continue;
         };
-        // No need to check the cases for ObjDataDiffKind::Replace and ObjDataDiffKind::None again.
+        // No need to check the cases for relocations being deleted or matching again.
         // They were already handled in the loop over the left relocs.
     }
     diffs

From 497e4a301e9cc962fab0e22dfe887fbd6329a3db Mon Sep 17 00:00:00 2001
From: LagoLunatic <LagoLunatic@users.noreply.github.com>
Date: Wed, 15 Jan 2025 02:08:39 -0500
Subject: [PATCH 6/9] Properly detect byte size of each relocation

---
 objdiff-core/src/arch/arm.rs   | 13 +++++++++++++
 objdiff-core/src/arch/arm64.rs | 15 +++++++++++++++
 objdiff-core/src/arch/mips.rs  | 11 +++++++++++
 objdiff-core/src/arch/mod.rs   |  2 ++
 objdiff-core/src/arch/ppc.rs   | 11 +++++++++++
 objdiff-core/src/arch/x86.rs   | 13 +++++++++++++
 objdiff-core/src/diff/data.rs  | 32 +++++++++++++++++++-------------
 7 files changed, 84 insertions(+), 13 deletions(-)

diff --git a/objdiff-core/src/arch/arm.rs b/objdiff-core/src/arch/arm.rs
index 28826c8e..84a21ccf 100644
--- a/objdiff-core/src/arch/arm.rs
+++ b/objdiff-core/src/arch/arm.rs
@@ -276,6 +276,19 @@ impl ObjArch for ObjArchArm {
     fn display_reloc(&self, flags: RelocationFlags) -> Cow<'static, str> {
         Cow::Owned(format!("<{flags:?}>"))
     }
+
+    fn get_reloc_byte_size(&self, flags: RelocationFlags) -> usize {
+        match flags {
+            RelocationFlags::Elf { r_type } => match r_type {
+                elf::R_ARM_ABS32 => 4,
+                elf::R_ARM_REL32 => 4,
+                elf::R_ARM_ABS16 => 2,
+                elf::R_ARM_ABS8 => 1,
+                _ => 1,
+            },
+            _ => 1,
+        }
+    }
 }
 
 #[derive(Clone, Copy, Debug)]
diff --git a/objdiff-core/src/arch/arm64.rs b/objdiff-core/src/arch/arm64.rs
index 7e396fa2..ec225f38 100644
--- a/objdiff-core/src/arch/arm64.rs
+++ b/objdiff-core/src/arch/arm64.rs
@@ -173,6 +173,21 @@ impl ObjArch for ObjArchArm64 {
             _ => Cow::Owned(format!("<{flags:?}>")),
         }
     }
+
+    fn get_reloc_byte_size(&self, flags: RelocationFlags) -> usize {
+        match flags {
+            RelocationFlags::Elf { r_type } => match r_type {
+                elf::R_AARCH64_ABS64 => 8,
+                elf::R_AARCH64_ABS32 => 4,
+                elf::R_AARCH64_ABS16 => 2,
+                elf::R_AARCH64_PREL64 => 8,
+                elf::R_AARCH64_PREL32 => 4,
+                elf::R_AARCH64_PREL16 => 2,
+                _ => 1,
+            },
+            _ => 1,
+        }
+    }
 }
 
 struct DisplayCtx<'a> {
diff --git a/objdiff-core/src/arch/mips.rs b/objdiff-core/src/arch/mips.rs
index 87b6bddc..2b89b23b 100644
--- a/objdiff-core/src/arch/mips.rs
+++ b/objdiff-core/src/arch/mips.rs
@@ -271,6 +271,17 @@ impl ObjArch for ObjArchMips {
             _ => Cow::Owned(format!("<{flags:?}>")),
         }
     }
+
+    fn get_reloc_byte_size(&self, flags: RelocationFlags) -> usize {
+        match flags {
+            RelocationFlags::Elf { r_type } => match r_type {
+                elf::R_MIPS_16 => 2,
+                elf::R_MIPS_32 => 4,
+                _ => 1,
+            },
+            _ => 1,
+        }
+    }
 }
 
 fn push_reloc(args: &mut Vec<ObjInsArg>, reloc: &ObjReloc) -> Result<()> {
diff --git a/objdiff-core/src/arch/mod.rs b/objdiff-core/src/arch/mod.rs
index 82579dd1..5135ae47 100644
--- a/objdiff-core/src/arch/mod.rs
+++ b/objdiff-core/src/arch/mod.rs
@@ -148,6 +148,8 @@ pub trait ObjArch: Send + Sync {
 
     fn display_reloc(&self, flags: RelocationFlags) -> Cow<'static, str>;
 
+    fn get_reloc_byte_size(&self, flags: RelocationFlags) -> usize;
+
     fn symbol_address(&self, symbol: &Symbol) -> u64 { symbol.address() }
 
     fn guess_data_type(&self, _instruction: &ObjIns) -> Option<DataType> { None }
diff --git a/objdiff-core/src/arch/ppc.rs b/objdiff-core/src/arch/ppc.rs
index 3768a530..d80eb3f6 100644
--- a/objdiff-core/src/arch/ppc.rs
+++ b/objdiff-core/src/arch/ppc.rs
@@ -193,6 +193,17 @@ impl ObjArch for ObjArchPpc {
         }
     }
 
+    fn get_reloc_byte_size(&self, flags: RelocationFlags) -> usize {
+        match flags {
+            RelocationFlags::Elf { r_type } => match r_type {
+                elf::R_PPC_ADDR32 => 4,
+                elf::R_PPC_UADDR32 => 4,
+                _ => 1,
+            },
+            _ => 1,
+        }
+    }
+
     fn guess_data_type(&self, instruction: &ObjIns) -> Option<super::DataType> {
         if instruction.reloc.as_ref().is_some_and(|r| r.target.name.starts_with("@stringBase")) {
             return Some(DataType::String);
diff --git a/objdiff-core/src/arch/x86.rs b/objdiff-core/src/arch/x86.rs
index 82399fb4..b292993f 100644
--- a/objdiff-core/src/arch/x86.rs
+++ b/objdiff-core/src/arch/x86.rs
@@ -162,6 +162,19 @@ impl ObjArch for ObjArchX86 {
             _ => Cow::Owned(format!("<{flags:?}>")),
         }
     }
+
+    fn get_reloc_byte_size(&self, flags: RelocationFlags) -> usize {
+        match flags {
+            RelocationFlags::Coff { typ } => match typ {
+                pe::IMAGE_REL_I386_DIR16 => 2,
+                pe::IMAGE_REL_I386_REL16 => 2,
+                pe::IMAGE_REL_I386_DIR32 => 4,
+                pe::IMAGE_REL_I386_REL32 => 4,
+                _ => 1,
+            },
+            _ => 1,
+        }
+    }
 }
 
 fn replace_arg(
diff --git a/objdiff-core/src/diff/data.rs b/objdiff-core/src/diff/data.rs
index 1c42ed53..ded4d6a4 100644
--- a/objdiff-core/src/diff/data.rs
+++ b/objdiff-core/src/diff/data.rs
@@ -205,7 +205,7 @@ pub fn diff_data_section(
                             ..Default::default()
                         });
                     }
-                    let reloc_diff_len = 4; // TODO don't hardcode
+                    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(),
@@ -229,7 +229,7 @@ pub fn diff_data_section(
                             ..Default::default()
                         });
                     }
-                    let reloc_diff_len = 4; // TODO don't hardcode
+                    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 {
@@ -372,22 +372,28 @@ pub fn diff_data_symbol(
     if !reloc_diffs.is_empty() {
         let mut total_reloc_bytes = 0;
         let mut matching_reloc_bytes = 0;
-        for (diff_kind, _, _) in reloc_diffs {
-            let reloc_diff_len: usize = 4; // TODO don't hardcode
+        for (diff_kind, left_reloc, right_reloc) in reloc_diffs {
+            let reloc_diff_len = match (left_reloc, right_reloc) {
+                (None, None) => unreachable!(),
+                (None, Some(right_reloc)) => right_obj.arch.get_reloc_byte_size(right_reloc.flags),
+                (Some(left_reloc), _) => left_obj.arch.get_reloc_byte_size(left_reloc.flags),
+            };
             total_reloc_bytes += reloc_diff_len;
             if diff_kind == ObjDataDiffKind::None {
                 matching_reloc_bytes += reloc_diff_len;
             }
         }
-        let relocs_match_ratio = matching_reloc_bytes as f32 / total_reloc_bytes as f32;
-        // Adjust the overall match ratio to include relocation differences.
-        // We calculate it so that bytes that contain a relocation are counted twice: once for the
-        // byte's raw value, and once for its relocation.
-        // e.g. An 8 byte symbol that has 8 matching raw bytes and a single 4 byte relocation that
-        // doesn't match would show as 66% (weighted average of 100% and 0%).
-        match_ratio = ((bytes_match_ratio * (left_data.len() as f32))
-            + (relocs_match_ratio * total_reloc_bytes as f32))
-            / (left_data.len() + total_reloc_bytes) as f32;
+        if total_reloc_bytes > 0 {
+            let relocs_match_ratio = matching_reloc_bytes as f32 / total_reloc_bytes as f32;
+            // Adjust the overall match ratio to include relocation differences.
+            // We calculate it so that bytes that contain a relocation are counted twice: once for the
+            // byte's raw value, and once for its relocation.
+            // e.g. An 8 byte symbol that has 8 matching raw bytes and a single 4 byte relocation that
+            // doesn't match would show as 66% (weighted average of 100% and 0%).
+            match_ratio = ((bytes_match_ratio * (left_data.len() as f32))
+                + (relocs_match_ratio * total_reloc_bytes as f32))
+                / (left_data.len() + total_reloc_bytes) as f32;
+        }
     }
 
     let match_percent = match_ratio * 100.0;

From b7ff0c9b4111f1576d1e26723335f4b7e63c74ca Mon Sep 17 00:00:00 2001
From: LagoLunatic <LagoLunatic@users.noreply.github.com>
Date: Wed, 15 Jan 2025 02:10:21 -0500
Subject: [PATCH 7/9] Data view: Add context menu for copying relocation target
 symbols

---
 objdiff-gui/src/views/data_diff.rs | 31 ++++++++++++++++++++++++++++--
 1 file changed, 29 insertions(+), 2 deletions(-)

diff --git a/objdiff-gui/src/views/data_diff.rs b/objdiff-gui/src/views/data_diff.rs
index 93778007..bca9bcb6 100644
--- a/objdiff-gui/src/views/data_diff.rs
+++ b/objdiff-gui/src/views/data_diff.rs
@@ -79,6 +79,32 @@ fn data_row_hover_ui(
     });
 }
 
+fn data_row_context_menu(ui: &mut egui::Ui, diffs: &[ObjDataDiff]) {
+    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 {
+                continue;
+            };
+
+            // TODO: This code is copy-pasted from ins_context_menu.
+            // Try to separate this out into a shared function.
+            if let Some(name) = &reloc.target.demangled_name {
+                if ui.button(format!("Copy \"{name}\"")).clicked() {
+                    ui.output_mut(|output| output.copied_text.clone_from(name));
+                    ui.close_menu();
+                }
+            }
+            if ui.button(format!("Copy \"{}\"", reloc.target.name)).clicked() {
+                ui.output_mut(|output| output.copied_text.clone_from(&reloc.target.name));
+                ui.close_menu();
+            }
+        }
+    });
+}
+
 fn data_row_ui(
     ui: &mut egui::Ui,
     obj: Option<&ObjInfo>,
@@ -159,8 +185,9 @@ fn data_row_ui(
 
     let response = Label::new(job).sense(Sense::click()).ui(ui);
     if let Some(obj) = obj {
-        response.on_hover_ui_at_pointer(|ui| data_row_hover_ui(ui, obj, diffs, appearance));
-        // .context_menu(|ui| data_row_context_menu(ui, ins)); // TODO
+        response
+            .on_hover_ui_at_pointer(|ui| data_row_hover_ui(ui, obj, diffs, appearance))
+            .context_menu(|ui| data_row_context_menu(ui, diffs));
     }
 }
 

From 4770583537d99926497294c2691ba18920cc0e14 Mon Sep 17 00:00:00 2001
From: LagoLunatic <LagoLunatic@users.noreply.github.com>
Date: Wed, 15 Jan 2025 02:33:24 -0500
Subject: [PATCH 8/9] Also show already-matching relocations on hover/right
 click

---
 objdiff-core/src/diff/data.rs | 9 +++------
 1 file changed, 3 insertions(+), 6 deletions(-)

diff --git a/objdiff-core/src/diff/data.rs b/objdiff-core/src/diff/data.rs
index ded4d6a4..35d6e761 100644
--- a/objdiff-core/src/diff/data.rs
+++ b/objdiff-core/src/diff/data.rs
@@ -177,7 +177,7 @@ pub fn diff_data_section(
             }
         };
         if kind == ObjDataDiffKind::None {
-            let mut found_any_reloc_diffs = false;
+            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(
@@ -188,10 +188,7 @@ pub fn diff_data_section(
                 left_range.clone(),
                 right_range.clone(),
             ) {
-                if diff_kind == ObjDataDiffKind::None {
-                    continue;
-                }
-                found_any_reloc_diffs = true;
+                found_any_relocs = true;
 
                 if let Some(left_reloc) = left_reloc {
                     let left_reloc_addr = left_reloc.address as usize;
@@ -243,7 +240,7 @@ pub fn diff_data_section(
                 }
             }
 
-            if found_any_reloc_diffs {
+            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];

From 161dadd21325f44a7d55597d7a916f75fa0d925a Mon Sep 17 00:00:00 2001
From: LagoLunatic <LagoLunatic@users.noreply.github.com>
Date: Wed, 15 Jan 2025 16:35:26 -0500
Subject: [PATCH 9/9] Change font color for nonmatching relocs on hover

---
 objdiff-gui/src/views/data_diff.rs | 25 +++++++++++--------------
 1 file changed, 11 insertions(+), 14 deletions(-)

diff --git a/objdiff-gui/src/views/data_diff.rs b/objdiff-gui/src/views/data_diff.rs
index bca9bcb6..d83eeab0 100644
--- a/objdiff-gui/src/views/data_diff.rs
+++ b/objdiff-gui/src/views/data_diff.rs
@@ -42,6 +42,12 @@ fn data_row_hover_ui(
                 continue;
             };
 
+            // 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,
+            };
+
             // 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)));
@@ -50,30 +56,21 @@ fn data_row_hover_ui(
                 Ordering::Less => format!("-{:x}", -reloc.addend),
                 _ => "".to_string(),
             };
-            ui.colored_label(
-                appearance.highlight_color,
-                format!("Name: {}{}", reloc.target.name, addend_str),
-            );
+            ui.colored_label(color, format!("Name: {}{}", reloc.target.name, addend_str));
             if let Some(orig_section_index) = reloc.target.orig_section_index {
                 if let Some(section) =
                     obj.sections.iter().find(|s| s.orig_index == orig_section_index)
                 {
-                    ui.colored_label(
-                        appearance.highlight_color,
-                        format!("Section: {}", section.name),
-                    );
+                    ui.colored_label(color, format!("Section: {}", section.name));
                 }
                 ui.colored_label(
-                    appearance.highlight_color,
+                    color,
                     format!("Address: {:x}{}", reloc.target.address, addend_str),
                 );
-                ui.colored_label(
-                    appearance.highlight_color,
-                    format!("Size: {:x}", reloc.target.size),
-                );
+                ui.colored_label(color, format!("Size: {:x}", reloc.target.size));
                 if reloc.addend >= 0 && reloc.target.bytes.len() > reloc.addend as usize {}
             } else {
-                ui.colored_label(appearance.highlight_color, "Extern".to_string());
+                ui.colored_label(color, "Extern".to_string());
             }
         }
     });