Fix diffing of relocations to symbols that have compiler-generated names

objdiff-core/src/diff/code.rs

@@ -12,9 +12,12 @@ use super::{
     InstructionBranchTo, InstructionDiffKind, InstructionDiffRow, PreferredStringEncoding,
     SymbolDiff, display::display_ins_data_literals,
 };
-use crate::obj::{
-    InstructionArg, InstructionArgValue, InstructionRef, Object, ResolvedInstructionRef,
-    ResolvedRelocation, ResolvedSymbol, SymbolKind,
+use crate::{
+    diff::{address_eq, section_name_eq, symbol_name_matches},
+    obj::{
+        InstructionArg, InstructionArgValue, InstructionRef, Object, ResolvedInstructionRef,
+        ResolvedRelocation, ResolvedSymbol, SymbolFlag, SymbolKind,
+    },
 };
 
 pub fn no_diff_code(
@@ -266,36 +269,6 @@ fn resolve_branches(ops: &[InstructionRef], rows: &mut [InstructionDiffRow]) {
     }
 }
 
-pub(crate) fn address_eq(left: ResolvedRelocation, right: ResolvedRelocation) -> bool {
-    if right.symbol.size == 0 && left.symbol.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's data, 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.symbol.address as i64 + left.relocation.addend
-            == right.symbol.address as i64 + right.relocation.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.symbol.address == right.symbol.address
-            && left.relocation.addend == right.relocation.addend
-    }
-}
-
-pub(crate) fn section_name_eq(
-    left_obj: &Object,
-    right_obj: &Object,
-    left_section_index: usize,
-    right_section_index: usize,
-) -> bool {
-    left_obj.sections.get(left_section_index).is_some_and(|left_section| {
-        right_obj
-            .sections
-            .get(right_section_index)
-            .is_some_and(|right_section| left_section.name == right_section.name)
-    })
-}
-
 fn ins_data_literals_eq(
     left_obj: &Object,
     right_obj: &Object,
@@ -338,19 +311,40 @@ fn reloc_eq(
         return true;
     }
 
-    let symbol_name_addend_matches = left_reloc.symbol.name == right_reloc.symbol.name
+    let symbol_name_addend_matches = symbol_name_matches(left_reloc.symbol, right_reloc.symbol)
         && left_reloc.relocation.addend == right_reloc.relocation.addend;
-    match (&left_reloc.symbol.section, &right_reloc.symbol.section) {
+    match (left_reloc.symbol.section, right_reloc.symbol.section) {
         (Some(sl), Some(sr)) => {
-            // Match if section and name or address match
-            section_name_eq(left_obj, right_obj, *sl, *sr)
-                && (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
-                    || right_reloc.symbol.size == 0 // Likely a pool symbol like ...data, don't treat this as a diff
-                    || ins_data_literals_eq(left_obj, right_obj, left_ins, right_ins, diff_config))
+            if !section_name_eq(left_obj, right_obj, sl, sr) {
+                return false;
+            };
+            let mut name_ok = false;
+            if diff_config.function_reloc_diffs == FunctionRelocDiffs::DataValue {
+                // Ignore names entirely
+                name_ok = true;
+            } else if left_reloc.symbol.flags.contains(SymbolFlag::CompilerGenerated)
+                && right_reloc.symbol.flags.contains(SymbolFlag::CompilerGenerated)
+            {
+                // Match if both symbol names are fully compiler-generated
+                name_ok = true;
+            } else if symbol_name_addend_matches || address_eq(left_reloc, right_reloc) {
+                // Match if name+addend or address match
+                name_ok = true;
+            }
+            let mut value_ok = false;
+            if diff_config.function_reloc_diffs == FunctionRelocDiffs::NameAddress {
+                // Ignore data values entirely
+                value_ok = true;
+            } else if left_reloc.symbol.kind != SymbolKind::Object {
+                // Not a data symbol, don't diff value
+                value_ok = true;
+            } else if right_reloc.symbol.size == 0 {
+                // Likely a pool symbol like ...data, don't treat this as a diff
+                value_ok = true;
+            } else if ins_data_literals_eq(left_obj, right_obj, left_ins, right_ins, diff_config) {
+                value_ok = true;
+            }
+            name_ok && value_ok
         }
         (Some(_), None) | (None, Some(_)) | (None, None) => symbol_name_addend_matches,
     }

objdiff-core/src/diff/data.rs

@@ -6,9 +6,11 @@ use similar::{Algorithm, capture_diff_slices, diff_ratio};
 
 use super::{
     DataDiff, DataDiffKind, DataDiffRow, DataRelocationDiff, ObjectDiff, SectionDiff, SymbolDiff,
-    code::{address_eq, section_name_eq},
 };
-use crate::obj::{Object, Relocation, ResolvedRelocation, Symbol, SymbolFlag, SymbolKind};
+use crate::{
+    diff::{address_eq, section_name_eq, symbol_name_matches},
+    obj::{Object, Relocation, ResolvedRelocation, Symbol, SymbolFlag, SymbolKind},
+};
 
 pub fn diff_bss_symbol(
     left_obj: &Object,
@@ -35,33 +37,34 @@ pub fn diff_bss_symbol(
     ))
 }
 
-pub fn symbol_name_matches(left: &Symbol, right: &Symbol) -> bool {
-    if let Some(left_name) = &left.normalized_name
-        && let Some(right_name) = &right.normalized_name
-    {
-        left_name == right_name
-    } else {
-        left.name == right.name
-    }
-}
-
 fn reloc_eq(
     left_obj: &Object,
     right_obj: &Object,
-    left: ResolvedRelocation,
-    right: ResolvedRelocation,
+    left_reloc: ResolvedRelocation,
+    right_reloc: ResolvedRelocation,
 ) -> bool {
-    if left.relocation.flags != right.relocation.flags {
+    if left_reloc.relocation.flags != right_reloc.relocation.flags {
         return false;
     }
 
-    let symbol_name_addend_matches = symbol_name_matches(left.symbol, right.symbol)
-        && left.relocation.addend == right.relocation.addend;
-    match (left.symbol.section, right.symbol.section) {
+    let symbol_name_addend_matches = symbol_name_matches(left_reloc.symbol, right_reloc.symbol)
+        && left_reloc.relocation.addend == right_reloc.relocation.addend;
+    match (left_reloc.symbol.section, right_reloc.symbol.section) {
         (Some(sl), Some(sr)) => {
-            // Match if section and name+addend or address match
-            section_name_eq(left_obj, right_obj, sl, sr)
-                && (symbol_name_addend_matches || address_eq(left, right))
+            if !section_name_eq(left_obj, right_obj, sl, sr) {
+                return false;
+            };
+            let mut name_ok = false;
+            if left_reloc.symbol.flags.contains(SymbolFlag::CompilerGenerated)
+                && right_reloc.symbol.flags.contains(SymbolFlag::CompilerGenerated)
+            {
+                // Match if both symbol names are fully compiler-generated
+                name_ok = true;
+            } else if symbol_name_addend_matches || address_eq(left_reloc, right_reloc) {
+                // Match if name+addend or address match
+                name_ok = true;
+            }
+            name_ok
         }
         (Some(_), None) | (None, Some(_)) | (None, None) => symbol_name_addend_matches,
     }

objdiff-core/src/diff/mod.rs

@@ -14,10 +14,11 @@ use crate::{
         data::{
             diff_bss_section, diff_bss_symbol, diff_data_section, diff_data_symbol,
             diff_generic_section, no_diff_bss_section, no_diff_data_section, no_diff_data_symbol,
-            symbol_name_matches,
         },
     },
-    obj::{InstructionRef, Object, Relocation, SectionKind, Symbol, SymbolFlag},
+    obj::{
+        InstructionRef, Object, Relocation, ResolvedRelocation, SectionKind, Symbol, SymbolFlag,
+    },
 };
 
 pub mod code;
@@ -814,3 +815,43 @@ pub enum DiffSide {
     /// The base side of the diff.
     Base,
 }
+
+pub(crate) fn address_eq(left: ResolvedRelocation, right: ResolvedRelocation) -> bool {
+    if right.symbol.size == 0 && left.symbol.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's data, 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.symbol.address as i64 + left.relocation.addend
+            == right.symbol.address as i64 + right.relocation.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.symbol.address == right.symbol.address
+            && left.relocation.addend == right.relocation.addend
+    }
+}
+
+pub(crate) fn section_name_eq(
+    left_obj: &Object,
+    right_obj: &Object,
+    left_section_index: usize,
+    right_section_index: usize,
+) -> bool {
+    left_obj.sections.get(left_section_index).is_some_and(|left_section| {
+        right_obj
+            .sections
+            .get(right_section_index)
+            .is_some_and(|right_section| left_section.name == right_section.name)
+    })
+}
+
+pub(crate) fn symbol_name_matches(left: &Symbol, right: &Symbol) -> bool {
+    if let Some(left_name) = &left.normalized_name
+        && let Some(right_name) = &right.normalized_name
+    {
+        left_name == right_name
+    } else {
+        left.name == right.name
+    }
+}