Improve data section diff logic

More accurate in more cases

Fixes #81

objdiff-core/src/diff/data.rs

@@ -11,27 +11,6 @@ use crate::{
     obj::{ObjInfo, ObjSection, SymbolRef},
 };
 
-/// Compare the addresses and sizes of each symbol in the BSS sections.
-pub fn diff_bss_section(
-    left: &ObjSection,
-    right: &ObjSection,
-) -> Result<(ObjSectionDiff, ObjSectionDiff)> {
-    let deadline = Instant::now() + Duration::from_secs(5);
-    let left_sizes = left.symbols.iter().map(|s| (s.section_address, s.size)).collect::<Vec<_>>();
-    let right_sizes = right.symbols.iter().map(|s| (s.section_address, s.size)).collect::<Vec<_>>();
-    let ops = capture_diff_slices_deadline(
-        Algorithm::Patience,
-        &left_sizes,
-        &right_sizes,
-        Some(deadline),
-    );
-    let match_percent = get_diff_ratio(&ops, left_sizes.len(), right_sizes.len()) * 100.0;
-    Ok((
-        ObjSectionDiff { symbols: vec![], data_diff: vec![], match_percent: Some(match_percent) },
-        ObjSectionDiff { symbols: vec![], data_diff: vec![], match_percent: Some(match_percent) },
-    ))
-}
-
 pub fn diff_bss_symbol(
     left_obj: &ObjInfo,
     right_obj: &ObjInfo,
@@ -65,6 +44,8 @@ pub fn no_diff_symbol(_obj: &ObjInfo, symbol_ref: SymbolRef) -> ObjSymbolDiff {
 pub fn diff_data_section(
     left: &ObjSection,
     right: &ObjSection,
+    left_section_diff: &ObjSectionDiff,
+    right_section_diff: &ObjSectionDiff,
 ) -> Result<(ObjSectionDiff, ObjSectionDiff)> {
     let deadline = Instant::now() + Duration::from_secs(5);
     let left_max = left.symbols.iter().map(|s| s.section_address + s.size).max().unwrap_or(0);
@@ -73,7 +54,6 @@ pub fn diff_data_section(
     let right_data = &right.data[..right_max as usize];
     let ops =
         capture_diff_slices_deadline(Algorithm::Patience, left_data, right_data, Some(deadline));
-    let match_percent = get_diff_ratio(&ops, left_data.len(), right_data.len()) * 100.0;
 
     let mut left_diff = Vec::<ObjDataDiff>::new();
     let mut right_diff = Vec::<ObjDataDiff>::new();
@@ -143,18 +123,11 @@ pub fn diff_data_section(
         }
     }
 
-    Ok((
+    let (mut left_section_diff, mut right_section_diff) =
-        ObjSectionDiff {
+        diff_generic_section(left, right, left_section_diff, right_section_diff)?;
-            symbols: vec![],
+    left_section_diff.data_diff = left_diff;
-            data_diff: left_diff,
+    right_section_diff.data_diff = right_diff;
-            match_percent: Some(match_percent),
+    Ok((left_section_diff, right_section_diff))
-        },
-        ObjSectionDiff {
-            symbols: vec![],
-            data_diff: right_diff,
-            match_percent: Some(match_percent),
-        },
-    ))
 }
 
 pub fn diff_data_symbol(
@@ -195,21 +168,24 @@ pub fn diff_data_symbol(
     ))
 }
 
-/// Compare the text sections of two object files.
+/// Compares a section of two object files.
-/// This essentially adds up the match percentage of each symbol in the text section.
+/// This essentially adds up the match percentage of each symbol in the section.
-pub fn diff_text_section(
+pub fn diff_generic_section(
     left: &ObjSection,
     _right: &ObjSection,
     left_diff: &ObjSectionDiff,
     _right_diff: &ObjSectionDiff,
 ) -> Result<(ObjSectionDiff, ObjSectionDiff)> {
-    let match_percent = left
+    let match_percent = if left_diff.symbols.iter().all(|d| d.match_percent == Some(100.0)) {
-        .symbols
+        100.0 // Avoid fp precision issues
-        .iter()
+    } else {
-        .zip(left_diff.symbols.iter())
+        left.symbols
-        .map(|(s, d)| d.match_percent.unwrap_or(0.0) * s.size as f32)
+            .iter()
-        .sum::<f32>()
+            .zip(left_diff.symbols.iter())
-        / left.size as f32;
+            .map(|(s, d)| d.match_percent.unwrap_or(0.0) * s.size as f32)
+            .sum::<f32>()
+            / left.size as f32
+    };
     Ok((
         ObjSectionDiff { symbols: vec![], data_diff: vec![], match_percent: Some(match_percent) },
         ObjSectionDiff { symbols: vec![], data_diff: vec![], match_percent: Some(match_percent) },

objdiff-core/src/diff/mod.rs

@@ -6,8 +6,8 @@ use crate::{
     diff::{
         code::{diff_code, no_diff_code, process_code_symbol},
         data::{
-            diff_bss_section, diff_bss_symbol, diff_data_section, diff_data_symbol,
+            diff_bss_symbol, diff_data_section, diff_data_symbol, diff_generic_section,
-            diff_text_section, no_diff_symbol,
+            no_diff_symbol,
         },
     },
     obj::{ObjInfo, ObjIns, ObjSection, ObjSectionKind, ObjSymbol, SymbolRef},
@@ -483,10 +483,10 @@ pub fn diff_objs(
             let left_section = &left_obj.sections[left_section_idx];
             let right_section = &right_obj.sections[right_section_idx];
             match section_kind {
-                ObjSectionKind::Code => {
+                ObjSectionKind::Code | ObjSectionKind::Bss => {
                     let left_section_diff = left_out.section_diff(left_section_idx);
                     let right_section_diff = right_out.section_diff(right_section_idx);
-                    let (left_diff, right_diff) = diff_text_section(
+                    let (left_diff, right_diff) = diff_generic_section(
                         left_section,
                         right_section,
                         left_section_diff,
@@ -496,12 +496,14 @@ pub fn diff_objs(
                     right_out.section_diff_mut(right_section_idx).merge(right_diff);
                 }
                 ObjSectionKind::Data => {
-                    let (left_diff, right_diff) = diff_data_section(left_section, right_section)?;
+                    let left_section_diff = left_out.section_diff(left_section_idx);
-                    left_out.section_diff_mut(left_section_idx).merge(left_diff);
+                    let right_section_diff = right_out.section_diff(right_section_idx);
-                    right_out.section_diff_mut(right_section_idx).merge(right_diff);
+                    let (left_diff, right_diff) = diff_data_section(
-                }
+                        left_section,
-                ObjSectionKind::Bss => {
+                        right_section,
-                    let (left_diff, right_diff) = diff_bss_section(left_section, right_section)?;
+                        left_section_diff,
+                        right_section_diff,
+                    )?;
                     left_out.section_diff_mut(left_section_idx).merge(left_diff);
                     right_out.section_diff_mut(right_section_idx).merge(right_diff);
                 }
@@ -630,6 +632,26 @@ fn find_symbol(
             }
         }
     }
+    // Match Metrowerks symbol$1234 against symbol$2345
+    if let Some((prefix, suffix)) = in_symbol.name.split_once('$') {
+        if !suffix.chars().all(char::is_numeric) {
+            return None;
+        }
+        for (section_idx, section) in obj.sections.iter().enumerate() {
+            if section.kind != in_section.kind {
+                continue;
+            }
+            if let Some(symbol_idx) = section.symbols.iter().position(|symbol| {
+                if let Some((p, s)) = symbol.name.split_once('$') {
+                    prefix == p && s.chars().all(char::is_numeric)
+                } else {
+                    false
+                }
+            }) {
+                return Some(SymbolRef { section_idx, symbol_idx });
+            }
+        }
+    }
     None
 }