[PATCH dwarves] bpf_encoder: Fix a verbose output issue

[PATCH dwarves] bpf_encoder: Fix a verbose output issue

[Yonghong Song]

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 <yonghong.song@linux.dev>
---
 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.
-- 
2.47.3

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

[Alan Maguire]

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 <yonghong.song@linux.dev>

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.

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

[Yonghong Song]

On 1/15/26 1:41 AM, Alan Maguire wrote:
> 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 <yonghong.song@linux.dev>
> 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!

Sure. Will take a look.

>
>> ---
>>   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.