Data diff view: Show relocs on hover and in context menu

LagoLunatic · LagoLunatic · commit 9bc6c45afc68 · 2025-03-04T02:11:58.000-05:00
This reimplements #154 Note that colorizing the text depending on the kind of diff has still not been reimplemented yet
diff --git a/objdiff-core/src/diff/data.rs b/objdiff-core/src/diff/data.rs
@@ -63,7 +63,7 @@ fn reloc_eq(
 }
 
 #[inline]
-fn resolve_relocation<'obj>(
+pub fn resolve_relocation<'obj>(
     obj: &'obj Object,
     reloc: &'obj Relocation,
 ) -> ResolvedRelocation<'obj> {
@@ -74,8 +74,6 @@ fn resolve_relocation<'obj>(
 /// Compares relocations contained with a certain data range.
 /// The DataDiffKind 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, 'right>(
     left_obj: &'left Object,
     right_obj: &'right Object,
@@ -254,13 +252,21 @@ pub fn diff_data_section(
             let len = left_obj.arch.data_reloc_size(left_reloc.relocation.flags);
             let range = left_reloc.relocation.address as usize
                 ..left_reloc.relocation.address as usize + len;
-            left_reloc_diffs.push(DataRelocationDiff { kind: diff_kind, range });
+            left_reloc_diffs.push(DataRelocationDiff {
+                reloc: left_reloc.relocation.clone(),
+                kind: diff_kind,
+                range,
+            });
         }
         if let Some(right_reloc) = right_reloc {
             let len = right_obj.arch.data_reloc_size(right_reloc.relocation.flags);
             let range = right_reloc.relocation.address as usize
                 ..right_reloc.relocation.address as usize + len;
-            right_reloc_diffs.push(DataRelocationDiff { kind: diff_kind, range });
+            right_reloc_diffs.push(DataRelocationDiff {
+                reloc: right_reloc.relocation.clone(),
+                kind: diff_kind,
+                range,
+            });
         }
     }
 
diff --git a/objdiff-core/src/diff/display.rs b/objdiff-core/src/diff/display.rs
@@ -15,7 +15,7 @@ use crate::{
     diff::{DiffObjConfig, InstructionDiffKind, InstructionDiffRow, ObjectDiff, SymbolDiff},
     obj::{
         InstructionArg, InstructionArgValue, Object, ParsedInstruction, ResolvedInstructionRef,
-        SectionFlag, SectionKind, Symbol, SymbolFlag, SymbolKind,
+        ResolvedRelocation, SectionFlag, SectionKind, Symbol, SymbolFlag, SymbolKind,
     },
 };
 
@@ -424,6 +424,42 @@ pub fn symbol_hover(obj: &Object, symbol_index: usize, addend: i64) -> Vec<Hover
     out
 }
 
+pub fn relocation_context(
+    obj: &Object,
+    reloc: ResolvedRelocation,
+    ins: Option<ResolvedInstructionRef>,
+) -> Vec<ContextItem> {
+    let mut out = Vec::new();
+    if let Some(ins) = ins {
+        for literal in display_ins_data_literals(obj, ins) {
+            out.push(ContextItem::Copy { value: literal, label: None });
+        }
+        out.push(ContextItem::Separator);
+    }
+    out.append(&mut symbol_context(obj, reloc.relocation.target_symbol));
+    out
+}
+
+pub fn relocation_hover(obj: &Object, reloc: ResolvedRelocation) -> Vec<HoverItem> {
+    let mut out = Vec::new();
+    if let Some(name) = obj.arch.reloc_name(reloc.relocation.flags) {
+        out.push(HoverItem::Text {
+            label: "Relocation type".into(),
+            value: name.to_string(),
+            color: HoverItemColor::Normal,
+        });
+    } else {
+        out.push(HoverItem::Text {
+            label: "Relocation type".into(),
+            value: format!("<{:?}>", reloc.relocation.flags),
+            color: HoverItemColor::Normal,
+        });
+    }
+    out.push(HoverItem::Separator);
+    out.append(&mut symbol_hover(obj, reloc.relocation.target_symbol, reloc.relocation.addend));
+    out
+}
+
 pub fn instruction_context(
     obj: &Object,
     resolved: ResolvedInstructionRef,
@@ -458,11 +494,7 @@ pub fn instruction_context(
         }
     }
     if let Some(reloc) = resolved.relocation {
-        for literal in display_ins_data_literals(obj, resolved) {
-            out.push(ContextItem::Copy { value: literal, label: None });
-        }
-        out.push(ContextItem::Separator);
-        out.append(&mut symbol_context(obj, reloc.relocation.target_symbol));
+        out.append(&mut relocation_context(obj, reloc, Some(resolved)));
     }
     out
 }
@@ -509,21 +541,7 @@ pub fn instruction_hover(
         }
     }
     if let Some(reloc) = resolved.relocation {
-        if let Some(name) = obj.arch.reloc_name(reloc.relocation.flags) {
-            out.push(HoverItem::Text {
-                label: "Relocation type".into(),
-                value: name.to_string(),
-                color: HoverItemColor::Normal,
-            });
-        } else {
-            out.push(HoverItem::Text {
-                label: "Relocation type".into(),
-                value: format!("<{:?}>", reloc.relocation.flags),
-                color: HoverItemColor::Normal,
-            });
-        }
-        out.push(HoverItem::Separator);
-        out.append(&mut symbol_hover(obj, reloc.relocation.target_symbol, reloc.relocation.addend));
+        out.append(&mut relocation_hover(obj, reloc));
         out.push(HoverItem::Separator);
         if let Some(ty) = obj.arch.guess_data_type(resolved) {
             for literal in display_ins_data_literals(obj, resolved) {
diff --git a/objdiff-core/src/diff/mod.rs b/objdiff-core/src/diff/mod.rs
@@ -16,7 +16,7 @@ use crate::{
             diff_generic_section,
         },
     },
-    obj::{InstructionRef, Object, SectionKind, Symbol, SymbolFlag},
+    obj::{InstructionRef, Object, Relocation, SectionKind, Symbol, SymbolFlag},
 };
 
 pub mod code;
@@ -93,6 +93,7 @@ pub struct DataDiff {
 
 #[derive(Debug, Clone)]
 pub struct DataRelocationDiff {
+    pub reloc: Relocation,
     pub kind: DataDiffKind,
     pub range: Range<usize>,
 }
diff --git a/objdiff-gui/src/views/data_diff.rs b/objdiff-gui/src/views/data_diff.rs
@@ -2,99 +2,89 @@ use std::{cmp::min, default::Default, mem::take};
 
 use egui::{text::LayoutJob, Label, Sense, Widget};
 use objdiff_core::{
-    diff::{DataDiff, DataDiffKind, DataRelocationDiff},
+    diff::{
+        data::resolve_relocation,
+        display::{relocation_context, relocation_hover, ContextItem, HoverItem},
+        DataDiff, DataDiffKind, DataRelocationDiff,
+    },
     obj::Object,
 };
 
+use super::diff::{context_menu_items_ui, hover_items_ui};
 use crate::views::{appearance::Appearance, write_text};
 
 pub(crate) const BYTES_PER_ROW: usize = 16;
 
+fn data_row_hover(obj: &Object, diffs: &[(DataDiff, Vec<DataRelocationDiff>)]) -> Vec<HoverItem> {
+    let mut out = Vec::new();
+    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: Change hover text color depending on Insert/Delete/Replace kind
+        // let color = get_color_for_diff_kind(reloc_diff.kind, appearance);
+
+        let reloc = resolve_relocation(obj, reloc);
+        out.append(&mut relocation_hover(obj, reloc));
+    }
+    out
+}
+
+fn data_row_context(
+    obj: &Object,
+    diffs: &[(DataDiff, Vec<DataRelocationDiff>)],
+) -> Vec<ContextItem> {
+    let mut out = Vec::new();
+    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);
+
+        let reloc = resolve_relocation(obj, reloc);
+        out.append(&mut relocation_context(obj, reloc, None));
+    }
+    out
+}
+
 fn data_row_hover_ui(
     ui: &mut egui::Ui,
-    _obj: &Object,
-    _diffs: &[(DataDiff, Vec<DataRelocationDiff>)],
-    _appearance: &Appearance,
+    obj: &Object,
+    diffs: &[(DataDiff, Vec<DataRelocationDiff>)],
+    appearance: &Appearance,
 ) {
     ui.scope(|ui| {
         ui.style_mut().override_text_style = Some(egui::TextStyle::Monospace);
         ui.style_mut().wrap_mode = Some(egui::TextWrapMode::Extend);
-
-        // TODO
-        // 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);
-        //
-        //     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),
-        //         _ => "".to_string(),
-        //     };
-        //     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(color, format!("Section: {}", section.name));
-        //         }
-        //         ui.colored_label(
-        //             color,
-        //             format!("Address: {:x}{}", reloc.target.address, addend_str),
-        //         );
-        //         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(color, "Extern".to_string());
-        //     }
-        // }
+        hover_items_ui(ui, data_row_hover(obj, diffs), appearance);
     });
 }
 
-fn data_row_context_menu(ui: &mut egui::Ui, _diffs: &[(DataDiff, Vec<DataRelocationDiff>)]) {
+fn data_row_context_menu(
+    ui: &mut egui::Ui,
+    obj: &Object,
+    diffs: &[(DataDiff, Vec<DataRelocationDiff>)],
+    column: usize,
+    appearance: &Appearance,
+) {
     ui.scope(|ui| {
         ui.style_mut().override_text_style = Some(egui::TextStyle::Monospace);
         ui.style_mut().wrap_mode = Some(egui::TextWrapMode::Extend);
-
-        // TODO
-        // 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.
-        //     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();
-        //     }
-        // }
+        context_menu_items_ui(ui, data_row_context(obj, diffs), column, appearance);
     });
 }
 
@@ -113,6 +103,7 @@ pub(crate) fn data_row_ui(
     address: usize,
     diffs: &[(DataDiff, Vec<DataRelocationDiff>)],
     appearance: &Appearance,
+    column: usize,
 ) {
     if diffs.iter().any(|(dd, rds)| {
         dd.kind != DataDiffKind::None || rds.iter().any(|rd| rd.kind != DataDiffKind::None)
@@ -191,9 +182,8 @@ pub(crate) 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, diffs));
+        response.context_menu(|ui| data_row_context_menu(ui, obj, diffs, column, appearance));
+        response.on_hover_ui_at_pointer(|ui| data_row_hover_ui(ui, obj, diffs, appearance));
     }
 }
 
diff --git a/objdiff-gui/src/views/diff.rs b/objdiff-gui/src/views/diff.rs
@@ -525,9 +525,23 @@ pub fn diff_view_ui(
                     let address = i * BYTES_PER_ROW;
                     row.col(|ui| {
                         if column == 0 {
-                            data_row_ui(ui, Some(left_obj), address, &left_diffs[i], appearance);
+                            data_row_ui(
+                                ui,
+                                Some(left_obj),
+                                address,
+                                &left_diffs[i],
+                                appearance,
+                                column,
+                            );
                         } else if column == 1 {
-                            data_row_ui(ui, Some(right_obj), address, &right_diffs[i], appearance);
+                            data_row_ui(
+                                ui,
+                                Some(right_obj),
+                                address,
+                                &right_diffs[i],
+                                appearance,
+                                column,
+                            );
                         }
                     });
                 },
@@ -667,7 +681,7 @@ fn diff_col_ui(
                     let i = row.index();
                     let address = i * BYTES_PER_ROW;
                     row.col(|ui| {
-                        data_row_ui(ui, Some(obj), address, &diffs[i], appearance);
+                        data_row_ui(ui, Some(obj), address, &diffs[i], appearance, column);
                     });
                 },
             );
