* [PATCH bpf-next 0/2] Handle -fms-extension in kernel structs
@ 2025-12-16 17:18 Alan Maguire
2025-12-16 17:18 ` [PATCH bpf-next 1/2] libbpf: add option to force-anonymize nested structs for BTF dump Alan Maguire
` (2 more replies)
0 siblings, 3 replies; 27+ messages in thread
From: Alan Maguire @ 2025-12-16 17:18 UTC (permalink / raw)
To: qmo
Cc: ast, daniel, andrii, martin.lau, eddyz87, song, yonghong.song,
john.fastabend, kpsingh, sdf, haoluo, jolsa, bpf, Alan Maguire
The kernel is using named structs embedded in other structs
with no member name (similar to anonymous struct usage).
Such an arrangement causes problems for compilers unless -fms-extension
is specified. With vmlinux.h generation we do not want to impose
that on the vmlinux.h consumer, so generate a compatible representation
using anonymous types.
Patch 1 adds libbpf support for this in BTF dump code; patch 2
adds bpftool support to use the new option.
Note that this was essentially
Suggested-by: Song Liu <song@kernel.org>
so feel free to add that to the series.
See [1] for more.
[1] https://lore.kernel.org/bpf/CAADnVQK9ZkPC7+R5VXKHVdtj8tumpMXm7BTp0u9CoiFLz_aPTg@mail.gmail.com/
Alan Maguire (2):
libbpf: add option to force-anonymize nested structs for BTF dump
bpftool: force-anonymize structs to avoid need for -fms-extension
tools/bpf/bpftool/btf.c | 4 +++-
tools/lib/bpf/btf.h | 25 ++++++++++++++++++++++++-
tools/lib/bpf/btf_dump.c | 25 ++++++++++++++++++++++---
3 files changed, 49 insertions(+), 5 deletions(-)
--
2.39.3
^ permalink raw reply [flat|nested] 27+ messages in thread* [PATCH bpf-next 1/2] libbpf: add option to force-anonymize nested structs for BTF dump 2025-12-16 17:18 [PATCH bpf-next 0/2] Handle -fms-extension in kernel structs Alan Maguire @ 2025-12-16 17:18 ` Alan Maguire 2025-12-16 19:00 ` Eduard Zingerman 2025-12-16 17:18 ` [PATCH bpf-next 2/2] bpftool: force-anonymize structs to avoid need for -fms-extension Alan Maguire 2025-12-16 19:20 ` [PATCH bpf-next 0/2] Handle -fms-extension in kernel structs Song Liu 2 siblings, 1 reply; 27+ messages in thread From: Alan Maguire @ 2025-12-16 17:18 UTC (permalink / raw) To: qmo Cc: ast, daniel, andrii, martin.lau, eddyz87, song, yonghong.song, john.fastabend, kpsingh, sdf, haoluo, jolsa, bpf, Alan Maguire When -fms-extension is used, it becomes possible to declare a non-anonymous nested struct without a member name; for example struct bar { int baz; }; struct foo { struct bar; }; These are used in the kernel now, and hence make it into BTF. For example: struct ns_tree { u64 ns_id; atomic_t __ns_ref_active; struct ns_tree_node ns_unified_node; struct ns_tree_node ns_tree_node; struct ns_tree_node ns_owner_node; struct ns_tree_root ns_owner_root; }; struct proc_ns_operations; struct ns_common { u32 ns_type; struct dentry *stashed; const struct proc_ns_operations *ops; unsigned int inum; refcount_t __ns_ref; union { >>>> struct ns_tree; <<<< struct callback_head ns_rcu; }; }; In order to generate a vmlinux.h from such BTF that does not force support of -fms-extension, provide an option to btf_dump__new() to replace structs that have names but no referring member name with an equivalent anonymous struct. bpftool can then make use of this feature. With this approach, the above would become: truct ns_common { u32 ns_type; struct dentry *stashed; const struct proc_ns_operations *ops; unsigned int inum; refcount_t __ns_ref; union { struct { u64 ns_id; atomic_t __ns_ref_active; struct ns_tree_node ns_unified_node; struct ns_tree_node ns_tree_node; struct ns_tree_node ns_owner_node; struct ns_tree_root ns_owner_root; }; struct callback_head ns_rcu; }; }; Fields are still accessible, size is the same but we have included an anonymous equivalent of the struct instead since this is more compatible. Signed-off-by: Alan Maguire <alan.maguire@oracle.com> --- tools/lib/bpf/btf.h | 25 ++++++++++++++++++++++++- tools/lib/bpf/btf_dump.c | 25 ++++++++++++++++++++++--- 2 files changed, 46 insertions(+), 4 deletions(-) diff --git a/tools/lib/bpf/btf.h b/tools/lib/bpf/btf.h index cc01494d6210..fc32e488c05c 100644 --- a/tools/lib/bpf/btf.h +++ b/tools/lib/bpf/btf.h @@ -285,8 +285,31 @@ struct btf_dump; struct btf_dump_opts { size_t sz; + /* when a struct declares another struct within it without a + * member name, force it to be an anonymous nested struct; + * for example instead of + * + * struct bar { + * int baz; + * }; + * struct foo { + * struct bar; + * ... + * }; + * + * use + * + * struct foo { + * struct { + * int baz; + * }; + * }; + * + * This is needed for compilation without -fms-extension . + */ + bool force_anon_struct_members; }; -#define btf_dump_opts__last_field sz +#define btf_dump_opts__last_field force_anon_struct_members typedef void (*btf_dump_printf_fn_t)(void *ctx, const char *fmt, va_list args); diff --git a/tools/lib/bpf/btf_dump.c b/tools/lib/bpf/btf_dump.c index 6388392f49a0..b37062247580 100644 --- a/tools/lib/bpf/btf_dump.c +++ b/tools/lib/bpf/btf_dump.c @@ -54,6 +54,8 @@ struct btf_dump_type_aux_state { __u8 name_resolved: 1; /* whether type is referenced from any other type */ __u8 referenced: 1; + /* whether named type should be anonymized */ + __u8 anonymize: 1; }; /* indent string length; one indent string is added for each indent level */ @@ -84,6 +86,7 @@ struct btf_dump { int ptr_sz; bool strip_mods; bool skip_anon_defs; + bool force_anon_struct_members; int last_id; /* per-type auxiliary state */ @@ -164,6 +167,7 @@ struct btf_dump *btf_dump__new(const struct btf *btf, if (!d) return libbpf_err_ptr(-ENOMEM); + d->force_anon_struct_members = OPTS_GET(opts, force_anon_struct_members, false); d->btf = btf; d->printf_fn = printf_fn; d->cb_ctx = ctx; @@ -1417,6 +1421,8 @@ static void btf_dump_emit_name(const struct btf_dump *d, btf_dump_printf(d, "%s%s", separate ? " " : "", name); } +static void btf_dump_set_anon_type(struct btf_dump *d, __u32 id, bool anon); + static void btf_dump_emit_type_chain(struct btf_dump *d, struct id_stack *decls, const char *fname, int lvl) @@ -1460,10 +1466,16 @@ static void btf_dump_emit_type_chain(struct btf_dump *d, case BTF_KIND_UNION: btf_dump_emit_mods(d, decls); /* inline anonymous struct/union */ - if (t->name_off == 0 && !d->skip_anon_defs) + if (t->name_off == 0 && !d->skip_anon_defs) { btf_dump_emit_struct_def(d, id, t, lvl); - else + } else if (decls->cnt == 0 && !fname[0] && d->force_anon_struct_members) { + /* anonymize nested struct and emit it */ + btf_dump_set_anon_type(d, id, true); + btf_dump_emit_struct_def(d, id, t, lvl); + btf_dump_set_anon_type(d, id, false); + } else { btf_dump_emit_struct_fwd(d, id, t); + } break; case BTF_KIND_ENUM: case BTF_KIND_ENUM64: @@ -1661,6 +1673,13 @@ static size_t btf_dump_name_dups(struct btf_dump *d, struct hashmap *name_map, return dup_cnt; } +static void btf_dump_set_anon_type(struct btf_dump *d, __u32 id, bool anon) +{ + struct btf_dump_type_aux_state *s = &d->type_states[id]; + + s->anonymize = anon; +} + static const char *btf_dump_resolve_name(struct btf_dump *d, __u32 id, struct hashmap *name_map) { @@ -1670,7 +1689,7 @@ static const char *btf_dump_resolve_name(struct btf_dump *d, __u32 id, const char **cached_name = &d->cached_names[id]; size_t dup_cnt; - if (t->name_off == 0) + if (t->name_off == 0 || s->anonymize) return ""; if (s->name_resolved) -- 2.39.3 ^ permalink raw reply related [flat|nested] 27+ messages in thread
* Re: [PATCH bpf-next 1/2] libbpf: add option to force-anonymize nested structs for BTF dump 2025-12-16 17:18 ` [PATCH bpf-next 1/2] libbpf: add option to force-anonymize nested structs for BTF dump Alan Maguire @ 2025-12-16 19:00 ` Eduard Zingerman 2025-12-16 19:08 ` Andrii Nakryiko 2025-12-16 19:46 ` Eduard Zingerman 0 siblings, 2 replies; 27+ messages in thread From: Eduard Zingerman @ 2025-12-16 19:00 UTC (permalink / raw) To: Alan Maguire, qmo Cc: ast, daniel, andrii, martin.lau, song, yonghong.song, john.fastabend, kpsingh, sdf, haoluo, jolsa, bpf On Tue, 2025-12-16 at 17:18 +0000, Alan Maguire wrote: [...] > @@ -1460,10 +1466,16 @@ static void btf_dump_emit_type_chain(struct btf_dump *d, > case BTF_KIND_UNION: > btf_dump_emit_mods(d, decls); > /* inline anonymous struct/union */ > - if (t->name_off == 0 && !d->skip_anon_defs) > + if (t->name_off == 0 && !d->skip_anon_defs) { > btf_dump_emit_struct_def(d, id, t, lvl); > - else > + } else if (decls->cnt == 0 && !fname[0] && d->force_anon_struct_members) { > + /* anonymize nested struct and emit it */ > + btf_dump_set_anon_type(d, id, true); > + btf_dump_emit_struct_def(d, id, t, lvl); > + btf_dump_set_anon_type(d, id, false); Hi Alan, I think this is a solid idea. It seems to me that with current implementation there would be a trouble in the following scenario: struct foo { struct foo *ptr; }; struct bar { struct foo; } Because state for 'foo' will be anonymize == true at the moment when 'ptr' field is printed. Maybe pass a direct parameter to btf_dump_emit_struct_def()? > + } else { > btf_dump_emit_struct_fwd(d, id, t); > + } > break; > case BTF_KIND_ENUM: > case BTF_KIND_ENUM64: [...] ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH bpf-next 1/2] libbpf: add option to force-anonymize nested structs for BTF dump 2025-12-16 19:00 ` Eduard Zingerman @ 2025-12-16 19:08 ` Andrii Nakryiko 2025-12-16 19:46 ` Eduard Zingerman 1 sibling, 0 replies; 27+ messages in thread From: Andrii Nakryiko @ 2025-12-16 19:08 UTC (permalink / raw) To: Eduard Zingerman Cc: Alan Maguire, qmo, ast, daniel, andrii, martin.lau, song, yonghong.song, john.fastabend, kpsingh, sdf, haoluo, jolsa, bpf On Tue, Dec 16, 2025 at 11:00 AM Eduard Zingerman <eddyz87@gmail.com> wrote: > > On Tue, 2025-12-16 at 17:18 +0000, Alan Maguire wrote: > > [...] > > > @@ -1460,10 +1466,16 @@ static void btf_dump_emit_type_chain(struct btf_dump *d, > > case BTF_KIND_UNION: > > btf_dump_emit_mods(d, decls); > > /* inline anonymous struct/union */ > > - if (t->name_off == 0 && !d->skip_anon_defs) > > + if (t->name_off == 0 && !d->skip_anon_defs) { > > btf_dump_emit_struct_def(d, id, t, lvl); > > - else > > + } else if (decls->cnt == 0 && !fname[0] && d->force_anon_struct_members) { > > + /* anonymize nested struct and emit it */ > > + btf_dump_set_anon_type(d, id, true); > > + btf_dump_emit_struct_def(d, id, t, lvl); Alan, please check that you are handling padding at the end of struct that you are inlining artificially. I suspect you'll need some special handling, because btf_dump_emit_bit_padding() (as far as I remember) is trying to minimize unnecessary explicit padding. But it does so under the assumption that we have a self-contained struct definition, so there will be no more fields at the end of struct. That's not the case anymore here, so I suspect we'll have improper field offsets in some corner cases. Let's add appropriate tests and make sure all this works. > > + btf_dump_set_anon_type(d, id, false); > > > Hi Alan, > > I think this is a solid idea. > > It seems to me that with current implementation there would be a > trouble in the following scenario: > > struct foo { struct foo *ptr; }; > struct bar { > struct foo; > } > > Because state for 'foo' will be anonymize == true at the moment when > 'ptr' field is printed. > > Maybe pass a direct parameter to btf_dump_emit_struct_def()? +1, that seems like a bit more robust approach. > > > > + } else { > > btf_dump_emit_struct_fwd(d, id, t); > > + } > > break; > > case BTF_KIND_ENUM: > > case BTF_KIND_ENUM64: > > [...] > I'll reply here just to not split discussion. I have two things I want to discuss. 1. I don't like unnecessary options. I don't think this anon struct field embedding should be an option, it should just happen. 2. The above would work even better if we can actually support both -fms-extensions and -fno-ms-extensions mode transparently. AI tells me that -fms-extensions should be detectable at compilation time with #ifdef __MS_EXTENSIONS__. I haven't checked, but please do. Assuming that works, we can (and probably should?) emit #ifdef __MS_EXTENSIONS__ struct inner; #else <inlined struct inner fields here> <whatever explicit padding we need to preserve layout> #endif This way #1 is a no-brainer, emitted definition will work correctly everywhere. Please check if that's possible. pw-bot: cr ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH bpf-next 1/2] libbpf: add option to force-anonymize nested structs for BTF dump 2025-12-16 19:00 ` Eduard Zingerman 2025-12-16 19:08 ` Andrii Nakryiko @ 2025-12-16 19:46 ` Eduard Zingerman 2025-12-17 16:06 ` Alan Maguire 1 sibling, 1 reply; 27+ messages in thread From: Eduard Zingerman @ 2025-12-16 19:46 UTC (permalink / raw) To: Alan Maguire, qmo Cc: ast, daniel, andrii, martin.lau, song, yonghong.song, john.fastabend, kpsingh, sdf, haoluo, jolsa, bpf On Tue, 2025-12-16 at 11:00 -0800, Eduard Zingerman wrote: > On Tue, 2025-12-16 at 17:18 +0000, Alan Maguire wrote: > > [...] > > > @@ -1460,10 +1466,16 @@ static void btf_dump_emit_type_chain(struct btf_dump *d, > > case BTF_KIND_UNION: > > btf_dump_emit_mods(d, decls); > > /* inline anonymous struct/union */ > > - if (t->name_off == 0 && !d->skip_anon_defs) > > + if (t->name_off == 0 && !d->skip_anon_defs) { > > btf_dump_emit_struct_def(d, id, t, lvl); > > - else > > + } else if (decls->cnt == 0 && !fname[0] && d->force_anon_struct_members) { > > + /* anonymize nested struct and emit it */ > > + btf_dump_set_anon_type(d, id, true); > > + btf_dump_emit_struct_def(d, id, t, lvl); > > + btf_dump_set_anon_type(d, id, false); > > > Hi Alan, > > I think this is a solid idea. > > It seems to me that with current implementation there would be a > trouble in the following scenario: > > struct foo { struct foo *ptr; }; > struct bar { > struct foo; > } > > Because state for 'foo' will be anonymize == true at the moment when > 'ptr' field is printed. > > Maybe pass a direct parameter to btf_dump_emit_struct_def()? Digging a bit more into this, here are a couple of weird examples: $ cat ~/tmp/ms-ext-test.c struct foo { struct foo *ptr; }; struct bar { struct foo; }; struct bar root; $ gcc -g -c -o ~/tmp/ms-ext-test.o ~/tmp/ms-ext-test.c $ pahole -J ~/tmp/ms-ext-test.o $ bpftool btf dump file ~/tmp/ms-ext-test.o format c #ifndef __VMLINUX_H__ #define __VMLINUX_H__ #ifndef BPF_NO_PRESERVE_ACCESS_INDEX #pragma clang attribute push (__attribute__((preserve_access_index)), apply_to = record) #endif #ifndef __ksym #define __ksym __attribute__((section(".ksyms"))) #endif #ifndef __weak #define __weak __attribute__((weak)) #endif #ifndef __bpf_fastcall #if __has_attribute(bpf_fastcall) #define __bpf_fastcall __attribute__((bpf_fastcall)) #else #define __bpf_fastcall #endif #endif struct foo { struct foo *ptr; }; /* BPF kfuncs */ #ifndef BPF_NO_KFUNC_PROTOTYPES #endif #ifndef BPF_NO_PRESERVE_ACCESS_INDEX #pragma clang attribute pop #endif #endif /* __VMLINUX_H__ */ $ cat ~/tmp/ms-ext-test.c struct foo { struct foo *ptr; }; struct bar { struct foo; int a; }; struct bar root; $ cgcc -fms-extensions -g -c -o ~/tmp/ms-ext-test.o ~/tmp/ms-ext-test.c $ pahole -J ~/tmp/ms-ext-test.o $ tools/bpf/bpftool/bpftool btf dump file ~/tmp/ms-ext-test.o format c #ifndef __VMLINUX_H__ #define __VMLINUX_H__ #ifndef BPF_NO_PRESERVE_ACCESS_INDEX #pragma clang attribute push (__attribute__((preserve_access_index)), apply_to = record) #endif #ifndef __ksym #define __ksym __attribute__((section(".ksyms"))) #endif #ifndef __weak #define __weak __attribute__((weak)) #endif #ifndef __bpf_fastcall #if __has_attribute(bpf_fastcall) #define __bpf_fastcall __attribute__((bpf_fastcall)) #else #define __bpf_fastcall #endif #endif struct foo { struct foo *ptr; }; struct bar { struct { struct *ptr; }; int a; }; /* BPF kfuncs */ #ifndef BPF_NO_KFUNC_PROTOTYPES #endif #ifndef BPF_NO_PRESERVE_ACCESS_INDEX #pragma clang attribute pop #endif #endif /* __VMLINUX_H__ */ ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH bpf-next 1/2] libbpf: add option to force-anonymize nested structs for BTF dump 2025-12-16 19:46 ` Eduard Zingerman @ 2025-12-17 16:06 ` Alan Maguire 2025-12-17 16:12 ` Alexei Starovoitov 2025-12-17 17:10 ` Andrii Nakryiko 0 siblings, 2 replies; 27+ messages in thread From: Alan Maguire @ 2025-12-17 16:06 UTC (permalink / raw) To: Eduard Zingerman, qmo, Andrii Nakryiko Cc: ast, daniel, andrii, martin.lau, song, yonghong.song, john.fastabend, kpsingh, sdf, haoluo, jolsa, bpf On 16/12/2025 19:46, Eduard Zingerman wrote: > On Tue, 2025-12-16 at 11:00 -0800, Eduard Zingerman wrote: >> On Tue, 2025-12-16 at 17:18 +0000, Alan Maguire wrote: >> >> [...] >> >>> @@ -1460,10 +1466,16 @@ static void btf_dump_emit_type_chain(struct btf_dump *d, >>> case BTF_KIND_UNION: >>> btf_dump_emit_mods(d, decls); >>> /* inline anonymous struct/union */ >>> - if (t->name_off == 0 && !d->skip_anon_defs) >>> + if (t->name_off == 0 && !d->skip_anon_defs) { >>> btf_dump_emit_struct_def(d, id, t, lvl); >>> - else >>> + } else if (decls->cnt == 0 && !fname[0] && d->force_anon_struct_members) { >>> + /* anonymize nested struct and emit it */ >>> + btf_dump_set_anon_type(d, id, true); >>> + btf_dump_emit_struct_def(d, id, t, lvl); >>> + btf_dump_set_anon_type(d, id, false); >> >> >> Hi Alan, >> >> I think this is a solid idea. >> >> It seems to me that with current implementation there would be a >> trouble in the following scenario: >> >> struct foo { struct foo *ptr; }; >> struct bar { >> struct foo; >> } >> >> Because state for 'foo' will be anonymize == true at the moment when >> 'ptr' field is printed. >> >> Maybe pass a direct parameter to btf_dump_emit_struct_def()? > > Digging a bit more into this, here are a couple of weird examples: > > $ cat ~/tmp/ms-ext-test.c > struct foo { > struct foo *ptr; > }; > > struct bar { > struct foo; > }; > > struct bar root; > $ gcc -g -c -o ~/tmp/ms-ext-test.o ~/tmp/ms-ext-test.c > $ pahole -J ~/tmp/ms-ext-test.o > $ bpftool btf dump file ~/tmp/ms-ext-test.o format c > #ifndef __VMLINUX_H__ > #define __VMLINUX_H__ > > #ifndef BPF_NO_PRESERVE_ACCESS_INDEX > #pragma clang attribute push (__attribute__((preserve_access_index)), apply_to = record) > #endif > > #ifndef __ksym > #define __ksym __attribute__((section(".ksyms"))) > #endif > > #ifndef __weak > #define __weak __attribute__((weak)) > #endif > > #ifndef __bpf_fastcall > #if __has_attribute(bpf_fastcall) > #define __bpf_fastcall __attribute__((bpf_fastcall)) > #else > #define __bpf_fastcall > #endif > #endif > > struct foo { > struct foo *ptr; > }; > > > /* BPF kfuncs */ > #ifndef BPF_NO_KFUNC_PROTOTYPES > #endif > > #ifndef BPF_NO_PRESERVE_ACCESS_INDEX > #pragma clang attribute pop > #endif > > #endif /* __VMLINUX_H__ */ > > > $ cat ~/tmp/ms-ext-test.c > struct foo { > struct foo *ptr; > }; > > struct bar { > struct foo; > int a; > }; > > struct bar root; > $ cgcc -fms-extensions -g -c -o ~/tmp/ms-ext-test.o ~/tmp/ms-ext-test.c > $ pahole -J ~/tmp/ms-ext-test.o > $ tools/bpf/bpftool/bpftool btf dump file ~/tmp/ms-ext-test.o format c > #ifndef __VMLINUX_H__ > #define __VMLINUX_H__ > > #ifndef BPF_NO_PRESERVE_ACCESS_INDEX > #pragma clang attribute push (__attribute__((preserve_access_index)), apply_to = record) > #endif > > #ifndef __ksym > #define __ksym __attribute__((section(".ksyms"))) > #endif > > #ifndef __weak > #define __weak __attribute__((weak)) > #endif > > #ifndef __bpf_fastcall > #if __has_attribute(bpf_fastcall) > #define __bpf_fastcall __attribute__((bpf_fastcall)) > #else > #define __bpf_fastcall > #endif > #endif > > struct foo { > struct foo *ptr; > }; > > struct bar { > struct { > struct *ptr; > }; > int a; > }; > > > /* BPF kfuncs */ > #ifndef BPF_NO_KFUNC_PROTOTYPES > #endif > > #ifndef BPF_NO_PRESERVE_ACCESS_INDEX > #pragma clang attribute pop > #endif > > #endif /* __VMLINUX_H__ */ > Ack to all suggestions; in particular handling both cases with #ifdef is really nice since it does away with the need for libbpf/bpftool options. With that in place - along with passing parameters rather than setting field values, and being more selective to ensure we only emit the #ifdef/#else/#endif for a composite type nested in a composite type - the above gives us the following: struct foo { struct foo *ptr; }; struct bar { #ifdef __MS_EXTENSIONS__ struct foo; #else struct { struct foo *ptr; }; #endif int a; }; I think that's the format we want. With respect to the padding behaviour, I may be missing something but I'm not sure these changes will impact that. We pad in two cases: 1. between struct fields, where the current offset is less than the member offset or we have alignment requirements to be met. 2. at the end of the struct, to pad it out the the size/alignment requirements. I don't see anything different here for the case where we force-anonymize-inline the struct; it still does 1 and 2 identical with the out-of-line declaration. To test this I augmented Eduard's example to add internal and whole struct alignment requirements: struct foo { struct foo *ptr; char __attribute__((__aligned__(16))) s; int t; } __attribute__((__aligned__(64))); struct bar { struct foo; int a; }; struct bar root; int main(int argc, char *argv[]) { struct bar *r = (struct bar *)argv[0]; } $ gcc -fms-extensions -g -c -o /tmp/ms-ext-test.o /tmp/ms-ext-test.c $ pahole -J /tmp/ms-ext-test.o $ bpftool btf dump file /tmp/ms-ext-test.o format c struct foo { struct foo *ptr; long: 64; char s; int t; long: 64; long: 64; long: 64; long: 64; long: 64; }; struct bar { #ifdef __MS_EXTENSIONS__ struct foo; #else struct { struct foo *ptr; long: 64; char s; int t; long: 64; long: 64; long: 64; long: 64; long: 64; }; #endif int a; long: 64; long: 64; long: 64; long: 64; long: 64; long: 64; long: 64; }; This looks equivalent to me, but there may some other condition you're thinking of here? Thanks! Alan ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH bpf-next 1/2] libbpf: add option to force-anonymize nested structs for BTF dump 2025-12-17 16:06 ` Alan Maguire @ 2025-12-17 16:12 ` Alexei Starovoitov 2025-12-17 17:06 ` Andrii Nakryiko 2025-12-17 17:10 ` Andrii Nakryiko 1 sibling, 1 reply; 27+ messages in thread From: Alexei Starovoitov @ 2025-12-17 16:12 UTC (permalink / raw) To: Alan Maguire Cc: Eduard Zingerman, Quentin Monnet, Andrii Nakryiko, Alexei Starovoitov, Daniel Borkmann, Martin KaFai Lau, Song Liu, Yonghong Song, John Fastabend, KP Singh, Stanislav Fomichev, Hao Luo, Jiri Olsa, bpf On Wed, Dec 17, 2025 at 8:06 AM Alan Maguire <alan.maguire@oracle.com> wrote: > > struct foo { > struct foo *ptr; > }; > > struct bar { > > #ifdef __MS_EXTENSIONS__ > struct foo; > #else > struct { > struct foo *ptr; > }; > #endif Did you test it ? I suspect AI invented it. I see nothing like this in gcc or llvm sources. ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH bpf-next 1/2] libbpf: add option to force-anonymize nested structs for BTF dump 2025-12-17 16:12 ` Alexei Starovoitov @ 2025-12-17 17:06 ` Andrii Nakryiko 2025-12-17 17:33 ` Alan Maguire 0 siblings, 1 reply; 27+ messages in thread From: Andrii Nakryiko @ 2025-12-17 17:06 UTC (permalink / raw) To: Alexei Starovoitov Cc: Alan Maguire, Eduard Zingerman, Quentin Monnet, Andrii Nakryiko, Alexei Starovoitov, Daniel Borkmann, Martin KaFai Lau, Song Liu, Yonghong Song, John Fastabend, KP Singh, Stanislav Fomichev, Hao Luo, Jiri Olsa, bpf On Wed, Dec 17, 2025 at 8:13 AM Alexei Starovoitov <alexei.starovoitov@gmail.com> wrote: > > On Wed, Dec 17, 2025 at 8:06 AM Alan Maguire <alan.maguire@oracle.com> wrote: > > > > struct foo { > > struct foo *ptr; > > }; > > > > struct bar { > > > > #ifdef __MS_EXTENSIONS__ > > struct foo; > > #else > > struct { > > struct foo *ptr; > > }; > > #endif > > Did you test it ? I suspect AI invented it. > I see nothing like this in gcc or llvm sources. Grepping a bit I suspect we need to check for _MSC_EXTENSIONS, worst case - _MSC_VER. But Alan, please double check in practice. ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH bpf-next 1/2] libbpf: add option to force-anonymize nested structs for BTF dump 2025-12-17 17:06 ` Andrii Nakryiko @ 2025-12-17 17:33 ` Alan Maguire 2025-12-17 17:52 ` Alexei Starovoitov 0 siblings, 1 reply; 27+ messages in thread From: Alan Maguire @ 2025-12-17 17:33 UTC (permalink / raw) To: Andrii Nakryiko, Alexei Starovoitov Cc: Eduard Zingerman, Quentin Monnet, Andrii Nakryiko, Alexei Starovoitov, Daniel Borkmann, Martin KaFai Lau, Song Liu, Yonghong Song, John Fastabend, KP Singh, Stanislav Fomichev, Hao Luo, Jiri Olsa, bpf On 17/12/2025 17:06, Andrii Nakryiko wrote: > On Wed, Dec 17, 2025 at 8:13 AM Alexei Starovoitov > <alexei.starovoitov@gmail.com> wrote: >> >> On Wed, Dec 17, 2025 at 8:06 AM Alan Maguire <alan.maguire@oracle.com> wrote: >>> >>> struct foo { >>> struct foo *ptr; >>> }; >>> >>> struct bar { >>> >>> #ifdef __MS_EXTENSIONS__ >>> struct foo; >>> #else >>> struct { >>> struct foo *ptr; >>> }; >>> #endif >> >> Did you test it ? I suspect AI invented it. >> I see nothing like this in gcc or llvm sources. > > Grepping a bit I suspect we need to check for _MSC_EXTENSIONS, worst > case - _MSC_VER. But Alan, please double check in practice. Thanks; I tried these too, no luck with either gcc or clang. Looks like the requests to merge them haven't landed yet, latest I could find on this was [1]/[2]. I guess the other approach would be to have the user explicitly #define a macro prior to vmlinux.h inclusion if they want to use the extensions, similar to how we handle BPF_NO_PRESERVE_ACCESS_INDEX; maybe call it BPF_USE_MS_EXTENSIONS or similar? It could default to true if _MSC_EXTENSIONS is set to future-proof it. [1] https://gcc.gnu.org/bugzilla/show_bug.cgi?id=110977 [2] https://reviews.llvm.org/D157334 ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH bpf-next 1/2] libbpf: add option to force-anonymize nested structs for BTF dump 2025-12-17 17:33 ` Alan Maguire @ 2025-12-17 17:52 ` Alexei Starovoitov 2025-12-17 18:41 ` Alan Maguire 0 siblings, 1 reply; 27+ messages in thread From: Alexei Starovoitov @ 2025-12-17 17:52 UTC (permalink / raw) To: Alan Maguire Cc: Andrii Nakryiko, Eduard Zingerman, Quentin Monnet, Andrii Nakryiko, Alexei Starovoitov, Daniel Borkmann, Martin KaFai Lau, Song Liu, Yonghong Song, John Fastabend, KP Singh, Stanislav Fomichev, Hao Luo, Jiri Olsa, bpf On Wed, Dec 17, 2025 at 9:33 AM Alan Maguire <alan.maguire@oracle.com> wrote: > > On 17/12/2025 17:06, Andrii Nakryiko wrote: > > On Wed, Dec 17, 2025 at 8:13 AM Alexei Starovoitov > > <alexei.starovoitov@gmail.com> wrote: > >> > >> On Wed, Dec 17, 2025 at 8:06 AM Alan Maguire <alan.maguire@oracle.com> wrote: > >>> > >>> struct foo { > >>> struct foo *ptr; > >>> }; > >>> > >>> struct bar { > >>> > >>> #ifdef __MS_EXTENSIONS__ > >>> struct foo; > >>> #else > >>> struct { > >>> struct foo *ptr; > >>> }; > >>> #endif > >> > >> Did you test it ? I suspect AI invented it. > >> I see nothing like this in gcc or llvm sources. > > > > Grepping a bit I suspect we need to check for _MSC_EXTENSIONS, worst > > case - _MSC_VER. But Alan, please double check in practice. > > Thanks; I tried these too, no luck with either gcc or clang. Looks like the > requests to merge them haven't landed yet, latest I could find on this was [1]/[2]. clang diff landed, but these defines are there only when clang is built for windows. After studying the code a bit the following works with clang on linux: #if __has_builtin(__builtin_FUNCSIG) but not with gcc. ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH bpf-next 1/2] libbpf: add option to force-anonymize nested structs for BTF dump 2025-12-17 17:52 ` Alexei Starovoitov @ 2025-12-17 18:41 ` Alan Maguire 2025-12-17 19:34 ` Eduard Zingerman 0 siblings, 1 reply; 27+ messages in thread From: Alan Maguire @ 2025-12-17 18:41 UTC (permalink / raw) To: Alexei Starovoitov Cc: Andrii Nakryiko, Eduard Zingerman, Quentin Monnet, Andrii Nakryiko, Alexei Starovoitov, Daniel Borkmann, Martin KaFai Lau, Song Liu, Yonghong Song, John Fastabend, KP Singh, Stanislav Fomichev, Hao Luo, Jiri Olsa, bpf On 17/12/2025 17:52, Alexei Starovoitov wrote: > On Wed, Dec 17, 2025 at 9:33 AM Alan Maguire <alan.maguire@oracle.com> wrote: >> >> On 17/12/2025 17:06, Andrii Nakryiko wrote: >>> On Wed, Dec 17, 2025 at 8:13 AM Alexei Starovoitov >>> <alexei.starovoitov@gmail.com> wrote: >>>> >>>> On Wed, Dec 17, 2025 at 8:06 AM Alan Maguire <alan.maguire@oracle.com> wrote: >>>>> >>>>> struct foo { >>>>> struct foo *ptr; >>>>> }; >>>>> >>>>> struct bar { >>>>> >>>>> #ifdef __MS_EXTENSIONS__ >>>>> struct foo; >>>>> #else >>>>> struct { >>>>> struct foo *ptr; >>>>> }; >>>>> #endif >>>> >>>> Did you test it ? I suspect AI invented it. >>>> I see nothing like this in gcc or llvm sources. >>> >>> Grepping a bit I suspect we need to check for _MSC_EXTENSIONS, worst >>> case - _MSC_VER. But Alan, please double check in practice. >> >> Thanks; I tried these too, no luck with either gcc or clang. Looks like the >> requests to merge them haven't landed yet, latest I could find on this was [1]/[2]. > > clang diff landed, but these defines are there only when > clang is built for windows. > > After studying the code a bit the following works with clang on linux: > > #if __has_builtin(__builtin_FUNCSIG) > > but not with gcc. So maybe the best we can do here is something like the following at the top of vmlinux.h: #ifndef BPF_USE_MS_EXTENSIONS #if __has_builtin(__builtin_FUNCSIG) || defined(_MSC_EXTENSIONS) #define BPF_USE_MS_EXTENSIONS #endif #endif ...and then guard using #ifdef BPF_USE_MS_EXTENSIONS That will work on clang and perhaps at some point work on gcc, but also gives the user the option to supply a macro to force use in cases where there is no detection available. ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH bpf-next 1/2] libbpf: add option to force-anonymize nested structs for BTF dump 2025-12-17 18:41 ` Alan Maguire @ 2025-12-17 19:34 ` Eduard Zingerman 2025-12-17 19:35 ` Eduard Zingerman 0 siblings, 1 reply; 27+ messages in thread From: Eduard Zingerman @ 2025-12-17 19:34 UTC (permalink / raw) To: Alan Maguire, Alexei Starovoitov Cc: Andrii Nakryiko, Quentin Monnet, Andrii Nakryiko, Alexei Starovoitov, Daniel Borkmann, Martin KaFai Lau, Song Liu, Yonghong Song, John Fastabend, KP Singh, Stanislav Fomichev, Hao Luo, Jiri Olsa, bpf On Wed, 2025-12-17 at 18:41 +0000, Alan Maguire wrote: [...] > So maybe the best we can do here is something like the following at the top > of vmlinux.h: > > #ifndef BPF_USE_MS_EXTENSIONS > #if __has_builtin(__builtin_FUNCSIG) || defined(_MSC_EXTENSIONS) > #define BPF_USE_MS_EXTENSIONS > #endif > #endif > > ...and then guard using #ifdef BPF_USE_MS_EXTENSIONS > > That will work on clang and perhaps at some point work on gcc, but also > gives the user the option to supply a macro to force use in cases where > there is no detection available. Are we sure we need such flexibility? Maybe just stick with current implementation and unroll the structures unconditionally? ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH bpf-next 1/2] libbpf: add option to force-anonymize nested structs for BTF dump 2025-12-17 19:34 ` Eduard Zingerman @ 2025-12-17 19:35 ` Eduard Zingerman 2025-12-17 20:50 ` Alan Maguire 0 siblings, 1 reply; 27+ messages in thread From: Eduard Zingerman @ 2025-12-17 19:35 UTC (permalink / raw) To: Alan Maguire, Alexei Starovoitov Cc: Andrii Nakryiko, Quentin Monnet, Andrii Nakryiko, Alexei Starovoitov, Daniel Borkmann, Martin KaFai Lau, Song Liu, Yonghong Song, John Fastabend, KP Singh, Stanislav Fomichev, Hao Luo, Jiri Olsa, bpf On Wed, 2025-12-17 at 11:34 -0800, Eduard Zingerman wrote: > On Wed, 2025-12-17 at 18:41 +0000, Alan Maguire wrote: > > [...] > > > So maybe the best we can do here is something like the following at the top > > of vmlinux.h: > > > > #ifndef BPF_USE_MS_EXTENSIONS > > #if __has_builtin(__builtin_FUNCSIG) || defined(_MSC_EXTENSIONS) > > #define BPF_USE_MS_EXTENSIONS > > #endif > > #endif > > > > ...and then guard using #ifdef BPF_USE_MS_EXTENSIONS > > > > That will work on clang and perhaps at some point work on gcc, but also > > gives the user the option to supply a macro to force use in cases where > > there is no detection available. > > Are we sure we need such flexibility? > Maybe just stick with current implementation and unroll the structures > unconditionally? I mean, the point of the extension is to make the code smaller. But here we are expanding it instead, so why bother? ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH bpf-next 1/2] libbpf: add option to force-anonymize nested structs for BTF dump 2025-12-17 19:35 ` Eduard Zingerman @ 2025-12-17 20:50 ` Alan Maguire 2025-12-17 21:02 ` Andrii Nakryiko 0 siblings, 1 reply; 27+ messages in thread From: Alan Maguire @ 2025-12-17 20:50 UTC (permalink / raw) To: Eduard Zingerman, Alexei Starovoitov Cc: Andrii Nakryiko, Quentin Monnet, Andrii Nakryiko, Alexei Starovoitov, Daniel Borkmann, Martin KaFai Lau, Song Liu, Yonghong Song, John Fastabend, KP Singh, Stanislav Fomichev, Hao Luo, Jiri Olsa, bpf On 17/12/2025 19:35, Eduard Zingerman wrote: > On Wed, 2025-12-17 at 11:34 -0800, Eduard Zingerman wrote: >> On Wed, 2025-12-17 at 18:41 +0000, Alan Maguire wrote: >> >> [...] >> >>> So maybe the best we can do here is something like the following at the top >>> of vmlinux.h: >>> >>> #ifndef BPF_USE_MS_EXTENSIONS >>> #if __has_builtin(__builtin_FUNCSIG) || defined(_MSC_EXTENSIONS) >>> #define BPF_USE_MS_EXTENSIONS >>> #endif >>> #endif >>> >>> ...and then guard using #ifdef BPF_USE_MS_EXTENSIONS >>> >>> That will work on clang and perhaps at some point work on gcc, but also >>> gives the user the option to supply a macro to force use in cases where >>> there is no detection available. >> >> Are we sure we need such flexibility? >> Maybe just stick with current implementation and unroll the structures >> unconditionally? > > I mean, the point of the extension is to make the code smaller. > But here we are expanding it instead, so why bother? Yeah, I'm happy either way; if we have agreement that we just use the nested anon struct without macro complications I'll send an updated patch. Alan ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH bpf-next 1/2] libbpf: add option to force-anonymize nested structs for BTF dump 2025-12-17 20:50 ` Alan Maguire @ 2025-12-17 21:02 ` Andrii Nakryiko 2025-12-17 21:27 ` Alexei Starovoitov 0 siblings, 1 reply; 27+ messages in thread From: Andrii Nakryiko @ 2025-12-17 21:02 UTC (permalink / raw) To: Alan Maguire Cc: Eduard Zingerman, Alexei Starovoitov, Quentin Monnet, Andrii Nakryiko, Alexei Starovoitov, Daniel Borkmann, Martin KaFai Lau, Song Liu, Yonghong Song, John Fastabend, KP Singh, Stanislav Fomichev, Hao Luo, Jiri Olsa, bpf On Wed, Dec 17, 2025 at 12:50 PM Alan Maguire <alan.maguire@oracle.com> wrote: > > On 17/12/2025 19:35, Eduard Zingerman wrote: > > On Wed, 2025-12-17 at 11:34 -0800, Eduard Zingerman wrote: > >> On Wed, 2025-12-17 at 18:41 +0000, Alan Maguire wrote: > >> > >> [...] > >> > >>> So maybe the best we can do here is something like the following at the top > >>> of vmlinux.h: > >>> > >>> #ifndef BPF_USE_MS_EXTENSIONS > >>> #if __has_builtin(__builtin_FUNCSIG) || defined(_MSC_EXTENSIONS) > >>> #define BPF_USE_MS_EXTENSIONS > >>> #endif > >>> #endif > >>> > >>> ...and then guard using #ifdef BPF_USE_MS_EXTENSIONS > >>> > >>> That will work on clang and perhaps at some point work on gcc, but also > >>> gives the user the option to supply a macro to force use in cases where > >>> there is no detection available. > >> > >> Are we sure we need such flexibility? > >> Maybe just stick with current implementation and unroll the structures > >> unconditionally? > > > > I mean, the point of the extension is to make the code smaller. > > But here we are expanding it instead, so why bother? > > Yeah, I'm happy either way; if we have agreement that we just use the nested anon > struct without macro complications I'll send an updated patch. There is a little bit of semantic meaning being lost when we inline the struct, but I guess that can't be helped. Let's just unconditionally inline then. Still better than having extra emit option, IMO. > > Alan ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH bpf-next 1/2] libbpf: add option to force-anonymize nested structs for BTF dump 2025-12-17 21:02 ` Andrii Nakryiko @ 2025-12-17 21:27 ` Alexei Starovoitov 2025-12-17 22:34 ` Eduard Zingerman 2025-12-17 23:52 ` Andrii Nakryiko 0 siblings, 2 replies; 27+ messages in thread From: Alexei Starovoitov @ 2025-12-17 21:27 UTC (permalink / raw) To: Andrii Nakryiko Cc: Alan Maguire, Eduard Zingerman, Quentin Monnet, Andrii Nakryiko, Alexei Starovoitov, Daniel Borkmann, Martin KaFai Lau, Song Liu, Yonghong Song, John Fastabend, KP Singh, Stanislav Fomichev, Hao Luo, Jiri Olsa, bpf On Wed, Dec 17, 2025 at 1:02 PM Andrii Nakryiko <andrii.nakryiko@gmail.com> wrote: > > On Wed, Dec 17, 2025 at 12:50 PM Alan Maguire <alan.maguire@oracle.com> wrote: > > > > On 17/12/2025 19:35, Eduard Zingerman wrote: > > > On Wed, 2025-12-17 at 11:34 -0800, Eduard Zingerman wrote: > > >> On Wed, 2025-12-17 at 18:41 +0000, Alan Maguire wrote: > > >> > > >> [...] > > >> > > >>> So maybe the best we can do here is something like the following at the top > > >>> of vmlinux.h: > > >>> > > >>> #ifndef BPF_USE_MS_EXTENSIONS > > >>> #if __has_builtin(__builtin_FUNCSIG) || defined(_MSC_EXTENSIONS) > > >>> #define BPF_USE_MS_EXTENSIONS > > >>> #endif > > >>> #endif > > >>> > > >>> ...and then guard using #ifdef BPF_USE_MS_EXTENSIONS > > >>> > > >>> That will work on clang and perhaps at some point work on gcc, but also > > >>> gives the user the option to supply a macro to force use in cases where > > >>> there is no detection available. > > >> > > >> Are we sure we need such flexibility? > > >> Maybe just stick with current implementation and unroll the structures > > >> unconditionally? > > > > > > I mean, the point of the extension is to make the code smaller. > > > But here we are expanding it instead, so why bother? > > > > Yeah, I'm happy either way; if we have agreement that we just use the nested anon > > struct without macro complications I'll send an updated patch. > > There is a little bit of semantic meaning being lost when we inline > the struct, but I guess that can't be helped. Let's just > unconditionally inline then. Still better than having extra emit > option, IMO. tbh I'm concerned about information loss. If it's not too hard I would do #ifndef BPF_USE_MS_EXTENSIONS #if __has_builtin(__builtin_FUNCSIG) #define BPF_USE_MS_EXTENSIONS #endif and it will guarantee to work for clang while gcc will have structs inlined. In one of the clang selftests they have this comment: clang/test/Preprocessor/feature_tests.c: #elif __has_builtin(__builtin_FUNCSIG) #error Clang should not have this without '-fms-extensions' #endif so this detection is a known approach. ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH bpf-next 1/2] libbpf: add option to force-anonymize nested structs for BTF dump 2025-12-17 21:27 ` Alexei Starovoitov @ 2025-12-17 22:34 ` Eduard Zingerman 2025-12-17 22:47 ` Eduard Zingerman 2025-12-17 23:52 ` Andrii Nakryiko 1 sibling, 1 reply; 27+ messages in thread From: Eduard Zingerman @ 2025-12-17 22:34 UTC (permalink / raw) To: Alexei Starovoitov, Andrii Nakryiko Cc: Alan Maguire, Quentin Monnet, Andrii Nakryiko, Alexei Starovoitov, Daniel Borkmann, Martin KaFai Lau, Song Liu, Yonghong Song, John Fastabend, KP Singh, Stanislav Fomichev, Hao Luo, Jiri Olsa, bpf On Wed, 2025-12-17 at 13:27 -0800, Alexei Starovoitov wrote: > On Wed, Dec 17, 2025 at 1:02 PM Andrii Nakryiko > <andrii.nakryiko@gmail.com> wrote: > > > > On Wed, Dec 17, 2025 at 12:50 PM Alan Maguire <alan.maguire@oracle.com> wrote: > > > > > > On 17/12/2025 19:35, Eduard Zingerman wrote: > > > > On Wed, 2025-12-17 at 11:34 -0800, Eduard Zingerman wrote: > > > > > On Wed, 2025-12-17 at 18:41 +0000, Alan Maguire wrote: > > > > > > > > > > [...] > > > > > > > > > > > So maybe the best we can do here is something like the following at the top > > > > > > of vmlinux.h: > > > > > > > > > > > > #ifndef BPF_USE_MS_EXTENSIONS > > > > > > #if __has_builtin(__builtin_FUNCSIG) || defined(_MSC_EXTENSIONS) > > > > > > #define BPF_USE_MS_EXTENSIONS > > > > > > #endif > > > > > > #endif > > > > > > > > > > > > ...and then guard using #ifdef BPF_USE_MS_EXTENSIONS > > > > > > > > > > > > That will work on clang and perhaps at some point work on gcc, but also > > > > > > gives the user the option to supply a macro to force use in cases where > > > > > > there is no detection available. > > > > > > > > > > Are we sure we need such flexibility? > > > > > Maybe just stick with current implementation and unroll the structures > > > > > unconditionally? > > > > > > > > I mean, the point of the extension is to make the code smaller. > > > > But here we are expanding it instead, so why bother? > > > > > > Yeah, I'm happy either way; if we have agreement that we just use the nested anon > > > struct without macro complications I'll send an updated patch. > > > > There is a little bit of semantic meaning being lost when we inline > > the struct, but I guess that can't be helped. Let's just > > unconditionally inline then. Still better than having extra emit > > option, IMO. > > tbh I'm concerned about information loss. > > If it's not too hard I would do > #ifndef BPF_USE_MS_EXTENSIONS > #if __has_builtin(__builtin_FUNCSIG) > #define BPF_USE_MS_EXTENSIONS > #endif > > and it will guarantee to work for clang while gcc will have structs inlined. > > In one of the clang selftests they have this comment: > clang/test/Preprocessor/feature_tests.c: > #elif __has_builtin(__builtin_FUNCSIG) > #error Clang should not have this without '-fms-extensions' > #endif > > so this detection is a known approach. Speaking of information loss. It appears that clang does the same trick internally: $ cat ms-ext-test2.c struct foo { int a; } __attribute__((preserve_access_index)); struct bar { struct foo; } __attribute__((preserve_access_index)); int buz(struct bar *bar) { return bar->a; } $ clang -O2 -g -fms-extensions --target=bpf -c ms-ext-test2.c ms-ext-test2.c:6:3: warning: anonymous structs are a Microsoft extension [-Wmicrosoft-anon-tag] 6 | struct foo; | ^~~~~~~~~~ 1 warning generated. $ llvm-objdump -Sdr ms-ext-test2.o ms-ext-test2.o: file format elf64-bpf Disassembly of section .text: 0000000000000000 <buz>: ; return bar->a; 0: 61 10 00 00 00 00 00 00 w0 = *(u32 *)(r1 + 0x0) 0000000000000000: CO-RE <byte_off> [2] struct bar::<anon 0>.a (0:0:0) 1: 95 00 00 00 00 00 00 00 exit Note the "<anon 0>" in the relocation. It appears that we loose no information if structures are unrolled. ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH bpf-next 1/2] libbpf: add option to force-anonymize nested structs for BTF dump 2025-12-17 22:34 ` Eduard Zingerman @ 2025-12-17 22:47 ` Eduard Zingerman 2025-12-17 23:34 ` Alexei Starovoitov 0 siblings, 1 reply; 27+ messages in thread From: Eduard Zingerman @ 2025-12-17 22:47 UTC (permalink / raw) To: Alexei Starovoitov, Andrii Nakryiko Cc: Alan Maguire, Quentin Monnet, Andrii Nakryiko, Alexei Starovoitov, Daniel Borkmann, Martin KaFai Lau, Song Liu, Yonghong Song, John Fastabend, KP Singh, Stanislav Fomichev, Hao Luo, Jiri Olsa, bpf On Wed, 2025-12-17 at 14:34 -0800, Eduard Zingerman wrote: > On Wed, 2025-12-17 at 13:27 -0800, Alexei Starovoitov wrote: > > On Wed, Dec 17, 2025 at 1:02 PM Andrii Nakryiko > > <andrii.nakryiko@gmail.com> wrote: > > > > > > On Wed, Dec 17, 2025 at 12:50 PM Alan Maguire <alan.maguire@oracle.com> wrote: > > > > > > > > On 17/12/2025 19:35, Eduard Zingerman wrote: > > > > > On Wed, 2025-12-17 at 11:34 -0800, Eduard Zingerman wrote: > > > > > > On Wed, 2025-12-17 at 18:41 +0000, Alan Maguire wrote: > > > > > > > > > > > > [...] > > > > > > > > > > > > > So maybe the best we can do here is something like the following at the top > > > > > > > of vmlinux.h: > > > > > > > > > > > > > > #ifndef BPF_USE_MS_EXTENSIONS > > > > > > > #if __has_builtin(__builtin_FUNCSIG) || defined(_MSC_EXTENSIONS) > > > > > > > #define BPF_USE_MS_EXTENSIONS > > > > > > > #endif > > > > > > > #endif > > > > > > > > > > > > > > ...and then guard using #ifdef BPF_USE_MS_EXTENSIONS > > > > > > > > > > > > > > That will work on clang and perhaps at some point work on gcc, but also > > > > > > > gives the user the option to supply a macro to force use in cases where > > > > > > > there is no detection available. > > > > > > > > > > > > Are we sure we need such flexibility? > > > > > > Maybe just stick with current implementation and unroll the structures > > > > > > unconditionally? > > > > > > > > > > I mean, the point of the extension is to make the code smaller. > > > > > But here we are expanding it instead, so why bother? > > > > > > > > Yeah, I'm happy either way; if we have agreement that we just use the nested anon > > > > struct without macro complications I'll send an updated patch. > > > > > > There is a little bit of semantic meaning being lost when we inline > > > the struct, but I guess that can't be helped. Let's just > > > unconditionally inline then. Still better than having extra emit > > > option, IMO. > > > > tbh I'm concerned about information loss. > > > > If it's not too hard I would do > > #ifndef BPF_USE_MS_EXTENSIONS > > #if __has_builtin(__builtin_FUNCSIG) > > #define BPF_USE_MS_EXTENSIONS > > #endif > > > > and it will guarantee to work for clang while gcc will have structs inlined. > > > > In one of the clang selftests they have this comment: > > clang/test/Preprocessor/feature_tests.c: > > #elif __has_builtin(__builtin_FUNCSIG) > > #error Clang should not have this without '-fms-extensions' > > #endif > > > > so this detection is a known approach. > > Speaking of information loss. > It appears that clang does the same trick internally: > > $ cat ms-ext-test2.c > struct foo { > int a; > } __attribute__((preserve_access_index)); > > struct bar { > struct foo; > } __attribute__((preserve_access_index)); > > int buz(struct bar *bar) { > return bar->a; > } > > $ clang -O2 -g -fms-extensions --target=bpf -c ms-ext-test2.c > ms-ext-test2.c:6:3: warning: anonymous structs are a Microsoft extension [-Wmicrosoft-anon-tag] > 6 | struct foo; > | ^~~~~~~~~~ > 1 warning generated. > > $ llvm-objdump -Sdr ms-ext-test2.o > > ms-ext-test2.o: file format elf64-bpf > > Disassembly of section .text: > > 0000000000000000 <buz>: > ; return bar->a; > 0: 61 10 00 00 00 00 00 00 w0 = *(u32 *)(r1 + 0x0) > 0000000000000000: CO-RE <byte_off> [2] struct bar::<anon 0>.a (0:0:0) > 1: 95 00 00 00 00 00 00 00 exit > > Note the "<anon 0>" in the relocation. > It appears that we loose no information if structures are unrolled. On the other hand, frontend knows that it deals with 'struct foo'. $ clang -Xclang -ast-dump -O2 -g -fms-extensions --target=bpf -c ms-ext-test2.c ... |-RecordDecl 0x4e0398 <line:5:1, line:7:1> line:5:8 struct bar definition | ... | |-FieldDecl 0x5200d8 <line:6:3, col:10> col:3 implicit referenced 'struct foo' | | `-BPFPreserveAccessIndexAttr 0x5201d8 <<invalid sloc>> Implicit | `-IndirectFieldDecl 0x520138 <line:2:7> col:7 implicit a 'int' | |-Field 0x5200d8 field_index 0 'struct foo' | |-Field 0x4e0298 'a' 'int' | `-BPFPreserveAccessIndexAttr 0x520180 <<invalid sloc>> Implicit `-FunctionDecl 0x5204a8 <line:9:1, line:11:1> line:9:5 buz 'int (struct bar *)' |-ParmVarDecl 0x520398 <col:9, col:21> col:21 used bar 'struct bar *' `-CompoundStmt 0x520668 <col:26, line:11:1> `-ReturnStmt 0x520658 <line:10:3, col:15> `-ImplicitCastExpr 0x520640 <col:10, col:15> 'int' <LValueToRValue> `-MemberExpr 0x520610 <col:10, col:15> 'int' lvalue .a 0x4e0298 `-MemberExpr 0x5205d8 <col:10, col:15> 'struct foo' lvalue -> 0x5200d8 `-ImplicitCastExpr 0x5205c0 <col:10> 'struct bar *' <LValueToRValue> `-DeclRefExpr 0x5205a0 <col:10> 'struct bar *' lvalue ParmVar 0x520398 'bar' 'struct bar *' And this relation is reflected in DWARF. So, there is a subtle difference. ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH bpf-next 1/2] libbpf: add option to force-anonymize nested structs for BTF dump 2025-12-17 22:47 ` Eduard Zingerman @ 2025-12-17 23:34 ` Alexei Starovoitov 2025-12-18 0:19 ` Eduard Zingerman 0 siblings, 1 reply; 27+ messages in thread From: Alexei Starovoitov @ 2025-12-17 23:34 UTC (permalink / raw) To: Eduard Zingerman Cc: Andrii Nakryiko, Alan Maguire, Quentin Monnet, Andrii Nakryiko, Alexei Starovoitov, Daniel Borkmann, Martin KaFai Lau, Song Liu, Yonghong Song, John Fastabend, KP Singh, Stanislav Fomichev, Hao Luo, Jiri Olsa, bpf On Wed, Dec 17, 2025 at 2:47 PM Eduard Zingerman <eddyz87@gmail.com> wrote: > > > $ cat ms-ext-test2.c > > struct foo { > > int a; > > } __attribute__((preserve_access_index)); > > > > struct bar { > > struct foo; > > } __attribute__((preserve_access_index)); > > > > int buz(struct bar *bar) { > > return bar->a; > > } > > > > $ clang -O2 -g -fms-extensions --target=bpf -c ms-ext-test2.c > > ms-ext-test2.c:6:3: warning: anonymous structs are a Microsoft extension [-Wmicrosoft-anon-tag] > > 6 | struct foo; > > | ^~~~~~~~~~ > > 1 warning generated. > > > > $ llvm-objdump -Sdr ms-ext-test2.o > > > > ms-ext-test2.o: file format elf64-bpf > > > > Disassembly of section .text: > > > > 0000000000000000 <buz>: > > ; return bar->a; > > 0: 61 10 00 00 00 00 00 00 w0 = *(u32 *)(r1 + 0x0) > > 0000000000000000: CO-RE <byte_off> [2] struct bar::<anon 0>.a (0:0:0) > > 1: 95 00 00 00 00 00 00 00 exit > > > > Note the "<anon 0>" in the relocation. > > It appears that we loose no information if structures are unrolled. Forgot to mention the CORE concern earlier... Does the above work with current logic in relo_core.c ? If not, we should definitely unconditionally unroll to avoid fixing CORE. ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH bpf-next 1/2] libbpf: add option to force-anonymize nested structs for BTF dump 2025-12-17 23:34 ` Alexei Starovoitov @ 2025-12-18 0:19 ` Eduard Zingerman 2025-12-18 0:39 ` Alexei Starovoitov 0 siblings, 1 reply; 27+ messages in thread From: Eduard Zingerman @ 2025-12-18 0:19 UTC (permalink / raw) To: Alexei Starovoitov Cc: Andrii Nakryiko, Alan Maguire, Quentin Monnet, Andrii Nakryiko, Alexei Starovoitov, Daniel Borkmann, Martin KaFai Lau, Song Liu, Yonghong Song, John Fastabend, KP Singh, Stanislav Fomichev, Hao Luo, Jiri Olsa, bpf On Wed, 2025-12-17 at 15:34 -0800, Alexei Starovoitov wrote: > On Wed, Dec 17, 2025 at 2:47 PM Eduard Zingerman <eddyz87@gmail.com> wrote: > > > > > $ cat ms-ext-test2.c > > > struct foo { > > > int a; > > > } __attribute__((preserve_access_index)); > > > > > > struct bar { > > > struct foo; > > > } __attribute__((preserve_access_index)); > > > > > > int buz(struct bar *bar) { > > > return bar->a; > > > } > > > > > > $ clang -O2 -g -fms-extensions --target=bpf -c ms-ext-test2.c > > > ms-ext-test2.c:6:3: warning: anonymous structs are a Microsoft extension [-Wmicrosoft-anon-tag] > > > 6 | struct foo; > > > | ^~~~~~~~~~ > > > 1 warning generated. > > > > > > $ llvm-objdump -Sdr ms-ext-test2.o > > > > > > ms-ext-test2.o: file format elf64-bpf > > > > > > Disassembly of section .text: > > > > > > 0000000000000000 <buz>: > > > ; return bar->a; > > > 0: 61 10 00 00 00 00 00 00 w0 = *(u32 *)(r1 + 0x0) > > > 0000000000000000: CO-RE <byte_off> [2] struct bar::<anon 0>.a (0:0:0) > > > 1: 95 00 00 00 00 00 00 00 exit > > > > > > Note the "<anon 0>" in the relocation. > > > It appears that we loose no information if structures are unrolled. > > Forgot to mention the CORE concern earlier... > Does the above work with current logic in relo_core.c ? > If not, we should definitely unconditionally unroll > to avoid fixing CORE. I think there might be an issue with CO-RE. Here is an example: struct foo { int a; } __attribute__((preserve_access_index)); struct bar { #ifdef USE_MS struct foo; #else struct { int a; }; #endif } __attribute__((preserve_access_index)); int buz(struct bar *bar) { return bar->a; } Here is what I get with USE_MS: $ llvm-objdump -Sdr ms-ext-test2.o ms-ext-test2.o: file format elf64-bpf Disassembly of section .text: 0000000000000000 <buz>: ; return bar->a; 0: 61 10 00 00 00 00 00 00 w0 = *(u32 *)(r1 + 0x0) 0000000000000000: CO-RE <byte_off> [2] struct bar::<anon 0>.a (0:0:0) 1: 95 00 00 00 00 00 00 00 exit $ bpftool btf dump file ms-ext-test2.o [1] PTR '(anon)' type_id=2 [2] STRUCT 'bar' size=4 vlen=1 '(anon)' type_id=3 bits_offset=0 [3] STRUCT 'foo' size=4 vlen=1 'a' type_id=4 bits_offset=0 [4] INT 'int' size=4 bits_offset=0 nr_bits=32 encoding=SIGNED [5] FUNC_PROTO '(anon)' ret_type_id=4 vlen=1 'bar' type_id=1 [6] FUNC 'buz' type_id=5 linkage=global And here is without USE_MS: $ llvm-objdump -Sdr ms-ext-test2.o ms-ext-test2.o: file format elf64-bpf Disassembly of section .text: 0000000000000000 <buz>: ; return bar->a; 0: 61 10 00 00 00 00 00 00 w0 = *(u32 *)(r1 + 0x0) 0000000000000000: CO-RE <byte_off> [2] struct bar::<anon 0>.a (0:0:0) 1: 95 00 00 00 00 00 00 00 exit $ bpftool btf dump file ms-ext-test2.o [1] PTR '(anon)' type_id=2 [2] STRUCT 'bar' size=4 vlen=1 '(anon)' type_id=3 bits_offset=0 [3] STRUCT '(anon)' size=4 vlen=1 'a' type_id=4 bits_offset=0 [4] INT 'int' size=4 bits_offset=0 nr_bits=32 encoding=SIGNED [5] FUNC_PROTO '(anon)' ret_type_id=4 vlen=1 'bar' type_id=1 [6] FUNC 'buz' type_id=5 linkage=global So, with USE_MS the relocation captures the offset inside 'struct foo'. And this is important for CO-RE offsets resolution. So unrolling structures is actually a problem. ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH bpf-next 1/2] libbpf: add option to force-anonymize nested structs for BTF dump 2025-12-18 0:19 ` Eduard Zingerman @ 2025-12-18 0:39 ` Alexei Starovoitov 2025-12-18 0:50 ` Eduard Zingerman 0 siblings, 1 reply; 27+ messages in thread From: Alexei Starovoitov @ 2025-12-18 0:39 UTC (permalink / raw) To: Eduard Zingerman Cc: Andrii Nakryiko, Alan Maguire, Quentin Monnet, Andrii Nakryiko, Alexei Starovoitov, Daniel Borkmann, Martin KaFai Lau, Song Liu, Yonghong Song, John Fastabend, KP Singh, Stanislav Fomichev, Hao Luo, Jiri Olsa, bpf On Wed, Dec 17, 2025 at 4:19 PM Eduard Zingerman <eddyz87@gmail.com> wrote: > > On Wed, 2025-12-17 at 15:34 -0800, Alexei Starovoitov wrote: > > On Wed, Dec 17, 2025 at 2:47 PM Eduard Zingerman <eddyz87@gmail.com> wrote: > > > > > > > $ cat ms-ext-test2.c > > > > struct foo { > > > > int a; > > > > } __attribute__((preserve_access_index)); > > > > > > > > struct bar { > > > > struct foo; > > > > } __attribute__((preserve_access_index)); > > > > > > > > int buz(struct bar *bar) { > > > > return bar->a; > > > > } > > > > > > > > $ clang -O2 -g -fms-extensions --target=bpf -c ms-ext-test2.c > > > > ms-ext-test2.c:6:3: warning: anonymous structs are a Microsoft extension [-Wmicrosoft-anon-tag] > > > > 6 | struct foo; > > > > | ^~~~~~~~~~ > > > > 1 warning generated. > > > > > > > > $ llvm-objdump -Sdr ms-ext-test2.o > > > > > > > > ms-ext-test2.o: file format elf64-bpf > > > > > > > > Disassembly of section .text: > > > > > > > > 0000000000000000 <buz>: > > > > ; return bar->a; > > > > 0: 61 10 00 00 00 00 00 00 w0 = *(u32 *)(r1 + 0x0) > > > > 0000000000000000: CO-RE <byte_off> [2] struct bar::<anon 0>.a (0:0:0) > > > > 1: 95 00 00 00 00 00 00 00 exit > > > > > > > > Note the "<anon 0>" in the relocation. > > > > It appears that we loose no information if structures are unrolled. > > > > Forgot to mention the CORE concern earlier... > > Does the above work with current logic in relo_core.c ? > > If not, we should definitely unconditionally unroll > > to avoid fixing CORE. > > I think there might be an issue with CO-RE. > Here is an example: > > struct foo { > int a; > } __attribute__((preserve_access_index)); > > struct bar { > #ifdef USE_MS > struct foo; > #else > struct { int a; }; > #endif > } __attribute__((preserve_access_index)); > > int buz(struct bar *bar) { > return bar->a; > } > > Here is what I get with USE_MS: > > $ llvm-objdump -Sdr ms-ext-test2.o > > ms-ext-test2.o: file format elf64-bpf > > Disassembly of section .text: > > 0000000000000000 <buz>: > ; return bar->a; > 0: 61 10 00 00 00 00 00 00 w0 = *(u32 *)(r1 + 0x0) > 0000000000000000: CO-RE <byte_off> [2] struct bar::<anon 0>.a (0:0:0) > 1: 95 00 00 00 00 00 00 00 exit > > $ bpftool btf dump file ms-ext-test2.o > [1] PTR '(anon)' type_id=2 > [2] STRUCT 'bar' size=4 vlen=1 > '(anon)' type_id=3 bits_offset=0 > [3] STRUCT 'foo' size=4 vlen=1 > 'a' type_id=4 bits_offset=0 > [4] INT 'int' size=4 bits_offset=0 nr_bits=32 encoding=SIGNED > [5] FUNC_PROTO '(anon)' ret_type_id=4 vlen=1 > 'bar' type_id=1 > [6] FUNC 'buz' type_id=5 linkage=global > > And here is without USE_MS: > > $ llvm-objdump -Sdr ms-ext-test2.o > > ms-ext-test2.o: file format elf64-bpf > > Disassembly of section .text: > > 0000000000000000 <buz>: > ; return bar->a; > 0: 61 10 00 00 00 00 00 00 w0 = *(u32 *)(r1 + 0x0) > 0000000000000000: CO-RE <byte_off> [2] struct bar::<anon 0>.a (0:0:0) > 1: 95 00 00 00 00 00 00 00 exit > > $ bpftool btf dump file ms-ext-test2.o > [1] PTR '(anon)' type_id=2 > [2] STRUCT 'bar' size=4 vlen=1 > '(anon)' type_id=3 bits_offset=0 > [3] STRUCT '(anon)' size=4 vlen=1 > 'a' type_id=4 bits_offset=0 > [4] INT 'int' size=4 bits_offset=0 nr_bits=32 encoding=SIGNED > [5] FUNC_PROTO '(anon)' ret_type_id=4 vlen=1 > 'bar' type_id=1 > [6] FUNC 'buz' type_id=5 linkage=global > > So, with USE_MS the relocation captures the offset inside 'struct foo'. > And this is important for CO-RE offsets resolution. > So unrolling structures is actually a problem. Sounds like we should silence the warning the way Song proposed and tell all users to add -fms-extension to their builds. ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH bpf-next 1/2] libbpf: add option to force-anonymize nested structs for BTF dump 2025-12-18 0:39 ` Alexei Starovoitov @ 2025-12-18 0:50 ` Eduard Zingerman 0 siblings, 0 replies; 27+ messages in thread From: Eduard Zingerman @ 2025-12-18 0:50 UTC (permalink / raw) To: Alexei Starovoitov Cc: Andrii Nakryiko, Alan Maguire, Quentin Monnet, Andrii Nakryiko, Alexei Starovoitov, Daniel Borkmann, Martin KaFai Lau, Song Liu, Yonghong Song, John Fastabend, KP Singh, Stanislav Fomichev, Hao Luo, Jiri Olsa, bpf On Wed, 2025-12-17 at 16:39 -0800, Alexei Starovoitov wrote: > On Wed, Dec 17, 2025 at 4:19 PM Eduard Zingerman <eddyz87@gmail.com> wrote: > > > > On Wed, 2025-12-17 at 15:34 -0800, Alexei Starovoitov wrote: > > > On Wed, Dec 17, 2025 at 2:47 PM Eduard Zingerman <eddyz87@gmail.com> wrote: > > > > > > > > > $ cat ms-ext-test2.c > > > > > struct foo { > > > > > int a; > > > > > } __attribute__((preserve_access_index)); > > > > > > > > > > struct bar { > > > > > struct foo; > > > > > } __attribute__((preserve_access_index)); > > > > > > > > > > int buz(struct bar *bar) { > > > > > return bar->a; > > > > > } > > > > > > > > > > $ clang -O2 -g -fms-extensions --target=bpf -c ms-ext-test2.c > > > > > ms-ext-test2.c:6:3: warning: anonymous structs are a Microsoft extension [-Wmicrosoft-anon-tag] > > > > > 6 | struct foo; > > > > > | ^~~~~~~~~~ > > > > > 1 warning generated. > > > > > > > > > > $ llvm-objdump -Sdr ms-ext-test2.o > > > > > > > > > > ms-ext-test2.o: file format elf64-bpf > > > > > > > > > > Disassembly of section .text: > > > > > > > > > > 0000000000000000 <buz>: > > > > > ; return bar->a; > > > > > 0: 61 10 00 00 00 00 00 00 w0 = *(u32 *)(r1 + 0x0) > > > > > 0000000000000000: CO-RE <byte_off> [2] struct bar::<anon 0>.a (0:0:0) > > > > > 1: 95 00 00 00 00 00 00 00 exit > > > > > > > > > > Note the "<anon 0>" in the relocation. > > > > > It appears that we loose no information if structures are unrolled. > > > > > > Forgot to mention the CORE concern earlier... > > > Does the above work with current logic in relo_core.c ? > > > If not, we should definitely unconditionally unroll > > > to avoid fixing CORE. > > > > I think there might be an issue with CO-RE. > > Here is an example: > > > > struct foo { > > int a; > > } __attribute__((preserve_access_index)); > > > > struct bar { > > #ifdef USE_MS > > struct foo; > > #else > > struct { int a; }; > > #endif > > } __attribute__((preserve_access_index)); > > > > int buz(struct bar *bar) { > > return bar->a; > > } > > > > Here is what I get with USE_MS: > > > > $ llvm-objdump -Sdr ms-ext-test2.o > > > > ms-ext-test2.o: file format elf64-bpf > > > > Disassembly of section .text: > > > > 0000000000000000 <buz>: > > ; return bar->a; > > 0: 61 10 00 00 00 00 00 00 w0 = *(u32 *)(r1 + 0x0) > > 0000000000000000: CO-RE <byte_off> [2] struct bar::<anon 0>.a (0:0:0) > > 1: 95 00 00 00 00 00 00 00 exit > > > > $ bpftool btf dump file ms-ext-test2.o > > [1] PTR '(anon)' type_id=2 > > [2] STRUCT 'bar' size=4 vlen=1 > > '(anon)' type_id=3 bits_offset=0 > > [3] STRUCT 'foo' size=4 vlen=1 > > 'a' type_id=4 bits_offset=0 > > [4] INT 'int' size=4 bits_offset=0 nr_bits=32 encoding=SIGNED > > [5] FUNC_PROTO '(anon)' ret_type_id=4 vlen=1 > > 'bar' type_id=1 > > [6] FUNC 'buz' type_id=5 linkage=global > > > > And here is without USE_MS: > > > > $ llvm-objdump -Sdr ms-ext-test2.o > > > > ms-ext-test2.o: file format elf64-bpf > > > > Disassembly of section .text: > > > > 0000000000000000 <buz>: > > ; return bar->a; > > 0: 61 10 00 00 00 00 00 00 w0 = *(u32 *)(r1 + 0x0) > > 0000000000000000: CO-RE <byte_off> [2] struct bar::<anon 0>.a (0:0:0) > > 1: 95 00 00 00 00 00 00 00 exit > > > > $ bpftool btf dump file ms-ext-test2.o > > [1] PTR '(anon)' type_id=2 > > [2] STRUCT 'bar' size=4 vlen=1 > > '(anon)' type_id=3 bits_offset=0 > > [3] STRUCT '(anon)' size=4 vlen=1 > > 'a' type_id=4 bits_offset=0 > > [4] INT 'int' size=4 bits_offset=0 nr_bits=32 encoding=SIGNED > > [5] FUNC_PROTO '(anon)' ret_type_id=4 vlen=1 > > 'bar' type_id=1 > > [6] FUNC 'buz' type_id=5 linkage=global > > > > So, with USE_MS the relocation captures the offset inside 'struct foo'. > > And this is important for CO-RE offsets resolution. > > So unrolling structures is actually a problem. > > Sounds like we should silence the warning the way Song proposed and > tell all users to add -fms-extension to their builds. Sounds like it. ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH bpf-next 1/2] libbpf: add option to force-anonymize nested structs for BTF dump 2025-12-17 21:27 ` Alexei Starovoitov 2025-12-17 22:34 ` Eduard Zingerman @ 2025-12-17 23:52 ` Andrii Nakryiko 2025-12-18 0:49 ` Eduard Zingerman 1 sibling, 1 reply; 27+ messages in thread From: Andrii Nakryiko @ 2025-12-17 23:52 UTC (permalink / raw) To: Alexei Starovoitov Cc: Alan Maguire, Eduard Zingerman, Quentin Monnet, Andrii Nakryiko, Alexei Starovoitov, Daniel Borkmann, Martin KaFai Lau, Song Liu, Yonghong Song, John Fastabend, KP Singh, Stanislav Fomichev, Hao Luo, Jiri Olsa, bpf On Wed, Dec 17, 2025 at 1:27 PM Alexei Starovoitov <alexei.starovoitov@gmail.com> wrote: > > On Wed, Dec 17, 2025 at 1:02 PM Andrii Nakryiko > <andrii.nakryiko@gmail.com> wrote: > > > > On Wed, Dec 17, 2025 at 12:50 PM Alan Maguire <alan.maguire@oracle.com> wrote: > > > > > > On 17/12/2025 19:35, Eduard Zingerman wrote: > > > > On Wed, 2025-12-17 at 11:34 -0800, Eduard Zingerman wrote: > > > >> On Wed, 2025-12-17 at 18:41 +0000, Alan Maguire wrote: > > > >> > > > >> [...] > > > >> > > > >>> So maybe the best we can do here is something like the following at the top > > > >>> of vmlinux.h: > > > >>> > > > >>> #ifndef BPF_USE_MS_EXTENSIONS > > > >>> #if __has_builtin(__builtin_FUNCSIG) || defined(_MSC_EXTENSIONS) > > > >>> #define BPF_USE_MS_EXTENSIONS > > > >>> #endif > > > >>> #endif > > > >>> > > > >>> ...and then guard using #ifdef BPF_USE_MS_EXTENSIONS > > > >>> > > > >>> That will work on clang and perhaps at some point work on gcc, but also > > > >>> gives the user the option to supply a macro to force use in cases where > > > >>> there is no detection available. > > > >> > > > >> Are we sure we need such flexibility? > > > >> Maybe just stick with current implementation and unroll the structures > > > >> unconditionally? > > > > > > > > I mean, the point of the extension is to make the code smaller. > > > > But here we are expanding it instead, so why bother? > > > > > > Yeah, I'm happy either way; if we have agreement that we just use the nested anon > > > struct without macro complications I'll send an updated patch. > > > > There is a little bit of semantic meaning being lost when we inline > > the struct, but I guess that can't be helped. Let's just > > unconditionally inline then. Still better than having extra emit > > option, IMO. > > tbh I'm concerned about information loss. > > If it's not too hard I would do > #ifndef BPF_USE_MS_EXTENSIONS > #if __has_builtin(__builtin_FUNCSIG) > #define BPF_USE_MS_EXTENSIONS > #endif > Concert I have with this is that we'd need to hard-code this bpftool/vmlinux.h-specific #ifdef/#else/#endif logic (with arbitrary and custom BPF_USE_MS_EXTENSIONS define use) for -fms-extension handling inside generic libbpf btf_dump API, which is not supposed to be vmlinux.h specific. Wasn't there a way to basically declare -fms-extensions using #pragma inside vmlinux.h itself? If yes, what's the problem with using it? Why do we need to work-around anything at all then? > and it will guarantee to work for clang while gcc will have structs inlined. > > In one of the clang selftests they have this comment: > clang/test/Preprocessor/feature_tests.c: > #elif __has_builtin(__builtin_FUNCSIG) > #error Clang should not have this without '-fms-extensions' > #endif > > so this detection is a known approach. ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH bpf-next 1/2] libbpf: add option to force-anonymize nested structs for BTF dump 2025-12-17 23:52 ` Andrii Nakryiko @ 2025-12-18 0:49 ` Eduard Zingerman 0 siblings, 0 replies; 27+ messages in thread From: Eduard Zingerman @ 2025-12-18 0:49 UTC (permalink / raw) To: Andrii Nakryiko, Alexei Starovoitov Cc: Alan Maguire, Quentin Monnet, Andrii Nakryiko, Alexei Starovoitov, Daniel Borkmann, Martin KaFai Lau, Song Liu, Yonghong Song, John Fastabend, KP Singh, Stanislav Fomichev, Hao Luo, Jiri Olsa, bpf On Wed, 2025-12-17 at 15:52 -0800, Andrii Nakryiko wrote: > On Wed, Dec 17, 2025 at 1:27 PM Alexei Starovoitov > <alexei.starovoitov@gmail.com> wrote: > > > > On Wed, Dec 17, 2025 at 1:02 PM Andrii Nakryiko > > <andrii.nakryiko@gmail.com> wrote: > > > > > > On Wed, Dec 17, 2025 at 12:50 PM Alan Maguire <alan.maguire@oracle.com> wrote: > > > > > > > > On 17/12/2025 19:35, Eduard Zingerman wrote: > > > > > On Wed, 2025-12-17 at 11:34 -0800, Eduard Zingerman wrote: > > > > > > On Wed, 2025-12-17 at 18:41 +0000, Alan Maguire wrote: > > > > > > > > > > > > [...] > > > > > > > > > > > > > So maybe the best we can do here is something like the following at the top > > > > > > > of vmlinux.h: > > > > > > > > > > > > > > #ifndef BPF_USE_MS_EXTENSIONS > > > > > > > #if __has_builtin(__builtin_FUNCSIG) || defined(_MSC_EXTENSIONS) > > > > > > > #define BPF_USE_MS_EXTENSIONS > > > > > > > #endif > > > > > > > #endif > > > > > > > > > > > > > > ...and then guard using #ifdef BPF_USE_MS_EXTENSIONS > > > > > > > > > > > > > > That will work on clang and perhaps at some point work on gcc, but also > > > > > > > gives the user the option to supply a macro to force use in cases where > > > > > > > there is no detection available. > > > > > > > > > > > > Are we sure we need such flexibility? > > > > > > Maybe just stick with current implementation and unroll the structures > > > > > > unconditionally? > > > > > > > > > > I mean, the point of the extension is to make the code smaller. > > > > > But here we are expanding it instead, so why bother? > > > > > > > > Yeah, I'm happy either way; if we have agreement that we just use the nested anon > > > > struct without macro complications I'll send an updated patch. > > > > > > There is a little bit of semantic meaning being lost when we inline > > > the struct, but I guess that can't be helped. Let's just > > > unconditionally inline then. Still better than having extra emit > > > option, IMO. > > > > tbh I'm concerned about information loss. > > > > If it's not too hard I would do > > #ifndef BPF_USE_MS_EXTENSIONS > > #if __has_builtin(__builtin_FUNCSIG) > > #define BPF_USE_MS_EXTENSIONS > > #endif > > > > Concert I have with this is that we'd need to hard-code this > bpftool/vmlinux.h-specific #ifdef/#else/#endif logic (with arbitrary > and custom BPF_USE_MS_EXTENSIONS define use) for -fms-extension > handling inside generic libbpf btf_dump API, which is not supposed to > be vmlinux.h specific. > > Wasn't there a way to basically declare -fms-extensions using #pragma > inside vmlinux.h itself? If yes, what's the problem with using it? Why > do we need to work-around anything at all then? Can't find anything relevant in [1] or [2]. [1] https://clang.llvm.org/docs/LanguageExtensions.html [2] https://clang.llvm.org/docs/UsersManual.html Google's LLM doesn't know about such pragmas either. > > and it will guarantee to work for clang while gcc will have structs inlined. > > > > In one of the clang selftests they have this comment: > > clang/test/Preprocessor/feature_tests.c: > > #elif __has_builtin(__builtin_FUNCSIG) > > #error Clang should not have this without '-fms-extensions' > > #endif > > > > so this detection is a known approach. ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH bpf-next 1/2] libbpf: add option to force-anonymize nested structs for BTF dump 2025-12-17 16:06 ` Alan Maguire 2025-12-17 16:12 ` Alexei Starovoitov @ 2025-12-17 17:10 ` Andrii Nakryiko 1 sibling, 0 replies; 27+ messages in thread From: Andrii Nakryiko @ 2025-12-17 17:10 UTC (permalink / raw) To: Alan Maguire Cc: Eduard Zingerman, qmo, Andrii Nakryiko, ast, daniel, martin.lau, song, yonghong.song, john.fastabend, kpsingh, sdf, haoluo, jolsa, bpf On Wed, Dec 17, 2025 at 8:06 AM Alan Maguire <alan.maguire@oracle.com> wrote: > > On 16/12/2025 19:46, Eduard Zingerman wrote: > > On Tue, 2025-12-16 at 11:00 -0800, Eduard Zingerman wrote: > >> On Tue, 2025-12-16 at 17:18 +0000, Alan Maguire wrote: > >> > >> [...] > >> > >>> @@ -1460,10 +1466,16 @@ static void btf_dump_emit_type_chain(struct btf_dump *d, > >>> case BTF_KIND_UNION: > >>> btf_dump_emit_mods(d, decls); > >>> /* inline anonymous struct/union */ > >>> - if (t->name_off == 0 && !d->skip_anon_defs) > >>> + if (t->name_off == 0 && !d->skip_anon_defs) { > >>> btf_dump_emit_struct_def(d, id, t, lvl); > >>> - else > >>> + } else if (decls->cnt == 0 && !fname[0] && d->force_anon_struct_members) { > >>> + /* anonymize nested struct and emit it */ > >>> + btf_dump_set_anon_type(d, id, true); > >>> + btf_dump_emit_struct_def(d, id, t, lvl); > >>> + btf_dump_set_anon_type(d, id, false); > >> > >> > >> Hi Alan, > >> > >> I think this is a solid idea. > >> > >> It seems to me that with current implementation there would be a > >> trouble in the following scenario: > >> > >> struct foo { struct foo *ptr; }; > >> struct bar { > >> struct foo; > >> } > >> > >> Because state for 'foo' will be anonymize == true at the moment when > >> 'ptr' field is printed. > >> > >> Maybe pass a direct parameter to btf_dump_emit_struct_def()? > > > > Digging a bit more into this, here are a couple of weird examples: > > > > $ cat ~/tmp/ms-ext-test.c > > struct foo { > > struct foo *ptr; > > }; > > > > struct bar { > > struct foo; > > }; > > > > struct bar root; > > $ gcc -g -c -o ~/tmp/ms-ext-test.o ~/tmp/ms-ext-test.c > > $ pahole -J ~/tmp/ms-ext-test.o > > $ bpftool btf dump file ~/tmp/ms-ext-test.o format c > > #ifndef __VMLINUX_H__ > > #define __VMLINUX_H__ > > > > #ifndef BPF_NO_PRESERVE_ACCESS_INDEX > > #pragma clang attribute push (__attribute__((preserve_access_index)), apply_to = record) > > #endif > > > > #ifndef __ksym > > #define __ksym __attribute__((section(".ksyms"))) > > #endif > > > > #ifndef __weak > > #define __weak __attribute__((weak)) > > #endif > > > > #ifndef __bpf_fastcall > > #if __has_attribute(bpf_fastcall) > > #define __bpf_fastcall __attribute__((bpf_fastcall)) > > #else > > #define __bpf_fastcall > > #endif > > #endif > > > > struct foo { > > struct foo *ptr; > > }; > > > > > > /* BPF kfuncs */ > > #ifndef BPF_NO_KFUNC_PROTOTYPES > > #endif > > > > #ifndef BPF_NO_PRESERVE_ACCESS_INDEX > > #pragma clang attribute pop > > #endif > > > > #endif /* __VMLINUX_H__ */ > > > > > > $ cat ~/tmp/ms-ext-test.c > > struct foo { > > struct foo *ptr; > > }; > > > > struct bar { > > struct foo; > > int a; > > }; > > > > struct bar root; > > $ cgcc -fms-extensions -g -c -o ~/tmp/ms-ext-test.o ~/tmp/ms-ext-test.c > > $ pahole -J ~/tmp/ms-ext-test.o > > $ tools/bpf/bpftool/bpftool btf dump file ~/tmp/ms-ext-test.o format c > > #ifndef __VMLINUX_H__ > > #define __VMLINUX_H__ > > > > #ifndef BPF_NO_PRESERVE_ACCESS_INDEX > > #pragma clang attribute push (__attribute__((preserve_access_index)), apply_to = record) > > #endif > > > > #ifndef __ksym > > #define __ksym __attribute__((section(".ksyms"))) > > #endif > > > > #ifndef __weak > > #define __weak __attribute__((weak)) > > #endif > > > > #ifndef __bpf_fastcall > > #if __has_attribute(bpf_fastcall) > > #define __bpf_fastcall __attribute__((bpf_fastcall)) > > #else > > #define __bpf_fastcall > > #endif > > #endif > > > > struct foo { > > struct foo *ptr; > > }; > > > > struct bar { > > struct { > > struct *ptr; > > }; > > int a; > > }; > > > > > > /* BPF kfuncs */ > > #ifndef BPF_NO_KFUNC_PROTOTYPES > > #endif > > > > #ifndef BPF_NO_PRESERVE_ACCESS_INDEX > > #pragma clang attribute pop > > #endif > > > > #endif /* __VMLINUX_H__ */ > > > > Ack to all suggestions; in particular handling both cases with #ifdef is really nice since > it does away with the need for libbpf/bpftool options. With that in place - along with passing > parameters rather than setting field values, and being more selective to ensure we only emit > the #ifdef/#else/#endif for a composite type nested in a composite type - the above > gives us the following: > > struct foo { > struct foo *ptr; > }; > > struct bar { > > #ifdef __MS_EXTENSIONS__ > struct foo; > #else > struct { > struct foo *ptr; > }; > #endif > > int a; > }; > > I think that's the format we want. > > With respect to the padding behaviour, I may be missing something but I'm not sure these changes > will impact that. We pad in two cases: > > 1. between struct fields, where the current offset is less than the member offset or we have > alignment requirements to be met. > 2. at the end of the struct, to pad it out the the size/alignment requirements. > > I don't see anything different here for the case where we force-anonymize-inline the struct; > it still does 1 and 2 identical with the out-of-line declaration. To test this I augmented > Eduard's example to add internal and whole struct alignment requirements: > > struct foo { > struct foo *ptr; > char __attribute__((__aligned__(16))) s; > int t; > } __attribute__((__aligned__(64))); > > struct bar { > struct foo; > int a; > }; > > struct bar root; > > int main(int argc, char *argv[]) > { > struct bar *r = (struct bar *)argv[0]; > } > > $ gcc -fms-extensions -g -c -o /tmp/ms-ext-test.o /tmp/ms-ext-test.c > $ pahole -J /tmp/ms-ext-test.o > $ bpftool btf dump file /tmp/ms-ext-test.o format c > > struct foo { > struct foo *ptr; > long: 64; > char s; > int t; > long: 64; > long: 64; > long: 64; > long: 64; > long: 64; > }; > > struct bar { > > #ifdef __MS_EXTENSIONS__ > struct foo; > #else > struct { > struct foo *ptr; > long: 64; > char s; > int t; > long: 64; > long: 64; > long: 64; > long: 64; > long: 64; > }; > #endif > > int a; > long: 64; > long: 64; > long: 64; > long: 64; > long: 64; > long: 64; > long: 64; > }; > > This looks equivalent to me, but there may some other condition you're thinking > of here? I initially assumed that we won't have outer anonymous struct {} and instead we'll just inline embedded structs directly. In that case it can happen that if the following field has smaller alignment requirement than embedded struct's alignment, then we can accidentally pack field more tightly. struct blah { long x; int y; /* 4 byte pad */ }; struct whatever { struct blah; char b; }; b will be at offset 16. But if we did (an arguably cleaner) struct whatever { long x; int y; /* here we should have long: 64 explicitly */ char b; } then without explicit padding before b, b would end up at offset 13. But all of this is not a concern if we keep an anonymous wrapping struct, so ignore me. > > Thanks! > > Alan ^ permalink raw reply [flat|nested] 27+ messages in thread
* [PATCH bpf-next 2/2] bpftool: force-anonymize structs to avoid need for -fms-extension 2025-12-16 17:18 [PATCH bpf-next 0/2] Handle -fms-extension in kernel structs Alan Maguire 2025-12-16 17:18 ` [PATCH bpf-next 1/2] libbpf: add option to force-anonymize nested structs for BTF dump Alan Maguire @ 2025-12-16 17:18 ` Alan Maguire 2025-12-16 19:20 ` [PATCH bpf-next 0/2] Handle -fms-extension in kernel structs Song Liu 2 siblings, 0 replies; 27+ messages in thread From: Alan Maguire @ 2025-12-16 17:18 UTC (permalink / raw) To: qmo Cc: ast, daniel, andrii, martin.lau, eddyz87, song, yonghong.song, john.fastabend, kpsingh, sdf, haoluo, jolsa, bpf, Alan Maguire Use libbpf dump option to ensure that a nested struct will be declared as an equivalent anonymous struct to the named struct. With this approach we do not need -fms-extension to support such structs in vmlinux.h. Signed-off-by: Alan Maguire <alan.maguire@oracle.com> --- tools/bpf/bpftool/btf.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/tools/bpf/bpftool/btf.c b/tools/bpf/bpftool/btf.c index 946612029dee..523c8bf0e53a 100644 --- a/tools/bpf/bpftool/btf.c +++ b/tools/bpf/bpftool/btf.c @@ -771,11 +771,13 @@ static struct sort_datum *sort_btf_c(const struct btf *btf) static int dump_btf_c(const struct btf *btf, __u32 *root_type_ids, int root_type_cnt, bool sort_dump) { + DECLARE_LIBBPF_OPTS(btf_dump_opts, opts, + .force_anon_struct_members = true); struct sort_datum *datums = NULL; struct btf_dump *d; int err = 0, i; - d = btf_dump__new(btf, btf_dump_printf, NULL, NULL); + d = btf_dump__new(btf, btf_dump_printf, NULL, &opts); if (!d) return -errno; -- 2.39.3 ^ permalink raw reply related [flat|nested] 27+ messages in thread
* Re: [PATCH bpf-next 0/2] Handle -fms-extension in kernel structs 2025-12-16 17:18 [PATCH bpf-next 0/2] Handle -fms-extension in kernel structs Alan Maguire 2025-12-16 17:18 ` [PATCH bpf-next 1/2] libbpf: add option to force-anonymize nested structs for BTF dump Alan Maguire 2025-12-16 17:18 ` [PATCH bpf-next 2/2] bpftool: force-anonymize structs to avoid need for -fms-extension Alan Maguire @ 2025-12-16 19:20 ` Song Liu 2 siblings, 0 replies; 27+ messages in thread From: Song Liu @ 2025-12-16 19:20 UTC (permalink / raw) To: Alan Maguire Cc: qmo, ast, daniel, andrii, martin.lau, eddyz87, yonghong.song, john.fastabend, kpsingh, sdf, haoluo, jolsa, bpf On Tue, Dec 16, 2025 at 9:20 AM Alan Maguire <alan.maguire@oracle.com> wrote: > > The kernel is using named structs embedded in other structs > with no member name (similar to anonymous struct usage). > Such an arrangement causes problems for compilers unless -fms-extension > is specified. With vmlinux.h generation we do not want to impose > that on the vmlinux.h consumer, so generate a compatible representation > using anonymous types. > > Patch 1 adds libbpf support for this in BTF dump code; patch 2 > adds bpftool support to use the new option. As Alexei suggested earlier, let's target the bpf tree. Please also test again upstream (v6.19-rc1). I am seeing some weird errors in my local tests on v6.19-rc1. Thanks, Song ^ permalink raw reply [flat|nested] 27+ messages in thread
end of thread, other threads:[~2025-12-18 0:50 UTC | newest] Thread overview: 27+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2025-12-16 17:18 [PATCH bpf-next 0/2] Handle -fms-extension in kernel structs Alan Maguire 2025-12-16 17:18 ` [PATCH bpf-next 1/2] libbpf: add option to force-anonymize nested structs for BTF dump Alan Maguire 2025-12-16 19:00 ` Eduard Zingerman 2025-12-16 19:08 ` Andrii Nakryiko 2025-12-16 19:46 ` Eduard Zingerman 2025-12-17 16:06 ` Alan Maguire 2025-12-17 16:12 ` Alexei Starovoitov 2025-12-17 17:06 ` Andrii Nakryiko 2025-12-17 17:33 ` Alan Maguire 2025-12-17 17:52 ` Alexei Starovoitov 2025-12-17 18:41 ` Alan Maguire 2025-12-17 19:34 ` Eduard Zingerman 2025-12-17 19:35 ` Eduard Zingerman 2025-12-17 20:50 ` Alan Maguire 2025-12-17 21:02 ` Andrii Nakryiko 2025-12-17 21:27 ` Alexei Starovoitov 2025-12-17 22:34 ` Eduard Zingerman 2025-12-17 22:47 ` Eduard Zingerman 2025-12-17 23:34 ` Alexei Starovoitov 2025-12-18 0:19 ` Eduard Zingerman 2025-12-18 0:39 ` Alexei Starovoitov 2025-12-18 0:50 ` Eduard Zingerman 2025-12-17 23:52 ` Andrii Nakryiko 2025-12-18 0:49 ` Eduard Zingerman 2025-12-17 17:10 ` Andrii Nakryiko 2025-12-16 17:18 ` [PATCH bpf-next 2/2] bpftool: force-anonymize structs to avoid need for -fms-extension Alan Maguire 2025-12-16 19:20 ` [PATCH bpf-next 0/2] Handle -fms-extension in kernel structs Song Liu
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox