[bpf-next] libbpf: fix no-args func prototype BTF dumping syntax

Commit Message

Andrii Nakryiko July 12, 2024, 10:44 p.m. UTC

For all these years libbpf's BTF dumper has been emitting not strictly
valid syntax for function prototypes that have no input arguments.

Instead of `int (*blah)()` we should emit `int (*blah)(void)`.

This is not normally a problem, but it manifests when we get kfuncs in
vmlinux.h that have no input arguments. Due to compiler internal
specifics, we get no BTF information for such kfuncs, if they are not
declared with proper `(void)`.

The fix is trivial. We also need to adjust a few ancient tests that
happily assumed `()` is correct.

Reported-by: Tejun Heo <[e-mail]>
Fixes: 351131b51c7a ("libbpf: add btf_dump API for BTF-to-C conversion")
Signed-off-by: Andrii Nakryiko <[e-mail]>
---
 tools/lib/bpf/btf_dump.c                                  | 8 +++++---
 .../selftests/bpf/progs/btf_dump_test_case_multidim.c     | 4 ++--
 .../selftests/bpf/progs/btf_dump_test_case_syntax.c       | 4 ++--
 3 files changed, 9 insertions(+), 7 deletions(-)

Comments

Stanislav Fomichev July 13, 2024, 1:01 a.m. UTC | #1

On 07/12, Andrii Nakryiko wrote:
> For all these years libbpf's BTF dumper has been emitting not strictly
> valid syntax for function prototypes that have no input arguments.
> 
> Instead of `int (*blah)()` we should emit `int (*blah)(void)`.
> 
> This is not normally a problem, but it manifests when we get kfuncs in
> vmlinux.h that have no input arguments. Due to compiler internal
> specifics, we get no BTF information for such kfuncs, if they are not
> declared with proper `(void)`.
> 
> The fix is trivial. We also need to adjust a few ancient tests that
> happily assumed `()` is correct.
> 
> Reported-by: Tejun Heo <[e-mail]>
> Fixes: 351131b51c7a ("libbpf: add btf_dump API for BTF-to-C conversion")
> Signed-off-by: Andrii Nakryiko <[e-mail]>

Acked-by: Stanislav Fomichev <[e-mail]>

[e-mail] July 17, 2024, 8:50 p.m. UTC | #2

Hello:

This patch was applied to bpf/bpf.git (master)
by Daniel Borkmann <[e-mail]>:

On Fri, 12 Jul 2024 15:44:42 -0700 you wrote:
> For all these years libbpf's BTF dumper has been emitting not strictly
> valid syntax for function prototypes that have no input arguments.
> 
> Instead of `int (*blah)()` we should emit `int (*blah)(void)`.
> 
> This is not normally a problem, but it manifests when we get kfuncs in
> vmlinux.h that have no input arguments. Due to compiler internal
> specifics, we get no BTF information for such kfuncs, if they are not
> declared with proper `(void)`.
> 
> [...]

Here is the summary with links:
  - [bpf-next] libbpf: fix no-args func prototype BTF dumping syntax
    https://git.kernel.org/bpf/bpf/c/189f1a976e42

You are awesome, thank you!

Patch

diff --git a/tools/lib/bpf/btf_dump.c b/tools/lib/bpf/btf_dump.c
index 5dbca76b953f..894860111ddb 100644
--- a/tools/lib/bpf/btf_dump.c
+++ b/tools/lib/bpf/btf_dump.c
@@ -1559,10 +1559,12 @@  static void btf_dump_emit_type_chain(struct btf_dump *d,
 			 * Clang for BPF target generates func_proto with no
 			 * args as a func_proto with a single void arg (e.g.,
 			 * `int (*f)(void)` vs just `int (*f)()`). We are
-			 * going to pretend there are no args for such case.
+			 * going to emit valid empty args (void) syntax for
+			 * such case. Similarly and conveniently, valid
+			 * no args case can be special-cased here as well.
 			 */
-			if (vlen == 1 && p->type == 0) {
-				btf_dump_printf(d, ")");
+			if (vlen == 0 || (vlen == 1 && p->type == 0)) {
+				btf_dump_printf(d, "void)");
 				return;
 			}
 
diff --git a/tools/testing/selftests/bpf/progs/btf_dump_test_case_multidim.c b/tools/testing/selftests/bpf/progs/btf_dump_test_case_multidim.c
index ba97165bdb28..a657651eba52 100644
--- a/tools/testing/selftests/bpf/progs/btf_dump_test_case_multidim.c
+++ b/tools/testing/selftests/bpf/progs/btf_dump_test_case_multidim.c
@@ -14,9 +14,9 @@  typedef int *ptr_arr_t[6];
 
 typedef int *ptr_multiarr_t[7][8][9][10];
 
-typedef int * (*fn_ptr_arr_t[11])();
+typedef int * (*fn_ptr_arr_t[11])(void);
 
-typedef int * (*fn_ptr_multiarr_t[12][13])();
+typedef int * (*fn_ptr_multiarr_t[12][13])(void);
 
 struct root_struct {
 	arr_t _1;
diff --git a/tools/testing/selftests/bpf/progs/btf_dump_test_case_syntax.c b/tools/testing/selftests/bpf/progs/btf_dump_test_case_syntax.c
index ad21ee8c7e23..29d01fff32bd 100644
--- a/tools/testing/selftests/bpf/progs/btf_dump_test_case_syntax.c
+++ b/tools/testing/selftests/bpf/progs/btf_dump_test_case_syntax.c
@@ -100,7 +100,7 @@  typedef void (*printf_fn_t)(const char *, ...);
  *   `int -> char *` function and returns pointer to a char. Equivalent:
  *   typedef char * (*fn_input_t)(int);
  *   typedef char * (*fn_output_outer_t)(fn_input_t);
- *   typedef const fn_output_outer_t (* fn_output_inner_t)();
+ *   typedef const fn_output_outer_t (* fn_output_inner_t)(void);
  *   typedef const fn_output_inner_t fn_ptr_arr2_t[5];
  */
 /* ----- START-EXPECTED-OUTPUT ----- */
@@ -127,7 +127,7 @@  typedef void (* (*signal_t)(int, void (*)(int)))(int);
 
 typedef char * (*fn_ptr_arr1_t[10])(int **);
 
-typedef char * (* (* const fn_ptr_arr2_t[5])())(char * (*)(int));
+typedef char * (* (* const fn_ptr_arr2_t[5])(void))(char * (*)(int));
 
 struct struct_w_typedefs {
 	int_t a;