Significantly improve diffing performance and fix minor bug with bss section match percents

objdiff-core/src/diff/data.rs

@@ -35,31 +35,13 @@ pub fn diff_bss_symbol(
     ))
 }
 
-pub fn symbol_name_matches(left_name: &str, right_name: &str) -> bool {
-    if let Some((left_prefix, left_suffix)) = left_name.split_once("@class$")
-        && let Some((right_prefix, right_suffix)) = right_name.split_once("@class$")
+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
     {
-        // Match Metrowerks anonymous class symbol names, ignoring the unique ID.
-        // e.g. __dt__Q29dCamera_c23@class$3665d_camera_cppFv
-        if left_prefix == right_prefix
-            && let Some(left_idx) = left_suffix.chars().position(|c| !c.is_numeric())
-            && let Some(right_idx) = right_suffix.chars().position(|c| !c.is_numeric())
-        {
-            // e.g. d_camera_cppFv (after the unique ID)
-            left_suffix[left_idx..] == right_suffix[right_idx..]
-        } else {
-            false
-        }
-    } else if let Some((prefix, suffix)) = left_name.split_once(['$', '.'])
-        && suffix.chars().all(char::is_numeric)
-    {
-        // Match Metrowerks symbol$1234 against symbol$2345
-        // and GCC symbol.1234 against symbol.2345
-        right_name
-            .split_once(['$', '.'])
-            .is_some_and(|(p, s)| p == prefix && s.chars().all(char::is_numeric))
-    } else {
         left_name == right_name
+    } else {
+        left.name == right.name
     }
 }
 
@@ -73,7 +55,7 @@ fn reloc_eq(
         return false;
     }
 
-    let symbol_name_addend_matches = symbol_name_matches(&left.symbol.name, &right.symbol.name)
+    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) {
         (Some(sl), Some(sr)) => {

objdiff-core/src/diff/mod.rs

@@ -7,7 +7,6 @@ use alloc::{
 use core::{num::NonZeroU32, ops::Range};
 
 use anyhow::Result;
-use itertools::Itertools;
 
 use crate::{
     diff::{
@@ -687,18 +686,6 @@ fn symbol_section_kind(obj: &Object, symbol: &Symbol) -> SectionKind {
     }
 }
 
-/// Check if a symbol is a compiler-generated like @1234 or _$E1234.
-fn is_symbol_compiler_generated(symbol: &Symbol) -> bool {
-    if symbol.name.starts_with('@') && symbol.name[1..].chars().all(char::is_numeric) {
-        // Exclude @stringBase0, @GUARD@, etc.
-        return true;
-    }
-    if symbol.name.starts_with("_$E") && symbol.name[3..].chars().all(char::is_numeric) {
-        return true;
-    }
-    false
-}
-
 fn find_symbol(
     obj: Option<&Object>,
     in_obj: &Object,
@@ -712,7 +699,7 @@ fn find_symbol(
 
     // Match compiler-generated symbols against each other (e.g. @251 -> @60)
     // If they are in the same section and have the same value
-    if is_symbol_compiler_generated(in_symbol)
+    if in_symbol.is_name_compiler_generated
         && matches!(section_kind, SectionKind::Code | SectionKind::Data | SectionKind::Bss)
     {
         let mut closest_match_symbol_idx = None;
@@ -724,7 +711,7 @@ fn find_symbol(
             if obj.sections[section_index].name != section_name {
                 continue;
             }
-            if !is_symbol_compiler_generated(symbol) {
+            if !symbol.is_name_compiler_generated {
                 continue;
             }
             match section_kind {
@@ -761,15 +748,11 @@ fn find_symbol(
     }
 
     // Try to find a symbol with a matching name
-    if let Some((symbol_idx, _)) = unmatched_symbols(obj, used)
-        .filter(|&(_, symbol)| {
-            symbol_name_matches(&in_symbol.name, &symbol.name)
-                && symbol_section_kind(obj, symbol) == section_kind
-                && symbol_section(obj, symbol).is_some_and(|(name, _)| name == section_name)
-        })
-        .sorted_unstable_by_key(|&(_, symbol)| (symbol.section, symbol.address))
-        .next()
-    {
+    if let Some((symbol_idx, _)) = unmatched_symbols(obj, used).find(|&(_, symbol)| {
+        symbol_name_matches(in_symbol, symbol)
+            && symbol_section_kind(obj, symbol) == section_kind
+            && symbol_section(obj, symbol).is_some_and(|(name, _)| name == section_name)
+    }) {
         return Some(symbol_idx);
     }
 

objdiff-core/src/obj/mod.rs

@@ -264,6 +264,8 @@ pub trait FlowAnalysisResult: core::fmt::Debug + Send {
 pub struct Symbol {
     pub name: String,
     pub demangled_name: Option<String>,
+    pub normalized_name: Option<String>,
+    pub is_name_compiler_generated: bool,
     pub address: u64,
     pub size: u64,
     pub kind: SymbolKind,
@@ -403,6 +405,8 @@ pub struct ResolvedInstructionRef<'obj> {
 static DUMMY_SYMBOL: Symbol = Symbol {
     name: String::new(),
     demangled_name: None,
+    normalized_name: None,
+    is_name_compiler_generated: false,
     address: 0,
     size: 0,
     kind: SymbolKind::Unknown,

objdiff-core/src/obj/read.rs

@@ -8,6 +8,7 @@ use alloc::{
 use core::{cmp::Ordering, num::NonZeroU64};
 
 use anyhow::{Context, Result, anyhow, bail, ensure};
+use itertools::Itertools;
 use object::{Object as _, ObjectSection as _, ObjectSymbol as _};
 
 use crate::{
@@ -35,6 +36,46 @@ fn map_section_kind(section: &object::Section) -> SectionKind {
     }
 }
 
+/// Check if a symbol's name is partially compiler-generated, and if so normalize it for pairing.
+/// e.g. symbol$1234 and symbol$2345 will both be replaced with symbol$0000 internally.
+fn get_normalized_symbol_name(name: &str) -> Option<String> {
+    const DUMMY_UNIQUE_ID: &str = "0000";
+    if let Some((prefix, suffix)) = name.split_once("@class$")
+        && let Some(idx) = suffix.chars().position(|c| !c.is_numeric())
+        && idx > 0
+    {
+        // Match Metrowerks anonymous class symbol names, ignoring the unique ID.
+        // e.g. __dt__Q29dCamera_c23@class$3665d_camera_cppFv
+        // and: __dt__Q29dCamera_c23@class$1727d_camera_cppFv
+        let suffix = &suffix[idx..];
+        Some(format!("{prefix}@class${DUMMY_UNIQUE_ID}{suffix}"))
+    } else if let Some((prefix, suffix)) = name.split_once('$')
+        && suffix.chars().all(char::is_numeric)
+    {
+        // Match Metrowerks symbol$1234 against symbol$2345
+        Some(format!("{prefix}${DUMMY_UNIQUE_ID}"))
+    } else if let Some((prefix, suffix)) = name.split_once('.')
+        && suffix.chars().all(char::is_numeric)
+    {
+        // Match GCC symbol.1234 against symbol.2345
+        Some(format!("{prefix}.{DUMMY_UNIQUE_ID}"))
+    } else {
+        None
+    }
+}
+
+/// Check if a symbol's name is entirely compiler-generated, such as @1234 or _$E1234.
+/// This enables pairing these symbols up by their value instead of their name.
+fn is_symbol_name_compiler_generated(name: &str) -> bool {
+    if name.starts_with('@') && name[1..].chars().all(char::is_numeric) {
+        // Exclude @stringBase0, @GUARD@, etc.
+        return true;
+    } else if name.starts_with("_$E") && name[3..].chars().all(char::is_numeric) {
+        return true;
+    }
+    false
+}
+
 fn map_symbol(
     arch: &dyn Arch,
     file: &object::File,
@@ -97,10 +138,14 @@ fn map_symbol(
         .and_then(|m| m.virtual_addresses.as_ref())
         .and_then(|v| v.get(symbol.index().0).cloned());
     let section = symbol.section_index().and_then(|i| section_indices.get(i.0).copied());
+    let normalized_name = get_normalized_symbol_name(&name);
+    let is_name_compiler_generated = is_symbol_name_compiler_generated(&name);
 
     Ok(Symbol {
         name,
         demangled_name,
+        normalized_name,
+        is_name_compiler_generated,
         address,
         size,
         kind,
@@ -122,7 +167,13 @@ fn map_symbols(
     let symbol_count = obj_file.symbols().count();
     let mut symbols = Vec::<Symbol>::with_capacity(symbol_count + obj_file.sections().count());
     let mut symbol_indices = Vec::<usize>::with_capacity(symbol_count + 1);
-    for obj_symbol in obj_file.symbols() {
+    let obj_symbols = obj_file.symbols();
+    // symbols() is not guaranteed to be sorted by address.
+    // We sort it here to fix pairing bugs with diff algorithms that assume the symbols are ordered.
+    // Sorting everything here once is less expensive than sorting subsets later in expensive loops.
+    let obj_symbols =
+        obj_symbols.sorted_by_key(|symbol| (symbol.section_index().map(|i| i.0), symbol.address()));
+    for obj_symbol in obj_symbols {
         if symbol_indices.len() <= obj_symbol.index().0 {
             symbol_indices.resize(obj_symbol.index().0 + 1, usize::MAX);
         }
@@ -172,6 +223,8 @@ fn add_section_symbols(sections: &[Section], symbols: &mut Vec<Symbol>) {
         symbols.push(Symbol {
             name,
             demangled_name: None,
+            normalized_name: None,
+            is_name_compiler_generated: false,
             address: 0,
             size,
             kind: SymbolKind::Section,

objdiff-core/src/obj/snapshots/objdiff_core__obj__read__test__combine_sections.snap

@@ -114,6 +114,8 @@ expression: "(sections, symbols)"
         Symbol {
             name: ".data",
             demangled_name: None,
+            normalized_name: None,
+            is_name_compiler_generated: false,
             address: 0,
             size: 0,
             kind: Section,
@@ -127,6 +129,8 @@ expression: "(sections, symbols)"
         Symbol {
             name: "symbol",
             demangled_name: None,
+            normalized_name: None,
+            is_name_compiler_generated: false,
             address: 4,
             size: 4,
             kind: Object,
@@ -140,6 +144,8 @@ expression: "(sections, symbols)"
         Symbol {
             name: "function",
             demangled_name: None,
+            normalized_name: None,
+            is_name_compiler_generated: false,
             address: 0,
             size: 8,
             kind: Function,
@@ -153,6 +159,8 @@ expression: "(sections, symbols)"
         Symbol {
             name: ".data",
             demangled_name: None,
+            normalized_name: None,
+            is_name_compiler_generated: false,
             address: 0,
             size: 0,
             kind: Unknown,

objdiff-core/tests/snapshots/arch_arm__combine_text_sections-2.snap

@@ -4,4 +4,4 @@ expression: output
 ---
 [(Line(90), Dim, 5), (Address(0), Dim, 5), (Spacing(4), Normal, 0), (Opcode("ldr", 32792), Normal, 10), (Argument(Opaque("r12")), Normal, 0), (Basic(", "), Normal, 0), (Basic("["), Normal, 0), (Argument(Opaque("pc")), Normal, 0), (Basic(", "), Normal, 0), (Basic("#"), Normal, 0), (Argument(Signed(0)), Normal, 0), (Basic("]"), Normal, 0), (Basic(" (->"), Normal, 0), (BranchDest(8), Normal, 0), (Basic(")"), Normal, 0), (Eol, Normal, 0)]
 [(Line(90), Dim, 5), (Address(4), Dim, 5), (Spacing(4), Normal, 0), (Opcode("bx", 32777), Normal, 10), (Argument(Opaque("r12")), Normal, 0), (Eol, Normal, 0)]
-[(Line(90), Dim, 5), (Address(8), Dim, 5), (Spacing(4), Normal, 0), (Opcode(".word", 65534), Normal, 10), (Symbol(Symbol { name: "esEnemyDraw", demangled_name: None, address: 0, size: 0, kind: Unknown, section: None, flags: FlagSet(Global), align: None, virtual_address: None }), Bright, 0), (Eol, Normal, 0)]
+[(Line(90), Dim, 5), (Address(8), Dim, 5), (Spacing(4), Normal, 0), (Opcode(".word", 65534), Normal, 10), (Symbol(Symbol { name: "esEnemyDraw", demangled_name: None, normalized_name: None, is_name_compiler_generated: false, address: 0, size: 0, kind: Unknown, section: None, flags: FlagSet(Global), align: None, virtual_address: None }), Bright, 0), (Eol, Normal, 0)]