qemu_arm_defconfig with LTO fails due to unaligned access

qemu_arm_defconfig with LTO fails due to unaligned access

[Heinrich Schuchardt]

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.

Best regards

Heinrich

Re: qemu_arm_defconfig with LTO fails due to unaligned access

[Ilias Apalodimas]

Hi Heinrich,

On Wed, 8 Mar 2023 at 06:12, Heinrich Schuchardt <xypron.glpk@gmx.de> 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?
>

I think it should

> 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.

Interesting... For the record, objdump agrees with what you are
seeing. The asm version is present only when compiling without LTO

>
> 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.

I am getting the same error using

# Ubuntu
arm-linux-gnueabihf-gcc --version
arm-linux-gnueabihf-gcc (Ubuntu 12.2.0-3ubuntu1) 12.2.0

#debian
arm-linux-gnueabihf-gcc --version
arm-linux-gnueabihf-gcc (GCC) 12.0.1 20220205 (experimental) [master
revision f49b8d25b1ff96e9cd09326666cc510b3a3accde]

Thanks
/Ilias

>
> Best regards
>
> Heinrich

Re: qemu_arm_defconfig with LTO fails due to unaligned access

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 2310 bytes --]

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.

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

Re: qemu_arm_defconfig with LTO fails due to unaligned access

[Heinrich Schuchardt]

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.

Best regards 

Heinrich

Re: qemu_arm_defconfig with LTO fails due to unaligned access

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 3009 bytes --]

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.

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]