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 lkft@linaro.org
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/tes... - https://storage.tuxsuite.com/public/linaro/lkft/builds/2eWtBPKv1yM8gfZJC8GkE...
-- Linaro LKFT https://lkft.linaro.org
On Wed, Apr 3, 2024 at 10:03 AM Naresh Kamboju naresh.kamboju@linaro.org 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 lkft@linaro.org
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/tes...
- https://storage.tuxsuite.com/public/linaro/lkft/builds/2eWtBPKv1yM8gfZJC8GkE...
-- 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 naresh.kamboju@linaro.org 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 lkft@linaro.org
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 arnd@arndb.de 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 naresh.kamboju@linaro.org 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 lkft@linaro.org
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 aleksander.lobakin@intel.com wrote:
From: Arnd Bergmann arnd@arndb.de 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 naresh.kamboju@linaro.org 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 lkft@linaro.org
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 aspsk@isovalent.com wrote:
On Wed, Apr 3, 2024 at 11:39 AM Alexander Lobakin aleksander.lobakin@intel.com wrote:
From: Arnd Bergmann arnd@arndb.de 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 naresh.kamboju@linaro.org 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 lkft@linaro.org
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 aspsk@isovalent.com Date: Wed, 3 Apr 2024 12:09:29 +0200
On Wed, Apr 3, 2024 at 11:57 AM Anton Protopopov aspsk@isovalent.com wrote:
On Wed, Apr 3, 2024 at 11:39 AM Alexander Lobakin aleksander.lobakin@intel.com wrote:
From: Arnd Bergmann arnd@arndb.de 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 naresh.kamboju@linaro.org 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 lkft@linaro.org
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 aspsk@isovalent.com 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