Re: [PATCH 1/2] btf_loader: infer alignment for zero-length arrays

[Alan Maguire <alan.maguire@oracle.com>]

On 25/06/2025 16:23, Antoine Tenart wrote:
> Zero sized arrays do not go through the bitfield fixup logic, which in
> turn makes them to skip the alignment infer logic. The result is
> zero-length arrays being incorrectly aligned when an explicit statement
> is needed. E.g. this happens for variable length arrays at end of
> structures, or for 0-length ones.
> 
> Before this patch we can see:
> 
>   struct skb_ext {
> 	refcount_t                 refcnt;               /*     0     4 */
> 	u8                         offset[4];            /*     4     4 */
> 	u8                         chunks;               /*     8     1 */
> 
> 	/* XXX 7 bytes hole, try to pack */
> 
> 	char                       data[];               /*    16     0 */
> 
> 	/* size: 16, cachelines: 1, members: 4 */
> 	/* sum members: 9, holes: 1, sum holes: 7 */
> 	/* last cacheline: 16 bytes */
>   } __attribute__((__aligned__(8)));
> 
>   offsetof(struct skb_ext, data) returns 9.
> 
> After this patch we get:
> 
>   struct skb_ext {
> 	refcount_t                 refcnt;               /*     0     4 */
> 	u8                         offset[4];            /*     4     4 */
> 	u8                         chunks;               /*     8     1 */
> 
> 	/* XXX 7 bytes hole, try to pack */
> 
> 	char                       data[] __attribute__((__aligned__(8))); /*    16     0 */
> 
> 	/* size: 16, cachelines: 1, members: 4 */
> 	/* sum members: 9, holes: 1, sum holes: 7 */
> 	/* forced alignments: 1, forced holes: 1, sum forced holes: 7 */
> 	/* last cacheline: 16 bytes */
>   } __attribute__((__aligned__(8)));
> 
>   offsetof(struct skb_ext, data) returns 16.
> 
> Signed-off-by: Antoine Tenart <atenart@kernel.org>

Reviewed-by: Alan Maguire <alan.maguire@oracle.com>

with a small suggestion below..

> ---
>  btf_loader.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/btf_loader.c b/btf_loader.c
> index f4f9f65289b5..76771afedd95 100644
> --- a/btf_loader.c
> +++ b/btf_loader.c
> @@ -648,6 +648,10 @@ static int class__fixup_btf_bitfields(const struct conf_load *conf, struct tag *
>  		/* if BTF data is incorrect and has size == 0, skip field,
>  		 * instead of crashing */

nit: could we update the comment here to reflect the change, adding
something like "however we still need to infer alignment for cases like
zero/variable-length arays".


>  		if (pos->byte_size == 0) {
> +			pos->alignment = class__infer_alignment(conf,
> +								pos->byte_offset,
> +								tag__natural_alignment(type, cu),
> +								smallest_offset);
>  			continue;
>  		}
>