Re: [PATCH 2/2] btf_loader: fix smallest offset in case of bitfields

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

On 25/06/2025 16:23, Antoine Tenart wrote:
> In case the member is a bitfield, using pos->byte_size can be misleading
> as it can be larger than the actual size. In such case we could miss an
> actual hole in the struct and not infer the alignment as expected.
> 
> Before this patch we can see:
> 
>   $ pahole nf_tables.btf -C nft_rule_dp
>   struct nft_rule_dp {
>   	u64                        is_last:1;            /*     0: 0  8 */
>   	u64                        dlen:12;              /*     0: 1  8 */
>   	u64                        handle:42;            /*     0:13  8 */
> 
>   	/* XXX 9 bits hole, try to pack */
> 
>   	unsigned char              data[];               /*     8     0 */
> 
>   	/* size: 8, cachelines: 1, members: 4 */
>   	/* sum bitfield members: 55 bits, bit holes: 1, sum bit holes: 9 bits */
>   	/* last cacheline: 8 bytes */
>   };
> 
>   offsetof(struct nft_rule_dp, data) returns 7.
> 
> Where `data` won't be aligned as expected, as the first member fits on
> the byte left after the bitfield.
> 
> After this patch we get:
> 
>   $ pahole nf_tables.btf -C nft_rule_dp
>   struct nft_rule_dp {
>   	u64                        is_last:1;            /*     0: 0  8 */
>   	u64                        dlen:12;              /*     0: 1  8 */
>   	u64                        handle:42;            /*     0:13  8 */
> 
>   	/* XXX 9 bits hole, try to pack */
> 
>   	unsigned char              data[] __attribute__((__aligned__(2))); /*     8     0 */
> 
>   	/* size: 8, cachelines: 1, members: 4 */
>   	/* sum bitfield members: 55 bits, bit holes: 1, sum bit holes: 9 bits */
>   	/* forced alignments: 1 */
>   	/* last cacheline: 8 bytes */
>   };
> 
>   offsetof(struct nft_rule_dp, data) returns 8.
> 
> Thanks to Paolo Valerio for helping, including narrowing down where the
> issue was (BTF decoder).
> 
> Signed-off-by: Antoine Tenart <atenart@kernel.org>

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

> ---
>  btf_loader.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/btf_loader.c b/btf_loader.c
> index 76771afedd95..3beb5bba5f94 100644
> --- a/btf_loader.c
> +++ b/btf_loader.c
> @@ -676,7 +676,10 @@ static int class__fixup_btf_bitfields(const struct conf_load *conf, struct tag *
>  							pos->byte_offset,
>  							tag__natural_alignment(type, cu),
>  							smallest_offset);
> -		smallest_offset = pos->byte_offset + pos->byte_size;
> +		smallest_offset = pos->byte_offset;
> +		smallest_offset += pos->bitfield_size ?
> +			(pos->bitfield_offset + pos->bitfield_size + 7) / 8 :
> +			pos->byte_size;


would be good to have a comment here explaining the situation too; it's
quite a subtle problem so anything that helps explain things would be
great. Thanks for tracking this down!

>  	}
>  
>  	tag_type->alignment = class__infer_alignment(conf,