Re: [PATCH bpf-next v1] libbpf: Fix inheritance of BTF pointer size

[Tony Ambardar <tony.ambardar@gmail.com>]

On Thu, May 22, 2025 at 09:48:30AM +0100, Alan Maguire wrote:
> On 22/05/2025 07:21, Tony Ambardar wrote:
> > Update btf_new_empty() to copy the pointer size from a provided base BTF.
> > This ensures split BTF works properly and fixes test failures seen on
> > 32-bit targets:
> > 
> >   root@qemu-armhf:/usr/libexec/kselftests-bpf# ./test_progs -a btf_split
> >   __test_btf_split:PASS:empty_main_btf 0 nsec
> >   __test_btf_split:PASS:main_ptr_sz 0 nsec
> >   __test_btf_split:PASS:empty_split_btf 0 nsec
> >   __test_btf_split:FAIL:inherit_ptr_sz unexpected inherit_ptr_sz: actual 4 != expected 8
> >   [...]
> >   #41/1    btf_split/single_split:FAIL
> > 
> > Fixes: ba451366bf44 ("libbpf: Implement basic split BTF support")
> > Signed-off-by: Tony Ambardar <tony.ambardar@gmail.com>
> 
> Great catch! Nit: should we use the btf_ptr_sz(base_btf) helper here?
> That would cover the edge case where base BTF does not have ptr_sz set,
> as in the absence of a base pointer size this will then determine the
> pointer size from the BTF itself. The reason I ask is it seems like the
> BTF raw parsing codepath may not set the pointer size, and we will use
> that raw parsing to parse vmlinux BTF from /sys/kernel/btf/vmlinux (ELF
> parsing uses the ELF class to set pointer size so we are good there I think)
> 
> Reviewed-by: Alan Maguire <alan.maguire@oracle.com>
> 

I tried to keep simple copy semantics consistent with inheritance &
heuristics working as expected, so that edge case should have been OK.
However, looking more closely after Andrii's prompting, I don't think the
original code really works... (see next email).

Regarding raw parsing, let me circle back since it's been a while I was
deep in that source.

Thanks,
Tony

> > ---
> >  tools/lib/bpf/btf.c | 1 +
> >  1 file changed, 1 insertion(+)
> > 
> > diff --git a/tools/lib/bpf/btf.c b/tools/lib/bpf/btf.c
> > index 8d0d0b645a75..b1977888b35e 100644
> > --- a/tools/lib/bpf/btf.c
> > +++ b/tools/lib/bpf/btf.c
> > @@ -995,6 +995,7 @@ static struct btf *btf_new_empty(struct btf *base_btf)
> >  
> >  	if (base_btf) {
> >  		btf->base_btf = base_btf;
> > +		btf->ptr_sz = base_btf->ptr_sz;
> >  		btf->start_id = btf__type_cnt(base_btf);
> >  		btf->start_str_off = base_btf->hdr->str_len + base_btf->start_str_off;
> >  		btf->swapped_endian = base_btf->swapped_endian;
>