Hi Ard: > Can you double check the object file? I suspect this is a linker relaxation > not a compiler issue.
With LTO in play, is there a way to check the object file? It's not in aarch64 assembly. Thanks, Jake ________________________________ From: Ard Biesheuvel <a...@kernel.org> Sent: Friday, October 27, 2023 9:46 AM To: devel@edk2.groups.io <devel@edk2.groups.io>; Jake Garver <j...@nvidia.com> Cc: Pedro Falcato <pedro.falc...@gmail.com>; rebe...@bsdio.com <rebe...@bsdio.com>; gaolim...@byosoft.com.cn <gaolim...@byosoft.com.cn>; bob.c.f...@intel.com <bob.c.f...@intel.com>; yuwei.c...@intel.com <yuwei.c...@intel.com>; ardb+tianoc...@kernel.org <ardb+tianoc...@kernel.org> Subject: Re: [edk2-devel] [PATCH] BaseTools/GenFw: Change opcode when converting ADR to ADRP External email: Use caution opening links or attachments Hello all, Apologies for the late response. On Fri, 27 Oct 2023 at 14:44, Jake Garver via groups.io <jake=nvidia....@groups.io> wrote: > > Thanks for your response, Pedro. > > I chased this as a toolchain bug originally, but concluded that the ADR > indeed works before GenFw rewrites it. But I see your point regarding the > relocation statement. > > As requested, below is the disassembled function along with relocations. > This was generated from the dll using "aarch64-linux-gnu-objdump -r -D". The > ADR in question is at 2ffc. > Can you double check the object file? I suspect this is a linker relaxation not a compiler issue. > This code was generated by the current version of the GCC 10.x toolchain on > Ubuntu20. So, if we're concluding this is a toolchain issue, then it's with > a fairly "stock" toolchain. > > 0000000000002fec <fdt_path_offset>: > 2fec: a9b97bfd stp x29, x30, [sp, #-112]! > 2ff0: 910003fd mov x29, sp > 2ff4: a90363f7 stp x23, x24, [sp, #48] > 2ff8: aa0003f8 mov x24, x0 > 2ffc: 10020020 adr x0, 7000 <_cont+0xe98> > 2ffc: R_AARCH64_ADR_GOT_PAGE __stack_chk_guard The nasty thing with relying on --emit-relocs is that they get out of sync. R_AARCH64_ADR_GOT_PAGE is documented as applying to ADRP only, so this code is non-compliant one way or the other. There are other relaxations that also confuse GenFw when the static relocs don't get updated accordingly. > 3000: a90153f3 stp x19, x20, [sp, #16] > 3004: aa0103f3 mov x19, x1 > 3008: f946cc00 ldr x0, [x0, #3480] > 3008: R_AARCH64_LD64_GOT_LO12_NC __stack_chk_guard > 300c: a9025bf5 stp x21, x22, [sp, #32] > 3010: a9046bf9 stp x25, x26, [sp, #64] > 3014: f9002bfb str x27, [sp, #80] > 3018: f9400001 ldr x1, [x0] > 301c: f90037e1 str x1, [sp, #104] > 3020: d2800001 mov x1, #0x0 // #0 > 3024: aa1303e0 mov x0, x19 > 3028: 97fffd04 bl 2438 <AsciiStrLen> > 3028: R_AARCH64_CALL26 .text+0x1438 > 302c: aa0003f5 mov x21, x0 > 3030: aa1803e0 mov x0, x24 > 3034: 97fff807 bl 1050 <fdt_check_header> > 3034: R_AARCH64_CALL26 .text+0x50 > 3038: 2a0003f4 mov w20, w0 > 303c: 35000260 cbnz w0, 3088 <fdt_path_offset+0x9c> > 3040: 39400260 ldrb w0, [x19] > 3044: 93407ea1 sxtw x1, w21 > 3048: 8b35c275 add x21, x19, w21, sxtw > 304c: 7100bc1f cmp w0, #0x2f > 3050: 54000420 b.eq 30d4 <fdt_path_offset+0xe8> // b.none > 3054: 528005e2 mov w2, #0x2f // #47 > 3058: aa1303e0 mov x0, x19 > 305c: 97ffff2f bl 2d18 <ScanMem8> > 305c: R_AARCH64_CALL26 .text+0x1d18 > 3060: f100001f cmp x0, #0x0 > 3064: 9a951016 csel x22, x0, x21, ne // ne = any > 3068: f0000001 adrp x1, 6000 <__stack_chk_fail+0x7c> > 3068: R_AARCH64_ADR_PREL_PG_HI21 .text+0x58b6 > 306c: 9122d821 add x1, x1, #0x8b6 > 306c: R_AARCH64_ADD_ABS_LO12_NC .text+0x58b6 > 3070: aa1803e0 mov x0, x24 > 3074: cb1302d4 sub x20, x22, x19 > 3078: 97ffffdd bl 2fec <fdt_path_offset> > 307c: 2a0003e1 mov w1, w0 > 3080: 36f80140 tbz w0, #31, 30a8 <fdt_path_offset+0xbc> > 3084: 12800094 mov w20, #0xfffffffb // #-5 > 3088: 90000020 adrp x0, 7000 <_cont+0xe98> > 3088: R_AARCH64_ADR_GOT_PAGE __stack_chk_guard > 308c: f946cc00 ldr x0, [x0, #3480] > 308c: R_AARCH64_LD64_GOT_LO12_NC __stack_chk_guard > 3090: f94037e1 ldr x1, [sp, #104] > 3094: f9400002 ldr x2, [x0] > 3098: eb020021 subs x1, x1, x2 > 309c: d2800002 mov x2, #0x0 // #0 > 30a0: 54000a00 b.eq 31e0 <fdt_path_offset+0x1f4> // b.none > 30a4: 94000bb8 bl 5f84 <__stack_chk_fail> > 30a4: R_AARCH64_CALL26 __stack_chk_fail > 30a8: 2a1403e3 mov w3, w20 > 30ac: aa1303e2 mov x2, x19 > 30b0: aa1803e0 mov x0, x24 > 30b4: d2800004 mov x4, #0x0 // #0 > 30b8: 97ffff72 bl 2e80 <fdt_get_property_namelen> > 30b8: R_AARCH64_CALL26 .text+0x1e80 > 30bc: b4fffe40 cbz x0, 3084 <fdt_path_offset+0x98> > 30c0: 91003001 add x1, x0, #0xc > 30c4: aa1603f3 mov x19, x22 > 30c8: aa1803e0 mov x0, x24 > 30cc: 97ffffc8 bl 2fec <fdt_path_offset> > 30d0: 2a0003f4 mov w20, w0 > 30d4: 910193fa add x26, sp, #0x64 > 30d8: 14000037 b 31b4 <fdt_path_offset+0x1c8> > 30dc: 910006d6 add x22, x22, #0x1 > 30e0: eb1602bf cmp x21, x22 > 30e4: 54fffd20 b.eq 3088 <fdt_path_offset+0x9c> // b.none > 30e8: 394002c0 ldrb w0, [x22] > 30ec: 7100bc1f cmp w0, #0x2f > 30f0: 54ffff60 b.eq 30dc <fdt_path_offset+0xf0> // b.none > 30f4: cb1602a1 sub x1, x21, x22 > 30f8: aa1603e0 mov x0, x22 > 30fc: 528005e2 mov w2, #0x2f // #47 > 3100: 97ffff06 bl 2d18 <ScanMem8> > 3100: R_AARCH64_CALL26 .text+0x1d18 > 3104: f100001f cmp x0, #0x0 > 3108: 9a951013 csel x19, x0, x21, ne // ne = any > 310c: aa1803e0 mov x0, x24 > 3110: 97fff7d0 bl 1050 <fdt_check_header> > 3110: R_AARCH64_CALL26 .text+0x50 > 3114: 35000620 cbnz w0, 31d8 <fdt_path_offset+0x1ec> > 3118: 4b160277 sub w23, w19, w22 > 311c: b90067ff str wzr, [sp, #100] > 3120: 110006fb add w27, w23, #0x1 > 3124: 93407ef7 sxtw x23, w23 > 3128: b94067e0 ldr w0, [sp, #100] > 312c: 7100029f cmp w20, #0x0 > 3130: 7a40a801 ccmp w0, #0x0, #0x1, ge // ge = tcont > 3134: 5400008a b.ge 3144 <fdt_path_offset+0x158> // b.tcont > 3138: 36f803c0 tbz w0, #31, 31b0 <fdt_path_offset+0x1c4> > 313c: 12800014 mov w20, #0xffffffff // #-1 > 3140: 17ffffd2 b 3088 <fdt_path_offset+0x9c> > 3144: 7100041f cmp w0, #0x1 > 3148: 540000e0 b.eq 3164 <fdt_path_offset+0x178> // b.none > 314c: 2a1403e1 mov w1, w20 > 3150: aa1a03e2 mov x2, x26 > 3154: aa1803e0 mov x0, x24 > 3158: 97fff87c bl 1348 <fdt_next_node> > 3158: R_AARCH64_CALL26 .text+0x348 > 315c: 2a0003f4 mov w20, w0 > 3160: 17fffff2 b 3128 <fdt_path_offset+0x13c> > 3164: 2a1b03e2 mov w2, w27 > 3168: 11001281 add w1, w20, #0x4 > 316c: aa1803e0 mov x0, x24 > 3170: 97fff7da bl 10d8 <fdt_offset_ptr> > 3170: R_AARCH64_CALL26 .text+0xd8 > 3174: aa0003f9 mov x25, x0 > 3178: b4fffea0 cbz x0, 314c <fdt_path_offset+0x160> > 317c: f10002ff cmp x23, #0x0 > 3180: fa4012c4 ccmp x22, x0, #0x4, ne // ne = any > 3184: 54000201 b.ne 31c4 <fdt_path_offset+0x1d8> // b.any > 3188: 38776b20 ldrb w0, [x25, x23] > 318c: 34000120 cbz w0, 31b0 <fdt_path_offset+0x1c4> > 3190: aa1703e1 mov x1, x23 > 3194: aa1603e0 mov x0, x22 > 3198: 52800802 mov w2, #0x40 // #64 > 319c: 97fffedf bl 2d18 <ScanMem8> > 319c: R_AARCH64_CALL26 .text+0x1d18 > 31a0: b5fffd60 cbnz x0, 314c <fdt_path_offset+0x160> > 31a4: 38776b20 ldrb w0, [x25, x23] > 31a8: 7101001f cmp w0, #0x40 > 31ac: 54fffd01 b.ne 314c <fdt_path_offset+0x160> // b.any > 31b0: 37fff6d4 tbnz w20, #31, 3088 <fdt_path_offset+0x9c> > 31b4: eb1302bf cmp x21, x19 > 31b8: 54fff689 b.ls 3088 <fdt_path_offset+0x9c> // b.plast > 31bc: aa1303f6 mov x22, x19 > 31c0: 17ffffca b 30e8 <fdt_path_offset+0xfc> > 31c4: aa1703e2 mov x2, x23 > 31c8: aa1603e1 mov x1, x22 > 31cc: 97fffef7 bl 2da8 <CompareMem.part.0> > 31cc: R_AARCH64_CALL26 .text+0x1da8 > 31d0: 35fffbe0 cbnz w0, 314c <fdt_path_offset+0x160> > 31d4: 17ffffed b 3188 <fdt_path_offset+0x19c> > 31d8: 2a0003f4 mov w20, w0 > 31dc: 17fffff5 b 31b0 <fdt_path_offset+0x1c4> > 31e0: 2a1403e0 mov w0, w20 > 31e4: a94153f3 ldp x19, x20, [sp, #16] > 31e8: a9425bf5 ldp x21, x22, [sp, #32] > 31ec: a94363f7 ldp x23, x24, [sp, #48] > 31f0: a9446bf9 ldp x25, x26, [sp, #64] > 31f4: f9402bfb ldr x27, [sp, #80] > 31f8: a8c77bfd ldp x29, x30, [sp], #112 > 31fc: d65f03c0 ret > 3200: 14000400 b 4200 <MmioWrite32.isra.0> > 3204: d503201f nop > 3208: f946cc00 ldr x0, [x0, #3480] > 320c: 17ffff80 b 300c <fdt_path_offset+0x20> > > Jake > ________________________________ > From: Pedro Falcato <pedro.falc...@gmail.com> > Sent: Thursday, October 26, 2023 2:46 PM > To: devel@edk2.groups.io <devel@edk2.groups.io>; Jake Garver <j...@nvidia.com> > Cc: rebe...@bsdio.com <rebe...@bsdio.com>; gaolim...@byosoft.com.cn > <gaolim...@byosoft.com.cn>; bob.c.f...@intel.com <bob.c.f...@intel.com>; > yuwei.c...@intel.com <yuwei.c...@intel.com>; ardb+tianoc...@kernel.org > <ardb+tianoc...@kernel.org> > Subject: Re: [edk2-devel] [PATCH] BaseTools/GenFw: Change opcode when > converting ADR to ADRP > > External email: Use caution opening links or attachments > > > On Thu, Oct 26, 2023 at 4:32 PM Jake Garver via groups.io > <jake=nvidia....@groups.io> wrote: > > > > In the R_AARCH64_ADR_GOT_PAGE case on AARCH64, be sure to change the > > opcode to ADRP. Prior to this change, we updated the address, but not > > the opcode. > > > > This resolves an issue experienced when building a StandaloneMm image > > with stack protection enabled on GCC 10.5. This scenario generates an > > ADR where an ADRP is more common in other versions of GCC tested. That > > explains the obscurity of the issue. However, an ADR is valid and > > should be handled by GenFw. > > Is this not a toolchain bug? > The aarch64 ELF ABI > (https://github.com/ARM-software/abi-aa/blob/main/aaelf64/aaelf64.rst) > says: > "Set the immediate value of an ADRP to bits [32:12] of X; check that > –232 <= X < 232" > > So it mentions this relocation pointing *to an ADRP* specifically. And > AFAIK there's no way > Page(G(GDAT(S+A)))-Page(P) > > would ever make sense if we're looking at an adr and not an adrp. > > Can you post the full disassembly for the function, plus relevant relocations? > > -- > Pedro > -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#110208): https://edk2.groups.io/g/devel/message/110208 Mute This Topic: https://groups.io/mt/102202314/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
