| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID:
<CO1PR02MB84601693B2ECBB323297B4ECB8E7A@CO1PR02MB8460.namprd02.prod.outlook.com>
Date: Thu, 2 Oct 2025 17:35:09 +0000
From: Andrew Pinski <apinski@...cinc.com>
To: Arnaldo Carvalho de Melo <acme@...nel.org>,
Yonghong Song
<yonghong.song@...ux.dev>
CC: "Andrew Pinski (QUIC)" <quic_apinski@...cinc.com>,
Namhyung Kim
<namhyung@...nel.org>, Sam James <sam@...too.org>,
Peter Zijlstra
<peterz@...radead.org>, Ingo Molnar <mingo@...hat.com>,
Mark Rutland
<mark.rutland@....com>,
Alexander Shishkin
<alexander.shishkin@...ux.intel.com>,
Jiri Olsa <jolsa@...nel.org>, Ian
Rogers <irogers@...gle.com>,
Adrian Hunter <adrian.hunter@...el.com>,
"Liang,
Kan" <kan.liang@...ux.intel.com>,
"linux-perf-users@...r.kernel.org"
<linux-perf-users@...r.kernel.org>,
"linux-kernel@...r.kernel.org"
<linux-kernel@...r.kernel.org>,
"bpf@...r.kernel.org" <bpf@...r.kernel.org>,
"Andrew.pinski@....qualcomm.com" <Andrew.pinski@....qualcomm.com>
Subject: RE: [PATCH] perf: use __builtin_preserve_field_info for GCC
compatibility
> -----Original Message-----
> From: Arnaldo Carvalho de Melo <acme@...nel.org>
> Sent: Thursday, October 2, 2025 10:32 AM
> To: Yonghong Song <yonghong.song@...ux.dev>
> Cc: Andrew Pinski (QUIC) <quic_apinski@...cinc.com>;
> Namhyung Kim <namhyung@...nel.org>; Sam James
> <sam@...too.org>; Peter Zijlstra <peterz@...radead.org>;
> Ingo Molnar <mingo@...hat.com>; Mark Rutland
> <mark.rutland@....com>; Alexander Shishkin
> <alexander.shishkin@...ux.intel.com>; Jiri Olsa
> <jolsa@...nel.org>; Ian Rogers <irogers@...gle.com>; Adrian
> Hunter <adrian.hunter@...el.com>; Liang, Kan
> <kan.liang@...ux.intel.com>; linux-perf-
> users@...r.kernel.org; linux-kernel@...r.kernel.org;
> bpf@...r.kernel.org
> Subject: Re: [PATCH] perf: use __builtin_preserve_field_info
> for GCC compatibility
>
> On Wed, Aug 06, 2025 at 05:27:02PM -0700, Yonghong Song
> wrote:
> > On 8/6/25 4:57 PM, Andrew Pinski wrote:
> > > > -----Original Message-----
> > > > From: Namhyung Kim <namhyung@...nel.org>
> > > > Sent: Wednesday, August 6, 2025 4:34 PM
> > > > To: Sam James <sam@...too.org>
> > > > Cc: Peter Zijlstra <peterz@...radead.org>; Ingo Molnar
> > > > <mingo@...hat.com>; Arnaldo Carvalho de Melo
> <acme@...nel.org>;
> > > > Mark Rutland <mark.rutland@....com>; Alexander
> Shishkin
> > > > <alexander.shishkin@...ux.intel.com>; Jiri Olsa
> > > > <jolsa@...nel.org>; Ian Rogers <irogers@...gle.com>;
> Adrian Hunter
> > > > <adrian.hunter@...el.com>; Liang, Kan
> <kan.liang@...ux.intel.com>;
> > > > Andrew Pinski <quic_apinski@...cinc.com>; linux-perf-
> > > > users@...r.kernel.org; linux-kernel@...r.kernel.org;
> > > > bpf@...r.kernel.org
> > > > Subject: Re: [PATCH] perf: use
> __builtin_preserve_field_info for
> > > > GCC compatibility
> > > >
> > > > Hello,
> > > >
> > > > On Wed, Aug 06, 2025 at 01:03:01AM +0100, Sam James
> > > > wrote:
> > > > > When exploring building bpf_skel with GCC's BPF
> support,
> > > > there was a
> > > > > buid failure because of bpf_core_field_exists vs the
> > > > mem_hops bitfield:
> > > > > ```
> > > > > In file included from
> util/bpf_skel/sample_filter.bpf.c:6:
> > > > > util/bpf_skel/sample_filter.bpf.c: In function
> > > > 'perf_get_sample':
> > > > > tools/perf/libbpf/include/bpf/bpf_core_read.h:169:42:
> > > > error: cannot take address of bit-field 'mem_hops'
> > > > > 169 | #define ___bpf_field_ref1(field) (&(field))
> > > > > | ^
> > > > > tools/perf/libbpf/include/bpf/bpf_helpers.h:222:29:
> note: in
> > > > expansion of macro '___bpf_field_ref1'
> > > > > 222 | #define ___bpf_concat(a, b) a ## b
> > > > > | ^
> > > > > tools/perf/libbpf/include/bpf/bpf_helpers.h:225:29:
> note: in
> > > > expansion of macro '___bpf_concat'
> > > > > 225 | #define ___bpf_apply(fn, n) ___bpf_concat(fn,
> n)
> > > > > | ^~~~~~~~~~~~~
> > > > > tools/perf/libbpf/include/bpf/bpf_core_read.h:173:9:
> note:
> > > > in expansion of macro '___bpf_apply'
> > > > > 173 | ___bpf_apply(___bpf_field_ref,
> > > > ___bpf_narg(args))(args)
> > > > > | ^~~~~~~~~~~~
> > > > > tools/perf/libbpf/include/bpf/bpf_core_read.h:188:39:
> note:
> > > > in expansion of macro '___bpf_field_ref'
> > > > > 188 |
> > > > __builtin_preserve_field_info(___bpf_field_ref(field),
> > > > BPF_FIELD_EXISTS)
> > > > > | ^~~~~~~~~~~~~~~~
> > > > > util/bpf_skel/sample_filter.bpf.c:167:29: note: in
> expansion
> > > > of macro 'bpf_core_field_exists'
> > > > > 167 | if (bpf_core_field_exists(data-
> > > > > mem_hops))
> > > > > | ^~~~~~~~~~~~~~~~~~~~~
> > > > > cc1: error: argument is not a field access ```
> > > > >
> > > > > ___bpf_field_ref1 was adapted for GCC in
> > > > > 12bbcf8e840f40b82b02981e96e0a5fbb0703ea9
> > > > > but the trick added for compatibility in
> > > > > 3a8b8fc3174891c4c12f5766d82184a82d4b2e3e
> > > > > isn't compatible with that as an address is used as an
> > > > argument.
> > > > > Workaround this by calling
> __builtin_preserve_field_info
> > > > directly as
> > > > > the bpf_core_field_exists macro does, but without the
> > > > ___bpf_field_ref use.
> > > >
> > > > IIUC GCC doesn't support bpf_core_fields_exists() for
> bitfield
> > > > members, right? Is it gonna change in the future?
> > > Let's discuss how __builtin_preserve_field_info is handled
> in the first place for BPF. Right now it seems it is passed some
> expression as the first argument is never evaluated.
> > > The problem is GCC's implementation of
> __builtin_preserve_field_info is all in the backend and the
> front end does not understand of the special rules here.
> > >
> > > GCC implements some "special" builtins in the front-end
> but not by the normal function call rules but parsing them
> separately; this is how __builtin_offsetof and a few others are
> implemented in both the C and C++ front-ends (and
> implemented separately). Now we could have add a hook to
> allow a backend to something similar and maybe that is the
> best way forward here.
> > > But it won't be __builtin_preserve_field_info but rather
> `__builtin_preserve_field_type_info(type,field,kind)` instead.
> > >
> > > __builtin_preserve_enum_type_value would also be
> added with the following:
> > > __builtin_preserve_enum_type_value(enum_type,
> enum_value, kind)
> > >
> > > And change all of the rest of the builtins to accept a true
> type argument rather than having to cast an null pointer to
> that type.
> > >
> > > Will clang implement a similar builtin?
> >
> > The clang only has one builtin for some related relocations:
> > __builtin_preserve_field_info(..., BPF_FIELD_EXISTS)
> > __builtin_preserve_field_info(...,
> BPF_FIELD_BYTE_OFFSET)
> > ...
> > They are all used in bpf_core_read.h.
> >
> > >
> > > Note this won't be done until at least GCC 16; maybe not
> until GCC 17 depending on if I or someone else gets time to
> implement the front-end parts which is acceptable to both the
> C and C++ front-ends.
>
> So I'm taking the patch as-is, ok?
>
> But first we need the Signed-off-by tag from Andrew Pinski as
> he is listed in a Co-authored-by, that I replaced with Co-
> developed-by as its the term used for this purpose in:
>
> Yonghong, can I add an Acked-by: you since you participated in
> this discussion agreeing with the original patch (If I'm not
> mistaken)?
Signed-off-by: Andrew Pinski <andrew.pinski@....qualcomm.com>
Note my email address for doing Linux and GCC development is Andrew.pinski@....qualcomm.com . It was apinski_quic@...cinc.com but that changed last month.
Thanks,
Andrew Pinski
>
Powered by blists - more mailing lists