Date:   Tue, 23 Mar 2021 19:59:55 +0100
From:   Oliver Hartkopp <socketcan@...tkopp.net>
To:     Rasmus Villemoes <linux@...musvillemoes.dk>,
        Marc Kleine-Budde <mkl@...gutronix.de>,
        Rong Chen <rong.a.chen@...el.com>,
        Patrick Menschel <menschel.p@...teo.de>
Cc:     kernel test robot <lkp@...el.com>, kbuild-all@...ts.01.org,
        linux-kernel@...r.kernel.org, linux-can <linux-can@...r.kernel.org>
Subject: Re: [kbuild-all] Re: include/linux/compiler_types.h:315:38: error:
 call to '__compiletime_assert_536' declared with attribute error:
 BUILD_BUG_ON failed: offsetof(struct can_frame, len) != offsetof(struct
 canfd_frame, len) || offsetof(struct can_frame, data) != offsetof(struc...



On 23.03.21 15:00, Rasmus Villemoes wrote:
> On 23/03/2021 13.49, Oliver Hartkopp wrote:
>>
>>
>> On 23.03.21 12:36, Rasmus Villemoes wrote:
>>>
>>> and more directly from the horse's mouth:
>>>
>>> https://developer.arm.com/documentation/dui0067/d/arm-compiler-reference/c-and-c---implementation-details/structures--unions--enumerations--and-bitfields
>>>
>>>
>>> Field alignment
>>>
>>>       Structures are arranged with the first-named component at the lowest
>>> address. Fields are aligned as follows:
>>>
>>>           A field with a char type is aligned to the next available byte.
>>>
>>>           A field with a short type is aligned to the next even-addressed
>>> byte.
>>>
>>>           Bitfield alignment depends on how the bitfield is declared. See
>>> Bitfields in packed structures for more information.
>>>
>>>           All other types are aligned on word boundaries.
>>>
>>> That anonymous union falls into the "All other types" bullet.
>>>
>>> __packed is the documented and standard way to overrule the
>>> compiler's/ABI's layout decisions.
>>
>> So why is there a difference between
>>
>> gcc version 10.2.0
>>
>> and
>>
>> gcc version 10.2.1 20210110 (Debian 10.2.1-6)
> 
> I'm guessing there's no difference between those (in this respect), but
> they are invoked differently.
> 
>> Would this mean that either STRUCTURE_SIZE_BOUNDARY or the command line
>> option -mstructure_size_boundary=<n>
>>
>> are set differently?
> 
> Yes, though very likely -mstructure_size_boundary is not set explicitly
> but via some other option.
> 
> gcc has a rather helpful but almost unknown feature that one can
> actually query for lots of different parameters and their
> default/current values. So on my Ubuntu system (20.04, gcc 9.3), for
> example, if I do
> 
> $ arm-linux-gnueabihf-gcc -O2 -Q --help=target | grep struct
>    -mstructure-size-boundary=            8
> 
> So that would seem to say that the union should work as expected.
> However, when I actually try to compile with the .config that kbuild
> reports failing, I do see that BUILD_BUG_ON triggering.
> 
> So let us inspect the actual command line used to build some other
> random .o file in net/can; look at net/can/.bcm.o.cmd
> 
> cmd_net/can/bcm.o := arm-linux-gnueabihf-gcc -Wp,-MMD,net/can/.bcm.o.d
> -nostdinc -isystem /usr/lib/gcc-cross/arm-linux-gnueabihf/9/include
> -I./arch/arm/include -I./arch/arm/include/generated  -I./include
> -I./arch/arm/include/uapi -I./arch/arm/include/generated/uapi
> -I./include/uapi -I./include/generated/uapi -include
> ./include/linux/compiler-version.h -include ./include/linux/kconfig.h
> -include ./include/linux/compiler_types.h -D__KERNEL__ -mlittle-endian
> -I./arch/arm/mach-footbridge/include -fmacro-prefix-map=./= -Wall
> -Wundef -Werror=strict-prototypes -Wno-trigraphs -fno-strict-aliasing
> -fno-common -fshort-wchar -fno-PIE -Werror=implicit-function-declaration
> -Werror=implicit-int -Werror=return-type -Wno-format-security -std=gnu89
> -fno-dwarf2-cfi-asm -mno-unaligned-access -fno-omit-frame-pointer -mapcs
> -mno-sched-prolog -fno-ipa-sra -mabi=apcs-gnu -mno-thumb-interwork -marm
> -Wa,-mno-warn-deprecated -D__LINUX_ARM_ARCH__=4 -march=armv4
> -mtune=strongarm110 -msoft-float -Uarm -fno-delete-null-pointer-checks
> -Wno-frame-address -Wno-format-truncation -Wno-format-overflow
> -Wno-address-of-packed-member -O2 --param=allow-store-data-races=0
> -Wframe-larger-than=1024 -fno-stack-protector
> -Wno-unused-but-set-variable -Wimplicit-fallthrough
> -Wno-unused-const-variable -fno-omit-frame-pointer
> -fno-optimize-sibling-calls -fno-inline-functions-called-once
> -Wdeclaration-after-statement -Wvla -Wno-pointer-sign
> -Wno-stringop-truncation -Wno-array-bounds -Wno-stringop-overflow
> -Wno-restrict -Wno-maybe-uninitialized -fno-strict-overflow
> -fno-stack-check -fconserve-stack -Werror=date-time
> -Werror=incompatible-pointer-types -Werror=designated-init
> -Wno-packed-not-aligned    -fsanitize-coverage=trace-pc
> -DKBUILD_MODFILE='"net/can/can-bcm"' -DKBUILD_BASENAME='"bcm"'
> -DKBUILD_MODNAME='"can_bcm"' -D__KBUILD_MODNAME=kmod_can_bcm -c -o
> net/can/bcm.o net/can/bcm.c
> 
> Lots of gunk. But just to see if one of those options have affected the
> -mstructure-size-boundary= value, just take that whole command line and
> throw in -Q --help=target at the end, and we get
> 
>    -mstructure-size-boundary=            32
> 
> So let us guess that it's the ABI choice -mabi=apcs-gnu
> 
> $ arm-linux-gnueabihf-gcc -O2 -msoft-float -mabi=apcs-gnu -Q
> --help=target | grep struct
>    -mstructure-size-boundary=            32
> 
> Bingo. (-msoft-float is also included just as in the real command line
> because gcc barfs otherwise).

Thanks for all the comprehensive explanations!

> Now what CONFIG_* knobs are responsible for putting -mabi=apcs-gnu in
> CFLAGS is left as an exercise for the reader. Regardless, it is not a
> bug in the compiler. The error is the assumption that this language
> 
> "Aggregates and Unions
> 
> Structures and unions assume the alignment of their most strictly
> aligned component.

(parse error in sentence)

> Each member is assigned to the lowest available offset with the appropriate
> alignment. The size of any object is always a multiple of the object‘s
> alignment."
> 
> from the x86-64 ABI applies on all other architectures/ABIs.
> 
>> I'm not a compiler expert but this does not seem to be consistent.
>>
>> Especially as we only have byte sizes (inside and outside of the union)
>> and "A field with a char type is aligned to the next available byte."
> 
> Yes, and that's exactly what you got before the anon union was introduced.

Before(!) the union there is nothing to pad.

>> The union is indeed aligned to the word boundary - but the following
>> byte is not aligned to the next available byte.
> 
> Yes it is, because the union occupies 4 bytes. The first byte is shared
> by the two char members, the remaining three bytes are padding.

But why is the union 4 bytes long here and adds a padding of three bytes 
at the end? IMO this is an error.

Thanks for your patience,
Oliver

Powered by blists - more mailing lists