From 5bfaaaaf650e92219939acb4f3bc3802660d3710 Mon Sep 17 00:00:00 2001 From: Luke Street Date: Mon, 20 May 2024 17:38:20 -0600 Subject: [PATCH 1/4] Instruction hover / context menu improvements --- objdiff-cli/src/cmd/report.rs | 2 +- objdiff-core/src/arch/mips.rs | 2 + objdiff-core/src/arch/ppc.rs | 2 + objdiff-core/src/arch/x86.rs | 4 +- objdiff-core/src/diff/display.rs | 6 +-- objdiff-core/src/obj/mod.rs | 2 + objdiff-core/src/obj/read.rs | 2 +- objdiff-gui/src/views/data_diff.rs | 4 +- objdiff-gui/src/views/function_diff.rs | 64 ++++++++++++++++++++------ objdiff-gui/src/views/symbol_diff.rs | 5 +- 10 files changed, 68 insertions(+), 25 deletions(-) diff --git a/objdiff-cli/src/cmd/report.rs b/objdiff-cli/src/cmd/report.rs index 475cb58..50c7929 100644 --- a/objdiff-cli/src/cmd/report.rs +++ b/objdiff-cli/src/cmd/report.rs @@ -476,7 +476,7 @@ fn read_report(path: &Path) -> Result { fn serialize_hex(x: &Option, s: S) -> Result where S: serde::Serializer { if let Some(x) = x { - s.serialize_str(&format!("{:#X}", x)) + s.serialize_str(&format!("{:#x}", x)) } else { s.serialize_none() } diff --git a/objdiff-core/src/arch/mips.rs b/objdiff-core/src/arch/mips.rs index 7abce86..ba4ca80 100644 --- a/objdiff-core/src/arch/mips.rs +++ b/objdiff-core/src/arch/mips.rs @@ -49,6 +49,7 @@ impl ObjArch for ObjArchMips { let code = self.endianness.read_u32_bytes(chunk.try_into()?); let instruction = Instruction::new(code, cur_addr, InstrCategory::CPU); + let formatted = instruction.disassemble(None, 0); let op = instruction.unique_id as u16; ops.push(op); @@ -123,6 +124,7 @@ impl ObjArch for ObjArchMips { reloc: reloc.cloned(), branch_dest, line, + formatted, orig: None, }); cur_addr += 4; diff --git a/objdiff-core/src/arch/ppc.rs b/objdiff-core/src/arch/ppc.rs index 39b463b..485ef6b 100644 --- a/objdiff-core/src/arch/ppc.rs +++ b/objdiff-core/src/arch/ppc.rs @@ -59,6 +59,7 @@ impl ObjArch for ObjArchPpc { let orig = ins.basic().to_string(); let simplified = ins.simplified(); + let formatted = simplified.to_string(); let mut reloc_arg = None; if let Some(reloc) = reloc { @@ -144,6 +145,7 @@ impl ObjArch for ObjArchPpc { op: ins.op as u16, branch_dest, line, + formatted, orig: Some(orig), }); } diff --git a/objdiff-core/src/arch/x86.rs b/objdiff-core/src/arch/x86.rs index 90501aa..89e0345 100644 --- a/objdiff-core/src/arch/x86.rs +++ b/objdiff-core/src/arch/x86.rs @@ -57,6 +57,7 @@ impl ObjArch for ObjArchX86 { reloc: None, branch_dest: None, line: None, + formatted: String::new(), orig: None, }, error: None, @@ -81,6 +82,7 @@ impl ObjArch for ObjArchX86 { reloc: reloc.cloned(), branch_dest: None, line: obj.line_info.as_ref().and_then(|m| m.get(&address).cloned()), + formatted: String::new(), orig: None, }; // Run the formatter, which will populate output.ins @@ -89,7 +91,7 @@ impl ObjArch for ObjArchX86 { return Err(error); } ensure!(output.ins_operands.len() == output.ins.args.len()); - output.ins.orig = Some(output.formatted.clone()); + output.ins.formatted.clone_from(&output.formatted); // Make sure we've put the relocation somewhere in the instruction if reloc.is_some() && !output.ins.args.iter().any(|a| matches!(a, ObjInsArg::Reloc)) { diff --git a/objdiff-core/src/diff/display.rs b/objdiff-core/src/diff/display.rs index ee097e0..95d7a38 100644 --- a/objdiff-core/src/diff/display.rs +++ b/objdiff-core/src/diff/display.rs @@ -5,7 +5,7 @@ use crate::{ obj::{ObjInsArg, ObjInsArgValue, ObjReloc, ObjSymbol}, }; -#[derive(Debug, Clone)] +#[derive(Debug, Copy, Clone)] pub enum DiffText<'a> { /// Basic text Basic(&'a str), @@ -95,8 +95,8 @@ fn display_reloc_name( ) -> Result<(), E> { cb(DiffText::Symbol(&reloc.target))?; match reloc.target.addend.cmp(&0i64) { - Ordering::Greater => cb(DiffText::Basic(&format!("+{:#X}", reloc.target.addend))), - Ordering::Less => cb(DiffText::Basic(&format!("-{:#X}", -reloc.target.addend))), + Ordering::Greater => cb(DiffText::Basic(&format!("+{:#x}", reloc.target.addend))), + Ordering::Less => cb(DiffText::Basic(&format!("-{:#x}", -reloc.target.addend))), _ => Ok(()), } } diff --git a/objdiff-core/src/obj/mod.rs b/objdiff-core/src/obj/mod.rs index cfa1d20..02f69b7 100644 --- a/objdiff-core/src/obj/mod.rs +++ b/objdiff-core/src/obj/mod.rs @@ -101,6 +101,8 @@ pub struct ObjIns { pub branch_dest: Option, /// Line number pub line: Option, + /// Formatted instruction + pub formatted: String, /// Original (unsimplified) instruction pub orig: Option, } diff --git a/objdiff-core/src/obj/read.rs b/objdiff-core/src/obj/read.rs index c65956f..017d232 100644 --- a/objdiff-core/src/obj/read.rs +++ b/objdiff-core/src/obj/read.rs @@ -252,7 +252,7 @@ fn relocations_by_section( } else { reloc.addend() }; - // println!("Reloc: {reloc:?}, symbol: {symbol:?}, addend: {addend:#X}"); + // println!("Reloc: {reloc:?}, symbol: {symbol:?}, addend: {addend:#x}"); let target = match symbol.kind() { SymbolKind::Text | SymbolKind::Data | SymbolKind::Label | SymbolKind::Unknown => { to_obj_symbol(arch, obj_file, &symbol, addend, split_meta) diff --git a/objdiff-gui/src/views/data_diff.rs b/objdiff-gui/src/views/data_diff.rs index 1ad8505..d30f6dc 100644 --- a/objdiff-gui/src/views/data_diff.rs +++ b/objdiff-gui/src/views/data_diff.rs @@ -26,7 +26,7 @@ fn data_row_ui(ui: &mut egui::Ui, address: usize, diffs: &[ObjDataDiff], appeara } let mut job = LayoutJob::default(); write_text( - format!("{address:08X}: ").as_str(), + format!("{address:08x}: ").as_str(), appearance.text_color, &mut job, appearance.code_font.clone(), @@ -47,7 +47,7 @@ fn data_row_ui(ui: &mut egui::Ui, address: usize, diffs: &[ObjDataDiff], appeara } else { let mut text = String::new(); for byte in &diff.data { - text.push_str(format!("{byte:02X} ").as_str()); + text.push_str(format!("{byte:02x} ").as_str()); cur_addr += 1; if cur_addr % 8 == 0 { text.push(' '); diff --git a/objdiff-gui/src/views/function_diff.rs b/objdiff-gui/src/views/function_diff.rs index 4c8bd92..8421782 100644 --- a/objdiff-gui/src/views/function_diff.rs +++ b/objdiff-gui/src/views/function_diff.rs @@ -1,6 +1,6 @@ use std::default::Default; -use egui::{text::LayoutJob, Align, Label, Layout, Sense, Vec2, Widget}; +use egui::{text::LayoutJob, Align, Label, Layout, Response, Sense, Vec2, Widget}; use egui_extras::{Column, TableBuilder, TableRow}; use objdiff_core::{ arch::ObjArch, @@ -27,6 +27,7 @@ fn ins_hover_ui( arch: &dyn ObjArch, section: &ObjSection, ins: &ObjIns, + symbol: &ObjSymbol, appearance: &Appearance, ) { ui.scope(|ui| { @@ -35,10 +36,18 @@ fn ins_hover_ui( let offset = ins.address - section.address; ui.label(format!( - "{:02X?}", + "{:02x?}", §ion.data[offset as usize..(offset + ins.size as u64) as usize] )); + if let Some(virtual_address) = symbol.virtual_address { + let offset = ins.address - symbol.address; + ui.colored_label( + appearance.replace_color, + format!("Virtual address: {:#x}", virtual_address + offset), + ); + } + if let Some(orig) = &ins.orig { ui.label(format!("Original: {}", orig)); } @@ -77,12 +86,33 @@ fn ins_hover_ui( }); } -fn ins_context_menu(ui: &mut egui::Ui, ins: &ObjIns) { +fn ins_context_menu(ui: &mut egui::Ui, section: &ObjSection, ins: &ObjIns, symbol: &ObjSymbol) { ui.scope(|ui| { ui.style_mut().override_text_style = Some(egui::TextStyle::Monospace); ui.style_mut().wrap = Some(false); - // if ui.button("Copy hex").clicked() {} + if ui.button(format!("Copy \"{}\"", ins.formatted)).clicked() { + ui.output_mut(|output| output.copied_text.clone_from(&ins.formatted)); + ui.close_menu(); + } + + let mut hex_string = "0x".to_string(); + for byte in §ion.data[ins.address as usize..(ins.address + ins.size as u64) as usize] { + hex_string.push_str(&format!("{:02x}", byte)); + } + if ui.button(format!("Copy \"{hex_string}\" (instruction bytes)")).clicked() { + ui.output_mut(|output| output.copied_text = hex_string); + ui.close_menu(); + } + + if let Some(virtual_address) = symbol.virtual_address { + let offset = ins.address - symbol.address; + let offset_string = format!("{:#x}", virtual_address + offset); + if ui.button(format!("Copy \"{offset_string}\" (virtual address)")).clicked() { + ui.output_mut(|output| output.copied_text = offset_string); + ui.close_menu(); + } + } for arg in &ins.args { if let ObjInsArg::Arg(arg) = arg { @@ -144,6 +174,7 @@ fn diff_text_ui( appearance: &Appearance, ins_view_state: &mut FunctionViewState, space_width: f32, + response_cb: impl Fn(Response) -> Response, ) { let label_text; let mut base_color = match ins_diff.kind { @@ -204,13 +235,13 @@ fn diff_text_ui( let len = label_text.len(); let highlight = ins_view_state.highlight == text; - let response = Label::new(LayoutJob::single_section( + let mut response = Label::new(LayoutJob::single_section( label_text, appearance.code_text_format(base_color, highlight), )) .sense(Sense::click()) .ui(ui); - response.context_menu(|ui| ins_context_menu(ui, ins_diff.ins.as_ref().unwrap())); + response = response_cb(response); if response.clicked() { if highlight { ins_view_state.highlight = HighlightKind::None; @@ -229,6 +260,7 @@ fn asm_row_ui( symbol: &ObjSymbol, appearance: &Appearance, ins_view_state: &mut FunctionViewState, + response_cb: impl Fn(Response) -> Response, ) { ui.spacing_mut().item_spacing.x = 0.0; if ins_diff.kind != ObjInsDiffKind::None { @@ -236,7 +268,7 @@ fn asm_row_ui( } let space_width = ui.fonts(|f| f.glyph_width(&appearance.code_font, ' ')); display_diff(ins_diff, symbol.address, |text| { - diff_text_ui(ui, text, ins_diff, appearance, ins_view_state, space_width); + diff_text_ui(ui, text, ins_diff, appearance, ins_view_state, space_width, &response_cb); Ok::<_, ()>(()) }) .unwrap(); @@ -251,14 +283,20 @@ fn asm_col_ui( ) { let (section, symbol) = obj.0.section_symbol(symbol_ref); let ins_diff = &obj.1.symbol_diff(symbol_ref).instructions[row.index()]; + let response_cb = |response: Response| { + if let Some(ins) = &ins_diff.ins { + response.context_menu(|ui| ins_context_menu(ui, section, ins, symbol)); + response.on_hover_ui_at_pointer(|ui| { + ins_hover_ui(ui, obj.0.arch.as_ref(), section, ins, symbol, appearance) + }) + } else { + response + } + }; let (_, response) = row.col(|ui| { - asm_row_ui(ui, ins_diff, symbol, appearance, ins_view_state); + asm_row_ui(ui, ins_diff, symbol, appearance, ins_view_state, response_cb); }); - if let Some(ins) = &ins_diff.ins { - response.on_hover_ui_at_pointer(|ui| { - ins_hover_ui(ui, obj.0.arch.as_ref(), section, ins, appearance) - }); - } + response_cb(response); } fn empty_col_ui(row: &mut TableRow<'_, '_>) { diff --git a/objdiff-gui/src/views/symbol_diff.rs b/objdiff-gui/src/views/symbol_diff.rs index b28ecb2..58d0736 100644 --- a/objdiff-gui/src/views/symbol_diff.rs +++ b/objdiff-gui/src/views/symbol_diff.rs @@ -171,10 +171,7 @@ fn symbol_hover_ui(ui: &mut Ui, symbol: &ObjSymbol, appearance: &Appearance) { ); } if let Some(address) = symbol.virtual_address { - ui.colored_label( - appearance.highlight_color, - format!("Virtual address: {:#x}", address), - ); + ui.colored_label(appearance.replace_color, format!("Virtual address: {:#x}", address)); } }); } From 854dc9e4f5f917d14d0c6c894804c340cc911cf2 Mon Sep 17 00:00:00 2001 From: Aetias <144526980+AetiasHax@users.noreply.github.com> Date: Tue, 21 May 2024 01:38:58 +0200 Subject: [PATCH 2/4] Use base arch when processing base code (#62) --- objdiff-core/src/diff/code.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/objdiff-core/src/diff/code.rs b/objdiff-core/src/diff/code.rs index 00f064c..f676440 100644 --- a/objdiff-core/src/diff/code.rs +++ b/objdiff-core/src/diff/code.rs @@ -39,7 +39,7 @@ pub fn diff_code( config: &DiffObjConfig, ) -> Result<(ObjSymbolDiff, ObjSymbolDiff)> { let left_out = left_obj.arch.process_code(left_obj, left_symbol_ref, config)?; - let right_out = left_obj.arch.process_code(right_obj, right_symbol_ref, config)?; + let right_out = right_obj.arch.process_code(right_obj, right_symbol_ref, config)?; let mut left_diff = Vec::::new(); let mut right_diff = Vec::::new(); From ee9cef4c6fe5d3cc7f4f34404840cde233dda681 Mon Sep 17 00:00:00 2001 From: Luke Street Date: Mon, 20 May 2024 21:05:43 -0600 Subject: [PATCH 3/4] Rework DWARF line info parsing - Properly handles multiple sections in DWARF 1 - line_info moved into ObjSection - DWARF 2 parser no longer errors with no .text section - Both parsers properly skip empty sections --- objdiff-core/src/arch/mips.rs | 11 ++-- objdiff-core/src/arch/ppc.rs | 11 ++-- objdiff-core/src/arch/x86.rs | 8 +-- objdiff-core/src/obj/mod.rs | 13 ++-- objdiff-core/src/obj/read.rs | 116 ++++++++++++++++++---------------- 5 files changed, 78 insertions(+), 81 deletions(-) diff --git a/objdiff-core/src/arch/mips.rs b/objdiff-core/src/arch/mips.rs index f0355bb..fa69c64 100644 --- a/objdiff-core/src/arch/mips.rs +++ b/objdiff-core/src/arch/mips.rs @@ -1,7 +1,7 @@ use std::borrow::Cow; use anyhow::{bail, Result}; -use object::{elf, Endian, Endianness, File, Object, Relocation, RelocationFlags, SectionIndex}; +use object::{elf, Endian, Endianness, File, Object, Relocation, RelocationFlags}; use rabbitizer::{config, Abi, InstrCategory, Instruction, OperandType}; use crate::{ @@ -38,9 +38,6 @@ impl ObjArch for ObjArchMips { let code = §ion.data [symbol.section_address as usize..(symbol.section_address + symbol.size) as usize]; - let line_info = - obj.line_info.as_ref().and_then(|map| map.get(&SectionIndex(section.orig_index))); - let start_address = symbol.address; let end_address = symbol.address + symbol.size; let ins_count = code.len() / 4; @@ -114,8 +111,10 @@ impl ObjArch for ObjArchMips { } } } - let line = - line_info.and_then(|map| map.range(..=cur_addr as u64).last().map(|(_, &b)| b)); + let line = section + .line_info + .as_ref() + .and_then(|map| map.range(..=cur_addr as u64).last().map(|(_, &b)| b)); insts.push(ObjIns { address: cur_addr as u64, size: 4, diff --git a/objdiff-core/src/arch/ppc.rs b/objdiff-core/src/arch/ppc.rs index f657787..6b21ea4 100644 --- a/objdiff-core/src/arch/ppc.rs +++ b/objdiff-core/src/arch/ppc.rs @@ -1,7 +1,7 @@ use std::borrow::Cow; use anyhow::{bail, Result}; -use object::{elf, File, Relocation, RelocationFlags, SectionIndex}; +use object::{elf, File, Relocation, RelocationFlags}; use ppc750cl::{Argument, InsIter, GPR}; use crate::{ @@ -39,9 +39,6 @@ impl ObjArch for ObjArchPpc { let code = §ion.data [symbol.section_address as usize..(symbol.section_address + symbol.size) as usize]; - let line_info = - obj.line_info.as_ref().and_then(|map| map.get(&SectionIndex(section.orig_index))); - let ins_count = code.len() / 4; let mut ops = Vec::::with_capacity(ins_count); let mut insts = Vec::::with_capacity(ins_count); @@ -135,8 +132,10 @@ impl ObjArch for ObjArchPpc { } ops.push(ins.op as u16); - let line = - line_info.and_then(|map| map.range(..=cur_addr as u64).last().map(|(_, &b)| b)); + let line = section + .line_info + .as_ref() + .and_then(|map| map.range(..=cur_addr as u64).last().map(|(_, &b)| b)); insts.push(ObjIns { address: cur_addr as u64, size: 4, diff --git a/objdiff-core/src/arch/x86.rs b/objdiff-core/src/arch/x86.rs index db961e0..cd65690 100644 --- a/objdiff-core/src/arch/x86.rs +++ b/objdiff-core/src/arch/x86.rs @@ -6,7 +6,7 @@ use iced_x86::{ GasFormatter, Instruction, IntelFormatter, MasmFormatter, NasmFormatter, NumberKind, OpKind, PrefixKind, Register, }; -use object::{pe, Endian, Endianness, File, Object, Relocation, RelocationFlags, SectionIndex}; +use object::{pe, Endian, Endianness, File, Object, Relocation, RelocationFlags}; use crate::{ arch::{ObjArch, ProcessCodeResult}, @@ -36,9 +36,6 @@ impl ObjArch for ObjArchX86 { let code = §ion.data [symbol.section_address as usize..(symbol.section_address + symbol.size) as usize]; - let line_info = - obj.line_info.as_ref().and_then(|map| map.get(&SectionIndex(section.orig_index))); - let mut result = ProcessCodeResult { ops: Vec::new(), insts: Vec::new() }; let mut decoder = Decoder::with_ip(self.bits, code, symbol.address, DecoderOptions::NONE); let mut formatter: Box = match config.x86_formatter { @@ -76,6 +73,7 @@ impl ObjArch for ObjArchX86 { .relocations .iter() .find(|r| r.address >= address && r.address < address + instruction.len() as u64); + let line = section.line_info.as_ref().and_then(|m| m.get(&address).cloned()); output.ins = ObjIns { address, size: instruction.len() as u8, @@ -84,7 +82,7 @@ impl ObjArch for ObjArchX86 { args: vec![], reloc: reloc.cloned(), branch_dest: None, - line: line_info.and_then(|m| m.get(&address).cloned()), + line, formatted: String::new(), orig: None, }; diff --git a/objdiff-core/src/obj/mod.rs b/objdiff-core/src/obj/mod.rs index df93d81..c8534c9 100644 --- a/objdiff-core/src/obj/mod.rs +++ b/objdiff-core/src/obj/mod.rs @@ -1,16 +1,11 @@ pub mod read; pub mod split_meta; -use std::{ - borrow::Cow, - collections::{BTreeMap, HashMap}, - fmt, - path::PathBuf, -}; +use std::{borrow::Cow, collections::BTreeMap, fmt, path::PathBuf}; use filetime::FileTime; use flagset::{flags, FlagSet}; -use object::{RelocationFlags, SectionIndex}; +use object::RelocationFlags; use split_meta::SplitMeta; use crate::{arch::ObjArch, util::ReallySigned}; @@ -44,6 +39,8 @@ pub struct ObjSection { pub symbols: Vec, pub relocations: Vec, pub virtual_address: Option, + /// Line number info (.line or .debug_line section) + pub line_info: Option>, } #[derive(Debug, Clone, Eq, PartialEq)] @@ -133,8 +130,6 @@ pub struct ObjInfo { pub sections: Vec, /// Common BSS symbols pub common: Vec, - /// Line number info (.line or .debug_line section) - pub line_info: Option>>, /// Split object metadata (.note.split section) pub split_meta: Option, } diff --git a/objdiff-core/src/obj/read.rs b/objdiff-core/src/obj/read.rs index 6bc3450..c89c03e 100644 --- a/objdiff-core/src/obj/read.rs +++ b/objdiff-core/src/obj/read.rs @@ -1,9 +1,4 @@ -use std::{ - collections::{BTreeMap, HashMap}, - fs, - io::Cursor, - path::Path, -}; +use std::{fs, io::Cursor, path::Path}; use anyhow::{anyhow, bail, ensure, Context, Result}; use byteorder::{BigEndian, ReadBytesExt}; @@ -116,6 +111,7 @@ fn filter_sections(obj_file: &File<'_>, split_meta: Option<&SplitMeta>) -> Resul symbols: Vec::new(), relocations: Vec::new(), virtual_address, + line_info: None, }); } result.sort_by(|a, b| a.name.cmp(&b.name)); @@ -273,46 +269,47 @@ fn relocations_by_section( Ok(relocations) } -fn line_info(obj_file: &File<'_>) -> Result>>> { - let mut map = HashMap::new(); - +fn line_info(obj_file: &File<'_>, sections: &mut [ObjSection]) -> Result<()> { // DWARF 1.1 if let Some(section) = obj_file.section_by_name(".line") { - if section.size() == 0 { - return Ok(None); - } - let text_section = obj_file - .sections() - .find(|s| s.kind() == SectionKind::Text) - .context("No text section found for line info")?; - let mut lines = BTreeMap::new(); - let data = section.uncompressed_data()?; let mut reader = Cursor::new(data.as_ref()); - let size = reader.read_u32::()?; - let base_address = reader.read_u32::()? as u64; - while reader.position() < size as u64 { - let line_number = reader.read_u32::()? as u64; - let statement_pos = reader.read_u16::()?; - if statement_pos != 0xFFFF { - log::warn!("Unhandled statement pos {}", statement_pos); + let mut text_sections = obj_file.sections().filter(|s| s.kind() == SectionKind::Text); + while reader.position() < data.len() as u64 { + let text_section_index = text_sections + .next() + .ok_or_else(|| anyhow!("Next text section not found for line info"))? + .index() + .0; + let start = reader.position(); + let size = reader.read_u32::()?; + let base_address = reader.read_u32::()? as u64; + let Some(out_section) = + sections.iter_mut().find(|s| s.orig_index == text_section_index) + else { + // Skip line info for sections we filtered out + reader.set_position(start + size as u64); + continue; + }; + let lines = out_section.line_info.get_or_insert_with(Default::default); + let end = start + size as u64; + while reader.position() < end { + let line_number = reader.read_u32::()? as u64; + let statement_pos = reader.read_u16::()?; + if statement_pos != 0xFFFF { + log::warn!("Unhandled statement pos {}", statement_pos); + } + let address_delta = reader.read_u32::()? as u64; + lines.insert(base_address + address_delta, line_number); + log::debug!("Line: {:#x} -> {}", base_address + address_delta, line_number); } - let address_delta = reader.read_u32::()? as u64; - lines.insert(base_address + address_delta, line_number); } - - map.insert(text_section.index(), lines); } // DWARF 2+ #[cfg(feature = "dwarf")] { - let mut text_sections = obj_file.sections().filter(|s| s.kind() == SectionKind::Text); - let first_section = text_sections.next().context("No text section found for line info")?; - map.insert(first_section.index(), BTreeMap::new()); - let mut lines = map.get_mut(&first_section.index()).unwrap(); - let dwarf_cow = gimli::DwarfSections::load(|id| { Ok::<_, gimli::Error>( obj_file @@ -327,32 +324,48 @@ fn line_info(obj_file: &File<'_>) -> Result Result { @@ -371,16 +384,9 @@ pub fn read(obj_path: &Path) -> Result { section.relocations = relocations_by_section(arch.as_ref(), &obj_file, section, split_meta.as_ref())?; } + 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, - line_info: line_info(&obj_file)?, - split_meta, - }) + Ok(ObjInfo { arch, path: obj_path.to_owned(), timestamp, sections, common, split_meta }) } pub fn has_function(obj_path: &Path, symbol_name: &str) -> Result { From 88af3211920e7ad87929606e8adf7b42e0235c78 Mon Sep 17 00:00:00 2001 From: Luke Street Date: Tue, 21 May 2024 09:24:06 -0600 Subject: [PATCH 4/4] Simplify line_info (no Option) --- objdiff-core/src/arch/mips.rs | 5 +---- objdiff-core/src/arch/ppc.rs | 5 +---- objdiff-core/src/arch/x86.rs | 2 +- objdiff-core/src/obj/mod.rs | 2 +- objdiff-core/src/obj/read.rs | 9 ++++----- 5 files changed, 8 insertions(+), 15 deletions(-) diff --git a/objdiff-core/src/arch/mips.rs b/objdiff-core/src/arch/mips.rs index fa69c64..109d0bb 100644 --- a/objdiff-core/src/arch/mips.rs +++ b/objdiff-core/src/arch/mips.rs @@ -111,10 +111,7 @@ impl ObjArch for ObjArchMips { } } } - let line = section - .line_info - .as_ref() - .and_then(|map| map.range(..=cur_addr as u64).last().map(|(_, &b)| b)); + let line = section.line_info.range(..=cur_addr as u64).last().map(|(_, &b)| b); insts.push(ObjIns { address: cur_addr as u64, size: 4, diff --git a/objdiff-core/src/arch/ppc.rs b/objdiff-core/src/arch/ppc.rs index 6b21ea4..d167c59 100644 --- a/objdiff-core/src/arch/ppc.rs +++ b/objdiff-core/src/arch/ppc.rs @@ -132,10 +132,7 @@ impl ObjArch for ObjArchPpc { } ops.push(ins.op as u16); - let line = section - .line_info - .as_ref() - .and_then(|map| map.range(..=cur_addr as u64).last().map(|(_, &b)| b)); + let line = section.line_info.range(..=cur_addr as u64).last().map(|(_, &b)| b); insts.push(ObjIns { address: cur_addr as u64, size: 4, diff --git a/objdiff-core/src/arch/x86.rs b/objdiff-core/src/arch/x86.rs index cd65690..f2fb028 100644 --- a/objdiff-core/src/arch/x86.rs +++ b/objdiff-core/src/arch/x86.rs @@ -73,7 +73,7 @@ impl ObjArch for ObjArchX86 { .relocations .iter() .find(|r| r.address >= address && r.address < address + instruction.len() as u64); - let line = section.line_info.as_ref().and_then(|m| m.get(&address).cloned()); + let line = section.line_info.range(..=address).last().map(|(_, &b)| b); output.ins = ObjIns { address, size: instruction.len() as u8, diff --git a/objdiff-core/src/obj/mod.rs b/objdiff-core/src/obj/mod.rs index c8534c9..59255ba 100644 --- a/objdiff-core/src/obj/mod.rs +++ b/objdiff-core/src/obj/mod.rs @@ -40,7 +40,7 @@ pub struct ObjSection { pub relocations: Vec, pub virtual_address: Option, /// Line number info (.line or .debug_line section) - pub line_info: Option>, + pub line_info: BTreeMap, } #[derive(Debug, Clone, Eq, PartialEq)] diff --git a/objdiff-core/src/obj/read.rs b/objdiff-core/src/obj/read.rs index c89c03e..8462da8 100644 --- a/objdiff-core/src/obj/read.rs +++ b/objdiff-core/src/obj/read.rs @@ -111,7 +111,7 @@ fn filter_sections(obj_file: &File<'_>, split_meta: Option<&SplitMeta>) -> Resul symbols: Vec::new(), relocations: Vec::new(), virtual_address, - line_info: None, + line_info: Default::default(), }); } result.sort_by(|a, b| a.name.cmp(&b.name)); @@ -292,7 +292,6 @@ fn line_info(obj_file: &File<'_>, sections: &mut [ObjSection]) -> Result<()> { reader.set_position(start + size as u64); continue; }; - let lines = out_section.line_info.get_or_insert_with(Default::default); let end = start + size as u64; while reader.position() < end { let line_number = reader.read_u32::()? as u64; @@ -301,7 +300,7 @@ fn line_info(obj_file: &File<'_>, sections: &mut [ObjSection]) -> Result<()> { log::warn!("Unhandled statement pos {}", statement_pos); } let address_delta = reader.read_u32::()? as u64; - lines.insert(base_address + address_delta, line_number); + out_section.line_info.insert(base_address + address_delta, line_number); log::debug!("Line: {:#x} -> {}", base_address + address_delta, line_number); } } @@ -337,7 +336,7 @@ fn line_info(obj_file: &File<'_>, sections: &mut [ObjSection]) -> Result<()> { let mut lines = sections .iter_mut() .find(|s| s.orig_index == section_index) - .map(|s| s.line_info.get_or_insert_with(Default::default)); + .map(|s| &mut s.line_info); let mut rows = program.rows(); while let Some((_header, row)) = rows.next_row()? { @@ -355,7 +354,7 @@ fn line_info(obj_file: &File<'_>, sections: &mut [ObjSection]) -> Result<()> { lines = sections .iter_mut() .find(|s| s.orig_index == section_index) - .map(|s| s.line_info.get_or_insert_with(Default::default)); + .map(|s| &mut s.line_info); } } }