Improve data section diffing more

This re-implements the older algorithm
used for data and BSS section match
percentages. We perform both and
choose the highest match percent
between the two options.

Resolves #84, #85

Author: encounter

objdiff-core/src/diff/data.rs

@@ -54,6 +54,7 @@ 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();
@@ -127,6 +128,13 @@ pub fn diff_data_section(
         diff_generic_section(left, right, left_section_diff, right_section_diff)?;
     left_section_diff.data_diff = left_diff;
     right_section_diff.data_diff = right_diff;
+    // Use the highest match percent between two options:
+    // - Left symbols matching right symbols by name
+    // - Diff of the data itself
+    if left_section_diff.match_percent.unwrap_or(-1.0) < match_percent {
+        left_section_diff.match_percent = Some(match_percent);
+        right_section_diff.match_percent = Some(match_percent);
+    }
     Ok((left_section_diff, right_section_diff))
 }
 
@@ -191,3 +199,35 @@ pub fn diff_generic_section(
         ObjSectionDiff { symbols: vec![], data_diff: vec![], match_percent: Some(match_percent) },
     ))
 }
+
+/// Compare the addresses and sizes of each symbol in the BSS sections.
+pub fn diff_bss_section(
+    left: &ObjSection,
+    right: &ObjSection,
+    left_diff: &ObjSectionDiff,
+    right_diff: &ObjSectionDiff,
+) -> 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 mut match_percent = get_diff_ratio(&ops, left_sizes.len(), right_sizes.len()) * 100.0;
+
+    // Use the highest match percent between two options:
+    // - Left symbols matching right symbols by name
+    // - Diff of the addresses and sizes of each symbol
+    let (generic_diff, _) = diff_generic_section(left, right, left_diff, right_diff)?;
+    if generic_diff.match_percent.unwrap_or(-1.0) > match_percent {
+        match_percent = generic_diff.match_percent.unwrap();
+    }
+
+    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_symbol, diff_data_section, diff_data_symbol, diff_generic_section,
-            no_diff_symbol,
+            diff_bss_section, diff_bss_symbol, diff_data_section, diff_data_symbol,
+            diff_generic_section, no_diff_symbol,
         },
     },
     obj::{ObjInfo, ObjIns, ObjSection, ObjSectionKind, ObjSymbol, SymbolRef},
@@ -483,7 +483,7 @@ 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::Bss => {
+                ObjSectionKind::Code => {
                     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_generic_section(
@@ -507,6 +507,18 @@ pub fn diff_objs(
                     left_out.section_diff_mut(left_section_idx).merge(left_diff);
                     right_out.section_diff_mut(right_section_idx).merge(right_diff);
                 }
+                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_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);
+                }
             }
         }
     }
@@ -546,8 +558,8 @@ fn matching_symbols(
             for (symbol_idx, symbol) in section.symbols.iter().enumerate() {
                 let symbol_match = SymbolMatch {
                     left: Some(SymbolRef { section_idx, symbol_idx }),
-                    right: find_symbol(right, symbol, section),
-                    prev: find_symbol(prev, symbol, section),
+                    right: find_symbol(right, symbol, section, Some(&right_used)),
+                    prev: find_symbol(prev, symbol, section, None),
                     section_kind: section.kind,
                 };
                 matches.push(symbol_match);
@@ -579,7 +591,7 @@ fn matching_symbols(
                 matches.push(SymbolMatch {
                     left: None,
                     right: Some(symbol_ref),
-                    prev: find_symbol(prev, symbol, section),
+                    prev: find_symbol(prev, symbol, section, None),
                     section_kind: section.kind,
                 });
             }
@@ -604,6 +616,7 @@ fn find_symbol(
     obj: Option<&ObjInfo>,
     in_symbol: &ObjSymbol,
     in_section: &ObjSection,
+    used: Option<&HashSet<SymbolRef>>,
 ) -> Option<SymbolRef> {
     let obj = obj?;
     // Try to find an exact name match
@@ -641,13 +654,21 @@ fn find_symbol(
             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
-                }
-            }) {
+            if let Some((symbol_idx, _)) =
+                section.symbols.iter().enumerate().find(|&(symbol_idx, symbol)| {
+                    if used
+                        .map(|u| u.contains(&SymbolRef { section_idx, symbol_idx }))
+                        .unwrap_or(false)
+                    {
+                        return false;
+                    }
+                    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 });
             }
         }

objdiff-core/src/obj/read.rs

@@ -149,7 +149,7 @@ fn symbols_by_section(
             }
         }
     }
-    result.sort_by_key(|v| v.address);
+    result.sort_by(|a, b| a.address.cmp(&b.address).then(a.size.cmp(&b.size)));
     let mut iter = result.iter_mut().peekable();
     while let Some(symbol) = iter.next() {
         if symbol.size == 0 {