ARM: Fix trailing relocations being ignored when inferring function sizes

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))
     }

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,
-        &sections,
-        &section_indices,
-        split_meta.as_ref(),
-        config,
-    )?;
+    let (mut symbols, symbol_indices) =
+        map_symbols(arch.as_ref(), &obj_file, &section_indices, split_meta.as_ref(), config)?;
     map_relocations(arch.as_ref(), &obj_file, &mut sections, &section_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, &sections)?;
     parse_line_info(&obj_file, &mut sections, &section_indices, data)?;
     if config.combine_data_sections || config.combine_text_sections {
         combine_sections(&mut sections, &mut symbols, config)?;

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);
+}

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)]

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: [],
+    },
+]