On Thu, Dec 22, 2022 at 12:04 PM Andrii Nakryiko <andrii.nakryiko@xxxxxxxxx> wrote: > > On Tue, Dec 20, 2022 at 9:59 PM James Hilliard > <james.hilliard1@xxxxxxxxx> wrote: > > > > Anonymous structs can't be declared inside function parameter > > definitions in current c standards, however clang doesn't detect this > > condition currently while GCC does. > > > > Details: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108189 > > > > Fixes errors like: > > progs/btf_dump_test_case_bitfields.c:85:7: error: anonymous struct declared inside parameter list will not be visible outside of this definition or declaration [-Werror] > > 85 | int f(struct { > > | ^~~~~~ > > > > Signed-off-by: James Hilliard <james.hilliard1@xxxxxxxxx> > > --- > > .../bpf/progs/btf_dump_test_case_bitfields.c | 9 ++++-- > > .../progs/btf_dump_test_case_namespacing.c | 10 ++++--- > > .../bpf/progs/btf_dump_test_case_packing.c | 10 ++++--- > > .../bpf/progs/btf_dump_test_case_padding.c | 10 ++++--- > > .../bpf/progs/btf_dump_test_case_syntax.c | 30 +++++++++++++------ > > 5 files changed, 46 insertions(+), 23 deletions(-) > > > > 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..c75f6bd06a49 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,16 @@ struct bitfield_flushed { > > long b: 16; > > }; > > > > -int f(struct { > > +/* ----- START-EXPECTED-OUTPUT ----- */ > > +struct root_struct { > > there is no need to make this struct part of expected output, just > keep it next to f? Seems to be required as the diff check fails otherwise. > > > > struct bitfields_only_mixed_types _1; > > struct bitfield_mixed_with_others _2; > > struct bitfield_flushed _3; > > -} *_) > > +}; > > + > > +/* ------ END-EXPECTED-OUTPUT ------ */ > > + > > +int f(struct root_struct *_) > > { > > 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..d7cf2a8487c9 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 > > @@ -49,9 +49,7 @@ typedef int Y; > > > > typedef int Z; > > > > -/*------ END-EXPECTED-OUTPUT ------ */ > > - > > -int f(struct { > > +struct root_struct { > > struct S _1; > > S _2; > > union U _3; > > @@ -67,7 +65,11 @@ int f(struct { > > X xx; > > Y yy; > > Z zz; > > -} *_) > > +}; > > same, that struct is only to preserve all the referenced types, so > keep it hidden from the output I wasn't able to find a way to keep it out of the output. The other tests with a root_struct seem to always have it in the output: https://github.com/torvalds/linux/blob/v6.1/tools/testing/selftests/bpf/progs/btf_dump_test_case_multidim.c#L21-L28 https://github.com/torvalds/linux/blob/v6.1/tools/testing/selftests/bpf/progs/btf_dump_test_case_ordering.c#L50-L56 https://github.com/torvalds/linux/blob/v6.1/tools/testing/selftests/bpf/progs/btf_dump_test_case_syntax.c#L222-L237 > > > + > > +/*------ END-EXPECTED-OUTPUT ------ */ > > + > > +int f(struct root_struct *_) > > { > > 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..e039ceb50c43 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 > > @@ -132,9 +132,7 @@ struct outer_packed_struct { > > struct nested_packed_struct b; > > } __attribute__((packed)); > > > > -/* ------ END-EXPECTED-OUTPUT ------ */ > > - > > -int f(struct { > > +struct root_struct { > > struct packed_trailing_space _1; > > struct non_packed_trailing_space _2; > > struct packed_fields _3; > > @@ -147,7 +145,11 @@ int f(struct { > > struct usb_host_endpoint _10; > > struct outer_nonpacked_struct _11; > > struct outer_packed_struct _12; > > -} *_) > > +}; > > + > > +/* ------ END-EXPECTED-OUTPUT ------ */ > > + > > +int f(struct root_struct *_) > > { > > 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..2ca46ad8d66a 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 > > @@ -220,9 +220,7 @@ struct outer_mixed_but_unpacked { > > struct nested_packed b2; > > }; > > > > -/* ------ END-EXPECTED-OUTPUT ------ */ > > - > > -int f(struct { > > +struct root_struct { > > struct padded_implicitly _1; > > struct padded_explicitly _2; > > struct padded_a_lot _3; > > @@ -243,7 +241,11 @@ int f(struct { > > struct ib_wc _201; > > struct acpi_object_method _202; > > struct outer_mixed_but_unpacked _203; > > -} *_) > > +} __attribute__((packed)); > > + > > +/* ------ END-EXPECTED-OUTPUT ------ */ > > + > > +int f(struct root_struct *_) > > { > > return 0; > > } > > 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 26fffb02ed10..3e31df7cecc6 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 > > @@ -104,24 +104,24 @@ typedef void (*printf_fn_t)(const char *, ...); > > * typedef const fn_output_inner_t fn_ptr_arr2_t[5]; > > */ > > /* ----- START-EXPECTED-OUTPUT ----- */ > > -typedef char * const * (*fn_ptr2_t)(struct { > > - int a; > > -}, int (*)(int)); > > +struct struct_a; > > + > > +typedef char * const * (*fn_ptr2_t)(struct struct_a, int (*)(int)); > > + > > +struct struct_c; > > + > > +struct struct_h; > > > > typedef struct { > > int a; > > - void (*b)(int, struct { > > - int c; > > - }, union { > > + void (*b)(int, struct struct_c, union { > > char d; > > int e[5]; > > }); > > } (*fn_complex_t)(union { > > void *f; > > char g[16]; > > -}, struct { > > - int h; > > -}); > > +}, struct struct_h); > > these do test some pieces of libbpf's btf_dump logic, so I'm way more > reluctant to remove these. If I understand correctly, this syntax will > be eventually supported by GCC, so is there any way to keep these > examples as is by requiring C23 mode or something? Or just skipping > compiling this one if GCC is used? I'm not sure, I'm having trouble finding a description in the C23 specification, I presume if it is in there then GCC will eventually support it. Maybe just keep the root_struct changes for now and hold off on this until it's clarified that this is valid C23 code or not? At the moment it appears there's a clang bug here as it shouldn't be valid C17 code(which AFAIU is clang's default). > > > > > typedef void (* (*signal_t)(int, void (*)(int)))(int); > > > > @@ -272,6 +272,18 @@ struct root_struct { > > struct float_struct _15; > > }; > > > > +struct struct_a { > > + int a; > > +}; > > + > > +struct struct_h { > > + int h; > > +}; > > + > > +struct struct_c { > > + int c; > > +}; > > + > > /* ------ END-EXPECTED-OUTPUT ------ */ > > > > int f(struct root_struct *s) > > -- > > 2.34.1 > >
