BPF List
 help / color / mirror / Atom feed
From: Eduard Zingerman <eddyz87@gmail.com>
To: Alexei Starovoitov <alexei.starovoitov@gmail.com>,
	Andrii Nakryiko <andrii.nakryiko@gmail.com>
Cc: Alan Maguire <alan.maguire@oracle.com>,
	Quentin Monnet <qmo@kernel.org>,
	Andrii Nakryiko <andrii@kernel.org>,
	Alexei Starovoitov <ast@kernel.org>,
	Daniel Borkmann <daniel@iogearbox.net>,
	 Martin KaFai Lau <martin.lau@linux.dev>,
	Song Liu <song@kernel.org>,
	Yonghong Song <yonghong.song@linux.dev>,
	John Fastabend <john.fastabend@gmail.com>,
	KP Singh <kpsingh@kernel.org>,
	Stanislav Fomichev	 <sdf@fomichev.me>,
	Hao Luo <haoluo@google.com>, Jiri Olsa <jolsa@kernel.org>,
	bpf	 <bpf@vger.kernel.org>
Subject: Re: [PATCH bpf-next 1/2] libbpf: add option to force-anonymize nested structs for BTF dump
Date: Wed, 17 Dec 2025 14:47:30 -0800	[thread overview]
Message-ID: <aeeae7e13ce401726ddce756268c0686d30eb3a9.camel@gmail.com> (raw)
In-Reply-To: <5022ccaf5591e5bb88fe3d7a08dbb3c4fb6c3132.camel@gmail.com>

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.

  reply	other threads:[~2025-12-17 22:47 UTC|newest]

Thread overview: 27+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
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 [this message]
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

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=aeeae7e13ce401726ddce756268c0686d30eb3a9.camel@gmail.com \
    --to=eddyz87@gmail.com \
    --cc=alan.maguire@oracle.com \
    --cc=alexei.starovoitov@gmail.com \
    --cc=andrii.nakryiko@gmail.com \
    --cc=andrii@kernel.org \
    --cc=ast@kernel.org \
    --cc=bpf@vger.kernel.org \
    --cc=daniel@iogearbox.net \
    --cc=haoluo@google.com \
    --cc=john.fastabend@gmail.com \
    --cc=jolsa@kernel.org \
    --cc=kpsingh@kernel.org \
    --cc=martin.lau@linux.dev \
    --cc=qmo@kernel.org \
    --cc=sdf@fomichev.me \
    --cc=song@kernel.org \
    --cc=yonghong.song@linux.dev \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox