Skip to content

Commit a19ffa8

Browse files
committed
Sort all symbols by address when reading the object
This fixes an underreport bug in diff_bss_section.
1 parent 4086cec commit a19ffa8

18 files changed

+3012
-3008
lines changed

objdiff-core/src/diff/mod.rs

Lines changed: 5 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -748,14 +748,11 @@ fn find_symbol(
748748
}
749749

750750
// Try to find a symbol with a matching name
751-
if let Some((symbol_idx, _)) = unmatched_symbols(obj, used)
752-
.filter(|&(_, symbol)| {
753-
symbol_name_matches(in_symbol, symbol)
754-
&& symbol_section_kind(obj, symbol) == section_kind
755-
&& symbol_section(obj, symbol).is_some_and(|(name, _)| name == section_name)
756-
})
757-
.min_by_key(|&(_, symbol)| (symbol.section, symbol.address))
758-
{
751+
if let Some((symbol_idx, _)) = unmatched_symbols(obj, used).find(|&(_, symbol)| {
752+
symbol_name_matches(in_symbol, symbol)
753+
&& symbol_section_kind(obj, symbol) == section_kind
754+
&& symbol_section(obj, symbol).is_some_and(|(name, _)| name == section_name)
755+
}) {
759756
return Some(symbol_idx);
760757
}
761758

objdiff-core/src/obj/read.rs

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ use alloc::{
88
use core::{cmp::Ordering, num::NonZeroU64};
99

1010
use anyhow::{Context, Result, anyhow, bail, ensure};
11+
use itertools::Itertools;
1112
use object::{Object as _, ObjectSection as _, ObjectSymbol as _};
1213

1314
use crate::{
@@ -166,7 +167,13 @@ fn map_symbols(
166167
let symbol_count = obj_file.symbols().count();
167168
let mut symbols = Vec::<Symbol>::with_capacity(symbol_count + obj_file.sections().count());
168169
let mut symbol_indices = Vec::<usize>::with_capacity(symbol_count + 1);
169-
for obj_symbol in obj_file.symbols() {
170+
let obj_symbols = obj_file.symbols();
171+
// symbols() is not guaranteed to be sorted by address.
172+
// We sort it here to fix pairing bugs with diff algorithms that assume the symbols are ordered.
173+
// Sorting everything here once is less expensive than sorting subsets later in expensive loops.
174+
let obj_symbols =
175+
obj_symbols.sorted_by_key(|symbol| (symbol.section_index().map(|i| i.0), symbol.address()));
176+
for obj_symbol in obj_symbols {
170177
if symbol_indices.len() <= obj_symbol.index().0 {
171178
symbol_indices.resize(obj_symbol.index().0 + 1, usize::MAX);
172179
}

0 commit comments

Comments
 (0)