Anonymous struct types in parameter lists in BPF selftests

["Jose E. Marchesi" <jose.marchesi@oracle.com>]

Hello.

In C functions whose declarations/definitions use struct types or enum
types (or pointers to them) in the parameter list, the scope of such
defined types is limited to the parameter list, which makes the
functions basically un-callable with type-correct arguments.

Therefore GCC has always emitted a warning when it finds such function
declarations, be them named:

  int f ( struct root { int i; } *arg)
  {
    return arg->i;
  }

  foo.c:1:9: warning: 'struct root' declared inside parameter list
             will not be visible outside of this definition or declaration
    1 |   int f(struct root { int i; } *_)
      |         ^~~~~~~~~~~

or anonymous:

  int f ( struct { int i; } *arg)
  {
    return arg->i;
  }

  foo.c:1:9: warning: anonymous struct declared inside parameter list
             will not be visible outside of this definition or declaration
    1 |   int f ( struct { int i; } *arg)
      |           ^~~~~~

This warning cannot be disabled.

Clang, on the other side, emits the warning by default when the type is
no anonymous (this warning can be disabled with -Wno-visibility):

  int f ( struct root { int i; } *arg)
  {
    return arg->i;
  }

  foo.c:1:18: warning: declaration of 'struct root' will not be visible
              outside of this function [-Wvisibility]
    int f ( struct root { int i; } *arg)

But it doesn't emit any warning if the type is anonymous, which is
puzzling to some (see
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108189).

Now, there are a few BPF selftests that contain function declarations
that get arguments of anonymous struct types defined inline:

  btf_dump_test_case_bitfields.c
  btf_dump_test_case_namespacing.c
  btf_dump_test_case_packing.c
  btf_dump_test_case_padding.c
  btf_dump_test_case_syntax.c

The first four tests can easily be changed to no longer use anonymous
definitions of struct types in the formal arguments, since their purpose
(as far as I can see) is to test quirks related to struct fields and
other unrelated issue.  This makes them buildable with GCC with -Werror.
See diff below.

However, btf_dump_test_case_syntax.c explicitly tests the dumping of a C
function like the above:

 * - `fn_ptr2_t`: function, taking anonymous struct as a first arg and pointer
 *   to a function, that takes int and returns int, as a second arg; returning
 *   a pointer to a const pointer to a char. Equivalent to:
 *	typedef struct { int a; } s_t;
 *	typedef int (*fn_t)(int);
 *	typedef char * const * (*fn_ptr2_t)(s_t, fn_t);

the function being:

  typedef char * const * (*fn_ptr2_t)(struct {
  	int a;
  }, int (*)(int));

which is not really equivalent to the above because one is an anonymous
struct type, the other is named, and also the scope issue described
above.

That makes me wonder, since this is testing the C generation from BTF,
what motivated this particular test?  Is there some particular code in
the kernel (or anywhere else) that uses anonymous struct types defined
in parameter lists?  If so, how are these functions used?

I understand the code above is legal C code, even if questionable in
practice, so perhaps the right thing to do is to build these selftests
with -Wno-error, because the warnings are actually expected?

diff --git a/tools/testing/selftests/bpf/progs/btf_dump_test_case_bitfields.c b/tools/testing/selftests/bpf/progs/btf_dump_test_case_bitfields.c
index e01690618e1e..7ee9f6fcb8d9 100644
--- a/tools/testing/selftests/bpf/progs/btf_dump_test_case_bitfields.c
+++ b/tools/testing/selftests/bpf/progs/btf_dump_test_case_bitfields.c
@@ -82,11 +82,12 @@ struct bitfield_flushed {
 	long b: 16;
 };
 
-int f(struct {
+struct root {
 	struct bitfields_only_mixed_types _1;
 	struct bitfield_mixed_with_others _2;
 	struct bitfield_flushed _3;
-} *_)
+};
+int f(struct root *_)
 {
 	return 0;
 }
diff --git a/tools/testing/selftests/bpf/progs/btf_dump_test_case_namespacing.c b/tools/testing/selftests/bpf/progs/btf_dump_test_case_namespacing.c
index 92a4ad428710..0472183ed56d 100644
--- a/tools/testing/selftests/bpf/progs/btf_dump_test_case_namespacing.c
+++ b/tools/testing/selftests/bpf/progs/btf_dump_test_case_namespacing.c
@@ -51,7 +51,7 @@ typedef int Z;
 
 /*------ END-EXPECTED-OUTPUT ------ */
 
-int f(struct {
+struct root {
 	struct S _1;
 	S _2;
 	union U _3;
@@ -67,7 +67,8 @@ int f(struct {
 	X xx;
 	Y yy;
 	Z zz;
-} *_)
+};
+int f(struct root *_)
 {
 	return 0;
 }
diff --git a/tools/testing/selftests/bpf/progs/btf_dump_test_case_packing.c b/tools/testing/selftests/bpf/progs/btf_dump_test_case_packing.c
index 7998f27df7dd..8a83f049029f 100644
--- a/tools/testing/selftests/bpf/progs/btf_dump_test_case_packing.c
+++ b/tools/testing/selftests/bpf/progs/btf_dump_test_case_packing.c
@@ -134,7 +134,7 @@ struct outer_packed_struct {
 
 /* ------ END-EXPECTED-OUTPUT ------ */
 
-int f(struct {
+struct root {
 	struct packed_trailing_space _1;
 	struct non_packed_trailing_space _2;
 	struct packed_fields _3;
@@ -147,7 +147,8 @@ int f(struct {
 	struct usb_host_endpoint _10;
 	struct outer_nonpacked_struct _11;
 	struct outer_packed_struct _12;
-} *_)
+};
+int f(struct root *_)
 {
 	return 0;
 }
diff --git a/tools/testing/selftests/bpf/progs/btf_dump_test_case_padding.c b/tools/testing/selftests/bpf/progs/btf_dump_test_case_padding.c
index 79276fbe454a..2e03d1455c12 100644
--- a/tools/testing/selftests/bpf/progs/btf_dump_test_case_padding.c
+++ b/tools/testing/selftests/bpf/progs/btf_dump_test_case_padding.c
@@ -222,7 +222,7 @@ struct outer_mixed_but_unpacked {
 
 /* ------ END-EXPECTED-OUTPUT ------ */
 
-int f(struct {
+struct root {
 	struct padded_implicitly _1;
 	struct padded_explicitly _2;
 	struct padded_a_lot _3;
@@ -243,7 +243,8 @@ int f(struct {
 	struct ib_wc _201;
 	struct acpi_object_method _202;
 	struct outer_mixed_but_unpacked _203;
-} *_)
+};
+int f(struct root *_)
 {
 	return 0;
 }