The arm footbridge_defconfig failed with gcc-13 and gcc-8 on Linux next
starting from next-20240328..next-20240402.
arm:
build:
* gcc-8-footbridge_defconfig - Failed
* gcc-13-footbridge_defconfig - Failed
Reported-by: Linux Kernel Functional Testing <[email protected]>
In file included from include/linux/bitfield.h:10,
from arch/arm/include/asm/ptrace.h:13,
from arch/arm/include/asm/processor.h:14,
from include/linux/prefetch.h:15,
from arch/arm/include/asm/atomic.h:12,
from include/linux/atomic.h:7,
from net/core/filter.c:20:
include/linux/build_bug.h:78:41: error: static assertion failed:
"struct bpf_fib_lookup size check"
78 | #define __static_assert(expr, msg, ...) _Static_assert(expr, msg)
| ^~~~~~~~~~~~~~
Steps to reproduce:
------
# tuxmake --runtime podman --target-arch arm --toolchain gcc-13
--kconfig footbridge_defconfig
Links:
- https://qa-reports.linaro.org/lkft/linux-next-master/build/next-20240402/testrun/23264362/suite/build/test/gcc-13-footbridge_defconfig/details/
- https://storage.tuxsuite.com/public/linaro/lkft/builds/2eWtBPKv1yM8gfZJC8GkEkxN2j8/
--
Linaro LKFT
https://lkft.linaro.org
On Wed, Apr 3, 2024 at 10:03 AM Naresh Kamboju
<[email protected]> wrote:
>
> The arm footbridge_defconfig failed with gcc-13 and gcc-8 on Linux next
> starting from next-20240328..next-20240402.
>
> arm:
> build:
> * gcc-8-footbridge_defconfig - Failed
> * gcc-13-footbridge_defconfig - Failed
>
> Reported-by: Linux Kernel Functional Testing <[email protected]>
>
> In file included from include/linux/bitfield.h:10,
> from arch/arm/include/asm/ptrace.h:13,
> from arch/arm/include/asm/processor.h:14,
> from include/linux/prefetch.h:15,
> from arch/arm/include/asm/atomic.h:12,
> from include/linux/atomic.h:7,
> from net/core/filter.c:20:
> include/linux/build_bug.h:78:41: error: static assertion failed:
> "struct bpf_fib_lookup size check"
> 78 | #define __static_assert(expr, msg, ...) _Static_assert(expr, msg)
> | ^~~~~~~~~~~~~~
Thanks, I will take a look today
> Steps to reproduce:
> ------
> # tuxmake --runtime podman --target-arch arm --toolchain gcc-13
> --kconfig footbridge_defconfig
>
> Links:
> - https://qa-reports.linaro.org/lkft/linux-next-master/build/next-20240402/testrun/23264362/suite/build/test/gcc-13-footbridge_defconfig/details/
> - https://storage.tuxsuite.com/public/linaro/lkft/builds/2eWtBPKv1yM8gfZJC8GkEkxN2j8/
>
> --
> Linaro LKFT
> https://lkft.linaro.org
On Wed, Apr 3, 2024, at 10:10, Anton Protopopov wrote:
> On Wed, Apr 3, 2024 at 10:03 AM Naresh Kamboju
> <[email protected]> wrote:
>>
>> The arm footbridge_defconfig failed with gcc-13 and gcc-8 on Linux next
>> starting from next-20240328..next-20240402.
>>
>> arm:
>> build:
>> * gcc-8-footbridge_defconfig - Failed
>> * gcc-13-footbridge_defconfig - Failed
>>
>> Reported-by: Linux Kernel Functional Testing <[email protected]>
>>
>> In file included from include/linux/bitfield.h:10,
>> from arch/arm/include/asm/ptrace.h:13,
>> from arch/arm/include/asm/processor.h:14,
>> from include/linux/prefetch.h:15,
>> from arch/arm/include/asm/atomic.h:12,
>> from include/linux/atomic.h:7,
>> from net/core/filter.c:20:
>> include/linux/build_bug.h:78:41: error: static assertion failed:
>> "struct bpf_fib_lookup size check"
>> 78 | #define __static_assert(expr, msg, ...) _Static_assert(expr, msg)
>> | ^~~~~~~~~~~~~~
>
> Thanks, I will take a look today
The problem is CONFIG_AEABI=n, which changes the alignment
of sub-word struct members. I had assumed that AEABI is enabled
by default for everything already, but it looks like footbridge
and a couple of other defconfigs still have it turned off:
$ git grep -l CONFIG_ARCH_MULTI_V7.is.not arch/arm/configs/* | xargs git grep -L AEABI
arch/arm/configs/assabet_defconfig
arch/arm/configs/collie_defconfig
arch/arm/configs/footbridge_defconfig
arch/arm/configs/h3600_defconfig
arch/arm/configs/jornada720_defconfig
arch/arm/configs/neponset_defconfig
arch/arm/configs/netwinder_defconfig
arch/arm/configs/rpc_defconfig
arch/arm/configs/spear3xx_defconfig
arch/arm/configs/spear6xx_defconfig
arch/arm/configs/spitz_defconfig
Russell still has machines with an OABI toolchain, but I'm not
aware of anyone else relying on it. It does cause other
problems as well, so I already turned it off a long time ago
for my randconfig testing.
We should probably make it the default for everything, except
whichever defconfig Russell uses:
--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig
@@ -1159,7 +1159,7 @@ config ARM_PATCH_IDIV
config AEABI
bool "Use the ARM EABI to compile the kernel" if !CPU_V7 && \
!CPU_V7M && !CPU_V6 && !CPU_V6K && !CC_IS_CLANG && !COMPILE_TEST
- default CPU_V7 || CPU_V7M || CPU_V6 || CPU_V6K || CC_IS_CLANG || COMPILE_TEST
+ default y
help
This option allows for the kernel to be compiled using the latest
ARM ABI (aka EABI). This is only useful if you are using a user
Or we could go one step further and make it 'depends on
EXPERT', short of removing it entirely.
Arnd
From: Arnd Bergmann <[email protected]>
Date: Wed, 03 Apr 2024 10:45:36 +0200
> On Wed, Apr 3, 2024, at 10:10, Anton Protopopov wrote:
>> On Wed, Apr 3, 2024 at 10:03 AM Naresh Kamboju
>> <[email protected]> wrote:
>>>
>>> The arm footbridge_defconfig failed with gcc-13 and gcc-8 on Linux next
>>> starting from next-20240328..next-20240402.
>>>
>>> arm:
>>> build:
>>> * gcc-8-footbridge_defconfig - Failed
>>> * gcc-13-footbridge_defconfig - Failed
>>>
>>> Reported-by: Linux Kernel Functional Testing <[email protected]>
>>>
>>> In file included from include/linux/bitfield.h:10,
>>> from arch/arm/include/asm/ptrace.h:13,
>>> from arch/arm/include/asm/processor.h:14,
>>> from include/linux/prefetch.h:15,
>>> from arch/arm/include/asm/atomic.h:12,
>>> from include/linux/atomic.h:7,
>>> from net/core/filter.c:20:
>>> include/linux/build_bug.h:78:41: error: static assertion failed:
>>> "struct bpf_fib_lookup size check"
>>> 78 | #define __static_assert(expr, msg, ...) _Static_assert(expr, msg)
>>> | ^~~~~~~~~~~~~~
>>
>> Thanks, I will take a look today
Naresh,
Could you please remove that static_assert() and dump bpf_bif_lookup
layout from pahole?
Anton unionized { smac, dmac } with __u32 mark. On x86_64, the offset of
smac was 52 (aligned to 4) already, so I don't really get what AEABI
does here. IIRC it aligns every structure to 8 bytes?
Maybe we could just add __attribute__((__packed__))
__attribute__((__aligned__(4))) to that anonymous union at the end.
>
> The problem is CONFIG_AEABI=n, which changes the alignment
> of sub-word struct members. I had assumed that AEABI is enabled
> by default for everything already, but it looks like footbridge
> and a couple of other defconfigs still have it turned off:
>
> $ git grep -l CONFIG_ARCH_MULTI_V7.is.not arch/arm/configs/* | xargs git grep -L AEABI
> arch/arm/configs/assabet_defconfig
> arch/arm/configs/collie_defconfig
> arch/arm/configs/footbridge_defconfig
> arch/arm/configs/h3600_defconfig
> arch/arm/configs/jornada720_defconfig
> arch/arm/configs/neponset_defconfig
> arch/arm/configs/netwinder_defconfig
> arch/arm/configs/rpc_defconfig
> arch/arm/configs/spear3xx_defconfig
> arch/arm/configs/spear6xx_defconfig
> arch/arm/configs/spitz_defconfig
>
> Russell still has machines with an OABI toolchain, but I'm not
> aware of anyone else relying on it. It does cause other
> problems as well, so I already turned it off a long time ago
> for my randconfig testing.
>
> We should probably make it the default for everything, except
> whichever defconfig Russell uses:
>
> --- a/arch/arm/Kconfig
> +++ b/arch/arm/Kconfig
> @@ -1159,7 +1159,7 @@ config ARM_PATCH_IDIV
> config AEABI
> bool "Use the ARM EABI to compile the kernel" if !CPU_V7 && \
> !CPU_V7M && !CPU_V6 && !CPU_V6K && !CC_IS_CLANG && !COMPILE_TEST
> - default CPU_V7 || CPU_V7M || CPU_V6 || CPU_V6K || CC_IS_CLANG || COMPILE_TEST
> + default y
> help
> This option allows for the kernel to be compiled using the latest
> ARM ABI (aka EABI). This is only useful if you are using a user
>
> Or we could go one step further and make it 'depends on
> EXPERT', short of removing it entirely.>
> Arnd
Thanks,
Olek
On Wed, Apr 3, 2024 at 11:39 AM Alexander Lobakin
<[email protected]> wrote:
>
> From: Arnd Bergmann <[email protected]>
> Date: Wed, 03 Apr 2024 10:45:36 +0200
>
> > On Wed, Apr 3, 2024, at 10:10, Anton Protopopov wrote:
> >> On Wed, Apr 3, 2024 at 10:03 AM Naresh Kamboju
> >> <[email protected]> wrote:
> >>>
> >>> The arm footbridge_defconfig failed with gcc-13 and gcc-8 on Linux next
> >>> starting from next-20240328..next-20240402.
> >>>
> >>> arm:
> >>> build:
> >>> * gcc-8-footbridge_defconfig - Failed
> >>> * gcc-13-footbridge_defconfig - Failed
> >>>
> >>> Reported-by: Linux Kernel Functional Testing <[email protected]>
> >>>
> >>> In file included from include/linux/bitfield.h:10,
> >>> from arch/arm/include/asm/ptrace.h:13,
> >>> from arch/arm/include/asm/processor.h:14,
> >>> from include/linux/prefetch.h:15,
> >>> from arch/arm/include/asm/atomic.h:12,
> >>> from include/linux/atomic.h:7,
> >>> from net/core/filter.c:20:
> >>> include/linux/build_bug.h:78:41: error: static assertion failed:
> >>> "struct bpf_fib_lookup size check"
> >>> 78 | #define __static_assert(expr, msg, ...) _Static_assert(expr, msg)
> >>> | ^~~~~~~~~~~~~~
> >>
> >> Thanks, I will take a look today
>
> Naresh,
>
> Could you please remove that static_assert() and dump bpf_bif_lookup
> layout from pahole?
>
> Anton unionized { smac, dmac } with __u32 mark. On x86_64, the offset of
> smac was 52 (aligned to 4) already, so I don't really get what AEABI
> does here. IIRC it aligns every structure to 8 bytes?
>
> Maybe we could just add __attribute__((__packed__))
> __attribute__((__aligned__(4))) to that anonymous union at the end.
Yeah, I am sending a patch for this right now. Better not to depend on
compiler options
On Wed, Apr 3, 2024 at 11:57 AM Anton Protopopov <[email protected]> wrote:
>
> On Wed, Apr 3, 2024 at 11:39 AM Alexander Lobakin
> <[email protected]> wrote:
> >
> > From: Arnd Bergmann <[email protected]>
> > Date: Wed, 03 Apr 2024 10:45:36 +0200
> >
> > > On Wed, Apr 3, 2024, at 10:10, Anton Protopopov wrote:
> > >> On Wed, Apr 3, 2024 at 10:03 AM Naresh Kamboju
> > >> <[email protected]> wrote:
> > >>>
> > >>> The arm footbridge_defconfig failed with gcc-13 and gcc-8 on Linux next
> > >>> starting from next-20240328..next-20240402.
> > >>>
> > >>> arm:
> > >>> build:
> > >>> * gcc-8-footbridge_defconfig - Failed
> > >>> * gcc-13-footbridge_defconfig - Failed
> > >>>
> > >>> Reported-by: Linux Kernel Functional Testing <[email protected]>
> > >>>
> > >>> In file included from include/linux/bitfield.h:10,
> > >>> from arch/arm/include/asm/ptrace.h:13,
> > >>> from arch/arm/include/asm/processor.h:14,
> > >>> from include/linux/prefetch.h:15,
> > >>> from arch/arm/include/asm/atomic.h:12,
> > >>> from include/linux/atomic.h:7,
> > >>> from net/core/filter.c:20:
> > >>> include/linux/build_bug.h:78:41: error: static assertion failed:
> > >>> "struct bpf_fib_lookup size check"
> > >>> 78 | #define __static_assert(expr, msg, ...) _Static_assert(expr, msg)
> > >>> | ^~~~~~~~~~~~~~
> > >>
> > >> Thanks, I will take a look today
> >
> > Naresh,
> >
> > Could you please remove that static_assert() and dump bpf_bif_lookup
> > layout from pahole?
> >
> > Anton unionized { smac, dmac } with __u32 mark. On x86_64, the offset of
> > smac was 52 (aligned to 4) already, so I don't really get what AEABI
> > does here. IIRC it aligns every structure to 8 bytes?
> >
> > Maybe we could just add __attribute__((__packed__))
> > __attribute__((__aligned__(4))) to that anonymous union at the end.
>
> Yeah, I am sending a patch for this right now. Better not to depend on
> compiler options
One __packed__ was not enough though. The problem was also with the
union of two __u16's which is padded to be 32 bits when AEABI=n and
the whole structure is packed (so total size is 66 in this case).
From: Anton Protopopov <[email protected]>
Date: Wed, 3 Apr 2024 12:09:29 +0200
> On Wed, Apr 3, 2024 at 11:57 AM Anton Protopopov <[email protected]> wrote:
>>
>> On Wed, Apr 3, 2024 at 11:39 AM Alexander Lobakin
>> <[email protected]> wrote:
>>>
>>> From: Arnd Bergmann <[email protected]>
>>> Date: Wed, 03 Apr 2024 10:45:36 +0200
>>>
>>>> On Wed, Apr 3, 2024, at 10:10, Anton Protopopov wrote:
>>>>> On Wed, Apr 3, 2024 at 10:03 AM Naresh Kamboju
>>>>> <[email protected]> wrote:
>>>>>>
>>>>>> The arm footbridge_defconfig failed with gcc-13 and gcc-8 on Linux next
>>>>>> starting from next-20240328..next-20240402.
>>>>>>
>>>>>> arm:
>>>>>> build:
>>>>>> * gcc-8-footbridge_defconfig - Failed
>>>>>> * gcc-13-footbridge_defconfig - Failed
>>>>>>
>>>>>> Reported-by: Linux Kernel Functional Testing <[email protected]>
>>>>>>
>>>>>> In file included from include/linux/bitfield.h:10,
>>>>>> from arch/arm/include/asm/ptrace.h:13,
>>>>>> from arch/arm/include/asm/processor.h:14,
>>>>>> from include/linux/prefetch.h:15,
>>>>>> from arch/arm/include/asm/atomic.h:12,
>>>>>> from include/linux/atomic.h:7,
>>>>>> from net/core/filter.c:20:
>>>>>> include/linux/build_bug.h:78:41: error: static assertion failed:
>>>>>> "struct bpf_fib_lookup size check"
>>>>>> 78 | #define __static_assert(expr, msg, ...) _Static_assert(expr, msg)
>>>>>> | ^~~~~~~~~~~~~~
>>>>>
>>>>> Thanks, I will take a look today
>>>
>>> Naresh,
>>>
>>> Could you please remove that static_assert() and dump bpf_bif_lookup
>>> layout from pahole?
>>>
>>> Anton unionized { smac, dmac } with __u32 mark. On x86_64, the offset of
>>> smac was 52 (aligned to 4) already, so I don't really get what AEABI
>>> does here. IIRC it aligns every structure to 8 bytes?
>>>
>>> Maybe we could just add __attribute__((__packed__))
>>> __attribute__((__aligned__(4))) to that anonymous union at the end.
>>
>> Yeah, I am sending a patch for this right now. Better not to depend on
>> compiler options
>
> One __packed__ was not enough though. The problem was also with the
> union of two __u16's which is padded to be 32 bits when AEABI=n and
> the whole structure is packed (so total size is 66 in this case).
Hmm, on my setup it's 64 bytes. Since it's UAPI, it always must be of
the same size. There's probably a padding somewhere in the middle.
Also, don't forget to always set __aligned__(4 or 8) together with
__packed__, otherwise the compilers generate terrible code (they assume
the structure alignment is 1 and access to every field can be unaligned).
Thanks,
Olek
On Wed, Apr 3, 2024, at 12:09, Anton Protopopov wrote:
> On Wed, Apr 3, 2024 at 11:57 AM Anton Protopopov <[email protected]> wrote:
end.
>>
>> Yeah, I am sending a patch for this right now. Better not to depend on
>> compiler options
>
> One __packed__ was not enough though. The problem was also with the
> union of two __u16's which is padded to be 32 bits when AEABI=n and
> the whole structure is packed (so total size is 66 in this case).
The __packed attribute is easy to misunderstand, in this case you
would need to mark every internal union and struct as well, since
you otherwise run into one or both of these problems:
- an inner aggregate with explicit packing still requires
32-bit alignment (and padding) for each member, even if the
outer struct puts it at an unaligned position
- You get a compiler warning if an internal structure
is follows the normal alignment constraints but is located
at an unaligned offset, since that violates the alignment
constraints of the C standard.
Arnd