Skip to content

Commit 758ec33

Browse files
committed
Clean up code for inferring section symbol size
1 parent e54c57d commit 758ec33

File tree

1 file changed

+19
-32
lines changed

1 file changed

+19
-32
lines changed

objdiff-core/src/obj/read.rs

Lines changed: 19 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -49,37 +49,11 @@ fn map_symbol(
4949
{
5050
let section_name = section.name().context("Failed to process section name")?;
5151
name = format!("[{section_name}]");
52-
53-
let section_kind = map_section_kind(&section);
54-
if section_kind == SectionKind::Data {
55-
// For data section symbols, the size would normally be zero and excluded from the diff.
56-
// Instead we make them the size of all the data in the section so that the user can
57-
// diff entire sections by clicking on the section symbol at the top of the section.
58-
// We only do this for data sections, as there would be no point for code or bss sections.
59-
if let Some(last_symbol) = file
60-
.symbols()
61-
.filter(|s| {
62-
s.section_index() == Some(section.index())
63-
&& s.kind() == object::SymbolKind::Data
64-
&& s.size() > 0
65-
})
66-
.max_by_key(|s| s.address())
67-
{
68-
// Use the address that the last symbol ends at as the section size.
69-
size = last_symbol.address() + last_symbol.size();
70-
} else {
71-
// `section.size()` can include extra padding that doesn't correspond to any symbol,
72-
// so only fall back to it if there are no symbols to look at.
73-
size = section.size();
74-
}
75-
} else {
76-
// For non-data section symbols, set the size to zero. If the size is non-zero, it will
77-
// be included in the diff. Most of the time, this is duplicative, given that we'll have
78-
// function or object symbols that cover the same range. In the case of an empty
79-
// section, the size inference logic below will set the size back to the section size,
80-
// thus acting as a placeholder symbol.
81-
size = 0;
82-
}
52+
// For section symbols, set the size to zero. If the size is non-zero, it will be included
53+
// in the diff. The size inference logic below may set the size back to the section size for
54+
// some sections, thus acting as a placeholder symbol to allow diffing an entire section at
55+
// once.
56+
size = 0;
8357
}
8458

8559
let mut flags = arch.extra_symbol_flags(symbol);
@@ -233,7 +207,20 @@ fn infer_symbol_sizes(arch: &dyn Arch, symbols: &mut [Symbol], sections: &[Secti
233207
let section = &sections[section_idx];
234208
let next_address =
235209
next_symbol.map(|s| s.address).unwrap_or_else(|| section.address + section.size);
236-
let new_size = if section.kind == SectionKind::Code {
210+
let new_size = if symbol.kind == SymbolKind::Section && section.kind == SectionKind::Data {
211+
// For data section symbols, set their size to the section size. Then the user can diff
212+
// the entire section at once by clicking on the section symbol at the top.
213+
// `section.size` can include extra padding, so instead prefer using the address that
214+
// the last symbol ends at when there are symbols in the section.
215+
symbols
216+
.iter()
217+
.filter(|s| {
218+
s.section == Some(section_idx) && s.kind == SymbolKind::Object && s.size > 0
219+
})
220+
.map(|s| s.address + s.size)
221+
.max()
222+
.unwrap_or(section.size)
223+
} else if section.kind == SectionKind::Code {
237224
arch.infer_function_size(symbol, section, next_address)?
238225
} else {
239226
next_address.saturating_sub(symbol.address)

0 commit comments

Comments
 (0)