Show relocation diffs in function view when the data's content differs (#153)

* Show reloc diff in func view when data content differs

* Add "Relax shifted data diffs" option

* Display fake pool relocations at end of line

* Diff reloc data by display string instead of raw bytes

This is to handle data symbols that contain multiple values in them at once, such as stringBase. If you compare the target symbol's bytes directly, then any part of the symbol having different bytes will cause *all* relocations to that symbol to show as a diff, even if the specific string being accessed is the same.

* Fix weak stripped symbols showing as a false diff

Fixed this by showing extern symbols correctly instead of skipping them.

* Add "Relax shifted data diffs" option to objdiff-cli

Includes both a command line argument and a keyboard shortcut (S).

* Remove addi string data hack and ... pool name hack

* Clippy fix

* PPC: Clear relocs from GPRs when overwritten

* PPC: Follow branches to improve pool detection accuracy

* PPC: Handle following bctr jump table control flow

* Clippy fixes

* PPC: Fix extern relocations not having their addend copied

* Add option to disable func data value diffing

* PPC: Handle lmw when clearing GPRs

* PPC: Handle moving reloc address with `add` inst

* Combine "relax reloc diffs" with other reloc diff options

* Add v3 config and migrate from v2

---------

Co-authored-by: Luke Street <luke@street.dev>

objdiff-cli/src/cmd/report.rs

@@ -169,7 +169,10 @@ fn report_object(
         }
         _ => {}
     }
-    let diff_config = diff::DiffObjConfig { relax_reloc_diffs: true, ..Default::default() };
+    let diff_config = diff::DiffObjConfig {
+        function_reloc_diffs: diff::FunctionRelocDiffs::None,
+        ..Default::default()
+    };
     let mapping_config = diff::MappingConfig::default();
     let target = object
         .target_path

objdiff-cli/src/views/function_diff.rs

@@ -3,7 +3,7 @@ use crossterm::event::{Event, KeyCode, KeyEventKind, KeyModifiers, MouseButton,
 use objdiff_core::{
     diff::{
         display::{display_diff, DiffText, HighlightKind},
-        ObjDiff, ObjInsDiffKind, ObjSymbolDiff,
+        FunctionRelocDiffs, ObjDiff, ObjInsDiffKind, ObjSymbolDiff,
     },
     obj::{ObjInfo, ObjSectionKind, ObjSymbol, SymbolRef},
 };
@@ -368,10 +368,15 @@ impl UiView for FunctionDiffUi {
                         self.scroll_x = self.scroll_x.saturating_sub(1);
                         result.redraw = true;
                     }
-                    // Toggle relax relocation diffs
+                    // Cycle through function relocation diff mode
                     KeyCode::Char('x') => {
-                        state.diff_obj_config.relax_reloc_diffs =
+                        state.diff_obj_config.function_reloc_diffs =
-                            !state.diff_obj_config.relax_reloc_diffs;
+                            match state.diff_obj_config.function_reloc_diffs {
+                                FunctionRelocDiffs::None => FunctionRelocDiffs::NameAddress,
+                                FunctionRelocDiffs::NameAddress => FunctionRelocDiffs::DataValue,
+                                FunctionRelocDiffs::DataValue => FunctionRelocDiffs::All,
+                                FunctionRelocDiffs::All => FunctionRelocDiffs::None,
+                            };
                         result.redraw = true;
                         return EventControlFlow::Reload;
                     }

objdiff-core/config-schema.json

@@ -1,11 +1,29 @@
 {
   "properties": [
     {
-      "id": "relaxRelocDiffs",
+      "id": "functionRelocDiffs",
-      "type": "boolean",
+      "type": "choice",
-      "default": false,
+      "default": "name_address",
-      "name": "Relax relocation diffs",
+      "name": "Function relocation diffs",
-      "description": "Ignores differences in relocation targets. (Address, name, etc)"
+      "description": "How relocation targets will be diffed in the function view.",
+      "items": [
+        {
+          "value": "none",
+          "name": "None"
+        },
+        {
+          "value": "name_address",
+          "name": "Name or address"
+        },
+        {
+          "value": "data_value",
+          "name": "Data value"
+        },
+        {
+          "value": "all",
+          "name": "Name or address, data value"
+        }
+      ]
     },
     {
       "id": "spaceBetweenArgs",
@@ -193,7 +211,7 @@
       "id": "general",
       "name": "General",
       "properties": [
-        "relaxRelocDiffs",
+        "functionRelocDiffs",
         "spaceBetweenArgs",
         "combineDataSections"
       ]

objdiff-core/src/arch/mod.rs

@@ -156,6 +156,17 @@ pub trait ObjArch: Send + Sync {
         Some(format!("Bytes: {:#x?}", bytes))
     }
 
+    fn display_ins_data(&self, ins: &ObjIns) -> Option<String> {
+        let reloc = ins.reloc.as_ref()?;
+        if reloc.addend >= 0 && reloc.target.bytes.len() > reloc.addend as usize {
+            self.guess_data_type(ins).and_then(|ty| {
+                self.display_data_type(ty, &reloc.target.bytes[reloc.addend as usize..])
+            })
+        } else {
+            None
+        }
+    }
+
     // Downcast methods
     #[cfg(feature = "ppc")]
     fn ppc(&self) -> Option<&ppc::ObjArchPpc> { None }

objdiff-core/src/arch/ppc.rs

@@ -1,6 +1,6 @@
 use std::{
     borrow::Cow,
-    collections::{BTreeMap, HashMap},
+    collections::{BTreeMap, HashMap, HashSet},
 };
 
 use anyhow::{bail, ensure, Result};
@@ -10,7 +10,7 @@ use object::{
     elf, File, Object, ObjectSection, ObjectSymbol, Relocation, RelocationFlags, RelocationTarget,
     Symbol, SymbolKind,
 };
-use ppc750cl::{Argument, InsIter, Opcode, ParsedIns, GPR};
+use ppc750cl::{Argument, Arguments, Ins, InsIter, Opcode, ParsedIns, GPR};
 
 use crate::{
     arch::{DataType, ObjArch, ProcessCodeResult},
@@ -143,6 +143,15 @@ impl ObjArch for ObjArchPpc {
                 }
             }
 
+            if reloc.is_none() {
+                if let Some(fake_pool_reloc) = fake_pool_reloc_for_addr.get(&cur_addr) {
+                    // If this instruction has a fake pool relocation, show it as a fake argument
+                    // at the end of the line.
+                    args.push(ObjInsArg::PlainText(" ".into()));
+                    push_reloc(&mut args, fake_pool_reloc)?;
+                }
+            }
+
             ops.push(ins.op as u16);
             let line = line_info.range(..=cur_addr as u64).last().map(|(_, &b)| b);
             insts.push(ObjIns {
@@ -198,19 +207,7 @@ impl ObjArch for ObjArchPpc {
             return Some(DataType::String);
         }
 
-        let op = Opcode::from(instruction.op as u8);
+        guess_data_type_from_load_store_inst_op(Opcode::from(instruction.op as u8))
-        if let Some(ty) = guess_data_type_from_load_store_inst_op(op) {
-            Some(ty)
-        } else if op == Opcode::Addi {
-            // Assume that any addi instruction that references a local symbol is loading a string.
-            // This hack is not ideal and results in tons of false positives where it will show
-            // garbage strings (e.g. misinterpreting arrays, float literals, etc).
-            // But not all strings are in the @stringBase pool, so the condition above that checks
-            // the target symbol name would miss some.
-            Some(DataType::String)
-        } else {
-            None
-        }
     }
 
     fn display_data_type(&self, ty: DataType, bytes: &[u8]) -> Option<String> {
@@ -248,6 +245,12 @@ fn push_reloc(args: &mut Vec<ObjInsArg>, reloc: &ObjReloc) -> Result<()> {
             elf::R_PPC_ADDR32 | elf::R_PPC_UADDR32 | elf::R_PPC_REL24 | elf::R_PPC_REL14 => {
                 args.push(ObjInsArg::Reloc);
             }
+            elf::R_PPC_NONE => {
+                // Fake pool relocation.
+                args.push(ObjInsArg::PlainText("<".into()));
+                args.push(ObjInsArg::Reloc);
+                args.push(ObjInsArg::PlainText(">".into()));
+            }
             _ => bail!("Unsupported ELF PPC relocation type {r_type}"),
         },
         flags => bail!("Unsupported PPC relocation kind: {flags:?}"),
@@ -442,16 +445,43 @@ fn get_offset_and_addr_gpr_for_possible_pool_reference(
                 Argument::Simm(simm),
             ) => Some((simm.0, addr_src_gpr, Some(addr_dst_gpr))),
             (
+                // `mr` or `mr.`
                 Opcode::Or,
                 Argument::GPR(addr_dst_gpr),
                 Argument::GPR(addr_src_gpr),
                 Argument::None,
-            ) => Some((0, addr_src_gpr, Some(addr_dst_gpr))), // `mr` or `mr.`
+            ) => Some((0, addr_src_gpr, Some(addr_dst_gpr))),
+            (
+                Opcode::Add,
+                Argument::GPR(addr_dst_gpr),
+                Argument::GPR(addr_src_gpr),
+                Argument::GPR(_offset_gpr),
+            ) => Some((0, addr_src_gpr, Some(addr_dst_gpr))),
             _ => None,
         }
     }
 }
 
+// Remove the relocation we're keeping track of in a particular register when an instruction reuses
+// that register to hold some other value, unrelated to pool relocation addresses.
+fn clear_overwritten_gprs(ins: Ins, gpr_pool_relocs: &mut HashMap<u8, ObjReloc>) {
+    let mut def_args = Arguments::default();
+    ins.parse_defs(&mut def_args);
+    for arg in def_args {
+        if let Argument::GPR(gpr) = arg {
+            if ins.op == Opcode::Lmw {
+                // `lmw` overwrites all registers from rd to r31.
+                // ppc750cl only returns rd itself, so we manually clear the rest of them.
+                for reg in gpr.0..31 {
+                    gpr_pool_relocs.remove(&reg);
+                }
+                break;
+            }
+            gpr_pool_relocs.remove(&gpr.0);
+        }
+    }
+}
+
 // We create a fake relocation for an instruction, vaguely simulating what the actual relocation
 // might have looked like if it wasn't pooled. This is so minimal changes are needed to display
 // pooled accesses vs non-pooled accesses. We set the relocation type to R_PPC_NONE to indicate that
@@ -461,14 +491,17 @@ fn get_offset_and_addr_gpr_for_possible_pool_reference(
 fn make_fake_pool_reloc(offset: i16, cur_addr: u32, pool_reloc: &ObjReloc) -> Option<ObjReloc> {
     let offset_from_pool = pool_reloc.addend + offset as i64;
     let target_address = pool_reloc.target.address.checked_add_signed(offset_from_pool)?;
-    let orig_section_index = pool_reloc.target.orig_section_index?;
+    let target;
-    // We also need to create a fake target symbol to go inside our fake relocation.
+    let addend;
-    // This is because we don't have access to list of all symbols in this section, so we can't find
+    if pool_reloc.target.orig_section_index.is_some() {
-    // the real symbol yet. Instead we make a placeholder that has the correct `orig_section_index`
+        // If the target symbol is within this current object, then we also need to create a fake
-    // and `address` fields, and then later on when this information is displayed to the user, we
+        // target symbol to go inside our fake relocation. This is because we don't have access to
-    // can find the real symbol by searching through the object's section's symbols for one that
+        // list of all symbols in this section, so we can't find the real symbol within the pool
-    // contains this address.
+        // based on its address yet. Instead we make a placeholder that has the correct
-    let fake_target_symbol = ObjSymbol {
+        // `orig_section_index` and `address` fields, and then later on when this information is
+        // displayed to the user, we can find the real symbol by searching through the object's
+        // section's symbols for one that contains this address.
+        target = ObjSymbol {
             name: "".to_string(),
             demangled_name: None,
             address: target_address,
@@ -477,19 +510,32 @@ fn make_fake_pool_reloc(offset: i16, cur_addr: u32, pool_reloc: &ObjReloc) -> Op
             size_known: false,
             kind: Default::default(),
             flags: Default::default(),
-        orig_section_index: Some(orig_section_index),
+            orig_section_index: pool_reloc.target.orig_section_index,
             virtual_address: None,
             original_index: None,
             bytes: vec![],
         };
         // The addend is also fake because we don't know yet if the `target_address` here is the exact
         // start of the symbol or if it's in the middle of it.
-    let fake_addend = 0;
+        addend = 0;
+    } else {
+        // But if the target symbol is in a different object (extern), then we simply copy the pool
+        // relocation's target. This is because it won't be possible to locate the actual symbol
+        // later on based only off of an offset without knowing the object or section it's in. And
+        // doing that for external symbols would also be unnecessary, because when the compiler
+        // generates an instruction that accesses an external "pool" plus some offset, that won't be
+        // a normal pool that contains other symbols within it that we want to display. It will be
+        // something like a vtable for a class with multiple inheritance (for example, dCcD_Cyl in
+        // The Wind Waker). So just showing that vtable symbol plus an addend to represent the
+        // offset into it works fine in this case, no fake symbol to hold an address is necessary.
+        target = pool_reloc.target.clone();
+        addend = pool_reloc.addend;
+    };
     Some(ObjReloc {
         flags: RelocationFlags::Elf { r_type: elf::R_PPC_NONE },
         address: cur_addr as u64,
-        target: fake_target_symbol,
+        target,
-        addend: fake_addend,
+        addend,
     })
 }
 
@@ -497,64 +543,137 @@ fn make_fake_pool_reloc(offset: i16, cur_addr: u32, pool_reloc: &ObjReloc) -> Op
 // of pooled data relocations in them, finding which instructions load data from those addresses,
 // and constructing a mapping of the address of that instruction to a "fake pool relocation" that
 // simulates what that instruction's relocation would look like if data hadn't been pooled.
-// Limitations: This method currently only goes through the instructions in a function in linear
+// This method tries to follow the function's proper control flow. It keeps track of a queue of
-// order, from start to finish. It does *not* follow any branches. This means that it could have
+// states it hasn't traversed yet, where each state holds an instruction address and a HashMap of
-// false positives or false negatives in determining which relocation is currently loaded in which
+// which registers hold which pool relocations at that point.
-// register at any given point in the function, as control flow is not respected.
+// When a conditional or unconditional branch is encountered, the destination of the branch is added
-// There are currently no known examples of this method producing inaccurate results in reality, but
+// to the queue. Conditional branches will traverse both the path where the branch is taken and the
-// if examples are found, it may be possible to update this method to also follow all branches so
+// one where it's not. Unconditional branches only follow the branch, ignoring any code immediately
-// that it produces more accurate results.
+// after the branch instruction.
+// Limitations: This method cannot read jump tables. This is because the jump tables are located in
+// the .data section, but ObjArch.process_code only has access to the .text section. In order to
+// work around this limitation and avoid completely missing most code inside switch statements that
+// use jump tables, we instead guess that any parts of a function we missed were switch cases, and
+// traverse them as if the last `bctr` before that address had branched there. This should be fairly
+// accurate in practice - in testing the only instructions it seems to miss are double branches that
+// the compiler generates in error which can never be reached during normal execution anyway.
 fn generate_fake_pool_reloc_for_addr_mapping(
-    address: u64,
+    func_address: u64,
     code: &[u8],
     relocations: &[ObjReloc],
 ) -> HashMap<u32, ObjReloc> {
-    let mut active_pool_relocs = HashMap::new();
+    let mut visited_ins_addrs = HashSet::new();
     let mut pool_reloc_for_addr = HashMap::new();
-    for (cur_addr, ins) in InsIter::new(code, address as u32) {
+    let mut ins_iters_with_gpr_state =
-        let simplified = ins.simplified();
+        vec![(InsIter::new(code, func_address as u32), HashMap::new())];
-        let reloc = relocations.iter().find(|r| (r.address as u32 & !3) == cur_addr);
+    let mut gpr_state_at_bctr = BTreeMap::new();
+    while let Some((ins_iter, mut gpr_pool_relocs)) = ins_iters_with_gpr_state.pop() {
+        for (cur_addr, ins) in ins_iter {
+            if visited_ins_addrs.contains(&cur_addr) {
+                // Avoid getting stuck in an infinite loop when following looping branches.
+                break;
+            }
+            visited_ins_addrs.insert(cur_addr);
 
+            let simplified = ins.simplified();
+
+            // First handle traversing the function's control flow.
+            let mut branch_dest = None;
+            for arg in simplified.args_iter() {
+                if let Argument::BranchDest(dest) = arg {
+                    let dest = cur_addr.wrapping_add_signed(dest.0);
+                    branch_dest = Some(dest);
+                    break;
+                }
+            }
+            if let Some(branch_dest) = branch_dest {
+                if branch_dest >= func_address as u32
+                    && (branch_dest - func_address as u32) < code.len() as u32
+                {
+                    let dest_offset_into_func = branch_dest - func_address as u32;
+                    let dest_code_slice = &code[dest_offset_into_func as usize..];
+                    match ins.op {
+                        Opcode::Bc => {
+                            // Conditional branch.
+                            // Add the branch destination to the queue to do later.
+                            ins_iters_with_gpr_state.push((
+                                InsIter::new(dest_code_slice, branch_dest),
+                                gpr_pool_relocs.clone(),
+                            ));
+                            // Then continue on with the current iterator.
+                        }
+                        Opcode::B => {
+                            if simplified.mnemonic != "bl" {
+                                // Unconditional branch.
+                                // Add the branch destination to the queue.
+                                ins_iters_with_gpr_state.push((
+                                    InsIter::new(dest_code_slice, branch_dest),
+                                    gpr_pool_relocs.clone(),
+                                ));
+                                // Break out of the current iterator so we can do the newly added one.
+                                break;
+                            }
+                        }
+                        _ => unreachable!(),
+                    }
+                }
+            }
+            if let Opcode::Bcctr = ins.op {
+                if simplified.mnemonic == "bctr" {
+                    // Unconditional branch to count register.
+                    // Likely a jump table.
+                    gpr_state_at_bctr.insert(cur_addr, gpr_pool_relocs.clone());
+                }
+            }
+
+            // Then handle keeping track of which GPR contains which pool relocation.
+            let reloc = relocations.iter().find(|r| (r.address as u32 & !3) == cur_addr);
             if let Some(reloc) = reloc {
                 // This instruction has a real relocation, so it may be a pool load we want to keep
                 // track of.
                 let args = &simplified.args;
                 match (ins.op, args[0], args[1], args[2]) {
                     (
+                        // `lis` + `addi`
                         Opcode::Addi,
                         Argument::GPR(addr_dst_gpr),
                         Argument::GPR(_addr_src_gpr),
                         Argument::Simm(_simm),
                     ) => {
-                    active_pool_relocs.insert(addr_dst_gpr.0, reloc.clone()); // `lis` + `addi`
+                        gpr_pool_relocs.insert(addr_dst_gpr.0, reloc.clone());
                     }
                     (
+                        // `lis` + `ori`
                         Opcode::Ori,
                         Argument::GPR(addr_dst_gpr),
                         Argument::GPR(_addr_src_gpr),
                         Argument::Uimm(_uimm),
                     ) => {
-                    active_pool_relocs.insert(addr_dst_gpr.0, reloc.clone()); // `lis` + `ori`
+                        gpr_pool_relocs.insert(addr_dst_gpr.0, reloc.clone());
                     }
                     (Opcode::B, _, _, _) => {
                         if simplified.mnemonic == "bl" {
                             // When encountering a function call, clear any active pool relocations from
                             // the volatile registers (r0, r3-r12), but not the nonvolatile registers.
-                        active_pool_relocs.remove(&0);
+                            gpr_pool_relocs.remove(&0);
                             for gpr in 3..12 {
-                            active_pool_relocs.remove(&gpr);
+                                gpr_pool_relocs.remove(&gpr);
                             }
                         }
                     }
-                _ => {}
+                    _ => {
+                        clear_overwritten_gprs(ins, &mut gpr_pool_relocs);
+                    }
                 }
             } else if let Some((offset, addr_src_gpr, addr_dst_gpr)) =
                 get_offset_and_addr_gpr_for_possible_pool_reference(ins.op, &simplified)
             {
                 // This instruction doesn't have a real relocation, so it may be a reference to one of
                 // the already-loaded pools.
-            if let Some(pool_reloc) = active_pool_relocs.get(&addr_src_gpr.0) {
+                if let Some(pool_reloc) = gpr_pool_relocs.get(&addr_src_gpr.0) {
-                if let Some(fake_pool_reloc) = make_fake_pool_reloc(offset, cur_addr, pool_reloc) {
+                    if let Some(fake_pool_reloc) =
+                        make_fake_pool_reloc(offset, cur_addr, pool_reloc)
+                    {
                         pool_reloc_for_addr.insert(cur_addr, fake_pool_reloc);
                     }
                     if let Some(addr_dst_gpr) = addr_dst_gpr {
@@ -568,7 +687,40 @@ fn generate_fake_pool_reloc_for_addr_mapping(
                         // Then the body of the loop will `lwzx` one of the array elements from r21.
                         let mut new_reloc = pool_reloc.clone();
                         new_reloc.addend += offset as i64;
-                    active_pool_relocs.insert(addr_dst_gpr.0, new_reloc);
+                        gpr_pool_relocs.insert(addr_dst_gpr.0, new_reloc);
+                    } else {
+                        clear_overwritten_gprs(ins, &mut gpr_pool_relocs);
+                    }
+                } else {
+                    clear_overwritten_gprs(ins, &mut gpr_pool_relocs);
+                }
+            } else {
+                clear_overwritten_gprs(ins, &mut gpr_pool_relocs);
+            }
+        }
+
+        // Finally, if we're about to finish the outer loop and don't have any more control flow to
+        // follow, we check if there are any instruction addresses in this function that we missed.
+        // If so, and if there were any `bctr` instructions before those points in this function,
+        // then we try to traverse those missing spots as switch cases.
+        if ins_iters_with_gpr_state.is_empty() {
+            let unseen_addrs = (func_address as u32..func_address as u32 + code.len() as u32)
+                .step_by(4)
+                .filter(|addr| !visited_ins_addrs.contains(addr));
+            for unseen_addr in unseen_addrs {
+                let prev_bctr_gpr_state = gpr_state_at_bctr
+                    .iter()
+                    .filter(|(&addr, _)| addr < unseen_addr)
+                    .min_by_key(|(&addr, _)| addr)
+                    .map(|(_, gpr_state)| gpr_state);
+                if let Some(gpr_pool_relocs) = prev_bctr_gpr_state {
+                    let dest_offset_into_func = unseen_addr - func_address as u32;
+                    let dest_code_slice = &code[dest_offset_into_func as usize..];
+                    ins_iters_with_gpr_state.push((
+                        InsIter::new(dest_code_slice, unseen_addr),
+                        gpr_pool_relocs.clone(),
+                    ));
+                    break;
                 }
             }
         }

objdiff-core/src/diff/code.rs

@@ -3,13 +3,17 @@ use std::{cmp::max, collections::BTreeMap};
 use anyhow::{anyhow, Result};
 use similar::{capture_diff_slices_deadline, Algorithm};
 
+use super::FunctionRelocDiffs;
 use crate::{
     arch::ProcessCodeResult,
     diff::{
         DiffObjConfig, ObjInsArgDiff, ObjInsBranchFrom, ObjInsBranchTo, ObjInsDiff, ObjInsDiffKind,
         ObjSymbolDiff,
     },
-    obj::{ObjInfo, ObjInsArg, ObjReloc, ObjSection, ObjSymbol, ObjSymbolFlags, SymbolRef},
+    obj::{
+        ObjInfo, ObjIns, ObjInsArg, ObjReloc, ObjSection, ObjSymbol, ObjSymbolFlags, ObjSymbolKind,
+        SymbolRef,
+    },
 };
 
 pub fn process_code_symbol(
@@ -215,16 +219,19 @@ fn reloc_eq(
     config: &DiffObjConfig,
     left_obj: &ObjInfo,
     right_obj: &ObjInfo,
-    left_reloc: Option<&ObjReloc>,
+    left_ins: Option<&ObjIns>,
-    right_reloc: Option<&ObjReloc>,
+    right_ins: Option<&ObjIns>,
 ) -> bool {
-    let (Some(left), Some(right)) = (left_reloc, right_reloc) else {
+    let (Some(left_ins), Some(right_ins)) = (left_ins, right_ins) else {
+        return false;
+    };
+    let (Some(left), Some(right)) = (&left_ins.reloc, &right_ins.reloc) else {
         return false;
     };
     if left.flags != right.flags {
         return false;
     }
-    if config.relax_reloc_diffs {
+    if config.function_reloc_diffs == FunctionRelocDiffs::None {
         return true;
     }
 
@@ -233,7 +240,13 @@ fn reloc_eq(
         (Some(sl), Some(sr)) => {
             // Match if section and name or address match
             section_name_eq(left_obj, right_obj, *sl, *sr)
-                && (symbol_name_matches || address_eq(left, right))
+                && (config.function_reloc_diffs == FunctionRelocDiffs::DataValue
+                    || symbol_name_matches
+                    || address_eq(left, right))
+                && (config.function_reloc_diffs == FunctionRelocDiffs::NameAddress
+                    || left.target.kind != ObjSymbolKind::Object
+                    || left_obj.arch.display_ins_data(left_ins)
+                        == left_obj.arch.display_ins_data(right_ins))
         }
         (Some(_), None) => false,
         (None, Some(_)) => {
@@ -262,7 +275,7 @@ fn arg_eq(
             ObjInsArg::Arg(r) => l.loose_eq(r),
             // If relocations are relaxed, match if left is a constant and right is a reloc
             // Useful for instances where the target object is created without relocations
-            ObjInsArg::Reloc => config.relax_reloc_diffs,
+            ObjInsArg::Reloc => config.function_reloc_diffs == FunctionRelocDiffs::None,
             _ => false,
         },
         ObjInsArg::Reloc => {
@@ -271,8 +284,8 @@ fn arg_eq(
                     config,
                     left_obj,
                     right_obj,
-                    left_diff.ins.as_ref().and_then(|i| i.reloc.as_ref()),
+                    left_diff.ins.as_ref(),
-                    right_diff.ins.as_ref().and_then(|i| i.reloc.as_ref()),
+                    right_diff.ins.as_ref(),
                 )
         }
         ObjInsArg::BranchDest(_) => match right {
@@ -283,7 +296,7 @@ fn arg_eq(
             }
             // If relocations are relaxed, match if left is a constant and right is a reloc
             // Useful for instances where the target object is created without relocations
-            ObjInsArg::Reloc => config.relax_reloc_diffs,
+            ObjInsArg::Reloc => config.function_reloc_diffs == FunctionRelocDiffs::None,
             _ => false,
         },
     }

objdiff-gui/src/app_config.rs

@@ -3,8 +3,11 @@ use std::path::PathBuf;
 use eframe::Storage;
 use globset::Glob;
 use objdiff_core::{
-    config::ScratchConfig,
+    config::{ScratchConfig, SymbolMappings},
-    diff::{ArmArchVersion, ArmR9Usage, DiffObjConfig, MipsAbi, MipsInstrCategory, X86Formatter},
+    diff::{
+        ArmArchVersion, ArmR9Usage, DiffObjConfig, FunctionRelocDiffs, MipsAbi, MipsInstrCategory,
+        X86Formatter,
+    },
 };
 
 use crate::app::{AppConfig, ObjectConfig, CONFIG_KEY};
@@ -15,7 +18,7 @@ pub struct AppConfigVersion {
 }
 
 impl Default for AppConfigVersion {
-    fn default() -> Self { Self { version: 2 } }
+    fn default() -> Self { Self { version: 3 } }
 }
 
 /// Deserialize the AppConfig from storage, handling upgrades from older versions.
@@ -23,7 +26,8 @@ pub fn deserialize_config(storage: &dyn Storage) -> Option<AppConfig> {
     let str = storage.get_string(CONFIG_KEY)?;
     match ron::from_str::<AppConfigVersion>(&str) {
         Ok(version) => match version.version {
-            2 => from_str::<AppConfig>(&str),
+            3 => from_str::<AppConfig>(&str),
+            2 => from_str::<AppConfigV2>(&str).map(|c| c.into_config()),
             1 => from_str::<AppConfigV1>(&str).map(|c| c.into_config()),
             _ => {
                 log::warn!("Unknown config version: {}", version.version);
@@ -49,6 +53,119 @@ where T: serde::de::DeserializeOwned {
     }
 }
 
+#[derive(serde::Deserialize, serde::Serialize)]
+pub struct ScratchConfigV2 {
+    #[serde(default)]
+    pub platform: Option<String>,
+    #[serde(default)]
+    pub compiler: Option<String>,
+    #[serde(default)]
+    pub c_flags: Option<String>,
+    #[serde(default)]
+    pub ctx_path: Option<PathBuf>,
+    #[serde(default)]
+    pub build_ctx: Option<bool>,
+    #[serde(default)]
+    pub preset_id: Option<u32>,
+}
+
+impl ScratchConfigV2 {
+    fn into_config(self) -> ScratchConfig {
+        ScratchConfig {
+            platform: self.platform,
+            compiler: self.compiler,
+            c_flags: self.c_flags,
+            ctx_path: self.ctx_path,
+            build_ctx: self.build_ctx,
+            preset_id: self.preset_id,
+        }
+    }
+}
+
+#[derive(serde::Deserialize, serde::Serialize)]
+pub struct ObjectConfigV2 {
+    pub name: String,
+    pub target_path: Option<PathBuf>,
+    pub base_path: Option<PathBuf>,
+    pub reverse_fn_order: Option<bool>,
+    pub complete: Option<bool>,
+    pub scratch: Option<ScratchConfigV2>,
+    pub source_path: Option<String>,
+    #[serde(default)]
+    pub symbol_mappings: SymbolMappings,
+}
+
+impl ObjectConfigV2 {
+    fn into_config(self) -> ObjectConfig {
+        ObjectConfig {
+            name: self.name,
+            target_path: self.target_path,
+            base_path: self.base_path,
+            reverse_fn_order: self.reverse_fn_order,
+            complete: self.complete,
+            scratch: self.scratch.map(|scratch| scratch.into_config()),
+            source_path: self.source_path,
+            symbol_mappings: self.symbol_mappings,
+        }
+    }
+}
+
+#[derive(serde::Deserialize, serde::Serialize)]
+pub struct AppConfigV2 {
+    pub version: u32,
+    #[serde(default)]
+    pub custom_make: Option<String>,
+    #[serde(default)]
+    pub custom_args: Option<Vec<String>>,
+    #[serde(default)]
+    pub selected_wsl_distro: Option<String>,
+    #[serde(default)]
+    pub project_dir: Option<PathBuf>,
+    #[serde(default)]
+    pub target_obj_dir: Option<PathBuf>,
+    #[serde(default)]
+    pub base_obj_dir: Option<PathBuf>,
+    #[serde(default)]
+    pub selected_obj: Option<ObjectConfigV2>,
+    #[serde(default = "bool_true")]
+    pub build_base: bool,
+    #[serde(default)]
+    pub build_target: bool,
+    #[serde(default = "bool_true")]
+    pub rebuild_on_changes: bool,
+    #[serde(default)]
+    pub auto_update_check: bool,
+    #[serde(default)]
+    pub watch_patterns: Vec<Glob>,
+    #[serde(default)]
+    pub recent_projects: Vec<PathBuf>,
+    #[serde(default)]
+    pub diff_obj_config: DiffObjConfigV1,
+}
+
+impl AppConfigV2 {
+    fn into_config(self) -> AppConfig {
+        log::info!("Upgrading configuration from v2");
+        AppConfig {
+            custom_make: self.custom_make,
+            custom_args: self.custom_args,
+            selected_wsl_distro: self.selected_wsl_distro,
+            project_dir: self.project_dir,
+            target_obj_dir: self.target_obj_dir,
+            base_obj_dir: self.base_obj_dir,
+            selected_obj: self.selected_obj.map(|obj| obj.into_config()),
+            build_base: self.build_base,
+            build_target: self.build_target,
+            rebuild_on_changes: self.rebuild_on_changes,
+            auto_update_check: self.auto_update_check,
+            watch_patterns: self.watch_patterns,
+            recent_projects: self.recent_projects,
+            diff_obj_config: self.diff_obj_config.into_config(),
+            ..Default::default()
+        }
+    }
+}
+
 #[derive(serde::Deserialize, serde::Serialize)]
 pub struct ScratchConfigV1 {
     #[serde(default)]
@@ -147,7 +264,11 @@ impl Default for DiffObjConfigV1 {
 impl DiffObjConfigV1 {
     fn into_config(self) -> DiffObjConfig {
         DiffObjConfig {
-            relax_reloc_diffs: self.relax_reloc_diffs,
+            function_reloc_diffs: if self.relax_reloc_diffs {
+                FunctionRelocDiffs::None
+            } else {
+                FunctionRelocDiffs::default()
+            },
             space_between_args: self.space_between_args,
             combine_data_sections: self.combine_data_sections,
             x86_formatter: self.x86_formatter,

objdiff-gui/src/views/function_diff.rs

@@ -149,13 +149,9 @@ fn ins_hover_ui(
                     appearance.highlight_color,
                     format!("Size: {:x}", reloc.target.size),
                 );
-                if reloc.addend >= 0 && reloc.target.bytes.len() > reloc.addend as usize {
+                if let Some(s) = obj.arch.display_ins_data(ins) {
-                    if let Some(s) = obj.arch.guess_data_type(ins).and_then(|ty| {
-                        obj.arch.display_data_type(ty, &reloc.target.bytes[reloc.addend as usize..])
-                    }) {
                     ui.colored_label(appearance.highlight_color, s);
                 }
-                }
             } else {
                 ui.colored_label(appearance.highlight_color, "Extern".to_string());
             }