Re: [PATCH dwarves] bpf_encoder: Fix a verbose output issue

On 15/01/2026 05:00, Yonghong Song wrote:
> For the following test.c:
>   $ cat test.c
>   unsigned tar(int a);
>   __attribute__((noinline)) static int foo(int a, int b)
>   {
>     return tar(a) + tar(a + 1);
>   }
>   __attribute__((noinline)) int bar(int a)
>   {
>     foo(a, 1);
>     return 0;
>   }
> The llvm compilation:
>   $ clang -O2 -g -c test.c
> And then
>   $ pahole -JV test.o
>   btf_encoder__new: 'test.o' doesn't have '.data..percpu' sectio  n
>   File test.o:
>   [1] INT unsigned int size=4 nr_bits=32 encoding=(none)
>   [2] INT int size=4 nr_bits=32 encoding=SIGNED
>   search cu 'test.c' for percpu global variables.
>   [3] FUNC_PROTO (anon) return=2 args=(2 a, [4] FUNC bar type_id=3
>   [5] FUNC_PROTO (anon) return=2 args=(2 a, 2 b, [6] FUNC foo type_id=5
> 
> The above confused format is due to btf_encoder__add_func_proto_for_state().
> The "is_last = param_idx == nr_params" is always false since param_idx
> starts from 0. The below change fixed the issue:
>   is_last = param_idx == (nr_params - 1)
> 
> With the fix, 'pahole -JV test.o' will produce the following:
>   ...
>   [3] FUNC_PROTO (anon) return=2 args=(2 a)
>   [4] FUNC bar type_id=3
>   [5] FUNC_PROTO (anon) return=2 args=(2 a, 2 b)
>   [6] FUNC foo type_id=5
>   ...
> 
> Signed-off-by: Yonghong Song <[email protected]>

This fix looks good but I _think_ we have another instance of this; see
btf_encoder__add_func_proto_for_ftype()

	ftype__for_each_parameter(ftype, param) {
                name = parameter__name(param);
                type_id = param->tag.type == 0 ? 0 : encoder->type_id_off + param->tag.type;
                ++param_idx;
                if (btf_encoder__add_func_param(encoder, name, type_id, param_idx == nr_params))
                        return -1;
        }

        ++param_idx;
        if (ftype->unspec_parms)
                if (btf_encoder__add_func_param(encoder, NULL, 0, param_idx == nr_params))
                        return -1;

Maybe I'm misreading but that last ++param_idx outside the loop is unneeded
I think? If I'm right would you mind fixing that one too? Thanks!

> ---
>  btf_encoder.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/btf_encoder.c b/btf_encoder.c
> index b37ee7f..09a5cda 100644
> --- a/btf_encoder.c
> +++ b/btf_encoder.c
> @@ -895,7 +895,7 @@ static int32_t btf_encoder__add_func_proto_for_state(struct btf_encoder *encoder
>  	for (param_idx = 0; param_idx < nr_params; param_idx++) {
>  		p = &state->parms[param_idx];
>  		name = btf__name_by_offset(btf, p->name_off);
> -		is_last = param_idx == nr_params;
> +		is_last = param_idx == (nr_params - 1);
>  
>  		/* adding BTF data may result in a move of the
>  		 * name string memory, so make a temporary copy.