diff --git a/objdiff-core/src/arch/arm.rs b/objdiff-core/src/arch/arm.rs index 35416c9f..d8fdd5e2 100644 --- a/objdiff-core/src/arch/arm.rs +++ b/objdiff-core/src/arch/arm.rs @@ -471,9 +471,13 @@ impl Arch for ArchArm { while next_address >= symbol.address + 2 && let Some(data) = section.data_range(next_address - 2, 2) && data == [0u8; 2] - && section.relocation_at(next_address - 2, 2).is_none() { next_address -= 2; + if let Some(relocation) = section.relocation_at(next_address, 2) { + // Avoid cutting trailing relocations in half. + next_address += self.data_reloc_size(relocation.flags) as u64; + break; + } } Ok(next_address.saturating_sub(symbol.address)) } diff --git a/objdiff-core/src/obj/read.rs b/objdiff-core/src/obj/read.rs index 6046070d..5cfb6d73 100644 --- a/objdiff-core/src/obj/read.rs +++ b/objdiff-core/src/obj/read.rs @@ -181,7 +181,6 @@ fn map_symbol( fn map_symbols( arch: &dyn Arch, obj_file: &object::File, - sections: &[Section], section_indices: &[usize], split_meta: Option<&SplitMeta>, config: &DiffObjConfig, @@ -223,9 +222,6 @@ fn map_symbols( symbols.push(symbol); } - // Infer symbol sizes for 0-size symbols - infer_symbol_sizes(arch, &mut symbols, sections)?; - Ok((symbols, symbol_indices)) } @@ -286,7 +282,7 @@ fn is_local_label(symbol: &Symbol) -> bool { } fn infer_symbol_sizes(arch: &dyn Arch, symbols: &mut [Symbol], sections: &[Section]) -> Result<()> { - // Above, we've sorted the symbols by section and then by address. + // Above, we've sorted the symbols by section and then by address, and also mapped section relocations. // Set symbol sizes based on the next symbol's address let mut iter_idx = 0; @@ -1075,15 +1071,11 @@ pub fn parse(data: &[u8], config: &DiffObjConfig, diff_side: DiffSide) -> Result let split_meta = parse_split_meta(&obj_file)?; let (mut sections, section_indices) = map_sections(arch.as_ref(), &obj_file, split_meta.as_ref())?; - let (mut symbols, symbol_indices) = map_symbols( - arch.as_ref(), - &obj_file, - §ions, - §ion_indices, - split_meta.as_ref(), - config, - )?; + let (mut symbols, symbol_indices) = + map_symbols(arch.as_ref(), &obj_file, §ion_indices, split_meta.as_ref(), config)?; map_relocations(arch.as_ref(), &obj_file, &mut sections, §ion_indices, &symbol_indices)?; + // Infer symbol sizes for 0-size symbols (must be done after map_relocations is called) + infer_symbol_sizes(arch.as_ref(), &mut symbols, §ions)?; parse_line_info(&obj_file, &mut sections, §ion_indices, data)?; if config.combine_data_sections || config.combine_text_sections { combine_sections(&mut sections, &mut symbols, config)?; diff --git a/objdiff-core/tests/arch_arm.rs b/objdiff-core/tests/arch_arm.rs index 9c507e1d..9756f9cd 100644 --- a/objdiff-core/tests/arch_arm.rs +++ b/objdiff-core/tests/arch_arm.rs @@ -98,3 +98,20 @@ fn trim_trailing_hword() { let output = common::display_diff(&obj, &diff, symbol_idx, &diff_config); insta::assert_snapshot!(output); } + +#[test] +#[cfg(feature = "arm")] +fn do_not_trim_trailing_relocations() { + let diff_config = diff::DiffObjConfig::default(); + let obj = obj::read::parse( + include_object!("data/arm/fake_tank.o"), + &diff_config, + diff::DiffSide::Base, + ) + .unwrap(); + let symbol_idx = obj.symbols.iter().position(|s| s.name == "FakeTankIdleInit").unwrap(); + let diff = diff::code::no_diff_code(&obj, symbol_idx, &diff_config).unwrap(); + insta::assert_debug_snapshot!(diff.instruction_rows); + let output = common::display_diff(&obj, &diff, symbol_idx, &diff_config); + insta::assert_snapshot!(output); +} diff --git a/objdiff-core/tests/data/arm/fake_tank.o b/objdiff-core/tests/data/arm/fake_tank.o new file mode 100644 index 00000000..2e986f4c Binary files /dev/null and b/objdiff-core/tests/data/arm/fake_tank.o differ diff --git a/objdiff-core/tests/snapshots/arch_arm__do_not_trim_trailing_relocations-2.snap b/objdiff-core/tests/snapshots/arch_arm__do_not_trim_trailing_relocations-2.snap new file mode 100644 index 00000000..8346e010 --- /dev/null +++ b/objdiff-core/tests/snapshots/arch_arm__do_not_trim_trailing_relocations-2.snap @@ -0,0 +1,15 @@ +--- +source: objdiff-core/tests/arch_arm.rs +expression: output +--- +[(Address(0), Dim, 5), (Spacing(4), Normal, 0), (Opcode("ldr", 24), Normal, 10), (Argument(Opaque("r1")), Normal, 0), (Basic(", "), Normal, 0), (Basic("["), Normal, 0), (Argument(Opaque("pc")), Normal, 0), (Basic(", "), Normal, 0), (Basic("#"), Normal, 0), (Argument(Signed(16)), Normal, 0), (Basic("]"), Normal, 0), (Basic(" (->"), Normal, 0), (BranchDest(20), Normal, 0), (Basic(")"), Normal, 0), (Eol, Normal, 0)] +[(Address(2), Dim, 5), (Spacing(4), Normal, 0), (Opcode("mov", 43), Normal, 10), (Argument(Opaque("r3")), Normal, 0), (Basic(", "), Normal, 0), (Argument(Opaque("r1")), Normal, 0), (Eol, Normal, 0)] +[(Address(4), Dim, 5), (Spacing(4), Normal, 0), (Opcode("add", 1), Normal, 10), (Argument(Opaque("r3")), Normal, 0), (Basic(", "), Normal, 0), (Basic("#"), Normal, 0), (Argument(Unsigned(36)), Normal, 0), (Eol, Normal, 0)] +[(Address(6), Dim, 5), (Spacing(4), Normal, 0), (Opcode("mov", 43), Normal, 10), (Argument(Opaque("r2")), Normal, 0), (Basic(", "), Normal, 0), (Basic("#"), Normal, 0), (Argument(Unsigned(0)), Normal, 0), (Eol, Normal, 0)] +[(Address(8), Dim, 5), (Spacing(4), Normal, 0), (Opcode("mov", 43), Normal, 10), (Argument(Opaque("r0")), Normal, 0), (Basic(", "), Normal, 0), (Basic("#"), Normal, 0), (Argument(Unsigned(2)), Normal, 0), (Eol, Normal, 0)] +[(Address(10), Dim, 5), (Spacing(4), Normal, 0), (Opcode("strb", 117), Normal, 10), (Argument(Opaque("r0")), Normal, 0), (Basic(", "), Normal, 0), (Basic("["), Normal, 0), (Argument(Opaque("r3")), Normal, 0), (Basic(", "), Normal, 0), (Basic("#"), Normal, 0), (Argument(Signed(0)), Normal, 0), (Basic("]"), Normal, 0), (Eol, Normal, 0)] +[(Address(12), Dim, 5), (Spacing(4), Normal, 0), (Opcode("strb", 117), Normal, 10), (Argument(Opaque("r2")), Normal, 0), (Basic(", "), Normal, 0), (Basic("["), Normal, 0), (Argument(Opaque("r1")), Normal, 0), (Basic(", "), Normal, 0), (Basic("#"), Normal, 0), (Argument(Signed(28)), Normal, 0), (Basic("]"), Normal, 0), (Eol, Normal, 0)] +[(Address(14), Dim, 5), (Spacing(4), Normal, 0), (Opcode("strh", 124), Normal, 10), (Argument(Opaque("r2")), Normal, 0), (Basic(", "), Normal, 0), (Basic("["), Normal, 0), (Argument(Opaque("r1")), Normal, 0), (Basic(", "), Normal, 0), (Basic("#"), Normal, 0), (Argument(Signed(22)), Normal, 0), (Basic("]"), Normal, 0), (Eol, Normal, 0)] +[(Address(16), Dim, 5), (Spacing(4), Normal, 0), (Opcode("bx", 9), Normal, 10), (Argument(Opaque("lr")), Normal, 0), (Eol, Normal, 0)] +[(Address(18), Dim, 5), (Spacing(4), Normal, 0), (Opcode(".hword", 65534), Normal, 10), (Argument(Unsigned(0)), Normal, 0), (Eol, Normal, 0)] +[(Address(20), Dim, 5), (Spacing(4), Normal, 0), (Opcode(".word", 65534), Normal, 10), (Symbol(Symbol { name: "gCurrentSprite", demangled_name: None, normalized_name: None, address: 0, size: 0, kind: Unknown, section: None, flags: FlagSet(Global), align: None, virtual_address: None }), Bright, 0), (Eol, Normal, 0)] diff --git a/objdiff-core/tests/snapshots/arch_arm__do_not_trim_trailing_relocations.snap b/objdiff-core/tests/snapshots/arch_arm__do_not_trim_trailing_relocations.snap new file mode 100644 index 00000000..b4d238f6 --- /dev/null +++ b/objdiff-core/tests/snapshots/arch_arm__do_not_trim_trailing_relocations.snap @@ -0,0 +1,160 @@ +--- +source: objdiff-core/tests/arch_arm.rs +expression: diff.instruction_rows +--- +[ + InstructionDiffRow { + ins_ref: Some( + InstructionRef { + address: 176, + size: 2, + opcode: 24, + branch_dest: None, + }, + ), + kind: None, + branch_from: None, + branch_to: None, + arg_diff: [], + }, + InstructionDiffRow { + ins_ref: Some( + InstructionRef { + address: 178, + size: 2, + opcode: 43, + branch_dest: None, + }, + ), + kind: None, + branch_from: None, + branch_to: None, + arg_diff: [], + }, + InstructionDiffRow { + ins_ref: Some( + InstructionRef { + address: 180, + size: 2, + opcode: 1, + branch_dest: None, + }, + ), + kind: None, + branch_from: None, + branch_to: None, + arg_diff: [], + }, + InstructionDiffRow { + ins_ref: Some( + InstructionRef { + address: 182, + size: 2, + opcode: 43, + branch_dest: None, + }, + ), + kind: None, + branch_from: None, + branch_to: None, + arg_diff: [], + }, + InstructionDiffRow { + ins_ref: Some( + InstructionRef { + address: 184, + size: 2, + opcode: 43, + branch_dest: None, + }, + ), + kind: None, + branch_from: None, + branch_to: None, + arg_diff: [], + }, + InstructionDiffRow { + ins_ref: Some( + InstructionRef { + address: 186, + size: 2, + opcode: 117, + branch_dest: None, + }, + ), + kind: None, + branch_from: None, + branch_to: None, + arg_diff: [], + }, + InstructionDiffRow { + ins_ref: Some( + InstructionRef { + address: 188, + size: 2, + opcode: 117, + branch_dest: None, + }, + ), + kind: None, + branch_from: None, + branch_to: None, + arg_diff: [], + }, + InstructionDiffRow { + ins_ref: Some( + InstructionRef { + address: 190, + size: 2, + opcode: 124, + branch_dest: None, + }, + ), + kind: None, + branch_from: None, + branch_to: None, + arg_diff: [], + }, + InstructionDiffRow { + ins_ref: Some( + InstructionRef { + address: 192, + size: 2, + opcode: 9, + branch_dest: None, + }, + ), + kind: None, + branch_from: None, + branch_to: None, + arg_diff: [], + }, + InstructionDiffRow { + ins_ref: Some( + InstructionRef { + address: 194, + size: 2, + opcode: 65534, + branch_dest: None, + }, + ), + kind: None, + branch_from: None, + branch_to: None, + arg_diff: [], + }, + InstructionDiffRow { + ins_ref: Some( + InstructionRef { + address: 196, + size: 4, + opcode: 65534, + branch_dest: None, + }, + ), + kind: None, + branch_from: None, + branch_to: None, + arg_diff: [], + }, +]