On Wed, Mar 08, 2023 at 08:19:32PM +0100, Heinrich Schuchardt wrote: > > > Am 8. März 2023 17:18:32 MEZ schrieb Tom Rini <[email protected]>: > >On Wed, Mar 08, 2023 at 05:12:26AM +0100, Heinrich Schuchardt wrote: > >> Hello Ilias, hello Tom, > >> > >> Tom tried to run qemu_arm_defconfig with CONFIG_LTO=y in gitlab. This > >> failed as shown in protocol > >> https://source.denx.de/u-boot/u-boot/-/jobs/589913/raw > >> > >> Executing 'HII database protocols' > >> test_hii_database_new_package_list: > >> data abort > >> pc : [<7ff39b98>] lr : [<7ff87328>] > >> reloc pc : [<00000b98>] lr : [<0004e328>] > >> sp : 7edf8cc0 ip : 0000000c fp : 7ffe60ec > >> r10: 00000000 r9 : 7eef8eb0 r8 : 7ffe0d02 > >> r7 : 00000000 r6 : 7ef0f8c8 r5 : 7ffe0cf0 r4 : 7ffe0cb4 > >> r3 : 7ffe0cef r2 : 00000000 r1 : ffffffff r0 : 00000000 > >> Flags: nzcv IRQs off FIQs off Mode SVC_32 > >> Code: e2403002 e3a00000 e1500001 012fff1e (e1f320b2) > >> UEFI image [0x00000000:0xffffffff] '/\selftest' > >> Resetting CPU .. > >> > >> Debugging shows: > >> > >> efi_hii_sibt_string_ucs2_block_next() calls u16_strnlen() for an > >> unaligned u16 string. Here "ldrh r2, [r3, #2]!" is executed for > >> unaligned r3. This should be allowable for SCTLR.A = 0. > >> > >> When the crash occurs SCRLR has value 0xc5187f. SCTLR.A is bit 1 with > >> value 1. > >> > >> The implementation of allow_unaligned() in > >> arch/arm/cpu/armv7 /sctlr.S should have set the flag to 0. > >> arch/arm/cpu/armv7/sctlr.S is compiled (as demonstrated by adding #error > >> to the code). > >> > >> If I remove the weak implementation of allow_unaligned() in > >> lib/efi_loader/efi_setup.c, the error does not occur. > >> > >> Shouldn't building with LTO ignore the weak implementation? > >> > >> If I add a printf() statement to the weak implemenation, the printf() > >> command is not executed but > >> > >> SCTLR 0xc5187d, SCTLR.A=0 > >> > >> The test passes as unaligned access is allowable. > >> > >> I was building inside the Docker image with the GCC downloaded by > >> buildman (gcc-12.2.0-nolibc/arm-linux-gnueabi). > >> > >> To me this looks like a compiler issue. > > > >Interesting, yes. It seems like it shouldn't be too hard to come up with > >a condensed example where the assembly function isn't used but instead > >the weak C function is. > > > >And as a work-around, re-doing the code so that path_to_uefi() just > >checks for ARM && !ARM64 before calling allow_unaligned() and not doing > >the weak function trick should also be fine. > > > > We have more places in our code using weak functions. There many cases not > covered by CI. > > Just looking at EFI may not be adequate.
Yes, there's a more general issue with weak functions where the non-weak version is in assembly rather than C. I think we've addressed those already, however, but it's good to note and should probably get documented. -- Tom
signature.asc
Description: PGP signature
