Reimplement function data value tooltips, function data value diffing, and data relocation diffing

objdiff-core/src/diff/code.rs

@@ -8,12 +8,13 @@ use alloc::{
 use anyhow::{anyhow, ensure, Context, Result};
 
 use super::{
-    DiffObjConfig, FunctionRelocDiffs, InstructionArgDiffIndex, InstructionBranchFrom,
-    InstructionBranchTo, InstructionDiffKind, InstructionDiffRow, SymbolDiff,
+    display::display_ins_data_literals, DiffObjConfig, FunctionRelocDiffs, InstructionArgDiffIndex,
+    InstructionBranchFrom, InstructionBranchTo, InstructionDiffKind, InstructionDiffRow,
+    SymbolDiff,
 };
 use crate::obj::{
-    InstructionArg, InstructionArgValue, InstructionRef, Object, ResolvedRelocation,
-    ScannedInstruction, SymbolFlag, SymbolKind,
+    InstructionArg, InstructionArgValue, InstructionRef, Object, ResolvedInstructionRef,
+    ResolvedRelocation, ScannedInstruction, SymbolFlag, SymbolKind,
 };
 
 pub fn no_diff_code(
@@ -291,12 +292,12 @@ pub(crate) fn section_name_eq(
 fn reloc_eq(
     left_obj: &Object,
     right_obj: &Object,
-    left_reloc: Option<ResolvedRelocation>,
-    right_reloc: Option<ResolvedRelocation>,
+    left_ins: ResolvedInstructionRef,
+    right_ins: ResolvedInstructionRef,
     diff_config: &DiffObjConfig,
 ) -> bool {
     let relax_reloc_diffs = diff_config.function_reloc_diffs == FunctionRelocDiffs::None;
-    let (left_reloc, right_reloc) = match (left_reloc, right_reloc) {
+    let (left_reloc, right_reloc) = match (left_ins.relocation, right_ins.relocation) {
         (Some(left_reloc), Some(right_reloc)) => (left_reloc, right_reloc),
         // If relocations are relaxed, match if left is missing a reloc
         (None, Some(_)) => return relax_reloc_diffs,
@@ -319,13 +320,10 @@ fn reloc_eq(
                 && (diff_config.function_reloc_diffs == FunctionRelocDiffs::DataValue
                     || symbol_name_addend_matches
                     || address_eq(left_reloc, right_reloc))
-                && (
-                    diff_config.function_reloc_diffs == FunctionRelocDiffs::NameAddress
-                        || left_reloc.symbol.kind != SymbolKind::Object
-                    // TODO
-                    // || left_obj.arch.display_ins_data_labels(left_ins)
-                    //     == left_obj.arch.display_ins_data_labels(right_ins))
-                )
+                && (diff_config.function_reloc_diffs == FunctionRelocDiffs::NameAddress
+                    || left_reloc.symbol.kind != SymbolKind::Object
+                    || display_ins_data_literals(left_obj, left_ins)
+                        == display_ins_data_literals(right_obj, right_ins))
         }
         (Some(_), None) => false,
         (None, Some(_)) => {
@@ -343,8 +341,8 @@ fn arg_eq(
     right_row: &InstructionDiffRow,
     left_arg: &InstructionArg,
     right_arg: &InstructionArg,
-    left_reloc: Option<ResolvedRelocation>,
-    right_reloc: Option<ResolvedRelocation>,
+    left_ins: ResolvedInstructionRef,
+    right_ins: ResolvedInstructionRef,
     diff_config: &DiffObjConfig,
 ) -> bool {
     match left_arg {
@@ -357,7 +355,7 @@ fn arg_eq(
         },
         InstructionArg::Reloc => {
             matches!(right_arg, InstructionArg::Reloc)
-                && reloc_eq(left_obj, right_obj, left_reloc, right_reloc, diff_config)
+                && reloc_eq(left_obj, right_obj, left_ins, right_ins, diff_config)
         }
         InstructionArg::BranchDest(_) => match right_arg {
             // Compare dest instruction idx after diffing
@@ -434,8 +432,10 @@ fn diff_instruction(
         .resolve_instruction_ref(right_symbol_idx, r)
         .context("Failed to resolve right instruction")?;
 
-    if left_resolved.code != right_resolved.code {
-        // If data doesn't match, process instructions and compare args
+    if left_resolved.code != right_resolved.code
+        || !reloc_eq(left_obj, right_obj, left_resolved, right_resolved, diff_config)
+    {
+        // If either the raw code bytes or relocations don't match, process instructions and compare args
         let left_ins = left_obj.arch.process_instruction(left_resolved, diff_config)?;
         let right_ins = left_obj.arch.process_instruction(right_resolved, diff_config)?;
         if left_ins.args.len() != right_ins.args.len() {
@@ -455,8 +455,8 @@ fn diff_instruction(
                 right_row,
                 a,
                 b,
-                left_resolved.relocation,
-                right_resolved.relocation,
+                left_resolved,
+                right_resolved,
                 diff_config,
             ) {
                 result.left_args_diff.push(InstructionArgDiffIndex::NONE);
@@ -500,19 +500,6 @@ fn diff_instruction(
         return Ok(result);
     }
 
-    // Compare relocations
-    if !reloc_eq(
-        left_obj,
-        right_obj,
-        left_resolved.relocation,
-        right_resolved.relocation,
-        diff_config,
-    ) {
-        state.diff_score += PENALTY_REG_DIFF;
-        // TODO add relocation diff to args
-        return Ok(InstructionDiffResult::new(InstructionDiffKind::ArgMismatch));
-    }
-
     Ok(InstructionDiffResult::new(InstructionDiffKind::None))
 }
 

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> {
@@ -72,10 +72,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 +250,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,
+            });
         }
     }
 

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,17 @@ 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.append(&mut relocation_hover(obj, reloc));
         out.push(HoverItem::Separator);
-        out.append(&mut symbol_hover(obj, reloc.relocation.target_symbol, reloc.relocation.addend));
+        if let Some(ty) = obj.arch.guess_data_type(resolved) {
+            for literal in display_ins_data_literals(obj, resolved) {
+                out.push(HoverItem::Text {
+                    label: format!("{}", ty),
+                    value: literal,
+                    color: HoverItemColor::Normal,
+                });
+            }
+        }
     }
     out
 }

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>,
 }