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.com@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.falcato@gmail.com>
> Sent: Thursday, October 26, 2023 2:46 PM
> To: devel@edk2.groups.io <devel@edk2.groups.io>; Jake Garver <jake@nvidia.com>
> Cc: rebecca@bsdio.com <rebecca@bsdio.com>; gaoliming@byosoft.com.cn <gaoliming@byosoft.com.cn>; bob.c.feng@intel.com <bob.c.feng@intel.com>; yuwei.chen@intel.com <yuwei.chen@intel.com>; ardb+tianocore@kernel.org <ardb+tianocore@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.com@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