[PATCH bpf-next v2] bpf_encoder: Fix a verbose output issue

Yonghong Song <[email protected]>
Newsgroups org.kernel.vger.dwarves,org.kernel.vger.bpf
Message-ID <[email protected]>
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 <[email protected]>
---
 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
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.