From 1fd901a8636bc41def406495892cfba8a0f15cf2 Mon Sep 17 00:00:00 2001 From: Aetias <144526980+AetiasHax@users.noreply.github.com> Date: Wed, 19 Jun 2024 06:05:24 +0200 Subject: [PATCH] Option to combine data sections (#76) Co-authored-by: Luke Street --- objdiff-cli/src/cmd/diff.rs | 25 +++++--- objdiff-cli/src/cmd/report.rs | 10 ++- objdiff-core/src/diff/mod.rs | 2 + objdiff-core/src/obj/read.rs | 106 +++++++++++++++++++++++++++++++- objdiff-gui/src/app.rs | 10 +++ objdiff-gui/src/jobs/objdiff.rs | 4 +- 6 files changed, 140 insertions(+), 17 deletions(-) diff --git a/objdiff-cli/src/cmd/diff.rs b/objdiff-cli/src/cmd/diff.rs index d0992a1..bc02acc 100644 --- a/objdiff-cli/src/cmd/diff.rs +++ b/objdiff-cli/src/cmd/diff.rs @@ -809,23 +809,28 @@ impl FunctionDiffUi { fn reload(&mut self) -> Result<()> { let prev = self.right_obj.take(); - let target = self - .target_path - .as_deref() - .map(|p| obj::read::read(p).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()))) - .transpose()?; 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, &config).with_context(|| format!("Loading {}", p.display())) + }) + .transpose()?; + let base = self + .base_path + .as_deref() + .map(|p| { + obj::read::read(p, &config).with_context(|| format!("Loading {}", p.display())) + }) + .transpose()?; 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)); diff --git a/objdiff-cli/src/cmd/report.rs b/objdiff-cli/src/cmd/report.rs index 2b671c9..40ed7df 100644 --- a/objdiff-cli/src/cmd/report.rs +++ b/objdiff-cli/src/cmd/report.rs @@ -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(), diff --git a/objdiff-core/src/diff/mod.rs b/objdiff-core/src/diff/mod.rs index a983fcc..0a83c52 100644 --- a/objdiff-core/src/diff/mod.rs +++ b/objdiff-core/src/diff/mod.rs @@ -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 @@ -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(), diff --git a/objdiff-core/src/obj/read.rs b/objdiff-core/src/obj/read.rs index 9b75923..cde880c 100644 --- a/objdiff-core/src/obj/read.rs +++ b/objdiff-core/src/obj/read.rs @@ -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}; @@ -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, @@ -361,7 +362,105 @@ fn line_info(obj_file: &File<'_>, sections: &mut [ObjSection]) -> Result<()> { Ok(()) } -pub fn read(obj_path: &Path) -> Result { +fn update_combined_symbol(symbol: ObjSymbol, address_change: i64) -> Result { + 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 { + 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? + 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) -> 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 { let (data, timestamp) = { let file = fs::File::open(obj_path)?; let timestamp = FileTime::from_last_modification_time(&file.metadata()?); @@ -377,6 +476,9 @@ pub fn read(obj_path: &Path) -> Result { 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 }) diff --git a/objdiff-gui/src/app.rs b/objdiff-gui/src/app.rs index 6cb6e50..6a4ac98 100644 --- a/objdiff-gui/src/app.rs +++ b/objdiff-gui/src/app.rs @@ -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; + } }); }); }); diff --git a/objdiff-gui/src/jobs/objdiff.rs b/objdiff-gui/src/jobs/objdiff.rs index cd89087..4535002 100644 --- a/objdiff-gui/src/jobs/objdiff.rs +++ b/objdiff-gui/src/jobs/objdiff.rs @@ -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()) })?) } @@ -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()))?, ) }