Skip to content

Commit dbabdbf

Browse files
LagoLunaticLagoLunatic
authored
ARM: Fix trailing relocations being ignored when inferring function sizes (#360)
* ARM: Fix relocations being ignored when inferring function sizes * ARM: Add test for trailing relocations * ARM: Fix trailing relocations being cut in half
1 parent 2fdba88 commit dbabdbf

6 files changed

Lines changed: 202 additions & 14 deletions

File tree

‎objdiff-core/src/arch/arm.rs‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -481,9 +481,13 @@ impl Arch for ArchArm {
481481
while next_address >= symbol.address + 2
482482
&& let Some(data) = section.data_range(next_address - 2, 2)
483483
&& data == [0u8; 2]
484-
&& section.relocation_at(next_address - 2, 2).is_none()
485484
{
486485
next_address -= 2;
486+
if let Some(relocation) = section.relocation_at(next_address, 2) {
487+
// Avoid cutting trailing relocations in half.
488+
next_address += self.data_reloc_size(relocation.flags) as u64;
489+
break;
490+
}
487491
}
488492
Ok(next_address.saturating_sub(symbol.address))
489493
}

‎objdiff-core/src/obj/read.rs‎

Lines changed: 5 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -183,7 +183,6 @@ fn map_symbol(
183183
fn map_symbols(
184184
arch: &dyn Arch,
185185
obj_file: &object::File,
186-
sections: &[Section],
187186
section_indices: &[usize],
188187
split_meta: Option<&SplitMeta>,
189188
config: &DiffObjConfig,
@@ -225,9 +224,6 @@ fn map_symbols(
225224
symbols.push(symbol);
226225
}
227226

228-
// Infer symbol sizes for 0-size symbols
229-
infer_symbol_sizes(arch, &mut symbols, sections)?;
230-
231227
Ok((symbols, symbol_indices))
232228
}
233229

@@ -288,7 +284,7 @@ fn is_local_label(symbol: &Symbol) -> bool {
288284
}
289285

290286
fn infer_symbol_sizes(arch: &dyn Arch, symbols: &mut [Symbol], sections: &[Section]) -> Result<()> {
291-
// Above, we've sorted the symbols by section and then by address.
287+
// Above, we've sorted the symbols by section and then by address, and also mapped section relocations.
292288

293289
// Set symbol sizes based on the next symbol's address
294290
let mut iter_idx = 0;
@@ -1077,15 +1073,11 @@ pub fn parse(data: &[u8], config: &DiffObjConfig, diff_side: DiffSide) -> Result
10771073
let split_meta = parse_split_meta(&obj_file)?;
10781074
let (mut sections, section_indices) =
10791075
map_sections(arch.as_ref(), &obj_file, split_meta.as_ref())?;
1080-
let (mut symbols, symbol_indices) = map_symbols(
1081-
arch.as_ref(),
1082-
&obj_file,
1083-
&sections,
1084-
&section_indices,
1085-
split_meta.as_ref(),
1086-
config,
1087-
)?;
1076+
let (mut symbols, symbol_indices) =
1077+
map_symbols(arch.as_ref(), &obj_file, &section_indices, split_meta.as_ref(), config)?;
10881078
map_relocations(arch.as_ref(), &obj_file, &mut sections, &section_indices, &symbol_indices)?;
1079+
// Infer symbol sizes for 0-size symbols (must be done after map_relocations is called)
1080+
infer_symbol_sizes(arch.as_ref(), &mut symbols, &sections)?;
10891081
parse_line_info(&obj_file, &mut sections, &section_indices, data)?;
10901082
if config.combine_data_sections || config.combine_text_sections {
10911083
combine_sections(&mut sections, &mut symbols, config)?;

‎objdiff-core/tests/arch_arm.rs‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -114,3 +114,20 @@ fn trim_trailing_hword() {
114114
let output = common::display_diff(&obj, &diff, symbol_idx, &diff_config);
115115
insta::assert_snapshot!(output);
116116
}
117+
118+
#[test]
119+
#[cfg(feature = "arm")]
120+
fn do_not_trim_trailing_relocations() {
121+
let diff_config = diff::DiffObjConfig::default();
122+
let obj = obj::read::parse(
123+
include_object!("data/arm/fake_tank.o"),
124+
&diff_config,
125+
diff::DiffSide::Base,
126+
)
127+
.unwrap();
128+
let symbol_idx = obj.symbols.iter().position(|s| s.name == "FakeTankIdleInit").unwrap();
129+
let diff = diff::code::no_diff_code(&obj, symbol_idx, &diff_config).unwrap();
130+
insta::assert_debug_snapshot!(diff.instruction_rows);
131+
let output = common::display_diff(&obj, &diff, symbol_idx, &diff_config);
132+
insta::assert_snapshot!(output);
133+
}

‎objdiff-core/tests/data/arm/fake_tank.o‎

8.93 KB
Binary file not shown.

‎objdiff-core/tests/snapshots/arch_arm__do_not_trim_trailing_relocations-2.snap‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
---
2+
source: objdiff-core/tests/arch_arm.rs
3+
expression: output
4+
---
5+
[(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)]
6+
[(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)]
7+
[(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)]
8+
[(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)]
9+
[(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)]
10+
[(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)]
11+
[(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)]
12+
[(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)]
13+
[(Address(16), Dim, 5), (Spacing(4), Normal, 0), (Opcode("bx", 9), Normal, 10), (Argument(Opaque("lr")), Normal, 0), (Eol, Normal, 0)]
14+
[(Address(18), Dim, 5), (Spacing(4), Normal, 0), (Opcode(".hword", 65534), Normal, 10), (Argument(Unsigned(0)), Normal, 0), (Eol, Normal, 0)]
15+
[(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‎

Lines changed: 160 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,160 @@
1+
---
2+
source: objdiff-core/tests/arch_arm.rs
3+
expression: diff.instruction_rows
4+
---
5+
[
6+
InstructionDiffRow {
7+
ins_ref: Some(
8+
InstructionRef {
9+
address: 176,
10+
size: 2,
11+
opcode: 24,
12+
branch_dest: None,
13+
},
14+
),
15+
kind: None,
16+
branch_from: None,
17+
branch_to: None,
18+
arg_diff: [],
19+
},
20+
InstructionDiffRow {
21+
ins_ref: Some(
22+
InstructionRef {
23+
address: 178,
24+
size: 2,
25+
opcode: 43,
26+
branch_dest: None,
27+
},
28+
),
29+
kind: None,
30+
branch_from: None,
31+
branch_to: None,
32+
arg_diff: [],
33+
},
34+
InstructionDiffRow {
35+
ins_ref: Some(
36+
InstructionRef {
37+
address: 180,
38+
size: 2,
39+
opcode: 1,
40+
branch_dest: None,
41+
},
42+
),
43+
kind: None,
44+
branch_from: None,
45+
branch_to: None,
46+
arg_diff: [],
47+
},
48+
InstructionDiffRow {
49+
ins_ref: Some(
50+
InstructionRef {
51+
address: 182,
52+
size: 2,
53+
opcode: 43,
54+
branch_dest: None,
55+
},
56+
),
57+
kind: None,
58+
branch_from: None,
59+
branch_to: None,
60+
arg_diff: [],
61+
},
62+
InstructionDiffRow {
63+
ins_ref: Some(
64+
InstructionRef {
65+
address: 184,
66+
size: 2,
67+
opcode: 43,
68+
branch_dest: None,
69+
},
70+
),
71+
kind: None,
72+
branch_from: None,
73+
branch_to: None,
74+
arg_diff: [],
75+
},
76+
InstructionDiffRow {
77+
ins_ref: Some(
78+
InstructionRef {
79+
address: 186,
80+
size: 2,
81+
opcode: 117,
82+
branch_dest: None,
83+
},
84+
),
85+
kind: None,
86+
branch_from: None,
87+
branch_to: None,
88+
arg_diff: [],
89+
},
90+
InstructionDiffRow {
91+
ins_ref: Some(
92+
InstructionRef {
93+
address: 188,
94+
size: 2,
95+
opcode: 117,
96+
branch_dest: None,
97+
},
98+
),
99+
kind: None,
100+
branch_from: None,
101+
branch_to: None,
102+
arg_diff: [],
103+
},
104+
InstructionDiffRow {
105+
ins_ref: Some(
106+
InstructionRef {
107+
address: 190,
108+
size: 2,
109+
opcode: 124,
110+
branch_dest: None,
111+
},
112+
),
113+
kind: None,
114+
branch_from: None,
115+
branch_to: None,
116+
arg_diff: [],
117+
},
118+
InstructionDiffRow {
119+
ins_ref: Some(
120+
InstructionRef {
121+
address: 192,
122+
size: 2,
123+
opcode: 9,
124+
branch_dest: None,
125+
},
126+
),
127+
kind: None,
128+
branch_from: None,
129+
branch_to: None,
130+
arg_diff: [],
131+
},
132+
InstructionDiffRow {
133+
ins_ref: Some(
134+
InstructionRef {
135+
address: 194,
136+
size: 2,
137+
opcode: 65534,
138+
branch_dest: None,
139+
},
140+
),
141+
kind: None,
142+
branch_from: None,
143+
branch_to: None,
144+
arg_diff: [],
145+
},
146+
InstructionDiffRow {
147+
ins_ref: Some(
148+
InstructionRef {
149+
address: 196,
150+
size: 4,
151+
opcode: 65534,
152+
branch_dest: None,
153+
},
154+
),
155+
kind: None,
156+
branch_from: None,
157+
branch_to: None,
158+
arg_diff: [],
159+
},
160+
]

0 commit comments

Comments
 (0)