Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Combine data sections with equal names #76

Merged
merged 2 commits into from
Jun 19, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 14 additions & 9 deletions objdiff-cli/src/cmd/diff.rs
Original file line number Diff line number Diff line change
Expand Up @@ -809,23 +809,28 @@ impl FunctionDiffUi {

fn reload(&mut self) -> Result<()> {
let prev = self.right_obj.take();
let config = diff::DiffObjConfig {
relax_reloc_diffs: self.relax_reloc_diffs,
space_between_args: true, // TODO
combine_data_sections: false, // TODO
x86_formatter: Default::default(), // TODO
mips_abi: Default::default(), // TODO
mips_instr_category: Default::default(), // TODO
};
let target = self
.target_path
.as_deref()
.map(|p| obj::read::read(p).with_context(|| format!("Loading {}", p.display())))
.map(|p| {
obj::read::read(p, &config).with_context(|| format!("Loading {}", p.display()))
})
.transpose()?;
let base = self
.base_path
.as_deref()
.map(|p| obj::read::read(p).with_context(|| format!("Loading {}", p.display())))
.map(|p| {
obj::read::read(p, &config).with_context(|| format!("Loading {}", p.display()))
})
.transpose()?;
let config = diff::DiffObjConfig {
relax_reloc_diffs: self.relax_reloc_diffs,
space_between_args: true, // TODO
x86_formatter: Default::default(), // TODO
mips_abi: Default::default(), // TODO
mips_instr_category: Default::default(), // TODO
};
let result = diff::diff_objs(&config, target.as_ref(), base.as_ref(), prev.as_ref())?;

let left_sym = target.as_ref().and_then(|o| find_function(o, &self.symbol_name));
Expand Down
10 changes: 7 additions & 3 deletions objdiff-cli/src/cmd/report.rs
Original file line number Diff line number Diff line change
Expand Up @@ -230,17 +230,21 @@ fn report_object(
}
_ => {}
}
let config = diff::DiffObjConfig { relax_reloc_diffs: true, ..Default::default() };
let target = object
.target_path
.as_ref()
.map(|p| obj::read::read(p).with_context(|| format!("Failed to open {}", p.display())))
.map(|p| {
obj::read::read(p, &config).with_context(|| format!("Failed to open {}", p.display()))
})
.transpose()?;
let base = object
.base_path
.as_ref()
.map(|p| obj::read::read(p).with_context(|| format!("Failed to open {}", p.display())))
.map(|p| {
obj::read::read(p, &config).with_context(|| format!("Failed to open {}", p.display()))
})
.transpose()?;
let config = diff::DiffObjConfig { relax_reloc_diffs: true, ..Default::default() };
let result = diff::diff_objs(&config, target.as_ref(), base.as_ref(), None)?;
let mut unit = ReportUnit {
name: object.name().to_string(),
Expand Down
2 changes: 2 additions & 0 deletions objdiff-core/src/diff/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,7 @@ pub struct DiffObjConfig {
pub relax_reloc_diffs: bool,
#[serde(default = "default_true")]
pub space_between_args: bool,
pub combine_data_sections: bool,
// x86
pub x86_formatter: X86Formatter,
// MIPS
Expand All @@ -114,6 +115,7 @@ impl Default for DiffObjConfig {
Self {
relax_reloc_diffs: false,
space_between_args: true,
combine_data_sections: false,
x86_formatter: Default::default(),
mips_abi: Default::default(),
mips_instr_category: Default::default(),
Expand Down
106 changes: 104 additions & 2 deletions objdiff-core/src/obj/read.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use std::{fs, io::Cursor, path::Path};
use std::{collections::HashSet, fs, io::Cursor, path::Path};

use anyhow::{anyhow, bail, ensure, Context, Result};
use byteorder::{BigEndian, ReadBytesExt};
Expand All @@ -11,6 +11,7 @@ use object::{

use crate::{
arch::{new_arch, ObjArch},
diff::DiffObjConfig,
obj::{
split_meta::{SplitMeta, SPLITMETA_SECTION},
ObjInfo, ObjReloc, ObjSection, ObjSectionKind, ObjSymbol, ObjSymbolFlagSet, ObjSymbolFlags,
Expand Down Expand Up @@ -361,7 +362,105 @@ fn line_info(obj_file: &File<'_>, sections: &mut [ObjSection]) -> Result<()> {
Ok(())
}

pub fn read(obj_path: &Path) -> Result<ObjInfo> {
fn update_combined_symbol(symbol: ObjSymbol, address_change: i64) -> Result<ObjSymbol> {
Ok(ObjSymbol {
name: symbol.name,
demangled_name: symbol.demangled_name,
address: (symbol.address as i64 + address_change).try_into()?,
section_address: (symbol.section_address as i64 + address_change).try_into()?,
size: symbol.size,
size_known: symbol.size_known,
flags: symbol.flags,
addend: symbol.addend,
virtual_address: if let Some(virtual_address) = symbol.virtual_address {
Some((virtual_address as i64 + address_change).try_into()?)
} else {
None
},
})
}

fn combine_sections(section: ObjSection, combine: ObjSection) -> Result<ObjSection> {
let mut data = section.data;
data.extend(combine.data);

let address_change: i64 = (section.address + section.size) as i64 - combine.address as i64;
let mut symbols = section.symbols;
for symbol in combine.symbols {
symbols.push(update_combined_symbol(symbol, address_change)?);
}

let mut relocations = section.relocations;
for reloc in combine.relocations {
relocations.push(ObjReloc {
flags: reloc.flags,
address: (reloc.address as i64 + address_change).try_into()?,
target: reloc.target, // TODO: Should be updated?
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, this is tricky. Would probably require keeping track of every symbol and their adjusted address. Luckily this only affects the relocation display on hover, so this can be revisited later.

target_section: reloc.target_section, // TODO: Same as above
});
}

let mut line_info = section.line_info;
for (addr, line) in combine.line_info {
let key = (addr as i64 + address_change).try_into()?;
line_info.insert(key, line);
}

Ok(ObjSection {
name: section.name,
kind: section.kind,
address: section.address,
size: section.size + combine.size,
data,
orig_index: section.orig_index,
symbols,
relocations,
virtual_address: section.virtual_address,
line_info,
})
}

fn combine_data_sections(sections: &mut Vec<ObjSection>) -> Result<()> {
let names_to_combine: HashSet<_> = sections
.iter()
.filter(|s| s.kind == ObjSectionKind::Data)
.map(|s| s.name.clone())
.collect();

for name in names_to_combine {
// Take section with lowest index
let (mut section_index, _) = sections
.iter()
.enumerate()
.filter(|(_, s)| s.name == name)
.min_by_key(|(_, s)| s.orig_index)
// Should not happen
.context("No combine section found with name")?;
let mut section = sections.remove(section_index);

// Remove equally named sections
let mut combines = vec![];
for i in (0..sections.len()).rev() {
if sections[i].name != name || sections[i].orig_index == section.orig_index {
continue;
}
combines.push(sections.remove(i));
if i < section_index {
section_index -= 1;
}
}

// Combine sections ordered by index
combines.sort_unstable_by_key(|c| c.orig_index);
for combine in combines {
section = combine_sections(section, combine)?;
}
sections.insert(section_index, section);
}
Ok(())
}

pub fn read(obj_path: &Path, config: &DiffObjConfig) -> Result<ObjInfo> {
let (data, timestamp) = {
let file = fs::File::open(obj_path)?;
let timestamp = FileTime::from_last_modification_time(&file.metadata()?);
Expand All @@ -377,6 +476,9 @@ pub fn read(obj_path: &Path) -> Result<ObjInfo> {
section.relocations =
relocations_by_section(arch.as_ref(), &obj_file, section, split_meta.as_ref())?;
}
if config.combine_data_sections {
combine_data_sections(&mut sections)?;
}
line_info(&obj_file, &mut sections)?;
let common = common_symbols(arch.as_ref(), &obj_file, split_meta.as_ref())?;
Ok(ObjInfo { arch, path: obj_path.to_owned(), timestamp, sections, common, split_meta })
Expand Down
10 changes: 10 additions & 0 deletions objdiff-gui/src/app.rs
Original file line number Diff line number Diff line change
Expand Up @@ -559,6 +559,16 @@ impl eframe::App for App {
{
config.queue_reload = true;
}
if ui
.checkbox(
&mut config.diff_obj_config.combine_data_sections,
"Combine data sections",
)
.on_hover_text("Combines data sections with equal names.")
.changed()
{
config.queue_reload = true;
}
});
});
});
Expand Down
4 changes: 2 additions & 2 deletions objdiff-gui/src/jobs/objdiff.rs
Original file line number Diff line number Diff line change
Expand Up @@ -236,7 +236,7 @@ fn run_build(
total,
&cancel,
)?;
Some(read::read(target_path).with_context(|| {
Some(read::read(target_path, &config.diff_obj_config).with_context(|| {
format!("Failed to read object '{}'", target_path.display())
})?)
}
Expand All @@ -253,7 +253,7 @@ fn run_build(
&cancel,
)?;
Some(
read::read(base_path)
read::read(base_path, &config.diff_obj_config)
.with_context(|| format!("Failed to read object '{}'", base_path.display()))?,
)
}
Expand Down