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

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 <[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!

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.