public inbox for dwarves@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH bpf-next v2] bpf_encoder: Fix a verbose output issue
@ 2026-01-16  2:02 Yonghong Song
  2026-01-16  2:36 ` Yonghong Song
  2026-01-16 12:24 ` Alan Maguire
  0 siblings, 2 replies; 3+ messages in thread
From: Yonghong Song @ 2026-01-16  2:02 UTC (permalink / raw)
  To: Alan Maguire, Arnaldo Carvalho de Melo, dwarves
  Cc: Alexei Starovoitov, Andrii Nakryiko, bpf, kernel-team

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

In addition, in btf_encoder__add_func_proto_for_ftype(), we have
  ++param_idx;
  if (ftype->unspec_parms) { ... }
This is correct but it is misleading since '++param_idx' is only needed
inside the above 'if' condition. So put '++param_idx' inside the
'if' condition to make code cleaner.

Signed-off-by: Yonghong Song <yonghong.song@linux.dev>
---
 btf_encoder.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/btf_encoder.c b/btf_encoder.c
index b37ee7f..ec6933e 100644
--- a/btf_encoder.c
+++ b/btf_encoder.c
@@ -865,10 +865,11 @@ static int32_t btf_encoder__add_func_proto_for_ftype(struct btf_encoder *encoder
 			return -1;
 	}
 
-	++param_idx;
-	if (ftype->unspec_parms)
+	if (ftype->unspec_parms) {
+		++param_idx;
 		if (btf_encoder__add_func_param(encoder, NULL, 0, param_idx == nr_params))
 			return -1;
+	}
 
 	return id;
 }
@@ -895,7 +896,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


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH bpf-next v2] bpf_encoder: Fix a verbose output issue
  2026-01-16  2:02 [PATCH bpf-next v2] bpf_encoder: Fix a verbose output issue Yonghong Song
@ 2026-01-16  2:36 ` Yonghong Song
  2026-01-16 12:24 ` Alan Maguire
  1 sibling, 0 replies; 3+ messages in thread
From: Yonghong Song @ 2026-01-16  2:36 UTC (permalink / raw)
  To: Alan Maguire, Arnaldo Carvalho de Melo, dwarves
  Cc: Alexei Starovoitov, Andrii Nakryiko, bpf, kernel-team



On 1/15/26 6:02 PM, 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
>    ...
>
> In addition, in btf_encoder__add_func_proto_for_ftype(), we have
>    ++param_idx;
>    if (ftype->unspec_parms) { ... }
> This is correct but it is misleading since '++param_idx' is only needed
> inside the above 'if' condition. So put '++param_idx' inside the
> 'if' condition to make code cleaner.
>
> Signed-off-by: Yonghong Song <yonghong.song@linux.dev>
> ---
>   btf_encoder.c | 7 ++++---
>   1 file changed, 4 insertions(+), 3 deletions(-)
>
> diff --git a/btf_encoder.c b/btf_encoder.c
> index b37ee7f..ec6933e 100644
> --- a/btf_encoder.c
> +++ b/btf_encoder.c
> @@ -865,10 +865,11 @@ static int32_t btf_encoder__add_func_proto_for_ftype(struct btf_encoder *encoder
>   			return -1;
>   	}
>   
> -	++param_idx;
> -	if (ftype->unspec_parms)
> +	if (ftype->unspec_parms) {
> +		++param_idx;
>   		if (btf_encoder__add_func_param(encoder, NULL, 0, param_idx == nr_params))
>   			return -1;
> +	}
>   
>   	return id;
>   }
> @@ -895,7 +896,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.

Sorry. The tag should be '[PATCH dwarves v2]' instead of '[PATCH bpf-next v2]'.


^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH bpf-next v2] bpf_encoder: Fix a verbose output issue
  2026-01-16  2:02 [PATCH bpf-next v2] bpf_encoder: Fix a verbose output issue Yonghong Song
  2026-01-16  2:36 ` Yonghong Song
@ 2026-01-16 12:24 ` Alan Maguire
  1 sibling, 0 replies; 3+ messages in thread
From: Alan Maguire @ 2026-01-16 12:24 UTC (permalink / raw)
  To: Yonghong Song, Arnaldo Carvalho de Melo, dwarves
  Cc: Alexei Starovoitov, Andrii Nakryiko, bpf, kernel-team

On 16/01/2026 02:02, 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
>   ...
> 
> In addition, in btf_encoder__add_func_proto_for_ftype(), we have
>   ++param_idx;
>   if (ftype->unspec_parms) { ... }
> This is correct but it is misleading since '++param_idx' is only needed
> inside the above 'if' condition. So put '++param_idx' inside the
> 'if' condition to make code cleaner.
> 
> Signed-off-by: Yonghong Song <yonghong.song@linux.dev>

applied, thanks!

Alan

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2026-01-16 12:24 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-01-16  2:02 [PATCH bpf-next v2] bpf_encoder: Fix a verbose output issue Yonghong Song
2026-01-16  2:36 ` Yonghong Song
2026-01-16 12:24 ` Alan Maguire

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox