From 93c049dc5624464b924cf24f7d0fcfb2b2262833 Mon Sep 17 00:00:00 2001 From: sozud <122322823+sozud@users.noreply.github.com> Date: Sat, 26 Apr 2025 15:27:51 -0700 Subject: [PATCH 1/4] Add data info for superh --- objdiff-core/src/arch/arm.rs | 1 + objdiff-core/src/arch/arm64.rs | 1 + objdiff-core/src/arch/mips.rs | 1 + objdiff-core/src/arch/mod.rs | 4 +- objdiff-core/src/arch/ppc.rs | 1 + objdiff-core/src/arch/superh/mod.rs | 155 +++++++++++++++++++++++++++- objdiff-core/src/arch/x86.rs | 8 ++ objdiff-core/src/diff/display.rs | 147 ++++++++++++++------------ 8 files changed, 251 insertions(+), 67 deletions(-) diff --git a/objdiff-core/src/arch/arm.rs b/objdiff-core/src/arch/arm.rs index 92ef5773..99d1853c 100644 --- a/objdiff-core/src/arch/arm.rs +++ b/objdiff-core/src/arch/arm.rs @@ -330,6 +330,7 @@ impl Arch for ArchArm { &self, resolved: ResolvedInstructionRef, diff_config: &DiffObjConfig, + _code: Option<&[u8]>, cb: &mut dyn FnMut(InstructionPart) -> Result<()>, ) -> Result<()> { let (ins, parsed_ins) = self.parse_ins_ref(resolved.ins_ref, resolved.code, diff_config)?; diff --git a/objdiff-core/src/arch/arm64.rs b/objdiff-core/src/arch/arm64.rs index a39c7273..d9eb9368 100644 --- a/objdiff-core/src/arch/arm64.rs +++ b/objdiff-core/src/arch/arm64.rs @@ -80,6 +80,7 @@ impl Arch for ArchArm64 { &self, resolved: ResolvedInstructionRef, _diff_config: &DiffObjConfig, + _code: Option<&[u8]>, cb: &mut dyn FnMut(InstructionPart) -> Result<()>, ) -> Result<()> { let mut reader = U8Reader::new(resolved.code); diff --git a/objdiff-core/src/arch/mips.rs b/objdiff-core/src/arch/mips.rs index 6f5147d7..9f94ce16 100644 --- a/objdiff-core/src/arch/mips.rs +++ b/objdiff-core/src/arch/mips.rs @@ -219,6 +219,7 @@ impl Arch for ArchMips { &self, resolved: ResolvedInstructionRef, diff_config: &DiffObjConfig, + _code: Option<&[u8]>, cb: &mut dyn FnMut(InstructionPart) -> Result<()>, ) -> Result<()> { let instruction = self.parse_ins_ref(resolved.ins_ref, resolved.code, diff_config)?; diff --git a/objdiff-core/src/arch/mod.rs b/objdiff-core/src/arch/mod.rs index 72e05c14..1fab861a 100644 --- a/objdiff-core/src/arch/mod.rs +++ b/objdiff-core/src/arch/mod.rs @@ -208,7 +208,7 @@ pub trait Arch: Send + Sync + Debug { ) -> Result { let mut mnemonic = None; let mut args = Vec::with_capacity(8); - self.display_instruction(resolved, diff_config, &mut |part| { + self.display_instruction(resolved, diff_config, None, &mut |part| { match part { InstructionPart::Opcode(m, _) => mnemonic = Some(Cow::Owned(m.into_owned())), InstructionPart::Arg(arg) => args.push(arg.into_static()), @@ -236,6 +236,7 @@ pub trait Arch: Send + Sync + Debug { &self, resolved: ResolvedInstructionRef, diff_config: &DiffObjConfig, + code: Option<&[u8]>, cb: &mut dyn FnMut(InstructionPart) -> Result<()>, ) -> Result<()>; @@ -344,6 +345,7 @@ impl Arch for ArchDummy { &self, _resolved: ResolvedInstructionRef, _diff_config: &DiffObjConfig, + _code: Option<&[u8]>, _cb: &mut dyn FnMut(InstructionPart) -> Result<()>, ) -> Result<()> { Ok(()) diff --git a/objdiff-core/src/arch/ppc.rs b/objdiff-core/src/arch/ppc.rs index 3e2a3c8e..ca1f6bb2 100644 --- a/objdiff-core/src/arch/ppc.rs +++ b/objdiff-core/src/arch/ppc.rs @@ -110,6 +110,7 @@ impl Arch for ArchPpc { &self, resolved: ResolvedInstructionRef, _diff_config: &DiffObjConfig, + _code: Option<&[u8]>, cb: &mut dyn FnMut(InstructionPart) -> Result<()>, ) -> Result<()> { let ins = self.parse_ins_ref(resolved)?.simplified(); diff --git a/objdiff-core/src/arch/superh/mod.rs b/objdiff-core/src/arch/superh/mod.rs index 5f9f852a..79f6d7f8 100644 --- a/objdiff-core/src/arch/superh/mod.rs +++ b/objdiff-core/src/arch/superh/mod.rs @@ -7,10 +7,12 @@ use crate::{ arch::{Arch, superh::disasm::sh2_disasm}, diff::{DiffObjConfig, display::InstructionPart}, obj::{ - InstructionRef, Relocation, RelocationFlags, ResolvedInstructionRef, ScannedInstruction, + InstructionRef, Relocation, RelocationFlags, ResolvedInstructionRef, ScannedInstruction }, }; +use std::collections::HashMap; + pub mod disasm; #[derive(Debug)] @@ -20,6 +22,12 @@ impl ArchSuperH { pub fn new(_file: &object::File) -> Result { Ok(Self {}) } } + +struct DataInfo { + address: u64, + size: u32, +} + impl Arch for ArchSuperH { fn scan_instructions( &self, @@ -31,6 +39,7 @@ impl Arch for ArchSuperH { ) -> Result> { let mut ops = Vec::::with_capacity(code.len() / 2); let mut offset = address; + for chunk in code.chunks_exact(2) { let opcode = u16::from_be_bytes(chunk.try_into().unwrap()); let mut parts: Vec = vec![]; @@ -62,13 +71,68 @@ impl Arch for ArchSuperH { &self, resolved: ResolvedInstructionRef, _diff_config: &DiffObjConfig, + _code: Option<&[u8]>, cb: &mut dyn FnMut(InstructionPart) -> Result<()>, ) -> Result<()> { let opcode = u16::from_be_bytes(resolved.code.try_into().unwrap()); let mut parts: Vec = vec![]; let mut branch_dest: Option = None; + sh2_disasm(0, opcode, true, &mut parts, &resolved, &mut branch_dest); + if let Some(code) = _code { + // scan for data + // map of instruction offsets to data target offsets + let mut data_offsets: HashMap = HashMap::::new(); + + let mut pos: u64 = 0; + for chunk in code.chunks_exact(2) { + let opcode = u16::from_be_bytes(chunk.try_into().unwrap()); + // mov.w + if (opcode & 0xf000) == 0x9000 { + let target = (opcode as u64 & 0xff) * 2 + 4 + pos; + let data_info = DataInfo { address: target, size: 2 }; + data_offsets.insert(pos, data_info); + } + // mov.l + else if (opcode & 0xf000) == 0xd000 { + let target = ((opcode as u64 & 0xff) * 4 + 4 + pos) & 0xfffffffc; + let data_info = DataInfo { address: target, size: 4 }; + data_offsets.insert(pos, data_info); + } + pos += 2; + } + + let pos = resolved.ins_ref.address - resolved.symbol.address; + let a = resolved.ins_ref.address; + let b = resolved.symbol.address; + + // add the data info + if let Some(value) = data_offsets.get(&pos) { + if value.size == 2 && value.address as usize + 1 < code.len() { + let data = u16::from_be_bytes( + code[value.address as usize..value.address as usize + 2] + .try_into() + .unwrap(), + ); + parts.push(InstructionPart::basic(" /* ")); + parts.push(InstructionPart::basic("0x")); + parts.push(InstructionPart::basic(format!("{:04X}", data))); + parts.push(InstructionPart::basic(" */")); + } else if value.size == 4 && value.address as usize + 3 < code.len() { + let data = u32::from_be_bytes( + code[value.address as usize..value.address as usize + 4] + .try_into() + .unwrap(), + ); + parts.push(InstructionPart::basic(" /* ")); + parts.push(InstructionPart::basic("0x")); + parts.push(InstructionPart::basic(format!("{:08X}", data))); + parts.push(InstructionPart::basic(" */")); + } + } + } + for part in parts { cb(part)?; } @@ -154,6 +218,7 @@ mod test { use super::*; use crate::obj::InstructionArg; + use crate::obj::Symbol; impl Display for InstructionPart<'_> { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { @@ -201,6 +266,7 @@ mod test { ..Default::default() }, &DiffObjConfig::default(), + None, &mut |part| { parts.push(part.into_static()); Ok(()) @@ -279,6 +345,7 @@ mod test { ..Default::default() }, &DiffObjConfig::default(), + None, &mut |part| { parts.push(part.into_static()); Ok(()) @@ -362,6 +429,7 @@ mod test { ..Default::default() }, &DiffObjConfig::default(), + None, &mut |part| { parts.push(part.into_static()); Ok(()) @@ -399,6 +467,7 @@ mod test { ..Default::default() }, &DiffObjConfig::default(), + None, &mut |part| { parts.push(part.into_static()); Ok(()) @@ -448,6 +517,7 @@ mod test { ..Default::default() }, &DiffObjConfig::default(), + None, &mut |part| { parts.push(part.into_static()); Ok(()) @@ -484,6 +554,7 @@ mod test { ..Default::default() }, &DiffObjConfig::default(), + None, &mut |part| { parts.push(part.into_static()); Ok(()) @@ -523,6 +594,7 @@ mod test { ..Default::default() }, &DiffObjConfig::default(), + None, &mut |part| { parts.push(part.into_static()); Ok(()) @@ -562,6 +634,7 @@ mod test { ..Default::default() }, &DiffObjConfig::default(), + None, &mut |part| { parts.push(part.into_static()); Ok(()) @@ -594,6 +667,7 @@ mod test { ..Default::default() }, &DiffObjConfig::default(), + None, &mut |part| { parts.push(part.into_static()); Ok(()) @@ -623,6 +697,85 @@ mod test { ..Default::default() }, &DiffObjConfig::default(), + None, + &mut |part| { + parts.push(part.into_static()); + Ok(()) + }, + ) + .unwrap(); + + let joined_str: String = parts.iter().map(|part| format!("{}", part)).collect(); + assert_eq!(joined_str, expected_str.to_string()); + } + } + + #[test] + fn test_func_0606_f378_mov_w_data_labeling() { + let arch = ArchSuperH {}; + let ops: &[(u16, u32, &str)] = &[(0x9000, 0x0606F378, "mov.w @(0x4, pc), r0 /* 0x00B0 */")]; + + let mut code = Vec::new(); + code.extend_from_slice(&0x9000_u16.to_be_bytes()); + code.extend_from_slice(&0x0009_u16.to_be_bytes()); + code.extend_from_slice(&0x00B0_u16.to_be_bytes()); + + for &(opcode, addr, expected_str) in ops { + let code_slice = &code; + let mut parts = Vec::new(); + + arch.display_instruction( + ResolvedInstructionRef { + ins_ref: InstructionRef { address: addr as u64, size: 2, opcode }, + code: &opcode.to_be_bytes(), + symbol: &Symbol { + address: 0x0606F378, // func base address + ..Default::default() + }, + ..Default::default() + }, + &DiffObjConfig::default(), + Some(code_slice), + &mut |part| { + parts.push(part.into_static()); + Ok(()) + }, + ) + .unwrap(); + + let joined_str: String = parts.iter().map(|part| format!("{}", part)).collect(); + assert_eq!(joined_str, expected_str.to_string()); + } + } + + #[test] + fn test_func_0606_f378_mov_l_data_labeling() { + let arch = ArchSuperH {}; + let ops: &[(u16, u32, &str)] = + &[(0xd000, 0x0606F378, "mov.l @(0x4, pc), r0 /* 0x00B000B0 */")]; + + let mut code = Vec::new(); + code.extend_from_slice(&0xd000_u16.to_be_bytes()); + code.extend_from_slice(&0x0009_u16.to_be_bytes()); + code.extend_from_slice(&0x00B0_u16.to_be_bytes()); + code.extend_from_slice(&0x00B0_u16.to_be_bytes()); + + for &(opcode, addr, expected_str) in ops { + let code_slice = &code; + let mut parts = Vec::new(); + + arch.display_instruction( + ResolvedInstructionRef { + ins_ref: InstructionRef { address: addr as u64, size: 2, opcode }, + code: &opcode.to_be_bytes(), + symbol: &Symbol { + address: 0x0606F378, // func base address + ..Default::default() + }, + ..Default::default() + }, + &DiffObjConfig::default(), + Some(code_slice), &mut |part| { parts.push(part.into_static()); Ok(()) diff --git a/objdiff-core/src/arch/x86.rs b/objdiff-core/src/arch/x86.rs index 42c330f0..40b3b62b 100644 --- a/objdiff-core/src/arch/x86.rs +++ b/objdiff-core/src/arch/x86.rs @@ -150,6 +150,7 @@ impl Arch for ArchX86 { &self, resolved: ResolvedInstructionRef, diff_config: &DiffObjConfig, + _code: Option<&[u8]>, cb: &mut dyn FnMut(InstructionPart) -> Result<()>, ) -> Result<()> { if resolved.ins_ref.opcode == DATA_OPCODE { @@ -482,6 +483,7 @@ mod test { ..Default::default() }, &DiffObjConfig::default(), + None, &mut |part| { parts.push(part.into_static()); Ok(()) @@ -527,6 +529,7 @@ mod test { ..Default::default() }, &DiffObjConfig::default(), + None, &mut |part| { parts.push(part.into_static()); Ok(()) @@ -572,6 +575,7 @@ mod test { ..Default::default() }, &DiffObjConfig::default(), + None, &mut |part| { parts.push(part.into_static()); Ok(()) @@ -615,6 +619,7 @@ mod test { ..Default::default() }, &DiffObjConfig::default(), + None, &mut |part| { parts.push(part.into_static()); Ok(()) @@ -646,6 +651,7 @@ mod test { ..Default::default() }, &DiffObjConfig::default(), + None, &mut |part| { parts.push(part.into_static()); Ok(()) @@ -685,6 +691,7 @@ mod test { ..Default::default() }, &DiffObjConfig::default(), + None, &mut |part| { parts.push(part.into_static()); Ok(()) @@ -724,6 +731,7 @@ mod test { ..Default::default() }, &DiffObjConfig::default(), + None, &mut |part| { parts.push(part.into_static()); Ok(()) diff --git a/objdiff-core/src/diff/display.rs b/objdiff-core/src/diff/display.rs index d5a53055..35ce3274 100644 --- a/objdiff-core/src/diff/display.rs +++ b/objdiff-core/src/diff/display.rs @@ -7,7 +7,7 @@ use alloc::{ }; use core::cmp::Ordering; -use anyhow::Result; +use anyhow::{Result, anyhow}; use itertools::Itertools; use regex::Regex; @@ -186,80 +186,97 @@ pub fn display_row( } let mut arg_idx = 0; let mut displayed_relocation = false; - obj.arch.display_instruction(resolved, diff_config, &mut |part| match part { - InstructionPart::Basic(text) => { - if text.chars().all(|c| c == ' ') { - cb(DiffTextSegment::spacing(text.len() as u8)) - } else { - cb(DiffTextSegment::basic(&text, base_color)) + + let symbol = &obj.symbols[symbol_index]; + let section_index = symbol.section.ok_or_else(|| anyhow!("Missing section for symbol"))?; + let section = &obj.sections[section_index]; + let code_data = section.data_range(symbol.address, symbol.size as usize).ok_or_else(|| { + anyhow!( + "Symbol data out of bounds: {:#x}..{:#x}", + symbol.address, + symbol.address + symbol.size + ) + })?; + + obj.arch.display_instruction( + resolved, + diff_config, + Some(code_data), + &mut |part| match part { + InstructionPart::Basic(text) => { + if text.chars().all(|c| c == ' ') { + cb(DiffTextSegment::spacing(text.len() as u8)) + } else { + cb(DiffTextSegment::basic(&text, base_color)) + } } - } - InstructionPart::Opcode(mnemonic, opcode) => cb(DiffTextSegment { - text: DiffText::Opcode(mnemonic.as_ref(), opcode), - color: match ins_row.kind { - InstructionDiffKind::OpMismatch => DiffTextColor::Replace, - _ => base_color, - }, - pad_to: 10, - }), - InstructionPart::Arg(arg) => { - let diff_index = ins_row.arg_diff.get(arg_idx).copied().unwrap_or_default(); - arg_idx += 1; - match arg { - InstructionArg::Value(value) => cb(DiffTextSegment { - text: DiffText::Argument(value), - color: diff_index - .get() - .map_or(base_color, |i| DiffTextColor::Rotating(i as u8)), - pad_to: 0, - }), - InstructionArg::Reloc => { - displayed_relocation = true; - let resolved = resolved.relocation.unwrap(); - let color = diff_index - .get() - .map_or(DiffTextColor::Bright, |i| DiffTextColor::Rotating(i as u8)); - cb(DiffTextSegment { - text: DiffText::Symbol(resolved.symbol), - color, + InstructionPart::Opcode(mnemonic, opcode) => cb(DiffTextSegment { + text: DiffText::Opcode(mnemonic.as_ref(), opcode), + color: match ins_row.kind { + InstructionDiffKind::OpMismatch => DiffTextColor::Replace, + _ => base_color, + }, + pad_to: 10, + }), + InstructionPart::Arg(arg) => { + let diff_index = ins_row.arg_diff.get(arg_idx).copied().unwrap_or_default(); + arg_idx += 1; + match arg { + InstructionArg::Value(value) => cb(DiffTextSegment { + text: DiffText::Argument(value), + color: diff_index + .get() + .map_or(base_color, |i| DiffTextColor::Rotating(i as u8)), pad_to: 0, - })?; - if resolved.relocation.addend != 0 { + }), + InstructionArg::Reloc => { + displayed_relocation = true; + let resolved = resolved.relocation.unwrap(); + let color = diff_index + .get() + .map_or(DiffTextColor::Bright, |i| DiffTextColor::Rotating(i as u8)); cb(DiffTextSegment { - text: DiffText::Addend(resolved.relocation.addend), + text: DiffText::Symbol(resolved.symbol), color, pad_to: 0, })?; + if resolved.relocation.addend != 0 { + cb(DiffTextSegment { + text: DiffText::Addend(resolved.relocation.addend), + color, + pad_to: 0, + })?; + } + Ok(()) } - Ok(()) - } - InstructionArg::BranchDest(dest) => { - if let Some(addr) = dest.checked_sub(resolved.symbol.address) { - cb(DiffTextSegment { - text: DiffText::BranchDest(addr), - color: diff_index - .get() - .map_or(base_color, |i| DiffTextColor::Rotating(i as u8)), - pad_to: 0, - }) - } else { - cb(DiffTextSegment { - text: DiffText::Argument(InstructionArgValue::Opaque(Cow::Borrowed( - "", - ))), - color: diff_index - .get() - .map_or(base_color, |i| DiffTextColor::Rotating(i as u8)), - pad_to: 0, - }) + InstructionArg::BranchDest(dest) => { + if let Some(addr) = dest.checked_sub(resolved.symbol.address) { + cb(DiffTextSegment { + text: DiffText::BranchDest(addr), + color: diff_index + .get() + .map_or(base_color, |i| DiffTextColor::Rotating(i as u8)), + pad_to: 0, + }) + } else { + cb(DiffTextSegment { + text: DiffText::Argument(InstructionArgValue::Opaque( + Cow::Borrowed(""), + )), + color: diff_index + .get() + .map_or(base_color, |i| DiffTextColor::Rotating(i as u8)), + pad_to: 0, + }) + } } } } - } - InstructionPart::Separator => { - cb(DiffTextSegment::basic(diff_config.separator(), base_color)) - } - })?; + InstructionPart::Separator => { + cb(DiffTextSegment::basic(diff_config.separator(), base_color)) + } + }, + )?; // Fallback for relocation that wasn't displayed if resolved.relocation.is_some() && !displayed_relocation { cb(DiffTextSegment::basic(" <", base_color))?; From 04534f521add401719c327f6c06da27ae7b25d9a Mon Sep 17 00:00:00 2001 From: sozud <122322823+sozud@users.noreply.github.com> Date: Sat, 26 Apr 2025 15:32:22 -0700 Subject: [PATCH 2/4] fmt --- objdiff-core/src/arch/superh/mod.rs | 11 +++-------- 1 file changed, 3 insertions(+), 8 deletions(-) diff --git a/objdiff-core/src/arch/superh/mod.rs b/objdiff-core/src/arch/superh/mod.rs index 79f6d7f8..5ccaa082 100644 --- a/objdiff-core/src/arch/superh/mod.rs +++ b/objdiff-core/src/arch/superh/mod.rs @@ -1,4 +1,5 @@ use alloc::{string::String, vec::Vec}; +use std::collections::HashMap; use anyhow::{Result, bail}; use object::elf; @@ -7,12 +8,10 @@ use crate::{ arch::{Arch, superh::disasm::sh2_disasm}, diff::{DiffObjConfig, display::InstructionPart}, obj::{ - InstructionRef, Relocation, RelocationFlags, ResolvedInstructionRef, ScannedInstruction + InstructionRef, Relocation, RelocationFlags, ResolvedInstructionRef, ScannedInstruction, }, }; -use std::collections::HashMap; - pub mod disasm; #[derive(Debug)] @@ -22,7 +21,6 @@ impl ArchSuperH { pub fn new(_file: &object::File) -> Result { Ok(Self {}) } } - struct DataInfo { address: u64, size: u32, @@ -104,8 +102,6 @@ impl Arch for ArchSuperH { } let pos = resolved.ins_ref.address - resolved.symbol.address; - let a = resolved.ins_ref.address; - let b = resolved.symbol.address; // add the data info if let Some(value) = data_offsets.get(&pos) { @@ -217,8 +213,7 @@ mod test { use std::fmt::{self, Display}; use super::*; - use crate::obj::InstructionArg; - use crate::obj::Symbol; + use crate::obj::{InstructionArg, Symbol}; impl Display for InstructionPart<'_> { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { From 87f4c5d5bb77940961f76ebb61d9f1cbca3297e3 Mon Sep 17 00:00:00 2001 From: Luke Street Date: Tue, 6 May 2025 22:04:48 -0600 Subject: [PATCH 3/4] Fetch symbol data dynamically from resolved.section --- objdiff-core/src/arch/arm.rs | 1 - objdiff-core/src/arch/arm64.rs | 1 - objdiff-core/src/arch/mips.rs | 1 - objdiff-core/src/arch/mod.rs | 4 +- objdiff-core/src/arch/ppc.rs | 1 - objdiff-core/src/arch/superh/mod.rs | 42 ++++---- objdiff-core/src/arch/x86.rs | 8 -- objdiff-core/src/diff/display.rs | 147 ++++++++++++---------------- objdiff-core/src/obj/mod.rs | 5 + 9 files changed, 92 insertions(+), 118 deletions(-) diff --git a/objdiff-core/src/arch/arm.rs b/objdiff-core/src/arch/arm.rs index 99d1853c..92ef5773 100644 --- a/objdiff-core/src/arch/arm.rs +++ b/objdiff-core/src/arch/arm.rs @@ -330,7 +330,6 @@ impl Arch for ArchArm { &self, resolved: ResolvedInstructionRef, diff_config: &DiffObjConfig, - _code: Option<&[u8]>, cb: &mut dyn FnMut(InstructionPart) -> Result<()>, ) -> Result<()> { let (ins, parsed_ins) = self.parse_ins_ref(resolved.ins_ref, resolved.code, diff_config)?; diff --git a/objdiff-core/src/arch/arm64.rs b/objdiff-core/src/arch/arm64.rs index d9eb9368..a39c7273 100644 --- a/objdiff-core/src/arch/arm64.rs +++ b/objdiff-core/src/arch/arm64.rs @@ -80,7 +80,6 @@ impl Arch for ArchArm64 { &self, resolved: ResolvedInstructionRef, _diff_config: &DiffObjConfig, - _code: Option<&[u8]>, cb: &mut dyn FnMut(InstructionPart) -> Result<()>, ) -> Result<()> { let mut reader = U8Reader::new(resolved.code); diff --git a/objdiff-core/src/arch/mips.rs b/objdiff-core/src/arch/mips.rs index 9f94ce16..6f5147d7 100644 --- a/objdiff-core/src/arch/mips.rs +++ b/objdiff-core/src/arch/mips.rs @@ -219,7 +219,6 @@ impl Arch for ArchMips { &self, resolved: ResolvedInstructionRef, diff_config: &DiffObjConfig, - _code: Option<&[u8]>, cb: &mut dyn FnMut(InstructionPart) -> Result<()>, ) -> Result<()> { let instruction = self.parse_ins_ref(resolved.ins_ref, resolved.code, diff_config)?; diff --git a/objdiff-core/src/arch/mod.rs b/objdiff-core/src/arch/mod.rs index 1fab861a..72e05c14 100644 --- a/objdiff-core/src/arch/mod.rs +++ b/objdiff-core/src/arch/mod.rs @@ -208,7 +208,7 @@ pub trait Arch: Send + Sync + Debug { ) -> Result { let mut mnemonic = None; let mut args = Vec::with_capacity(8); - self.display_instruction(resolved, diff_config, None, &mut |part| { + self.display_instruction(resolved, diff_config, &mut |part| { match part { InstructionPart::Opcode(m, _) => mnemonic = Some(Cow::Owned(m.into_owned())), InstructionPart::Arg(arg) => args.push(arg.into_static()), @@ -236,7 +236,6 @@ pub trait Arch: Send + Sync + Debug { &self, resolved: ResolvedInstructionRef, diff_config: &DiffObjConfig, - code: Option<&[u8]>, cb: &mut dyn FnMut(InstructionPart) -> Result<()>, ) -> Result<()>; @@ -345,7 +344,6 @@ impl Arch for ArchDummy { &self, _resolved: ResolvedInstructionRef, _diff_config: &DiffObjConfig, - _code: Option<&[u8]>, _cb: &mut dyn FnMut(InstructionPart) -> Result<()>, ) -> Result<()> { Ok(()) diff --git a/objdiff-core/src/arch/ppc.rs b/objdiff-core/src/arch/ppc.rs index ca1f6bb2..3e2a3c8e 100644 --- a/objdiff-core/src/arch/ppc.rs +++ b/objdiff-core/src/arch/ppc.rs @@ -110,7 +110,6 @@ impl Arch for ArchPpc { &self, resolved: ResolvedInstructionRef, _diff_config: &DiffObjConfig, - _code: Option<&[u8]>, cb: &mut dyn FnMut(InstructionPart) -> Result<()>, ) -> Result<()> { let ins = self.parse_ins_ref(resolved)?.simplified(); diff --git a/objdiff-core/src/arch/superh/mod.rs b/objdiff-core/src/arch/superh/mod.rs index 5ccaa082..870f1c62 100644 --- a/objdiff-core/src/arch/superh/mod.rs +++ b/objdiff-core/src/arch/superh/mod.rs @@ -69,7 +69,6 @@ impl Arch for ArchSuperH { &self, resolved: ResolvedInstructionRef, _diff_config: &DiffObjConfig, - _code: Option<&[u8]>, cb: &mut dyn FnMut(InstructionPart) -> Result<()>, ) -> Result<()> { let opcode = u16::from_be_bytes(resolved.code.try_into().unwrap()); @@ -78,13 +77,13 @@ impl Arch for ArchSuperH { sh2_disasm(0, opcode, true, &mut parts, &resolved, &mut branch_dest); - if let Some(code) = _code { + if let Some(symbol_data) = resolved.section.symbol_data(resolved.symbol) { // scan for data // map of instruction offsets to data target offsets let mut data_offsets: HashMap = HashMap::::new(); let mut pos: u64 = 0; - for chunk in code.chunks_exact(2) { + for chunk in symbol_data.chunks_exact(2) { let opcode = u16::from_be_bytes(chunk.try_into().unwrap()); // mov.w if (opcode & 0xf000) == 0x9000 { @@ -105,9 +104,9 @@ impl Arch for ArchSuperH { // add the data info if let Some(value) = data_offsets.get(&pos) { - if value.size == 2 && value.address as usize + 1 < code.len() { + if value.size == 2 && value.address as usize + 1 < symbol_data.len() { let data = u16::from_be_bytes( - code[value.address as usize..value.address as usize + 2] + symbol_data[value.address as usize..value.address as usize + 2] .try_into() .unwrap(), ); @@ -115,9 +114,9 @@ impl Arch for ArchSuperH { parts.push(InstructionPart::basic("0x")); parts.push(InstructionPart::basic(format!("{:04X}", data))); parts.push(InstructionPart::basic(" */")); - } else if value.size == 4 && value.address as usize + 3 < code.len() { + } else if value.size == 4 && value.address as usize + 3 < symbol_data.len() { let data = u32::from_be_bytes( - code[value.address as usize..value.address as usize + 4] + symbol_data[value.address as usize..value.address as usize + 4] .try_into() .unwrap(), ); @@ -213,7 +212,7 @@ mod test { use std::fmt::{self, Display}; use super::*; - use crate::obj::{InstructionArg, Symbol}; + use crate::obj::{InstructionArg, Section, SectionData, Symbol}; impl Display for InstructionPart<'_> { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { @@ -261,7 +260,6 @@ mod test { ..Default::default() }, &DiffObjConfig::default(), - None, &mut |part| { parts.push(part.into_static()); Ok(()) @@ -340,7 +338,6 @@ mod test { ..Default::default() }, &DiffObjConfig::default(), - None, &mut |part| { parts.push(part.into_static()); Ok(()) @@ -424,7 +421,6 @@ mod test { ..Default::default() }, &DiffObjConfig::default(), - None, &mut |part| { parts.push(part.into_static()); Ok(()) @@ -462,7 +458,6 @@ mod test { ..Default::default() }, &DiffObjConfig::default(), - None, &mut |part| { parts.push(part.into_static()); Ok(()) @@ -512,7 +507,6 @@ mod test { ..Default::default() }, &DiffObjConfig::default(), - None, &mut |part| { parts.push(part.into_static()); Ok(()) @@ -549,7 +543,6 @@ mod test { ..Default::default() }, &DiffObjConfig::default(), - None, &mut |part| { parts.push(part.into_static()); Ok(()) @@ -589,7 +582,6 @@ mod test { ..Default::default() }, &DiffObjConfig::default(), - None, &mut |part| { parts.push(part.into_static()); Ok(()) @@ -629,7 +621,6 @@ mod test { ..Default::default() }, &DiffObjConfig::default(), - None, &mut |part| { parts.push(part.into_static()); Ok(()) @@ -662,7 +653,6 @@ mod test { ..Default::default() }, &DiffObjConfig::default(), - None, &mut |part| { parts.push(part.into_static()); Ok(()) @@ -692,7 +682,6 @@ mod test { ..Default::default() }, &DiffObjConfig::default(), - None, &mut |part| { parts.push(part.into_static()); Ok(()) @@ -716,7 +705,6 @@ mod test { code.extend_from_slice(&0x00B0_u16.to_be_bytes()); for &(opcode, addr, expected_str) in ops { - let code_slice = &code; let mut parts = Vec::new(); arch.display_instruction( @@ -725,12 +713,18 @@ mod test { code: &opcode.to_be_bytes(), symbol: &Symbol { address: 0x0606F378, // func base address + size: code.len() as u64, + ..Default::default() + }, + section: &Section { + address: 0x0606F378, + size: code.len() as u64, + data: SectionData(code.clone()), ..Default::default() }, ..Default::default() }, &DiffObjConfig::default(), - Some(code_slice), &mut |part| { parts.push(part.into_static()); Ok(()) @@ -765,12 +759,18 @@ mod test { code: &opcode.to_be_bytes(), symbol: &Symbol { address: 0x0606F378, // func base address + size: code.len() as u64, + ..Default::default() + }, + section: &Section { + address: 0x0606F378, + size: code.len() as u64, + data: SectionData(code.clone()), ..Default::default() }, ..Default::default() }, &DiffObjConfig::default(), - Some(code_slice), &mut |part| { parts.push(part.into_static()); Ok(()) diff --git a/objdiff-core/src/arch/x86.rs b/objdiff-core/src/arch/x86.rs index 40b3b62b..42c330f0 100644 --- a/objdiff-core/src/arch/x86.rs +++ b/objdiff-core/src/arch/x86.rs @@ -150,7 +150,6 @@ impl Arch for ArchX86 { &self, resolved: ResolvedInstructionRef, diff_config: &DiffObjConfig, - _code: Option<&[u8]>, cb: &mut dyn FnMut(InstructionPart) -> Result<()>, ) -> Result<()> { if resolved.ins_ref.opcode == DATA_OPCODE { @@ -483,7 +482,6 @@ mod test { ..Default::default() }, &DiffObjConfig::default(), - None, &mut |part| { parts.push(part.into_static()); Ok(()) @@ -529,7 +527,6 @@ mod test { ..Default::default() }, &DiffObjConfig::default(), - None, &mut |part| { parts.push(part.into_static()); Ok(()) @@ -575,7 +572,6 @@ mod test { ..Default::default() }, &DiffObjConfig::default(), - None, &mut |part| { parts.push(part.into_static()); Ok(()) @@ -619,7 +615,6 @@ mod test { ..Default::default() }, &DiffObjConfig::default(), - None, &mut |part| { parts.push(part.into_static()); Ok(()) @@ -651,7 +646,6 @@ mod test { ..Default::default() }, &DiffObjConfig::default(), - None, &mut |part| { parts.push(part.into_static()); Ok(()) @@ -691,7 +685,6 @@ mod test { ..Default::default() }, &DiffObjConfig::default(), - None, &mut |part| { parts.push(part.into_static()); Ok(()) @@ -731,7 +724,6 @@ mod test { ..Default::default() }, &DiffObjConfig::default(), - None, &mut |part| { parts.push(part.into_static()); Ok(()) diff --git a/objdiff-core/src/diff/display.rs b/objdiff-core/src/diff/display.rs index 35ce3274..d5a53055 100644 --- a/objdiff-core/src/diff/display.rs +++ b/objdiff-core/src/diff/display.rs @@ -7,7 +7,7 @@ use alloc::{ }; use core::cmp::Ordering; -use anyhow::{Result, anyhow}; +use anyhow::Result; use itertools::Itertools; use regex::Regex; @@ -186,97 +186,80 @@ pub fn display_row( } let mut arg_idx = 0; let mut displayed_relocation = false; - - let symbol = &obj.symbols[symbol_index]; - let section_index = symbol.section.ok_or_else(|| anyhow!("Missing section for symbol"))?; - let section = &obj.sections[section_index]; - let code_data = section.data_range(symbol.address, symbol.size as usize).ok_or_else(|| { - anyhow!( - "Symbol data out of bounds: {:#x}..{:#x}", - symbol.address, - symbol.address + symbol.size - ) - })?; - - obj.arch.display_instruction( - resolved, - diff_config, - Some(code_data), - &mut |part| match part { - InstructionPart::Basic(text) => { - if text.chars().all(|c| c == ' ') { - cb(DiffTextSegment::spacing(text.len() as u8)) - } else { - cb(DiffTextSegment::basic(&text, base_color)) - } + obj.arch.display_instruction(resolved, diff_config, &mut |part| match part { + InstructionPart::Basic(text) => { + if text.chars().all(|c| c == ' ') { + cb(DiffTextSegment::spacing(text.len() as u8)) + } else { + cb(DiffTextSegment::basic(&text, base_color)) } - InstructionPart::Opcode(mnemonic, opcode) => cb(DiffTextSegment { - text: DiffText::Opcode(mnemonic.as_ref(), opcode), - color: match ins_row.kind { - InstructionDiffKind::OpMismatch => DiffTextColor::Replace, - _ => base_color, - }, - pad_to: 10, - }), - InstructionPart::Arg(arg) => { - let diff_index = ins_row.arg_diff.get(arg_idx).copied().unwrap_or_default(); - arg_idx += 1; - match arg { - InstructionArg::Value(value) => cb(DiffTextSegment { - text: DiffText::Argument(value), - color: diff_index - .get() - .map_or(base_color, |i| DiffTextColor::Rotating(i as u8)), + } + InstructionPart::Opcode(mnemonic, opcode) => cb(DiffTextSegment { + text: DiffText::Opcode(mnemonic.as_ref(), opcode), + color: match ins_row.kind { + InstructionDiffKind::OpMismatch => DiffTextColor::Replace, + _ => base_color, + }, + pad_to: 10, + }), + InstructionPart::Arg(arg) => { + let diff_index = ins_row.arg_diff.get(arg_idx).copied().unwrap_or_default(); + arg_idx += 1; + match arg { + InstructionArg::Value(value) => cb(DiffTextSegment { + text: DiffText::Argument(value), + color: diff_index + .get() + .map_or(base_color, |i| DiffTextColor::Rotating(i as u8)), + pad_to: 0, + }), + InstructionArg::Reloc => { + displayed_relocation = true; + let resolved = resolved.relocation.unwrap(); + let color = diff_index + .get() + .map_or(DiffTextColor::Bright, |i| DiffTextColor::Rotating(i as u8)); + cb(DiffTextSegment { + text: DiffText::Symbol(resolved.symbol), + color, pad_to: 0, - }), - InstructionArg::Reloc => { - displayed_relocation = true; - let resolved = resolved.relocation.unwrap(); - let color = diff_index - .get() - .map_or(DiffTextColor::Bright, |i| DiffTextColor::Rotating(i as u8)); + })?; + if resolved.relocation.addend != 0 { cb(DiffTextSegment { - text: DiffText::Symbol(resolved.symbol), + text: DiffText::Addend(resolved.relocation.addend), color, pad_to: 0, })?; - if resolved.relocation.addend != 0 { - cb(DiffTextSegment { - text: DiffText::Addend(resolved.relocation.addend), - color, - pad_to: 0, - })?; - } - Ok(()) } - InstructionArg::BranchDest(dest) => { - if let Some(addr) = dest.checked_sub(resolved.symbol.address) { - cb(DiffTextSegment { - text: DiffText::BranchDest(addr), - color: diff_index - .get() - .map_or(base_color, |i| DiffTextColor::Rotating(i as u8)), - pad_to: 0, - }) - } else { - cb(DiffTextSegment { - text: DiffText::Argument(InstructionArgValue::Opaque( - Cow::Borrowed(""), - )), - color: diff_index - .get() - .map_or(base_color, |i| DiffTextColor::Rotating(i as u8)), - pad_to: 0, - }) - } + Ok(()) + } + InstructionArg::BranchDest(dest) => { + if let Some(addr) = dest.checked_sub(resolved.symbol.address) { + cb(DiffTextSegment { + text: DiffText::BranchDest(addr), + color: diff_index + .get() + .map_or(base_color, |i| DiffTextColor::Rotating(i as u8)), + pad_to: 0, + }) + } else { + cb(DiffTextSegment { + text: DiffText::Argument(InstructionArgValue::Opaque(Cow::Borrowed( + "", + ))), + color: diff_index + .get() + .map_or(base_color, |i| DiffTextColor::Rotating(i as u8)), + pad_to: 0, + }) } } } - InstructionPart::Separator => { - cb(DiffTextSegment::basic(diff_config.separator(), base_color)) - } - }, - )?; + } + InstructionPart::Separator => { + cb(DiffTextSegment::basic(diff_config.separator(), base_color)) + } + })?; // Fallback for relocation that wasn't displayed if resolved.relocation.is_some() && !displayed_relocation { cb(DiffTextSegment::basic(" <", base_color))?; diff --git a/objdiff-core/src/obj/mod.rs b/objdiff-core/src/obj/mod.rs index f3f18a20..d6ded17c 100644 --- a/objdiff-core/src/obj/mod.rs +++ b/objdiff-core/src/obj/mod.rs @@ -105,6 +105,11 @@ impl Section { } } + pub fn symbol_data(&self, symbol: &Symbol) -> Option<&[u8]> { + let offset = symbol.address.checked_sub(self.address)?; + self.data.get(offset as usize..offset as usize + symbol.size as usize) + } + pub fn relocation_at<'obj>( &'obj self, obj: &'obj Object, From b0d355708d9853b66a518e815e73181b05663255 Mon Sep 17 00:00:00 2001 From: Luke Street Date: Tue, 6 May 2025 22:11:13 -0600 Subject: [PATCH 4/4] Remove unused var --- objdiff-core/src/arch/superh/mod.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/objdiff-core/src/arch/superh/mod.rs b/objdiff-core/src/arch/superh/mod.rs index 870f1c62..be00d738 100644 --- a/objdiff-core/src/arch/superh/mod.rs +++ b/objdiff-core/src/arch/superh/mod.rs @@ -750,7 +750,6 @@ mod test { code.extend_from_slice(&0x00B0_u16.to_be_bytes()); for &(opcode, addr, expected_str) in ops { - let code_slice = &code; let mut parts = Vec::new(); arch.display_instruction(