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 trini@konsulko.com:
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.