From dbabdbff1190bd83b85ca6a11cf4dfb90c27b3ff Mon Sep 17 00:00:00 2001 From: LagoLunatic Date: Fri, 10 Jul 2026 01:29:51 -0400 Subject: [PATCH] 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 --- objdiff-core/src/arch/arm.rs | 6 +- objdiff-core/src/obj/read.rs | 18 +- objdiff-core/tests/arch_arm.rs | 17 ++ objdiff-core/tests/data/arm/fake_tank.o | Bin 0 -> 9144 bytes ...m__do_not_trim_trailing_relocations-2.snap | 15 ++ ...arm__do_not_trim_trailing_relocations.snap | 160 ++++++++++++++++++ 6 files changed, 202 insertions(+), 14 deletions(-) create mode 100644 objdiff-core/tests/data/arm/fake_tank.o create mode 100644 objdiff-core/tests/snapshots/arch_arm__do_not_trim_trailing_relocations-2.snap create mode 100644 objdiff-core/tests/snapshots/arch_arm__do_not_trim_trailing_relocations.snap diff --git a/objdiff-core/src/arch/arm.rs b/objdiff-core/src/arch/arm.rs index 184b26b..78e93b8 100644 --- a/objdiff-core/src/arch/arm.rs +++ b/objdiff-core/src/arch/arm.rs @@ -481,9 +481,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 1b2589a..6000159 100644 --- a/objdiff-core/src/obj/read.rs +++ b/objdiff-core/src/obj/read.rs @@ -183,7 +183,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, @@ -225,9 +224,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)) } @@ -288,7 +284,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; @@ -1077,15 +1073,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 37cc838..96bc7e1 100644 --- a/objdiff-core/tests/arch_arm.rs +++ b/objdiff-core/tests/arch_arm.rs @@ -114,3 +114,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 0000000000000000000000000000000000000000..2e986f4cafcb6a2c49001247b5ab38eaf3a60c63 GIT binary patch literal 9144 zcmb<-^>JflWMqH=Mg|QA1do9sKnX0vY67M-7&sX?7!0=l|Niekrr0u6Q#z?v8&JzCt<4H#4el$Z(<-82k2lsObQ3KHBj z4VVfTRE0ef-8>q58WjvqG%6@?H1U?7Y2x?ttUtlvAmzZ&Ai>a)=+@C>V#ra!?95$| z;HGN8S|DJ_Q4y)Zb1_PRqd-R?(d|S72h^?u{~>lVDsU7S>|xx);LgCn;>j(eV$7h# zq##kiReq|ez#XDSU@Mo8BFxnqTo(^098d)LjDd|ogZYA@jEbSS0!M)W)D#8=9&afb z6(a^k2?c=y$?}^`k}|4>Odu6Tf)(EtI2;5DSUno6m=!o2BxMv0ISOF*fNcYZ8OU5_ z4;2GRMaBY%+dy(0P;;de1PUZ%3=J3yxQuKR1PU09Aojq_DcH*9B`Bk3$Wg(d!Eu4n zf}jh07^mp7X@TIEchNs$aol_$}n93$uK?;0Lw7(FexxhkdP6S5m1s) z2E`Yf=ZOwRixUkDEDRb<7gY@96gUbPHz2}SU@Mmgvy6eUR0WI7aV^Fh0WuN=5*3Fy zz;O?@oAttijs^)9s2dk_G)OIIP?Rw=kU|?WnVBle8U=U$tU{GLX zU{HbDp#`O>ZRcPz&w!PI!Gx88!HSiE!H$)I!G)E90TeHco=hGciI&h5z`(%lF(DBo z4#Ij+QDiY#dPWwLf~o`Q0b!vBAd(+Uf>>N|7Au&=t`ln3T!DC<@BfjzPZWAuhh23~WpsjGV~TIHb@)BAHQY zc?Je%24*B#5T6B21tSBf=+J?yVr1ZBU|=vn;!7|vFswl0gPIOnNPJZW28J0(e2@c? z_3JY*Fw8-cw`5>oSb)TLW?*1gg2V@zKMjcwG7nvT9g@5+0|UbmBtA&}1|&YnyhBKQ zko*ZGemDaI!xtoeA_D`%J0w0xKeBu_0|SE&$Plm^MsRrk0JA{^BSR@D{E*~9`n8bw zAf^e304Zl+WT!}gv19iD?kKP zel7z8!xSX`a!~vs@i#IsFx)`mgUp+R#0N2FfC#90yBQc59w70LGB7YaLE@ihU|`sW z#0Qyo4~c)1fq_AS32Ybx0|O*HUm(eY)T7JGAgO=Mz`$UE#DB}cz+i*K|IWa`;DE#j zHH%%4_}q*P3~@+&QAP%a1|&YnelsLK$Z5#>ysAoEut@j>QqK;pYIGBDgh;s-J^FdRYRM>8@oJV4^7GBPkcLE?kVzlFpHnUAbK zpOJy#3zB>#BLf2$yx3r5Xl7(!;6vi~GBPkIAn`%wt0D10=4&AFr!z7zI3V#CGBPl@ zAn{i-LfS%*6u`&;GS3c49%LS}`mG@KNb>s`85m-a_#pjJNc@wGkoFhM{L74xwit|m z7ZhHgLJ%y#$ncyI(nbSw85kKpGBPlL+gD%#Muy)Y2Z8EmD4&%H((VFt85kM(nILU2 z7+;bJ(jJ5Hm6;%IGZg5=7I6! znIP>S7$4+ZP}>v6&tzg?xP#>1VkQQL2S|KS_#w;JGBGfGL6QfV2WrQ{%mb z25|chYyl&~Lr~sh2XnyX-)m6%LE?V}g%1+{KNAB3sQm`f!N|bL3~9^3_`=MPb{&i_ z%M5Ab!T9RTkoF#oZ^#U3`@#6u%nS^mHXw}e%FMt3YA?a~{>+fJ6O13p3~5Kf_{q$W zHWiGY%M58>!T6x?1GQ~o{BmXn1|B5;H8L|W2q5u6;fE~W&CI}{f+P+`=W?*I5%nYjaAbcSPMur3MvWtO1fPsPG4jTV88vj2t0|The2AKfTFTldUa1z8} zU|<08W#M@fVxB7t1H(@w`6x8`bQT5%Q;?^j`YX`n7qBoefa)NS5g_w7voJ7#>I@kF z5SseiEDQ`Uk<0^?6|$f-4>g|^o;M)wlSkuQqVb(s85lMq=?`XQV9o6z`Y(D--I_%G1-f6(|`Yzz#MP%)5uq}V_q1Hqv9G)3bFpz#@0N*Lk|EDTJX zTn(LJEF%LL%hG~DB?ZQ@uz<7RN-YseO<=N4aF#2a<>CsJb%R^u2Diq|*bF9X3^Uoy z45ru30>Og0z|9%Xa)G(P%>}N`5o(^ZfdN#Vvw;zU1+~N3z!<96*}xd8*V(`X>QZL| zbGWP-T(22iuO)&7*Xs-ygN1>!0nB~Q1~B(I8^FTF*}%;a;&W#MH<-PKhA?{#;qEhp zyU*AJE{nv1*=uMDlQnXNv*7j`xxmyJxx##88-UuE-M(}Vkf`^8Y8_ZS425`5Sz)d!TxzrezHk?i2ab%3dg6p+_ z=`ex&(iHADQ@9J{d@RXcX zB*UFEQj@cT6LU+8{ZbQ)ASU>vrj-O`re~DEO$krT&df^>EdZO8?wKEynx0u)l3IkY z2c`nuXppI{c_}CkcFfDng&XabQ<<5U4mKF(M2HYWF)BaOH@_@3H#HC8Du_DA^2ACk z+Cjn$=@9{mMTy0kC8;5aMd_&}kpcO|nI)O|c^EPgaGBD)?07?Cv&3Y$6Ma$>%QExQ z!F<=e)S~oCP%I(&G_)Wku_V>4C^fYzH6$}PwFu&Ngj)Z^+<0U26bsX2Trw6WxMY%& zp*Fd|0}2vNU_MkXxFE4SFEcOQttdY?f&uPpc)0jx78hscfKwO59wY&X<5GHj9R>yluEmgX5)dCW{sE#5 z7#SFtmNGDyf%=TgA>$SxanSe#hz5-_fN0S80MjZ+KOe*g_1Br!Li*`U>mdDeka!Oh z0|VE3NPii`2labFv;{K*1J@={m>0;M~kbPtrC0Hvou=^0RZ4wPO1rI$eI6;OH&l->ZPw?OF~PV*9OS=1J^dlxC7UA$an+D9iX`tt{sr^1+JZtemBTn86fjFLHgAoKB)f- z3Kvj67er5Cg`_V~+Z)861LZG((o3N93Mjn>N^gMDTcGp~D7^(q z(lJmv0ZOMp=?o~H1EmX~bP1HMfYLQkx&cbJKiS4?yW7Q2GRvJ_DsMKP&yLE{*pd;uDt0Hu4-xCPU01_lXsNd5%P-GTBaXuJX>t^*QZ0vWFXAVs_f~KxOU42-W7PP*k13nhQ0IF`lrbsg|FmQs#<-t4# z1`R0&h8OG%pzH*mPXP@fa4;}1faaD!=0Usn4118w0%--wfoN?81_qFNLh3;CQp^kt z1t2{j3^Gpv#DHR$c_13pHi8N>++k;6c)$&wl4StRpMcy4>dS-t0J9UMc8@RvLpIFw f;L$vgCARAO;kJ>;o|Y7dz>& literal 0 HcmV?d00001 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 0000000..8346e01 --- /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 0000000..b4d238f --- /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: [], + }, +]