2022-06-09 06:50:23

by James Hilliard

[permalink] [raw]
Subject: [PATCH v3 1/1] libbpf: fix broken gcc SEC pragma macro

It seems the gcc preprocessor breaks unless pragmas are wrapped
individually inside macros when surrounding __attribute__.

Fixes errors like:
error: expected identifier or '(' before '#pragma'
106 | SEC("cgroup/bind6")
| ^~~

error: expected '=', ',', ';', 'asm' or '__attribute__' before '#pragma'
114 | char _license[] SEC("license") = "GPL";
| ^~~

Signed-off-by: James Hilliard <[email protected]>
---
Changes v2 -> v3:
- just fix SEC pragma
Changes v1 -> v2:
- replace typeof with __typeof__ instead of changing pragma macros
---
tools/lib/bpf/bpf_helpers.h | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/tools/lib/bpf/bpf_helpers.h b/tools/lib/bpf/bpf_helpers.h
index fb04eaf367f1..66d23c47c206 100644
--- a/tools/lib/bpf/bpf_helpers.h
+++ b/tools/lib/bpf/bpf_helpers.h
@@ -22,11 +22,12 @@
* To allow use of SEC() with externs (e.g., for extern .maps declarations),
* make sure __attribute__((unused)) doesn't trigger compilation warning.
*/
+#define DO_PRAGMA(x) _Pragma(#x)
#define SEC(name) \
- _Pragma("GCC diagnostic push") \
- _Pragma("GCC diagnostic ignored \"-Wignored-attributes\"") \
+ DO_PRAGMA("GCC diagnostic push") \
+ DO_PRAGMA("GCC diagnostic ignored \"-Wignored-attributes\"") \
__attribute__((section(name), used)) \
- _Pragma("GCC diagnostic pop") \
+ DO_PRAGMA("GCC diagnostic pop") \

/* Avoid 'linux/stddef.h' definition of '__always_inline'. */
#undef __always_inline
--
2.25.1


2022-06-09 19:23:16

by Andrii Nakryiko

[permalink] [raw]
Subject: Re: [PATCH v3 1/1] libbpf: fix broken gcc SEC pragma macro

On Wed, Jun 8, 2022 at 11:24 PM James Hilliard
<[email protected]> wrote:
>
> It seems the gcc preprocessor breaks unless pragmas are wrapped
> individually inside macros when surrounding __attribute__.
>
> Fixes errors like:
> error: expected identifier or '(' before '#pragma'
> 106 | SEC("cgroup/bind6")
> | ^~~
>
> error: expected '=', ',', ';', 'asm' or '__attribute__' before '#pragma'
> 114 | char _license[] SEC("license") = "GPL";
> | ^~~
>
> Signed-off-by: James Hilliard <[email protected]>
> ---
> Changes v2 -> v3:
> - just fix SEC pragma
> Changes v1 -> v2:
> - replace typeof with __typeof__ instead of changing pragma macros
> ---
> tools/lib/bpf/bpf_helpers.h | 7 ++++---
> 1 file changed, 4 insertions(+), 3 deletions(-)
>
> diff --git a/tools/lib/bpf/bpf_helpers.h b/tools/lib/bpf/bpf_helpers.h
> index fb04eaf367f1..66d23c47c206 100644
> --- a/tools/lib/bpf/bpf_helpers.h
> +++ b/tools/lib/bpf/bpf_helpers.h
> @@ -22,11 +22,12 @@
> * To allow use of SEC() with externs (e.g., for extern .maps declarations),
> * make sure __attribute__((unused)) doesn't trigger compilation warning.
> */
> +#define DO_PRAGMA(x) _Pragma(#x)
> #define SEC(name) \
> - _Pragma("GCC diagnostic push") \
> - _Pragma("GCC diagnostic ignored \"-Wignored-attributes\"") \
> + DO_PRAGMA("GCC diagnostic push") \
> + DO_PRAGMA("GCC diagnostic ignored \"-Wignored-attributes\"") \
> __attribute__((section(name), used)) \
> - _Pragma("GCC diagnostic pop") \
> + DO_PRAGMA("GCC diagnostic pop") \
>

I'm not going to accept this unless I can repro it in the first place.
Using -std=c17 doesn't trigger such issue. Please provide the repro
first. Building systemd is not a repro, unfortunately. Please try to
do it based on libbpf-bootstrap ([0])

[0] https://github.com/libbpf/libbpf-bootstrap

> /* Avoid 'linux/stddef.h' definition of '__always_inline'. */
> #undef __always_inline
> --
> 2.25.1
>

2022-06-09 23:54:04

by James Hilliard

[permalink] [raw]
Subject: Re: [PATCH v3 1/1] libbpf: fix broken gcc SEC pragma macro

On Thu, Jun 9, 2022 at 12:13 PM Andrii Nakryiko
<[email protected]> wrote:
>
> On Wed, Jun 8, 2022 at 11:24 PM James Hilliard
> <[email protected]> wrote:
> >
> > It seems the gcc preprocessor breaks unless pragmas are wrapped
> > individually inside macros when surrounding __attribute__.
> >
> > Fixes errors like:
> > error: expected identifier or '(' before '#pragma'
> > 106 | SEC("cgroup/bind6")
> > | ^~~
> >
> > error: expected '=', ',', ';', 'asm' or '__attribute__' before '#pragma'
> > 114 | char _license[] SEC("license") = "GPL";
> > | ^~~
> >
> > Signed-off-by: James Hilliard <[email protected]>
> > ---
> > Changes v2 -> v3:
> > - just fix SEC pragma
> > Changes v1 -> v2:
> > - replace typeof with __typeof__ instead of changing pragma macros
> > ---
> > tools/lib/bpf/bpf_helpers.h | 7 ++++---
> > 1 file changed, 4 insertions(+), 3 deletions(-)
> >
> > diff --git a/tools/lib/bpf/bpf_helpers.h b/tools/lib/bpf/bpf_helpers.h
> > index fb04eaf367f1..66d23c47c206 100644
> > --- a/tools/lib/bpf/bpf_helpers.h
> > +++ b/tools/lib/bpf/bpf_helpers.h
> > @@ -22,11 +22,12 @@
> > * To allow use of SEC() with externs (e.g., for extern .maps declarations),
> > * make sure __attribute__((unused)) doesn't trigger compilation warning.
> > */
> > +#define DO_PRAGMA(x) _Pragma(#x)
> > #define SEC(name) \
> > - _Pragma("GCC diagnostic push") \
> > - _Pragma("GCC diagnostic ignored \"-Wignored-attributes\"") \
> > + DO_PRAGMA("GCC diagnostic push") \
> > + DO_PRAGMA("GCC diagnostic ignored \"-Wignored-attributes\"") \
> > __attribute__((section(name), used)) \
> > - _Pragma("GCC diagnostic pop") \
> > + DO_PRAGMA("GCC diagnostic pop") \
> >
>
> I'm not going to accept this unless I can repro it in the first place.
> Using -std=c17 doesn't trigger such issue. Please provide the repro
> first. Building systemd is not a repro, unfortunately. Please try to
> do it based on libbpf-bootstrap ([0])
>
> [0] https://github.com/libbpf/libbpf-bootstrap

Seems to reproduce just fine already there with:
https://github.com/libbpf/libbpf-bootstrap/blob/31face36d469a0e3e4c4ac1cafc66747d3150930/examples/c/minimal.bpf.c

See here:
$ /home/buildroot/buildroot/output/per-package/libbpf/host/bin/bpf-gcc
-Winline -O2 -mframe-limit=32767 -mco-re -gbtf -std=gnu17 -v
-D__x86_64__ -mlittle-endian -I
/home/buildroot/buildroot/output/per-package/libbpf/host/x86_64-buildroot-linux-gnu/sysroot/usr/include
minimal.bpf.c -o minimal.bpf.o
Using built-in specs.
COLLECT_GCC=/home/buildroot/buildroot/output/per-package/libbpf/host/bin/bpf-gcc.br_real
COLLECT_LTO_WRAPPER=/home/buildroot/buildroot/output/per-package/libbpf/host/bin/../libexec/gcc/bpf-buildroot-none/12.1.0/lto-wrapper
Target: bpf-buildroot-none
Configured with: ./configure
--prefix=/home/buildroot/buildroot/output/per-package/host-gcc-bpf/host
--sysconfdir=/home/buildroot/buildroot/output/per-package/host-gcc-bpf/host/etc
--localstatedir=/home/buildroot/buildroot/output/per-package/host-gcc-bpf/host/var
--enable-shared --disable-static --disable-gtk-doc
--disable-gtk-doc-html --disable-doc --disable-docs
--disable-documentation --disable-debug --with-xmlto=no --with-fop=no
--disable-nls --disable-dependency-tracking
--target=bpf-buildroot-none
--prefix=/home/buildroot/buildroot/output/per-package/host-gcc-bpf/host
--sysconfdir=/home/buildroot/buildroot/output/per-package/host-gcc-bpf/host/etc
--enable-languages=c --with-gnu-ld --enable-static
--disable-decimal-float --disable-gcov --disable-libssp
--disable-multilib --disable-shared
--with-gmp=/home/buildroot/buildroot/output/per-package/host-gcc-bpf/host
--with-mpc=/home/buildroot/buildroot/output/per-package/host-gcc-bpf/host
--with-mpfr=/home/buildroot/buildroot/output/per-package/host-gcc-bpf/host
--with-pkgversion='Buildroot 2022.05-118-ge052166011-dirty'
--with-bugurl=http://bugs.buildroot.net/ --without-zstd --without-isl
--without-cloog
Thread model: single
Supported LTO compression algorithms: zlib
gcc version 12.1.0 (Buildroot 2022.05-118-ge052166011-dirty)
COLLECT_GCC_OPTIONS='--sysroot=/home/buildroot/buildroot/output/per-package/libbpf/host/x86_64-buildroot-linux-gnu/sysroot'
'-Winline' '-O2' '-mframe-limit=32767' '-mco-re' '-gbtf' '-std=gnu17'
'-v' '-D' '__x86_64__' '-mlittle-endian' '-I'
'/home/buildroot/buildroot/output/per-package/libbpf/host/x86_64-buildroot-linux-gnu/sysroot/usr/include'
'-o' 'minimal.bpf.o' '-dumpdir' 'minimal.bpf.o-'
/home/buildroot/buildroot/output/per-package/libbpf/host/bin/../libexec/gcc/bpf-buildroot-none/12.1.0/cc1
-quiet -v -I /home/buildroot/buildroot/output/per-package/libbpf/host/x86_64-buildroot-linux-gnu/sysroot/usr/include
-iprefix /home/buildroot/buildroot/output/per-package/libbpf/host/bin/../lib/gcc/bpf-buildroot-none/12.1.0/
-isysroot /home/buildroot/buildroot/output/per-package/libbpf/host/x86_64-buildroot-linux-gnu/sysroot
-D __x86_64__ minimal.bpf.c -quiet -dumpdir minimal.bpf.o- -dumpbase
minimal.bpf.c -dumpbase-ext .c -mframe-limit=32767 -mco-re
-mlittle-endian -gbtf -O2 -Winline -std=gnu17 -version -o
/tmp/cct4AXvg.s
GNU C17 (Buildroot 2022.05-118-ge052166011-dirty) version 12.1.0
(bpf-buildroot-none)
compiled by GNU C version 12.1.0, GMP version 6.2.1, MPFR version
4.1.0, MPC version 1.2.1, isl version none
GGC heuristics: --param ggc-min-expand=100 --param ggc-min-heapsize=131072
ignoring nonexistent directory
"/home/buildroot/buildroot/output/per-package/libbpf/host/bin/../lib/gcc/bpf-buildroot-none/12.1.0/../../../../bpf-buildroot-none/sys-include"
ignoring nonexistent directory
"/home/buildroot/buildroot/output/per-package/libbpf/host/bin/../lib/gcc/bpf-buildroot-none/12.1.0/../../../../bpf-buildroot-none/include"
ignoring duplicate directory
"/home/buildroot/buildroot/output/per-package/libbpf/host/bin/../lib/gcc/../../lib/gcc/bpf-buildroot-none/12.1.0/include"
ignoring duplicate directory
"/home/buildroot/buildroot/output/per-package/libbpf/host/bin/../lib/gcc/../../lib/gcc/bpf-buildroot-none/12.1.0/include-fixed"
ignoring nonexistent directory
"/home/buildroot/buildroot/output/per-package/libbpf/host/bin/../lib/gcc/../../lib/gcc/bpf-buildroot-none/12.1.0/../../../../bpf-buildroot-none/sys-include"
ignoring nonexistent directory
"/home/buildroot/buildroot/output/per-package/libbpf/host/bin/../lib/gcc/../../lib/gcc/bpf-buildroot-none/12.1.0/../../../../bpf-buildroot-none/include"
#include "..." search starts here:
#include <...> search starts here:
/home/buildroot/buildroot/output/per-package/libbpf/host/x86_64-buildroot-linux-gnu/sysroot/usr/include
/home/buildroot/buildroot/output/per-package/libbpf/host/bin/../lib/gcc/bpf-buildroot-none/12.1.0/include
/home/buildroot/buildroot/output/per-package/libbpf/host/bin/../lib/gcc/bpf-buildroot-none/12.1.0/include-fixed
End of search list.
GNU C17 (Buildroot 2022.05-118-ge052166011-dirty) version 12.1.0
(bpf-buildroot-none)
compiled by GNU C version 12.1.0, GMP version 6.2.1, MPFR version
4.1.0, MPC version 1.2.1, isl version none
GGC heuristics: --param ggc-min-expand=100 --param ggc-min-heapsize=131072
Compiler executable checksum: 9bf241ca1a2dd4ffd7652c5e247c9be8
minimal.bpf.c:6:1: error: expected '=', ',', ';', 'asm' or
'__attribute__' before '#pragma'
6 | char LICENSE[] SEC("license") = "Dual BSD/GPL";
| ^~~
minimal.bpf.c:6:1: error: expected identifier or '(' before '#pragma'
minimal.bpf.c:10:1: error: expected identifier or '(' before '#pragma'
10 | SEC("tp/syscalls/sys_enter_write")
| ^~~

>
> > /* Avoid 'linux/stddef.h' definition of '__always_inline'. */
> > #undef __always_inline
> > --
> > 2.25.1
> >

2022-06-28 00:16:33

by Andrii Nakryiko

[permalink] [raw]
Subject: Re: [PATCH v3 1/1] libbpf: fix broken gcc SEC pragma macro

On Thu, Jun 9, 2022 at 4:27 PM James Hilliard <[email protected]> wrote:
>
> On Thu, Jun 9, 2022 at 12:13 PM Andrii Nakryiko
> <[email protected]> wrote:
> >
> > On Wed, Jun 8, 2022 at 11:24 PM James Hilliard
> > <[email protected]> wrote:
> > >
> > > It seems the gcc preprocessor breaks unless pragmas are wrapped
> > > individually inside macros when surrounding __attribute__.
> > >
> > > Fixes errors like:
> > > error: expected identifier or '(' before '#pragma'
> > > 106 | SEC("cgroup/bind6")
> > > | ^~~
> > >
> > > error: expected '=', ',', ';', 'asm' or '__attribute__' before '#pragma'
> > > 114 | char _license[] SEC("license") = "GPL";
> > > | ^~~
> > >
> > > Signed-off-by: James Hilliard <[email protected]>
> > > ---
> > > Changes v2 -> v3:
> > > - just fix SEC pragma
> > > Changes v1 -> v2:
> > > - replace typeof with __typeof__ instead of changing pragma macros
> > > ---
> > > tools/lib/bpf/bpf_helpers.h | 7 ++++---
> > > 1 file changed, 4 insertions(+), 3 deletions(-)
> > >
> > > diff --git a/tools/lib/bpf/bpf_helpers.h b/tools/lib/bpf/bpf_helpers.h
> > > index fb04eaf367f1..66d23c47c206 100644
> > > --- a/tools/lib/bpf/bpf_helpers.h
> > > +++ b/tools/lib/bpf/bpf_helpers.h
> > > @@ -22,11 +22,12 @@
> > > * To allow use of SEC() with externs (e.g., for extern .maps declarations),
> > > * make sure __attribute__((unused)) doesn't trigger compilation warning.
> > > */
> > > +#define DO_PRAGMA(x) _Pragma(#x)
> > > #define SEC(name) \
> > > - _Pragma("GCC diagnostic push") \
> > > - _Pragma("GCC diagnostic ignored \"-Wignored-attributes\"") \
> > > + DO_PRAGMA("GCC diagnostic push") \
> > > + DO_PRAGMA("GCC diagnostic ignored \"-Wignored-attributes\"") \
> > > __attribute__((section(name), used)) \
> > > - _Pragma("GCC diagnostic pop") \
> > > + DO_PRAGMA("GCC diagnostic pop") \
> > >
> >
> > I'm not going to accept this unless I can repro it in the first place.
> > Using -std=c17 doesn't trigger such issue. Please provide the repro
> > first. Building systemd is not a repro, unfortunately. Please try to
> > do it based on libbpf-bootstrap ([0])
> >
> > [0] https://github.com/libbpf/libbpf-bootstrap
>
> Seems to reproduce just fine already there with:
> https://github.com/libbpf/libbpf-bootstrap/blob/31face36d469a0e3e4c4ac1cafc66747d3150930/examples/c/minimal.bpf.c
>
> See here:
> $ /home/buildroot/buildroot/output/per-package/libbpf/host/bin/bpf-gcc
> -Winline -O2 -mframe-limit=32767 -mco-re -gbtf -std=gnu17 -v
> -D__x86_64__ -mlittle-endian -I
> /home/buildroot/buildroot/output/per-package/libbpf/host/x86_64-buildroot-linux-gnu/sysroot/usr/include
> minimal.bpf.c -o minimal.bpf.o
> Using built-in specs.
> COLLECT_GCC=/home/buildroot/buildroot/output/per-package/libbpf/host/bin/bpf-gcc.br_real
> COLLECT_LTO_WRAPPER=/home/buildroot/buildroot/output/per-package/libbpf/host/bin/../libexec/gcc/bpf-buildroot-none/12.1.0/lto-wrapper
> Target: bpf-buildroot-none
> Configured with: ./configure
> --prefix=/home/buildroot/buildroot/output/per-package/host-gcc-bpf/host
> --sysconfdir=/home/buildroot/buildroot/output/per-package/host-gcc-bpf/host/etc
> --localstatedir=/home/buildroot/buildroot/output/per-package/host-gcc-bpf/host/var
> --enable-shared --disable-static --disable-gtk-doc
> --disable-gtk-doc-html --disable-doc --disable-docs
> --disable-documentation --disable-debug --with-xmlto=no --with-fop=no
> --disable-nls --disable-dependency-tracking
> --target=bpf-buildroot-none
> --prefix=/home/buildroot/buildroot/output/per-package/host-gcc-bpf/host
> --sysconfdir=/home/buildroot/buildroot/output/per-package/host-gcc-bpf/host/etc
> --enable-languages=c --with-gnu-ld --enable-static
> --disable-decimal-float --disable-gcov --disable-libssp
> --disable-multilib --disable-shared
> --with-gmp=/home/buildroot/buildroot/output/per-package/host-gcc-bpf/host
> --with-mpc=/home/buildroot/buildroot/output/per-package/host-gcc-bpf/host
> --with-mpfr=/home/buildroot/buildroot/output/per-package/host-gcc-bpf/host
> --with-pkgversion='Buildroot 2022.05-118-ge052166011-dirty'
> --with-bugurl=http://bugs.buildroot.net/ --without-zstd --without-isl
> --without-cloog
> Thread model: single
> Supported LTO compression algorithms: zlib
> gcc version 12.1.0 (Buildroot 2022.05-118-ge052166011-dirty)
> COLLECT_GCC_OPTIONS='--sysroot=/home/buildroot/buildroot/output/per-package/libbpf/host/x86_64-buildroot-linux-gnu/sysroot'
> '-Winline' '-O2' '-mframe-limit=32767' '-mco-re' '-gbtf' '-std=gnu17'
> '-v' '-D' '__x86_64__' '-mlittle-endian' '-I'
> '/home/buildroot/buildroot/output/per-package/libbpf/host/x86_64-buildroot-linux-gnu/sysroot/usr/include'
> '-o' 'minimal.bpf.o' '-dumpdir' 'minimal.bpf.o-'
> /home/buildroot/buildroot/output/per-package/libbpf/host/bin/../libexec/gcc/bpf-buildroot-none/12.1.0/cc1
> -quiet -v -I /home/buildroot/buildroot/output/per-package/libbpf/host/x86_64-buildroot-linux-gnu/sysroot/usr/include
> -iprefix /home/buildroot/buildroot/output/per-package/libbpf/host/bin/../lib/gcc/bpf-buildroot-none/12.1.0/
> -isysroot /home/buildroot/buildroot/output/per-package/libbpf/host/x86_64-buildroot-linux-gnu/sysroot
> -D __x86_64__ minimal.bpf.c -quiet -dumpdir minimal.bpf.o- -dumpbase
> minimal.bpf.c -dumpbase-ext .c -mframe-limit=32767 -mco-re
> -mlittle-endian -gbtf -O2 -Winline -std=gnu17 -version -o
> /tmp/cct4AXvg.s
> GNU C17 (Buildroot 2022.05-118-ge052166011-dirty) version 12.1.0
> (bpf-buildroot-none)
> compiled by GNU C version 12.1.0, GMP version 6.2.1, MPFR version
> 4.1.0, MPC version 1.2.1, isl version none
> GGC heuristics: --param ggc-min-expand=100 --param ggc-min-heapsize=131072
> ignoring nonexistent directory
> "/home/buildroot/buildroot/output/per-package/libbpf/host/bin/../lib/gcc/bpf-buildroot-none/12.1.0/../../../../bpf-buildroot-none/sys-include"
> ignoring nonexistent directory
> "/home/buildroot/buildroot/output/per-package/libbpf/host/bin/../lib/gcc/bpf-buildroot-none/12.1.0/../../../../bpf-buildroot-none/include"
> ignoring duplicate directory
> "/home/buildroot/buildroot/output/per-package/libbpf/host/bin/../lib/gcc/../../lib/gcc/bpf-buildroot-none/12.1.0/include"
> ignoring duplicate directory
> "/home/buildroot/buildroot/output/per-package/libbpf/host/bin/../lib/gcc/../../lib/gcc/bpf-buildroot-none/12.1.0/include-fixed"
> ignoring nonexistent directory
> "/home/buildroot/buildroot/output/per-package/libbpf/host/bin/../lib/gcc/../../lib/gcc/bpf-buildroot-none/12.1.0/../../../../bpf-buildroot-none/sys-include"
> ignoring nonexistent directory
> "/home/buildroot/buildroot/output/per-package/libbpf/host/bin/../lib/gcc/../../lib/gcc/bpf-buildroot-none/12.1.0/../../../../bpf-buildroot-none/include"
> #include "..." search starts here:
> #include <...> search starts here:
> /home/buildroot/buildroot/output/per-package/libbpf/host/x86_64-buildroot-linux-gnu/sysroot/usr/include
> /home/buildroot/buildroot/output/per-package/libbpf/host/bin/../lib/gcc/bpf-buildroot-none/12.1.0/include
> /home/buildroot/buildroot/output/per-package/libbpf/host/bin/../lib/gcc/bpf-buildroot-none/12.1.0/include-fixed
> End of search list.
> GNU C17 (Buildroot 2022.05-118-ge052166011-dirty) version 12.1.0
> (bpf-buildroot-none)
> compiled by GNU C version 12.1.0, GMP version 6.2.1, MPFR version
> 4.1.0, MPC version 1.2.1, isl version none
> GGC heuristics: --param ggc-min-expand=100 --param ggc-min-heapsize=131072
> Compiler executable checksum: 9bf241ca1a2dd4ffd7652c5e247c9be8
> minimal.bpf.c:6:1: error: expected '=', ',', ';', 'asm' or
> '__attribute__' before '#pragma'
> 6 | char LICENSE[] SEC("license") = "Dual BSD/GPL";
> | ^~~
> minimal.bpf.c:6:1: error: expected identifier or '(' before '#pragma'
> minimal.bpf.c:10:1: error: expected identifier or '(' before '#pragma'
> 10 | SEC("tp/syscalls/sys_enter_write")
> | ^~~

So this is a bug (hard to call this a feature) in gcc (not even
bpf-gcc, I could repro with a simple gcc). Is there a bug reported for
this somewhere? Are GCC folks aware and working on the fix?

What's curious is that the only thing that allows to bypass this is
adding #x in macro, having #define DO_PRAGMA(x) _Pragma(x) doesn't
help.

So ideally GCC can fix this? But either way your patch as is
erroneously passing extra quoted strings to _Pragma().

I'm pondering whether it's just cleaner to define SEC() without
pragmas for GCC? It will only cause compiler warning about unnecessary
unused attribute for extern *variable* declarations, which are very
rare. Instead of relying on this quirky "fix" approach. Ideally,
though, GCC just fixes _Pragma() handling, of course.

>
> >
> > > /* Avoid 'linux/stddef.h' definition of '__always_inline'. */
> > > #undef __always_inline
> > > --
> > > 2.25.1
> > >

2022-06-28 04:50:22

by James Hilliard

[permalink] [raw]
Subject: Re: [PATCH v3 1/1] libbpf: fix broken gcc SEC pragma macro

On Mon, Jun 27, 2022 at 5:16 PM Andrii Nakryiko
<[email protected]> wrote:
>
> On Thu, Jun 9, 2022 at 4:27 PM James Hilliard <[email protected]> wrote:
> >
> > On Thu, Jun 9, 2022 at 12:13 PM Andrii Nakryiko
> > <[email protected]> wrote:
> > >
> > > On Wed, Jun 8, 2022 at 11:24 PM James Hilliard
> > > <[email protected]> wrote:
> > > >
> > > > It seems the gcc preprocessor breaks unless pragmas are wrapped
> > > > individually inside macros when surrounding __attribute__.
> > > >
> > > > Fixes errors like:
> > > > error: expected identifier or '(' before '#pragma'
> > > > 106 | SEC("cgroup/bind6")
> > > > | ^~~
> > > >
> > > > error: expected '=', ',', ';', 'asm' or '__attribute__' before '#pragma'
> > > > 114 | char _license[] SEC("license") = "GPL";
> > > > | ^~~
> > > >
> > > > Signed-off-by: James Hilliard <[email protected]>
> > > > ---
> > > > Changes v2 -> v3:
> > > > - just fix SEC pragma
> > > > Changes v1 -> v2:
> > > > - replace typeof with __typeof__ instead of changing pragma macros
> > > > ---
> > > > tools/lib/bpf/bpf_helpers.h | 7 ++++---
> > > > 1 file changed, 4 insertions(+), 3 deletions(-)
> > > >
> > > > diff --git a/tools/lib/bpf/bpf_helpers.h b/tools/lib/bpf/bpf_helpers.h
> > > > index fb04eaf367f1..66d23c47c206 100644
> > > > --- a/tools/lib/bpf/bpf_helpers.h
> > > > +++ b/tools/lib/bpf/bpf_helpers.h
> > > > @@ -22,11 +22,12 @@
> > > > * To allow use of SEC() with externs (e.g., for extern .maps declarations),
> > > > * make sure __attribute__((unused)) doesn't trigger compilation warning.
> > > > */
> > > > +#define DO_PRAGMA(x) _Pragma(#x)
> > > > #define SEC(name) \
> > > > - _Pragma("GCC diagnostic push") \
> > > > - _Pragma("GCC diagnostic ignored \"-Wignored-attributes\"") \
> > > > + DO_PRAGMA("GCC diagnostic push") \
> > > > + DO_PRAGMA("GCC diagnostic ignored \"-Wignored-attributes\"") \
> > > > __attribute__((section(name), used)) \
> > > > - _Pragma("GCC diagnostic pop") \
> > > > + DO_PRAGMA("GCC diagnostic pop") \
> > > >
> > >
> > > I'm not going to accept this unless I can repro it in the first place.
> > > Using -std=c17 doesn't trigger such issue. Please provide the repro
> > > first. Building systemd is not a repro, unfortunately. Please try to
> > > do it based on libbpf-bootstrap ([0])
> > >
> > > [0] https://github.com/libbpf/libbpf-bootstrap
> >
> > Seems to reproduce just fine already there with:
> > https://github.com/libbpf/libbpf-bootstrap/blob/31face36d469a0e3e4c4ac1cafc66747d3150930/examples/c/minimal.bpf.c
> >
> > See here:
> > $ /home/buildroot/buildroot/output/per-package/libbpf/host/bin/bpf-gcc
> > -Winline -O2 -mframe-limit=32767 -mco-re -gbtf -std=gnu17 -v
> > -D__x86_64__ -mlittle-endian -I
> > /home/buildroot/buildroot/output/per-package/libbpf/host/x86_64-buildroot-linux-gnu/sysroot/usr/include
> > minimal.bpf.c -o minimal.bpf.o
> > Using built-in specs.
> > COLLECT_GCC=/home/buildroot/buildroot/output/per-package/libbpf/host/bin/bpf-gcc.br_real
> > COLLECT_LTO_WRAPPER=/home/buildroot/buildroot/output/per-package/libbpf/host/bin/../libexec/gcc/bpf-buildroot-none/12.1.0/lto-wrapper
> > Target: bpf-buildroot-none
> > Configured with: ./configure
> > --prefix=/home/buildroot/buildroot/output/per-package/host-gcc-bpf/host
> > --sysconfdir=/home/buildroot/buildroot/output/per-package/host-gcc-bpf/host/etc
> > --localstatedir=/home/buildroot/buildroot/output/per-package/host-gcc-bpf/host/var
> > --enable-shared --disable-static --disable-gtk-doc
> > --disable-gtk-doc-html --disable-doc --disable-docs
> > --disable-documentation --disable-debug --with-xmlto=no --with-fop=no
> > --disable-nls --disable-dependency-tracking
> > --target=bpf-buildroot-none
> > --prefix=/home/buildroot/buildroot/output/per-package/host-gcc-bpf/host
> > --sysconfdir=/home/buildroot/buildroot/output/per-package/host-gcc-bpf/host/etc
> > --enable-languages=c --with-gnu-ld --enable-static
> > --disable-decimal-float --disable-gcov --disable-libssp
> > --disable-multilib --disable-shared
> > --with-gmp=/home/buildroot/buildroot/output/per-package/host-gcc-bpf/host
> > --with-mpc=/home/buildroot/buildroot/output/per-package/host-gcc-bpf/host
> > --with-mpfr=/home/buildroot/buildroot/output/per-package/host-gcc-bpf/host
> > --with-pkgversion='Buildroot 2022.05-118-ge052166011-dirty'
> > --with-bugurl=http://bugs.buildroot.net/ --without-zstd --without-isl
> > --without-cloog
> > Thread model: single
> > Supported LTO compression algorithms: zlib
> > gcc version 12.1.0 (Buildroot 2022.05-118-ge052166011-dirty)
> > COLLECT_GCC_OPTIONS='--sysroot=/home/buildroot/buildroot/output/per-package/libbpf/host/x86_64-buildroot-linux-gnu/sysroot'
> > '-Winline' '-O2' '-mframe-limit=32767' '-mco-re' '-gbtf' '-std=gnu17'
> > '-v' '-D' '__x86_64__' '-mlittle-endian' '-I'
> > '/home/buildroot/buildroot/output/per-package/libbpf/host/x86_64-buildroot-linux-gnu/sysroot/usr/include'
> > '-o' 'minimal.bpf.o' '-dumpdir' 'minimal.bpf.o-'
> > /home/buildroot/buildroot/output/per-package/libbpf/host/bin/../libexec/gcc/bpf-buildroot-none/12.1.0/cc1
> > -quiet -v -I /home/buildroot/buildroot/output/per-package/libbpf/host/x86_64-buildroot-linux-gnu/sysroot/usr/include
> > -iprefix /home/buildroot/buildroot/output/per-package/libbpf/host/bin/../lib/gcc/bpf-buildroot-none/12.1.0/
> > -isysroot /home/buildroot/buildroot/output/per-package/libbpf/host/x86_64-buildroot-linux-gnu/sysroot
> > -D __x86_64__ minimal.bpf.c -quiet -dumpdir minimal.bpf.o- -dumpbase
> > minimal.bpf.c -dumpbase-ext .c -mframe-limit=32767 -mco-re
> > -mlittle-endian -gbtf -O2 -Winline -std=gnu17 -version -o
> > /tmp/cct4AXvg.s
> > GNU C17 (Buildroot 2022.05-118-ge052166011-dirty) version 12.1.0
> > (bpf-buildroot-none)
> > compiled by GNU C version 12.1.0, GMP version 6.2.1, MPFR version
> > 4.1.0, MPC version 1.2.1, isl version none
> > GGC heuristics: --param ggc-min-expand=100 --param ggc-min-heapsize=131072
> > ignoring nonexistent directory
> > "/home/buildroot/buildroot/output/per-package/libbpf/host/bin/../lib/gcc/bpf-buildroot-none/12.1.0/../../../../bpf-buildroot-none/sys-include"
> > ignoring nonexistent directory
> > "/home/buildroot/buildroot/output/per-package/libbpf/host/bin/../lib/gcc/bpf-buildroot-none/12.1.0/../../../../bpf-buildroot-none/include"
> > ignoring duplicate directory
> > "/home/buildroot/buildroot/output/per-package/libbpf/host/bin/../lib/gcc/../../lib/gcc/bpf-buildroot-none/12.1.0/include"
> > ignoring duplicate directory
> > "/home/buildroot/buildroot/output/per-package/libbpf/host/bin/../lib/gcc/../../lib/gcc/bpf-buildroot-none/12.1.0/include-fixed"
> > ignoring nonexistent directory
> > "/home/buildroot/buildroot/output/per-package/libbpf/host/bin/../lib/gcc/../../lib/gcc/bpf-buildroot-none/12.1.0/../../../../bpf-buildroot-none/sys-include"
> > ignoring nonexistent directory
> > "/home/buildroot/buildroot/output/per-package/libbpf/host/bin/../lib/gcc/../../lib/gcc/bpf-buildroot-none/12.1.0/../../../../bpf-buildroot-none/include"
> > #include "..." search starts here:
> > #include <...> search starts here:
> > /home/buildroot/buildroot/output/per-package/libbpf/host/x86_64-buildroot-linux-gnu/sysroot/usr/include
> > /home/buildroot/buildroot/output/per-package/libbpf/host/bin/../lib/gcc/bpf-buildroot-none/12.1.0/include
> > /home/buildroot/buildroot/output/per-package/libbpf/host/bin/../lib/gcc/bpf-buildroot-none/12.1.0/include-fixed
> > End of search list.
> > GNU C17 (Buildroot 2022.05-118-ge052166011-dirty) version 12.1.0
> > (bpf-buildroot-none)
> > compiled by GNU C version 12.1.0, GMP version 6.2.1, MPFR version
> > 4.1.0, MPC version 1.2.1, isl version none
> > GGC heuristics: --param ggc-min-expand=100 --param ggc-min-heapsize=131072
> > Compiler executable checksum: 9bf241ca1a2dd4ffd7652c5e247c9be8
> > minimal.bpf.c:6:1: error: expected '=', ',', ';', 'asm' or
> > '__attribute__' before '#pragma'
> > 6 | char LICENSE[] SEC("license") = "Dual BSD/GPL";
> > | ^~~
> > minimal.bpf.c:6:1: error: expected identifier or '(' before '#pragma'
> > minimal.bpf.c:10:1: error: expected identifier or '(' before '#pragma'
> > 10 | SEC("tp/syscalls/sys_enter_write")
> > | ^~~
>
> So this is a bug (hard to call this a feature) in gcc (not even
> bpf-gcc, I could repro with a simple gcc). Is there a bug reported for
> this somewhere? Are GCC folks aware and working on the fix?

Yeah, saw a few issues that looked relevant:
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=55578
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=90400

>
> What's curious is that the only thing that allows to bypass this is
> adding #x in macro, having #define DO_PRAGMA(x) _Pragma(x) doesn't
> help.
>
> So ideally GCC can fix this?

From the reported issues...it doesn't sound like a fix is going to be
coming all that
soon in GCC.

> But either way your patch as is
> erroneously passing extra quoted strings to _Pragma().

I recall the extra quotes were needed to make this work, does it work for you
without them?

>
> I'm pondering whether it's just cleaner to define SEC() without
> pragmas for GCC? It will only cause compiler warning about unnecessary
> unused attribute for extern *variable* declarations, which are very
> rare. Instead of relying on this quirky "fix" approach. Ideally,
> though, GCC just fixes _Pragma() handling, of course.

I mean, as long as this workaround is reliable I'd say using it is the
best option
for backwards compatibility, especially since it's only needed in one place from
the looks of it.

>
> >
> > >
> > > > /* Avoid 'linux/stddef.h' definition of '__always_inline'. */
> > > > #undef __always_inline
> > > > --
> > > > 2.25.1
> > > >

2022-06-30 22:32:49

by Andrii Nakryiko

[permalink] [raw]
Subject: Re: [PATCH v3 1/1] libbpf: fix broken gcc SEC pragma macro

On Mon, Jun 27, 2022 at 9:43 PM James Hilliard
<[email protected]> wrote:
>
> On Mon, Jun 27, 2022 at 5:16 PM Andrii Nakryiko
> <[email protected]> wrote:
> >
> > On Thu, Jun 9, 2022 at 4:27 PM James Hilliard <[email protected]> wrote:
> > >
> > > On Thu, Jun 9, 2022 at 12:13 PM Andrii Nakryiko
> > > <[email protected]> wrote:
> > > >
> > > > On Wed, Jun 8, 2022 at 11:24 PM James Hilliard
> > > > <[email protected]> wrote:
> > > > >
> > > > > It seems the gcc preprocessor breaks unless pragmas are wrapped
> > > > > individually inside macros when surrounding __attribute__.
> > > > >
> > > > > Fixes errors like:
> > > > > error: expected identifier or '(' before '#pragma'
> > > > > 106 | SEC("cgroup/bind6")
> > > > > | ^~~
> > > > >
> > > > > error: expected '=', ',', ';', 'asm' or '__attribute__' before '#pragma'
> > > > > 114 | char _license[] SEC("license") = "GPL";
> > > > > | ^~~
> > > > >
> > > > > Signed-off-by: James Hilliard <[email protected]>
> > > > > ---
> > > > > Changes v2 -> v3:
> > > > > - just fix SEC pragma
> > > > > Changes v1 -> v2:
> > > > > - replace typeof with __typeof__ instead of changing pragma macros
> > > > > ---
> > > > > tools/lib/bpf/bpf_helpers.h | 7 ++++---
> > > > > 1 file changed, 4 insertions(+), 3 deletions(-)
> > > > >
> > > > > diff --git a/tools/lib/bpf/bpf_helpers.h b/tools/lib/bpf/bpf_helpers.h
> > > > > index fb04eaf367f1..66d23c47c206 100644
> > > > > --- a/tools/lib/bpf/bpf_helpers.h
> > > > > +++ b/tools/lib/bpf/bpf_helpers.h
> > > > > @@ -22,11 +22,12 @@
> > > > > * To allow use of SEC() with externs (e.g., for extern .maps declarations),
> > > > > * make sure __attribute__((unused)) doesn't trigger compilation warning.
> > > > > */
> > > > > +#define DO_PRAGMA(x) _Pragma(#x)
> > > > > #define SEC(name) \
> > > > > - _Pragma("GCC diagnostic push") \
> > > > > - _Pragma("GCC diagnostic ignored \"-Wignored-attributes\"") \
> > > > > + DO_PRAGMA("GCC diagnostic push") \
> > > > > + DO_PRAGMA("GCC diagnostic ignored \"-Wignored-attributes\"") \
> > > > > __attribute__((section(name), used)) \
> > > > > - _Pragma("GCC diagnostic pop") \
> > > > > + DO_PRAGMA("GCC diagnostic pop") \
> > > > >
> > > >
> > > > I'm not going to accept this unless I can repro it in the first place.
> > > > Using -std=c17 doesn't trigger such issue. Please provide the repro
> > > > first. Building systemd is not a repro, unfortunately. Please try to
> > > > do it based on libbpf-bootstrap ([0])
> > > >
> > > > [0] https://github.com/libbpf/libbpf-bootstrap
> > >
> > > Seems to reproduce just fine already there with:
> > > https://github.com/libbpf/libbpf-bootstrap/blob/31face36d469a0e3e4c4ac1cafc66747d3150930/examples/c/minimal.bpf.c
> > >
> > > See here:
> > > $ /home/buildroot/buildroot/output/per-package/libbpf/host/bin/bpf-gcc
> > > -Winline -O2 -mframe-limit=32767 -mco-re -gbtf -std=gnu17 -v
> > > -D__x86_64__ -mlittle-endian -I
> > > /home/buildroot/buildroot/output/per-package/libbpf/host/x86_64-buildroot-linux-gnu/sysroot/usr/include
> > > minimal.bpf.c -o minimal.bpf.o
> > > Using built-in specs.
> > > COLLECT_GCC=/home/buildroot/buildroot/output/per-package/libbpf/host/bin/bpf-gcc.br_real
> > > COLLECT_LTO_WRAPPER=/home/buildroot/buildroot/output/per-package/libbpf/host/bin/../libexec/gcc/bpf-buildroot-none/12.1.0/lto-wrapper
> > > Target: bpf-buildroot-none
> > > Configured with: ./configure
> > > --prefix=/home/buildroot/buildroot/output/per-package/host-gcc-bpf/host
> > > --sysconfdir=/home/buildroot/buildroot/output/per-package/host-gcc-bpf/host/etc
> > > --localstatedir=/home/buildroot/buildroot/output/per-package/host-gcc-bpf/host/var
> > > --enable-shared --disable-static --disable-gtk-doc
> > > --disable-gtk-doc-html --disable-doc --disable-docs
> > > --disable-documentation --disable-debug --with-xmlto=no --with-fop=no
> > > --disable-nls --disable-dependency-tracking
> > > --target=bpf-buildroot-none
> > > --prefix=/home/buildroot/buildroot/output/per-package/host-gcc-bpf/host
> > > --sysconfdir=/home/buildroot/buildroot/output/per-package/host-gcc-bpf/host/etc
> > > --enable-languages=c --with-gnu-ld --enable-static
> > > --disable-decimal-float --disable-gcov --disable-libssp
> > > --disable-multilib --disable-shared
> > > --with-gmp=/home/buildroot/buildroot/output/per-package/host-gcc-bpf/host
> > > --with-mpc=/home/buildroot/buildroot/output/per-package/host-gcc-bpf/host
> > > --with-mpfr=/home/buildroot/buildroot/output/per-package/host-gcc-bpf/host
> > > --with-pkgversion='Buildroot 2022.05-118-ge052166011-dirty'
> > > --with-bugurl=http://bugs.buildroot.net/ --without-zstd --without-isl
> > > --without-cloog
> > > Thread model: single
> > > Supported LTO compression algorithms: zlib
> > > gcc version 12.1.0 (Buildroot 2022.05-118-ge052166011-dirty)
> > > COLLECT_GCC_OPTIONS='--sysroot=/home/buildroot/buildroot/output/per-package/libbpf/host/x86_64-buildroot-linux-gnu/sysroot'
> > > '-Winline' '-O2' '-mframe-limit=32767' '-mco-re' '-gbtf' '-std=gnu17'
> > > '-v' '-D' '__x86_64__' '-mlittle-endian' '-I'
> > > '/home/buildroot/buildroot/output/per-package/libbpf/host/x86_64-buildroot-linux-gnu/sysroot/usr/include'
> > > '-o' 'minimal.bpf.o' '-dumpdir' 'minimal.bpf.o-'
> > > /home/buildroot/buildroot/output/per-package/libbpf/host/bin/../libexec/gcc/bpf-buildroot-none/12.1.0/cc1
> > > -quiet -v -I /home/buildroot/buildroot/output/per-package/libbpf/host/x86_64-buildroot-linux-gnu/sysroot/usr/include
> > > -iprefix /home/buildroot/buildroot/output/per-package/libbpf/host/bin/../lib/gcc/bpf-buildroot-none/12.1.0/
> > > -isysroot /home/buildroot/buildroot/output/per-package/libbpf/host/x86_64-buildroot-linux-gnu/sysroot
> > > -D __x86_64__ minimal.bpf.c -quiet -dumpdir minimal.bpf.o- -dumpbase
> > > minimal.bpf.c -dumpbase-ext .c -mframe-limit=32767 -mco-re
> > > -mlittle-endian -gbtf -O2 -Winline -std=gnu17 -version -o
> > > /tmp/cct4AXvg.s
> > > GNU C17 (Buildroot 2022.05-118-ge052166011-dirty) version 12.1.0
> > > (bpf-buildroot-none)
> > > compiled by GNU C version 12.1.0, GMP version 6.2.1, MPFR version
> > > 4.1.0, MPC version 1.2.1, isl version none
> > > GGC heuristics: --param ggc-min-expand=100 --param ggc-min-heapsize=131072
> > > ignoring nonexistent directory
> > > "/home/buildroot/buildroot/output/per-package/libbpf/host/bin/../lib/gcc/bpf-buildroot-none/12.1.0/../../../../bpf-buildroot-none/sys-include"
> > > ignoring nonexistent directory
> > > "/home/buildroot/buildroot/output/per-package/libbpf/host/bin/../lib/gcc/bpf-buildroot-none/12.1.0/../../../../bpf-buildroot-none/include"
> > > ignoring duplicate directory
> > > "/home/buildroot/buildroot/output/per-package/libbpf/host/bin/../lib/gcc/../../lib/gcc/bpf-buildroot-none/12.1.0/include"
> > > ignoring duplicate directory
> > > "/home/buildroot/buildroot/output/per-package/libbpf/host/bin/../lib/gcc/../../lib/gcc/bpf-buildroot-none/12.1.0/include-fixed"
> > > ignoring nonexistent directory
> > > "/home/buildroot/buildroot/output/per-package/libbpf/host/bin/../lib/gcc/../../lib/gcc/bpf-buildroot-none/12.1.0/../../../../bpf-buildroot-none/sys-include"
> > > ignoring nonexistent directory
> > > "/home/buildroot/buildroot/output/per-package/libbpf/host/bin/../lib/gcc/../../lib/gcc/bpf-buildroot-none/12.1.0/../../../../bpf-buildroot-none/include"
> > > #include "..." search starts here:
> > > #include <...> search starts here:
> > > /home/buildroot/buildroot/output/per-package/libbpf/host/x86_64-buildroot-linux-gnu/sysroot/usr/include
> > > /home/buildroot/buildroot/output/per-package/libbpf/host/bin/../lib/gcc/bpf-buildroot-none/12.1.0/include
> > > /home/buildroot/buildroot/output/per-package/libbpf/host/bin/../lib/gcc/bpf-buildroot-none/12.1.0/include-fixed
> > > End of search list.
> > > GNU C17 (Buildroot 2022.05-118-ge052166011-dirty) version 12.1.0
> > > (bpf-buildroot-none)
> > > compiled by GNU C version 12.1.0, GMP version 6.2.1, MPFR version
> > > 4.1.0, MPC version 1.2.1, isl version none
> > > GGC heuristics: --param ggc-min-expand=100 --param ggc-min-heapsize=131072
> > > Compiler executable checksum: 9bf241ca1a2dd4ffd7652c5e247c9be8
> > > minimal.bpf.c:6:1: error: expected '=', ',', ';', 'asm' or
> > > '__attribute__' before '#pragma'
> > > 6 | char LICENSE[] SEC("license") = "Dual BSD/GPL";
> > > | ^~~
> > > minimal.bpf.c:6:1: error: expected identifier or '(' before '#pragma'
> > > minimal.bpf.c:10:1: error: expected identifier or '(' before '#pragma'
> > > 10 | SEC("tp/syscalls/sys_enter_write")
> > > | ^~~
> >
> > So this is a bug (hard to call this a feature) in gcc (not even
> > bpf-gcc, I could repro with a simple gcc). Is there a bug reported for
> > this somewhere? Are GCC folks aware and working on the fix?
>
> Yeah, saw a few issues that looked relevant:
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=55578
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=90400
>
> >
> > What's curious is that the only thing that allows to bypass this is
> > adding #x in macro, having #define DO_PRAGMA(x) _Pragma(x) doesn't
> > help.
> >
> > So ideally GCC can fix this?
>
> From the reported issues...it doesn't sound like a fix is going to be
> coming all that
> soon in GCC.
>
> > But either way your patch as is
> > erroneously passing extra quoted strings to _Pragma().
>
> I recall the extra quotes were needed to make this work, does it work for you
> without them?
>
> >
> > I'm pondering whether it's just cleaner to define SEC() without
> > pragmas for GCC? It will only cause compiler warning about unnecessary
> > unused attribute for extern *variable* declarations, which are very
> > rare. Instead of relying on this quirky "fix" approach. Ideally,
> > though, GCC just fixes _Pragma() handling, of course.
>
> I mean, as long as this workaround is reliable I'd say using it is the
> best option
> for backwards compatibility, especially since it's only needed in one place from
> the looks of it.

Is it reliable, though? Adding those quotes breaks Clang (I checked)
and it doesn't work as expected with GCC as well. It stops complaining
about #pragma, but it also doesn't push -Wignored-attributes. Here's
the test:

#define DO_PRAGMA(x) _Pragma(#x)

#define SEC(name) \
DO_PRAGMA("GCC diagnostic push") \
DO_PRAGMA("GCC diagnostic ignored \"-Wignored-attributes\"") \
__attribute__((section(name), used)) \
DO_PRAGMA("GCC diagnostic pop") \

extern int something SEC("whatever");

int main()
{
return something;
}


Used like this you get same warning:

$ cc test.c
test.c:10:1: warning: ‘used’ attribute ignored [-Wattributes]
10 | extern int something SEC("whatever");
| ^~~~~~

Removing quotes fixes Clang (linker error is expected)

$ clang test.c
/opt/rh/gcc-toolset-11/root/usr/lib/gcc/x86_64-redhat-linux/11/../../../../bin/ld:
/tmp/test-4eec0b.o: in function `main':
test.c:(.text+0xe): undefined reference to `something'

But we get back to the original problem with GCC:

$ cc test.c
test.c:10:1: error: expected ‘=’, ‘,’, ‘;’, ‘asm’ or ‘__attribute__’
before ‘#pragma’
10 | extern int something SEC("whatever");
| ^~~
test.c:10:1: error: expected identifier or ‘(’ before ‘#pragma’
test.c: In function ‘main’:
test.c:14:16: error: ‘something’ undeclared (first use in this function)
14 | return something;
| ^~~~~~~~~


So the best way forward I can propose for you is this:


#if __GNUC__ && !__clang__

#define SEC(name) __attribute__((section(name), used))

#else

#define SEC(name) \
_Pragma("GCC diagnostic push") \
_Pragma("GCC diagnostic ignored \"-Wignored-attributes\"") \
__attribute__((section(name), used)) \
_Pragma("GCC diagnostic pop") \

#endif

extern int something SEC("whatever");

int main()
{
return something;
}


With some comments explaining how broken GCC is w.r.t. _Pragma. And
just live with compiler warning about used if used with externs.


>
> >
> > >
> > > >
> > > > > /* Avoid 'linux/stddef.h' definition of '__always_inline'. */
> > > > > #undef __always_inline
> > > > > --
> > > > > 2.25.1
> > > > >

2022-07-01 17:18:37

by James Hilliard

[permalink] [raw]
Subject: Re: [PATCH v3 1/1] libbpf: fix broken gcc SEC pragma macro

On Thu, Jun 30, 2022 at 3:51 PM Andrii Nakryiko
<[email protected]> wrote:
>
> On Mon, Jun 27, 2022 at 9:43 PM James Hilliard
> <[email protected]> wrote:
> >
> > On Mon, Jun 27, 2022 at 5:16 PM Andrii Nakryiko
> > <[email protected]> wrote:
> > >
> > > On Thu, Jun 9, 2022 at 4:27 PM James Hilliard <[email protected]> wrote:
> > > >
> > > > On Thu, Jun 9, 2022 at 12:13 PM Andrii Nakryiko
> > > > <[email protected]> wrote:
> > > > >
> > > > > On Wed, Jun 8, 2022 at 11:24 PM James Hilliard
> > > > > <[email protected]> wrote:
> > > > > >
> > > > > > It seems the gcc preprocessor breaks unless pragmas are wrapped
> > > > > > individually inside macros when surrounding __attribute__.
> > > > > >
> > > > > > Fixes errors like:
> > > > > > error: expected identifier or '(' before '#pragma'
> > > > > > 106 | SEC("cgroup/bind6")
> > > > > > | ^~~
> > > > > >
> > > > > > error: expected '=', ',', ';', 'asm' or '__attribute__' before '#pragma'
> > > > > > 114 | char _license[] SEC("license") = "GPL";
> > > > > > | ^~~
> > > > > >
> > > > > > Signed-off-by: James Hilliard <[email protected]>
> > > > > > ---
> > > > > > Changes v2 -> v3:
> > > > > > - just fix SEC pragma
> > > > > > Changes v1 -> v2:
> > > > > > - replace typeof with __typeof__ instead of changing pragma macros
> > > > > > ---
> > > > > > tools/lib/bpf/bpf_helpers.h | 7 ++++---
> > > > > > 1 file changed, 4 insertions(+), 3 deletions(-)
> > > > > >
> > > > > > diff --git a/tools/lib/bpf/bpf_helpers.h b/tools/lib/bpf/bpf_helpers.h
> > > > > > index fb04eaf367f1..66d23c47c206 100644
> > > > > > --- a/tools/lib/bpf/bpf_helpers.h
> > > > > > +++ b/tools/lib/bpf/bpf_helpers.h
> > > > > > @@ -22,11 +22,12 @@
> > > > > > * To allow use of SEC() with externs (e.g., for extern .maps declarations),
> > > > > > * make sure __attribute__((unused)) doesn't trigger compilation warning.
> > > > > > */
> > > > > > +#define DO_PRAGMA(x) _Pragma(#x)
> > > > > > #define SEC(name) \
> > > > > > - _Pragma("GCC diagnostic push") \
> > > > > > - _Pragma("GCC diagnostic ignored \"-Wignored-attributes\"") \
> > > > > > + DO_PRAGMA("GCC diagnostic push") \
> > > > > > + DO_PRAGMA("GCC diagnostic ignored \"-Wignored-attributes\"") \
> > > > > > __attribute__((section(name), used)) \
> > > > > > - _Pragma("GCC diagnostic pop") \
> > > > > > + DO_PRAGMA("GCC diagnostic pop") \
> > > > > >
> > > > >
> > > > > I'm not going to accept this unless I can repro it in the first place.
> > > > > Using -std=c17 doesn't trigger such issue. Please provide the repro
> > > > > first. Building systemd is not a repro, unfortunately. Please try to
> > > > > do it based on libbpf-bootstrap ([0])
> > > > >
> > > > > [0] https://github.com/libbpf/libbpf-bootstrap
> > > >
> > > > Seems to reproduce just fine already there with:
> > > > https://github.com/libbpf/libbpf-bootstrap/blob/31face36d469a0e3e4c4ac1cafc66747d3150930/examples/c/minimal.bpf.c
> > > >
> > > > See here:
> > > > $ /home/buildroot/buildroot/output/per-package/libbpf/host/bin/bpf-gcc
> > > > -Winline -O2 -mframe-limit=32767 -mco-re -gbtf -std=gnu17 -v
> > > > -D__x86_64__ -mlittle-endian -I
> > > > /home/buildroot/buildroot/output/per-package/libbpf/host/x86_64-buildroot-linux-gnu/sysroot/usr/include
> > > > minimal.bpf.c -o minimal.bpf.o
> > > > Using built-in specs.
> > > > COLLECT_GCC=/home/buildroot/buildroot/output/per-package/libbpf/host/bin/bpf-gcc.br_real
> > > > COLLECT_LTO_WRAPPER=/home/buildroot/buildroot/output/per-package/libbpf/host/bin/../libexec/gcc/bpf-buildroot-none/12.1.0/lto-wrapper
> > > > Target: bpf-buildroot-none
> > > > Configured with: ./configure
> > > > --prefix=/home/buildroot/buildroot/output/per-package/host-gcc-bpf/host
> > > > --sysconfdir=/home/buildroot/buildroot/output/per-package/host-gcc-bpf/host/etc
> > > > --localstatedir=/home/buildroot/buildroot/output/per-package/host-gcc-bpf/host/var
> > > > --enable-shared --disable-static --disable-gtk-doc
> > > > --disable-gtk-doc-html --disable-doc --disable-docs
> > > > --disable-documentation --disable-debug --with-xmlto=no --with-fop=no
> > > > --disable-nls --disable-dependency-tracking
> > > > --target=bpf-buildroot-none
> > > > --prefix=/home/buildroot/buildroot/output/per-package/host-gcc-bpf/host
> > > > --sysconfdir=/home/buildroot/buildroot/output/per-package/host-gcc-bpf/host/etc
> > > > --enable-languages=c --with-gnu-ld --enable-static
> > > > --disable-decimal-float --disable-gcov --disable-libssp
> > > > --disable-multilib --disable-shared
> > > > --with-gmp=/home/buildroot/buildroot/output/per-package/host-gcc-bpf/host
> > > > --with-mpc=/home/buildroot/buildroot/output/per-package/host-gcc-bpf/host
> > > > --with-mpfr=/home/buildroot/buildroot/output/per-package/host-gcc-bpf/host
> > > > --with-pkgversion='Buildroot 2022.05-118-ge052166011-dirty'
> > > > --with-bugurl=http://bugs.buildroot.net/ --without-zstd --without-isl
> > > > --without-cloog
> > > > Thread model: single
> > > > Supported LTO compression algorithms: zlib
> > > > gcc version 12.1.0 (Buildroot 2022.05-118-ge052166011-dirty)
> > > > COLLECT_GCC_OPTIONS='--sysroot=/home/buildroot/buildroot/output/per-package/libbpf/host/x86_64-buildroot-linux-gnu/sysroot'
> > > > '-Winline' '-O2' '-mframe-limit=32767' '-mco-re' '-gbtf' '-std=gnu17'
> > > > '-v' '-D' '__x86_64__' '-mlittle-endian' '-I'
> > > > '/home/buildroot/buildroot/output/per-package/libbpf/host/x86_64-buildroot-linux-gnu/sysroot/usr/include'
> > > > '-o' 'minimal.bpf.o' '-dumpdir' 'minimal.bpf.o-'
> > > > /home/buildroot/buildroot/output/per-package/libbpf/host/bin/../libexec/gcc/bpf-buildroot-none/12.1.0/cc1
> > > > -quiet -v -I /home/buildroot/buildroot/output/per-package/libbpf/host/x86_64-buildroot-linux-gnu/sysroot/usr/include
> > > > -iprefix /home/buildroot/buildroot/output/per-package/libbpf/host/bin/../lib/gcc/bpf-buildroot-none/12.1.0/
> > > > -isysroot /home/buildroot/buildroot/output/per-package/libbpf/host/x86_64-buildroot-linux-gnu/sysroot
> > > > -D __x86_64__ minimal.bpf.c -quiet -dumpdir minimal.bpf.o- -dumpbase
> > > > minimal.bpf.c -dumpbase-ext .c -mframe-limit=32767 -mco-re
> > > > -mlittle-endian -gbtf -O2 -Winline -std=gnu17 -version -o
> > > > /tmp/cct4AXvg.s
> > > > GNU C17 (Buildroot 2022.05-118-ge052166011-dirty) version 12.1.0
> > > > (bpf-buildroot-none)
> > > > compiled by GNU C version 12.1.0, GMP version 6.2.1, MPFR version
> > > > 4.1.0, MPC version 1.2.1, isl version none
> > > > GGC heuristics: --param ggc-min-expand=100 --param ggc-min-heapsize=131072
> > > > ignoring nonexistent directory
> > > > "/home/buildroot/buildroot/output/per-package/libbpf/host/bin/../lib/gcc/bpf-buildroot-none/12.1.0/../../../../bpf-buildroot-none/sys-include"
> > > > ignoring nonexistent directory
> > > > "/home/buildroot/buildroot/output/per-package/libbpf/host/bin/../lib/gcc/bpf-buildroot-none/12.1.0/../../../../bpf-buildroot-none/include"
> > > > ignoring duplicate directory
> > > > "/home/buildroot/buildroot/output/per-package/libbpf/host/bin/../lib/gcc/../../lib/gcc/bpf-buildroot-none/12.1.0/include"
> > > > ignoring duplicate directory
> > > > "/home/buildroot/buildroot/output/per-package/libbpf/host/bin/../lib/gcc/../../lib/gcc/bpf-buildroot-none/12.1.0/include-fixed"
> > > > ignoring nonexistent directory
> > > > "/home/buildroot/buildroot/output/per-package/libbpf/host/bin/../lib/gcc/../../lib/gcc/bpf-buildroot-none/12.1.0/../../../../bpf-buildroot-none/sys-include"
> > > > ignoring nonexistent directory
> > > > "/home/buildroot/buildroot/output/per-package/libbpf/host/bin/../lib/gcc/../../lib/gcc/bpf-buildroot-none/12.1.0/../../../../bpf-buildroot-none/include"
> > > > #include "..." search starts here:
> > > > #include <...> search starts here:
> > > > /home/buildroot/buildroot/output/per-package/libbpf/host/x86_64-buildroot-linux-gnu/sysroot/usr/include
> > > > /home/buildroot/buildroot/output/per-package/libbpf/host/bin/../lib/gcc/bpf-buildroot-none/12.1.0/include
> > > > /home/buildroot/buildroot/output/per-package/libbpf/host/bin/../lib/gcc/bpf-buildroot-none/12.1.0/include-fixed
> > > > End of search list.
> > > > GNU C17 (Buildroot 2022.05-118-ge052166011-dirty) version 12.1.0
> > > > (bpf-buildroot-none)
> > > > compiled by GNU C version 12.1.0, GMP version 6.2.1, MPFR version
> > > > 4.1.0, MPC version 1.2.1, isl version none
> > > > GGC heuristics: --param ggc-min-expand=100 --param ggc-min-heapsize=131072
> > > > Compiler executable checksum: 9bf241ca1a2dd4ffd7652c5e247c9be8
> > > > minimal.bpf.c:6:1: error: expected '=', ',', ';', 'asm' or
> > > > '__attribute__' before '#pragma'
> > > > 6 | char LICENSE[] SEC("license") = "Dual BSD/GPL";
> > > > | ^~~
> > > > minimal.bpf.c:6:1: error: expected identifier or '(' before '#pragma'
> > > > minimal.bpf.c:10:1: error: expected identifier or '(' before '#pragma'
> > > > 10 | SEC("tp/syscalls/sys_enter_write")
> > > > | ^~~
> > >
> > > So this is a bug (hard to call this a feature) in gcc (not even
> > > bpf-gcc, I could repro with a simple gcc). Is there a bug reported for
> > > this somewhere? Are GCC folks aware and working on the fix?
> >
> > Yeah, saw a few issues that looked relevant:
> > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=55578
> > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=90400
> >
> > >
> > > What's curious is that the only thing that allows to bypass this is
> > > adding #x in macro, having #define DO_PRAGMA(x) _Pragma(x) doesn't
> > > help.
> > >
> > > So ideally GCC can fix this?
> >
> > From the reported issues...it doesn't sound like a fix is going to be
> > coming all that
> > soon in GCC.
> >
> > > But either way your patch as is
> > > erroneously passing extra quoted strings to _Pragma().
> >
> > I recall the extra quotes were needed to make this work, does it work for you
> > without them?
> >
> > >
> > > I'm pondering whether it's just cleaner to define SEC() without
> > > pragmas for GCC? It will only cause compiler warning about unnecessary
> > > unused attribute for extern *variable* declarations, which are very
> > > rare. Instead of relying on this quirky "fix" approach. Ideally,
> > > though, GCC just fixes _Pragma() handling, of course.
> >
> > I mean, as long as this workaround is reliable I'd say using it is the
> > best option
> > for backwards compatibility, especially since it's only needed in one place from
> > the looks of it.
>
> Is it reliable, though? Adding those quotes breaks Clang (I checked)
> and it doesn't work as expected with GCC as well. It stops complaining
> about #pragma, but it also doesn't push -Wignored-attributes. Here's
> the test:

Ok, yeah, guess my hack doesn't really work then.

>
> #define DO_PRAGMA(x) _Pragma(#x)
>
> #define SEC(name) \
> DO_PRAGMA("GCC diagnostic push") \
> DO_PRAGMA("GCC diagnostic ignored \"-Wignored-attributes\"") \
> __attribute__((section(name), used)) \
> DO_PRAGMA("GCC diagnostic pop") \
>
> extern int something SEC("whatever");
>
> int main()
> {
> return something;
> }
>
>
> Used like this you get same warning:
>
> $ cc test.c
> test.c:10:1: warning: ‘used’ attribute ignored [-Wattributes]
> 10 | extern int something SEC("whatever");
> | ^~~~~~
>
> Removing quotes fixes Clang (linker error is expected)
>
> $ clang test.c
> /opt/rh/gcc-toolset-11/root/usr/lib/gcc/x86_64-redhat-linux/11/../../../../bin/ld:

FYI I was testing with GCC 12.1.

> /tmp/test-4eec0b.o: in function `main':
> test.c:(.text+0xe): undefined reference to `something'
>
> But we get back to the original problem with GCC:
>
> $ cc test.c
> test.c:10:1: error: expected ‘=’, ‘,’, ‘;’, ‘asm’ or ‘__attribute__’
> before ‘#pragma’
> 10 | extern int something SEC("whatever");
> | ^~~
> test.c:10:1: error: expected identifier or ‘(’ before ‘#pragma’
> test.c: In function ‘main’:
> test.c:14:16: error: ‘something’ undeclared (first use in this function)
> 14 | return something;
> | ^~~~~~~~~
>
>
> So the best way forward I can propose for you is this:

Yeah, probably the best option for now.

>
>
> #if __GNUC__ && !__clang__
>
> #define SEC(name) __attribute__((section(name), used))
>
> #else
>
> #define SEC(name) \
> _Pragma("GCC diagnostic push") \
> _Pragma("GCC diagnostic ignored \"-Wignored-attributes\"") \
> __attribute__((section(name), used)) \
> _Pragma("GCC diagnostic pop") \
>
> #endif
>
> extern int something SEC("whatever");
>
> int main()
> {
> return something;
> }
>
>
> With some comments explaining how broken GCC is w.r.t. _Pragma. And
> just live with compiler warning about used if used with externs.

Yeah, do you want to spin a patch with that? I think you probably have a better
understanding of the issue at this point than I do.

>
>
> >
> > >
> > > >
> > > > >
> > > > > > /* Avoid 'linux/stddef.h' definition of '__always_inline'. */
> > > > > > #undef __always_inline
> > > > > > --
> > > > > > 2.25.1
> > > > > >

2022-07-06 04:45:37

by Andrii Nakryiko

[permalink] [raw]
Subject: Re: [PATCH v3 1/1] libbpf: fix broken gcc SEC pragma macro

On Fri, Jul 1, 2022 at 10:12 AM James Hilliard
<[email protected]> wrote:
>
> On Thu, Jun 30, 2022 at 3:51 PM Andrii Nakryiko
> <[email protected]> wrote:
> >
> > On Mon, Jun 27, 2022 at 9:43 PM James Hilliard
> > <[email protected]> wrote:
> > >
> > > On Mon, Jun 27, 2022 at 5:16 PM Andrii Nakryiko
> > > <[email protected]> wrote:
> > > >
> > > > On Thu, Jun 9, 2022 at 4:27 PM James Hilliard <[email protected]> wrote:
> > > > >
> > > > > On Thu, Jun 9, 2022 at 12:13 PM Andrii Nakryiko
> > > > > <[email protected]> wrote:
> > > > > >
> > > > > > On Wed, Jun 8, 2022 at 11:24 PM James Hilliard
> > > > > > <[email protected]> wrote:
> > > > > > >
> > > > > > > It seems the gcc preprocessor breaks unless pragmas are wrapped
> > > > > > > individually inside macros when surrounding __attribute__.
> > > > > > >
> > > > > > > Fixes errors like:
> > > > > > > error: expected identifier or '(' before '#pragma'
> > > > > > > 106 | SEC("cgroup/bind6")
> > > > > > > | ^~~
> > > > > > >
> > > > > > > error: expected '=', ',', ';', 'asm' or '__attribute__' before '#pragma'
> > > > > > > 114 | char _license[] SEC("license") = "GPL";
> > > > > > > | ^~~
> > > > > > >
> > > > > > > Signed-off-by: James Hilliard <[email protected]>
> > > > > > > ---
> > > > > > > Changes v2 -> v3:
> > > > > > > - just fix SEC pragma
> > > > > > > Changes v1 -> v2:
> > > > > > > - replace typeof with __typeof__ instead of changing pragma macros
> > > > > > > ---
> > > > > > > tools/lib/bpf/bpf_helpers.h | 7 ++++---
> > > > > > > 1 file changed, 4 insertions(+), 3 deletions(-)
> > > > > > >
> > > > > > > diff --git a/tools/lib/bpf/bpf_helpers.h b/tools/lib/bpf/bpf_helpers.h
> > > > > > > index fb04eaf367f1..66d23c47c206 100644
> > > > > > > --- a/tools/lib/bpf/bpf_helpers.h
> > > > > > > +++ b/tools/lib/bpf/bpf_helpers.h
> > > > > > > @@ -22,11 +22,12 @@
> > > > > > > * To allow use of SEC() with externs (e.g., for extern .maps declarations),
> > > > > > > * make sure __attribute__((unused)) doesn't trigger compilation warning.
> > > > > > > */
> > > > > > > +#define DO_PRAGMA(x) _Pragma(#x)
> > > > > > > #define SEC(name) \
> > > > > > > - _Pragma("GCC diagnostic push") \
> > > > > > > - _Pragma("GCC diagnostic ignored \"-Wignored-attributes\"") \
> > > > > > > + DO_PRAGMA("GCC diagnostic push") \
> > > > > > > + DO_PRAGMA("GCC diagnostic ignored \"-Wignored-attributes\"") \
> > > > > > > __attribute__((section(name), used)) \
> > > > > > > - _Pragma("GCC diagnostic pop") \
> > > > > > > + DO_PRAGMA("GCC diagnostic pop") \
> > > > > > >
> > > > > >
> > > > > > I'm not going to accept this unless I can repro it in the first place.
> > > > > > Using -std=c17 doesn't trigger such issue. Please provide the repro
> > > > > > first. Building systemd is not a repro, unfortunately. Please try to
> > > > > > do it based on libbpf-bootstrap ([0])
> > > > > >
> > > > > > [0] https://github.com/libbpf/libbpf-bootstrap
> > > > >
> > > > > Seems to reproduce just fine already there with:
> > > > > https://github.com/libbpf/libbpf-bootstrap/blob/31face36d469a0e3e4c4ac1cafc66747d3150930/examples/c/minimal.bpf.c
> > > > >
> > > > > See here:
> > > > > $ /home/buildroot/buildroot/output/per-package/libbpf/host/bin/bpf-gcc
> > > > > -Winline -O2 -mframe-limit=32767 -mco-re -gbtf -std=gnu17 -v
> > > > > -D__x86_64__ -mlittle-endian -I
> > > > > /home/buildroot/buildroot/output/per-package/libbpf/host/x86_64-buildroot-linux-gnu/sysroot/usr/include
> > > > > minimal.bpf.c -o minimal.bpf.o
> > > > > Using built-in specs.
> > > > > COLLECT_GCC=/home/buildroot/buildroot/output/per-package/libbpf/host/bin/bpf-gcc.br_real
> > > > > COLLECT_LTO_WRAPPER=/home/buildroot/buildroot/output/per-package/libbpf/host/bin/../libexec/gcc/bpf-buildroot-none/12.1.0/lto-wrapper
> > > > > Target: bpf-buildroot-none
> > > > > Configured with: ./configure
> > > > > --prefix=/home/buildroot/buildroot/output/per-package/host-gcc-bpf/host
> > > > > --sysconfdir=/home/buildroot/buildroot/output/per-package/host-gcc-bpf/host/etc
> > > > > --localstatedir=/home/buildroot/buildroot/output/per-package/host-gcc-bpf/host/var
> > > > > --enable-shared --disable-static --disable-gtk-doc
> > > > > --disable-gtk-doc-html --disable-doc --disable-docs
> > > > > --disable-documentation --disable-debug --with-xmlto=no --with-fop=no
> > > > > --disable-nls --disable-dependency-tracking
> > > > > --target=bpf-buildroot-none
> > > > > --prefix=/home/buildroot/buildroot/output/per-package/host-gcc-bpf/host
> > > > > --sysconfdir=/home/buildroot/buildroot/output/per-package/host-gcc-bpf/host/etc
> > > > > --enable-languages=c --with-gnu-ld --enable-static
> > > > > --disable-decimal-float --disable-gcov --disable-libssp
> > > > > --disable-multilib --disable-shared
> > > > > --with-gmp=/home/buildroot/buildroot/output/per-package/host-gcc-bpf/host
> > > > > --with-mpc=/home/buildroot/buildroot/output/per-package/host-gcc-bpf/host
> > > > > --with-mpfr=/home/buildroot/buildroot/output/per-package/host-gcc-bpf/host
> > > > > --with-pkgversion='Buildroot 2022.05-118-ge052166011-dirty'
> > > > > --with-bugurl=http://bugs.buildroot.net/ --without-zstd --without-isl
> > > > > --without-cloog
> > > > > Thread model: single
> > > > > Supported LTO compression algorithms: zlib
> > > > > gcc version 12.1.0 (Buildroot 2022.05-118-ge052166011-dirty)
> > > > > COLLECT_GCC_OPTIONS='--sysroot=/home/buildroot/buildroot/output/per-package/libbpf/host/x86_64-buildroot-linux-gnu/sysroot'
> > > > > '-Winline' '-O2' '-mframe-limit=32767' '-mco-re' '-gbtf' '-std=gnu17'
> > > > > '-v' '-D' '__x86_64__' '-mlittle-endian' '-I'
> > > > > '/home/buildroot/buildroot/output/per-package/libbpf/host/x86_64-buildroot-linux-gnu/sysroot/usr/include'
> > > > > '-o' 'minimal.bpf.o' '-dumpdir' 'minimal.bpf.o-'
> > > > > /home/buildroot/buildroot/output/per-package/libbpf/host/bin/../libexec/gcc/bpf-buildroot-none/12.1.0/cc1
> > > > > -quiet -v -I /home/buildroot/buildroot/output/per-package/libbpf/host/x86_64-buildroot-linux-gnu/sysroot/usr/include
> > > > > -iprefix /home/buildroot/buildroot/output/per-package/libbpf/host/bin/../lib/gcc/bpf-buildroot-none/12.1.0/
> > > > > -isysroot /home/buildroot/buildroot/output/per-package/libbpf/host/x86_64-buildroot-linux-gnu/sysroot
> > > > > -D __x86_64__ minimal.bpf.c -quiet -dumpdir minimal.bpf.o- -dumpbase
> > > > > minimal.bpf.c -dumpbase-ext .c -mframe-limit=32767 -mco-re
> > > > > -mlittle-endian -gbtf -O2 -Winline -std=gnu17 -version -o
> > > > > /tmp/cct4AXvg.s
> > > > > GNU C17 (Buildroot 2022.05-118-ge052166011-dirty) version 12.1.0
> > > > > (bpf-buildroot-none)
> > > > > compiled by GNU C version 12.1.0, GMP version 6.2.1, MPFR version
> > > > > 4.1.0, MPC version 1.2.1, isl version none
> > > > > GGC heuristics: --param ggc-min-expand=100 --param ggc-min-heapsize=131072
> > > > > ignoring nonexistent directory
> > > > > "/home/buildroot/buildroot/output/per-package/libbpf/host/bin/../lib/gcc/bpf-buildroot-none/12.1.0/../../../../bpf-buildroot-none/sys-include"
> > > > > ignoring nonexistent directory
> > > > > "/home/buildroot/buildroot/output/per-package/libbpf/host/bin/../lib/gcc/bpf-buildroot-none/12.1.0/../../../../bpf-buildroot-none/include"
> > > > > ignoring duplicate directory
> > > > > "/home/buildroot/buildroot/output/per-package/libbpf/host/bin/../lib/gcc/../../lib/gcc/bpf-buildroot-none/12.1.0/include"
> > > > > ignoring duplicate directory
> > > > > "/home/buildroot/buildroot/output/per-package/libbpf/host/bin/../lib/gcc/../../lib/gcc/bpf-buildroot-none/12.1.0/include-fixed"
> > > > > ignoring nonexistent directory
> > > > > "/home/buildroot/buildroot/output/per-package/libbpf/host/bin/../lib/gcc/../../lib/gcc/bpf-buildroot-none/12.1.0/../../../../bpf-buildroot-none/sys-include"
> > > > > ignoring nonexistent directory
> > > > > "/home/buildroot/buildroot/output/per-package/libbpf/host/bin/../lib/gcc/../../lib/gcc/bpf-buildroot-none/12.1.0/../../../../bpf-buildroot-none/include"
> > > > > #include "..." search starts here:
> > > > > #include <...> search starts here:
> > > > > /home/buildroot/buildroot/output/per-package/libbpf/host/x86_64-buildroot-linux-gnu/sysroot/usr/include
> > > > > /home/buildroot/buildroot/output/per-package/libbpf/host/bin/../lib/gcc/bpf-buildroot-none/12.1.0/include
> > > > > /home/buildroot/buildroot/output/per-package/libbpf/host/bin/../lib/gcc/bpf-buildroot-none/12.1.0/include-fixed
> > > > > End of search list.
> > > > > GNU C17 (Buildroot 2022.05-118-ge052166011-dirty) version 12.1.0
> > > > > (bpf-buildroot-none)
> > > > > compiled by GNU C version 12.1.0, GMP version 6.2.1, MPFR version
> > > > > 4.1.0, MPC version 1.2.1, isl version none
> > > > > GGC heuristics: --param ggc-min-expand=100 --param ggc-min-heapsize=131072
> > > > > Compiler executable checksum: 9bf241ca1a2dd4ffd7652c5e247c9be8
> > > > > minimal.bpf.c:6:1: error: expected '=', ',', ';', 'asm' or
> > > > > '__attribute__' before '#pragma'
> > > > > 6 | char LICENSE[] SEC("license") = "Dual BSD/GPL";
> > > > > | ^~~
> > > > > minimal.bpf.c:6:1: error: expected identifier or '(' before '#pragma'
> > > > > minimal.bpf.c:10:1: error: expected identifier or '(' before '#pragma'
> > > > > 10 | SEC("tp/syscalls/sys_enter_write")
> > > > > | ^~~
> > > >
> > > > So this is a bug (hard to call this a feature) in gcc (not even
> > > > bpf-gcc, I could repro with a simple gcc). Is there a bug reported for
> > > > this somewhere? Are GCC folks aware and working on the fix?
> > >
> > > Yeah, saw a few issues that looked relevant:
> > > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=55578
> > > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=90400
> > >
> > > >
> > > > What's curious is that the only thing that allows to bypass this is
> > > > adding #x in macro, having #define DO_PRAGMA(x) _Pragma(x) doesn't
> > > > help.
> > > >
> > > > So ideally GCC can fix this?
> > >
> > > From the reported issues...it doesn't sound like a fix is going to be
> > > coming all that
> > > soon in GCC.
> > >
> > > > But either way your patch as is
> > > > erroneously passing extra quoted strings to _Pragma().
> > >
> > > I recall the extra quotes were needed to make this work, does it work for you
> > > without them?
> > >
> > > >
> > > > I'm pondering whether it's just cleaner to define SEC() without
> > > > pragmas for GCC? It will only cause compiler warning about unnecessary
> > > > unused attribute for extern *variable* declarations, which are very
> > > > rare. Instead of relying on this quirky "fix" approach. Ideally,
> > > > though, GCC just fixes _Pragma() handling, of course.
> > >
> > > I mean, as long as this workaround is reliable I'd say using it is the
> > > best option
> > > for backwards compatibility, especially since it's only needed in one place from
> > > the looks of it.
> >
> > Is it reliable, though? Adding those quotes breaks Clang (I checked)
> > and it doesn't work as expected with GCC as well. It stops complaining
> > about #pragma, but it also doesn't push -Wignored-attributes. Here's
> > the test:
>
> Ok, yeah, guess my hack doesn't really work then.
>
> >
> > #define DO_PRAGMA(x) _Pragma(#x)
> >
> > #define SEC(name) \
> > DO_PRAGMA("GCC diagnostic push") \
> > DO_PRAGMA("GCC diagnostic ignored \"-Wignored-attributes\"") \
> > __attribute__((section(name), used)) \
> > DO_PRAGMA("GCC diagnostic pop") \
> >
> > extern int something SEC("whatever");
> >
> > int main()
> > {
> > return something;
> > }
> >
> >
> > Used like this you get same warning:
> >
> > $ cc test.c
> > test.c:10:1: warning: ‘used’ attribute ignored [-Wattributes]
> > 10 | extern int something SEC("whatever");
> > | ^~~~~~
> >
> > Removing quotes fixes Clang (linker error is expected)
> >
> > $ clang test.c
> > /opt/rh/gcc-toolset-11/root/usr/lib/gcc/x86_64-redhat-linux/11/../../../../bin/ld:
>
> FYI I was testing with GCC 12.1.
>
> > /tmp/test-4eec0b.o: in function `main':
> > test.c:(.text+0xe): undefined reference to `something'
> >
> > But we get back to the original problem with GCC:
> >
> > $ cc test.c
> > test.c:10:1: error: expected ‘=’, ‘,’, ‘;’, ‘asm’ or ‘__attribute__’
> > before ‘#pragma’
> > 10 | extern int something SEC("whatever");
> > | ^~~
> > test.c:10:1: error: expected identifier or ‘(’ before ‘#pragma’
> > test.c: In function ‘main’:
> > test.c:14:16: error: ‘something’ undeclared (first use in this function)
> > 14 | return something;
> > | ^~~~~~~~~
> >
> >
> > So the best way forward I can propose for you is this:
>
> Yeah, probably the best option for now.
>
> >
> >
> > #if __GNUC__ && !__clang__
> >
> > #define SEC(name) __attribute__((section(name), used))
> >
> > #else
> >
> > #define SEC(name) \
> > _Pragma("GCC diagnostic push") \
> > _Pragma("GCC diagnostic ignored \"-Wignored-attributes\"") \
> > __attribute__((section(name), used)) \
> > _Pragma("GCC diagnostic pop") \
> >
> > #endif
> >
> > extern int something SEC("whatever");
> >
> > int main()
> > {
> > return something;
> > }
> >
> >
> > With some comments explaining how broken GCC is w.r.t. _Pragma. And
> > just live with compiler warning about used if used with externs.
>
> Yeah, do you want to spin a patch with that? I think you probably have a better
> understanding of the issue at this point than I do.

I'd appreciate it if you do that and test selftests/bpf compilation
and execution with bpf-gcc (which I don't have locally). Our CI will
take care of testing Clang compilation. Thanks!

>
> >
> >
> > >
> > > >
> > > > >
> > > > > >
> > > > > > > /* Avoid 'linux/stddef.h' definition of '__always_inline'. */
> > > > > > > #undef __always_inline
> > > > > > > --
> > > > > > > 2.25.1
> > > > > > >

2022-07-06 12:35:56

by James Hilliard

[permalink] [raw]
Subject: Re: [PATCH v3 1/1] libbpf: fix broken gcc SEC pragma macro

On Tue, Jul 5, 2022 at 10:36 PM Andrii Nakryiko
<[email protected]> wrote:
>
> On Fri, Jul 1, 2022 at 10:12 AM James Hilliard
> <[email protected]> wrote:
> >
> > On Thu, Jun 30, 2022 at 3:51 PM Andrii Nakryiko
> > <[email protected]> wrote:
> > >
> > > On Mon, Jun 27, 2022 at 9:43 PM James Hilliard
> > > <[email protected]> wrote:
> > > >
> > > > On Mon, Jun 27, 2022 at 5:16 PM Andrii Nakryiko
> > > > <[email protected]> wrote:
> > > > >
> > > > > On Thu, Jun 9, 2022 at 4:27 PM James Hilliard <[email protected]> wrote:
> > > > > >
> > > > > > On Thu, Jun 9, 2022 at 12:13 PM Andrii Nakryiko
> > > > > > <[email protected]> wrote:
> > > > > > >
> > > > > > > On Wed, Jun 8, 2022 at 11:24 PM James Hilliard
> > > > > > > <[email protected]> wrote:
> > > > > > > >
> > > > > > > > It seems the gcc preprocessor breaks unless pragmas are wrapped
> > > > > > > > individually inside macros when surrounding __attribute__.
> > > > > > > >
> > > > > > > > Fixes errors like:
> > > > > > > > error: expected identifier or '(' before '#pragma'
> > > > > > > > 106 | SEC("cgroup/bind6")
> > > > > > > > | ^~~
> > > > > > > >
> > > > > > > > error: expected '=', ',', ';', 'asm' or '__attribute__' before '#pragma'
> > > > > > > > 114 | char _license[] SEC("license") = "GPL";
> > > > > > > > | ^~~
> > > > > > > >
> > > > > > > > Signed-off-by: James Hilliard <[email protected]>
> > > > > > > > ---
> > > > > > > > Changes v2 -> v3:
> > > > > > > > - just fix SEC pragma
> > > > > > > > Changes v1 -> v2:
> > > > > > > > - replace typeof with __typeof__ instead of changing pragma macros
> > > > > > > > ---
> > > > > > > > tools/lib/bpf/bpf_helpers.h | 7 ++++---
> > > > > > > > 1 file changed, 4 insertions(+), 3 deletions(-)
> > > > > > > >
> > > > > > > > diff --git a/tools/lib/bpf/bpf_helpers.h b/tools/lib/bpf/bpf_helpers.h
> > > > > > > > index fb04eaf367f1..66d23c47c206 100644
> > > > > > > > --- a/tools/lib/bpf/bpf_helpers.h
> > > > > > > > +++ b/tools/lib/bpf/bpf_helpers.h
> > > > > > > > @@ -22,11 +22,12 @@
> > > > > > > > * To allow use of SEC() with externs (e.g., for extern .maps declarations),
> > > > > > > > * make sure __attribute__((unused)) doesn't trigger compilation warning.
> > > > > > > > */
> > > > > > > > +#define DO_PRAGMA(x) _Pragma(#x)
> > > > > > > > #define SEC(name) \
> > > > > > > > - _Pragma("GCC diagnostic push") \
> > > > > > > > - _Pragma("GCC diagnostic ignored \"-Wignored-attributes\"") \
> > > > > > > > + DO_PRAGMA("GCC diagnostic push") \
> > > > > > > > + DO_PRAGMA("GCC diagnostic ignored \"-Wignored-attributes\"") \
> > > > > > > > __attribute__((section(name), used)) \
> > > > > > > > - _Pragma("GCC diagnostic pop") \
> > > > > > > > + DO_PRAGMA("GCC diagnostic pop") \
> > > > > > > >
> > > > > > >
> > > > > > > I'm not going to accept this unless I can repro it in the first place.
> > > > > > > Using -std=c17 doesn't trigger such issue. Please provide the repro
> > > > > > > first. Building systemd is not a repro, unfortunately. Please try to
> > > > > > > do it based on libbpf-bootstrap ([0])
> > > > > > >
> > > > > > > [0] https://github.com/libbpf/libbpf-bootstrap
> > > > > >
> > > > > > Seems to reproduce just fine already there with:
> > > > > > https://github.com/libbpf/libbpf-bootstrap/blob/31face36d469a0e3e4c4ac1cafc66747d3150930/examples/c/minimal.bpf.c
> > > > > >
> > > > > > See here:
> > > > > > $ /home/buildroot/buildroot/output/per-package/libbpf/host/bin/bpf-gcc
> > > > > > -Winline -O2 -mframe-limit=32767 -mco-re -gbtf -std=gnu17 -v
> > > > > > -D__x86_64__ -mlittle-endian -I
> > > > > > /home/buildroot/buildroot/output/per-package/libbpf/host/x86_64-buildroot-linux-gnu/sysroot/usr/include
> > > > > > minimal.bpf.c -o minimal.bpf.o
> > > > > > Using built-in specs.
> > > > > > COLLECT_GCC=/home/buildroot/buildroot/output/per-package/libbpf/host/bin/bpf-gcc.br_real
> > > > > > COLLECT_LTO_WRAPPER=/home/buildroot/buildroot/output/per-package/libbpf/host/bin/../libexec/gcc/bpf-buildroot-none/12.1.0/lto-wrapper
> > > > > > Target: bpf-buildroot-none
> > > > > > Configured with: ./configure
> > > > > > --prefix=/home/buildroot/buildroot/output/per-package/host-gcc-bpf/host
> > > > > > --sysconfdir=/home/buildroot/buildroot/output/per-package/host-gcc-bpf/host/etc
> > > > > > --localstatedir=/home/buildroot/buildroot/output/per-package/host-gcc-bpf/host/var
> > > > > > --enable-shared --disable-static --disable-gtk-doc
> > > > > > --disable-gtk-doc-html --disable-doc --disable-docs
> > > > > > --disable-documentation --disable-debug --with-xmlto=no --with-fop=no
> > > > > > --disable-nls --disable-dependency-tracking
> > > > > > --target=bpf-buildroot-none
> > > > > > --prefix=/home/buildroot/buildroot/output/per-package/host-gcc-bpf/host
> > > > > > --sysconfdir=/home/buildroot/buildroot/output/per-package/host-gcc-bpf/host/etc
> > > > > > --enable-languages=c --with-gnu-ld --enable-static
> > > > > > --disable-decimal-float --disable-gcov --disable-libssp
> > > > > > --disable-multilib --disable-shared
> > > > > > --with-gmp=/home/buildroot/buildroot/output/per-package/host-gcc-bpf/host
> > > > > > --with-mpc=/home/buildroot/buildroot/output/per-package/host-gcc-bpf/host
> > > > > > --with-mpfr=/home/buildroot/buildroot/output/per-package/host-gcc-bpf/host
> > > > > > --with-pkgversion='Buildroot 2022.05-118-ge052166011-dirty'
> > > > > > --with-bugurl=http://bugs.buildroot.net/ --without-zstd --without-isl
> > > > > > --without-cloog
> > > > > > Thread model: single
> > > > > > Supported LTO compression algorithms: zlib
> > > > > > gcc version 12.1.0 (Buildroot 2022.05-118-ge052166011-dirty)
> > > > > > COLLECT_GCC_OPTIONS='--sysroot=/home/buildroot/buildroot/output/per-package/libbpf/host/x86_64-buildroot-linux-gnu/sysroot'
> > > > > > '-Winline' '-O2' '-mframe-limit=32767' '-mco-re' '-gbtf' '-std=gnu17'
> > > > > > '-v' '-D' '__x86_64__' '-mlittle-endian' '-I'
> > > > > > '/home/buildroot/buildroot/output/per-package/libbpf/host/x86_64-buildroot-linux-gnu/sysroot/usr/include'
> > > > > > '-o' 'minimal.bpf.o' '-dumpdir' 'minimal.bpf.o-'
> > > > > > /home/buildroot/buildroot/output/per-package/libbpf/host/bin/../libexec/gcc/bpf-buildroot-none/12.1.0/cc1
> > > > > > -quiet -v -I /home/buildroot/buildroot/output/per-package/libbpf/host/x86_64-buildroot-linux-gnu/sysroot/usr/include
> > > > > > -iprefix /home/buildroot/buildroot/output/per-package/libbpf/host/bin/../lib/gcc/bpf-buildroot-none/12.1.0/
> > > > > > -isysroot /home/buildroot/buildroot/output/per-package/libbpf/host/x86_64-buildroot-linux-gnu/sysroot
> > > > > > -D __x86_64__ minimal.bpf.c -quiet -dumpdir minimal.bpf.o- -dumpbase
> > > > > > minimal.bpf.c -dumpbase-ext .c -mframe-limit=32767 -mco-re
> > > > > > -mlittle-endian -gbtf -O2 -Winline -std=gnu17 -version -o
> > > > > > /tmp/cct4AXvg.s
> > > > > > GNU C17 (Buildroot 2022.05-118-ge052166011-dirty) version 12.1.0
> > > > > > (bpf-buildroot-none)
> > > > > > compiled by GNU C version 12.1.0, GMP version 6.2.1, MPFR version
> > > > > > 4.1.0, MPC version 1.2.1, isl version none
> > > > > > GGC heuristics: --param ggc-min-expand=100 --param ggc-min-heapsize=131072
> > > > > > ignoring nonexistent directory
> > > > > > "/home/buildroot/buildroot/output/per-package/libbpf/host/bin/../lib/gcc/bpf-buildroot-none/12.1.0/../../../../bpf-buildroot-none/sys-include"
> > > > > > ignoring nonexistent directory
> > > > > > "/home/buildroot/buildroot/output/per-package/libbpf/host/bin/../lib/gcc/bpf-buildroot-none/12.1.0/../../../../bpf-buildroot-none/include"
> > > > > > ignoring duplicate directory
> > > > > > "/home/buildroot/buildroot/output/per-package/libbpf/host/bin/../lib/gcc/../../lib/gcc/bpf-buildroot-none/12.1.0/include"
> > > > > > ignoring duplicate directory
> > > > > > "/home/buildroot/buildroot/output/per-package/libbpf/host/bin/../lib/gcc/../../lib/gcc/bpf-buildroot-none/12.1.0/include-fixed"
> > > > > > ignoring nonexistent directory
> > > > > > "/home/buildroot/buildroot/output/per-package/libbpf/host/bin/../lib/gcc/../../lib/gcc/bpf-buildroot-none/12.1.0/../../../../bpf-buildroot-none/sys-include"
> > > > > > ignoring nonexistent directory
> > > > > > "/home/buildroot/buildroot/output/per-package/libbpf/host/bin/../lib/gcc/../../lib/gcc/bpf-buildroot-none/12.1.0/../../../../bpf-buildroot-none/include"
> > > > > > #include "..." search starts here:
> > > > > > #include <...> search starts here:
> > > > > > /home/buildroot/buildroot/output/per-package/libbpf/host/x86_64-buildroot-linux-gnu/sysroot/usr/include
> > > > > > /home/buildroot/buildroot/output/per-package/libbpf/host/bin/../lib/gcc/bpf-buildroot-none/12.1.0/include
> > > > > > /home/buildroot/buildroot/output/per-package/libbpf/host/bin/../lib/gcc/bpf-buildroot-none/12.1.0/include-fixed
> > > > > > End of search list.
> > > > > > GNU C17 (Buildroot 2022.05-118-ge052166011-dirty) version 12.1.0
> > > > > > (bpf-buildroot-none)
> > > > > > compiled by GNU C version 12.1.0, GMP version 6.2.1, MPFR version
> > > > > > 4.1.0, MPC version 1.2.1, isl version none
> > > > > > GGC heuristics: --param ggc-min-expand=100 --param ggc-min-heapsize=131072
> > > > > > Compiler executable checksum: 9bf241ca1a2dd4ffd7652c5e247c9be8
> > > > > > minimal.bpf.c:6:1: error: expected '=', ',', ';', 'asm' or
> > > > > > '__attribute__' before '#pragma'
> > > > > > 6 | char LICENSE[] SEC("license") = "Dual BSD/GPL";
> > > > > > | ^~~
> > > > > > minimal.bpf.c:6:1: error: expected identifier or '(' before '#pragma'
> > > > > > minimal.bpf.c:10:1: error: expected identifier or '(' before '#pragma'
> > > > > > 10 | SEC("tp/syscalls/sys_enter_write")
> > > > > > | ^~~
> > > > >
> > > > > So this is a bug (hard to call this a feature) in gcc (not even
> > > > > bpf-gcc, I could repro with a simple gcc). Is there a bug reported for
> > > > > this somewhere? Are GCC folks aware and working on the fix?
> > > >
> > > > Yeah, saw a few issues that looked relevant:
> > > > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=55578
> > > > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=90400
> > > >
> > > > >
> > > > > What's curious is that the only thing that allows to bypass this is
> > > > > adding #x in macro, having #define DO_PRAGMA(x) _Pragma(x) doesn't
> > > > > help.
> > > > >
> > > > > So ideally GCC can fix this?
> > > >
> > > > From the reported issues...it doesn't sound like a fix is going to be
> > > > coming all that
> > > > soon in GCC.
> > > >
> > > > > But either way your patch as is
> > > > > erroneously passing extra quoted strings to _Pragma().
> > > >
> > > > I recall the extra quotes were needed to make this work, does it work for you
> > > > without them?
> > > >
> > > > >
> > > > > I'm pondering whether it's just cleaner to define SEC() without
> > > > > pragmas for GCC? It will only cause compiler warning about unnecessary
> > > > > unused attribute for extern *variable* declarations, which are very
> > > > > rare. Instead of relying on this quirky "fix" approach. Ideally,
> > > > > though, GCC just fixes _Pragma() handling, of course.
> > > >
> > > > I mean, as long as this workaround is reliable I'd say using it is the
> > > > best option
> > > > for backwards compatibility, especially since it's only needed in one place from
> > > > the looks of it.
> > >
> > > Is it reliable, though? Adding those quotes breaks Clang (I checked)
> > > and it doesn't work as expected with GCC as well. It stops complaining
> > > about #pragma, but it also doesn't push -Wignored-attributes. Here's
> > > the test:
> >
> > Ok, yeah, guess my hack doesn't really work then.
> >
> > >
> > > #define DO_PRAGMA(x) _Pragma(#x)
> > >
> > > #define SEC(name) \
> > > DO_PRAGMA("GCC diagnostic push") \
> > > DO_PRAGMA("GCC diagnostic ignored \"-Wignored-attributes\"") \
> > > __attribute__((section(name), used)) \
> > > DO_PRAGMA("GCC diagnostic pop") \
> > >
> > > extern int something SEC("whatever");
> > >
> > > int main()
> > > {
> > > return something;
> > > }
> > >
> > >
> > > Used like this you get same warning:
> > >
> > > $ cc test.c
> > > test.c:10:1: warning: ‘used’ attribute ignored [-Wattributes]
> > > 10 | extern int something SEC("whatever");
> > > | ^~~~~~
> > >
> > > Removing quotes fixes Clang (linker error is expected)
> > >
> > > $ clang test.c
> > > /opt/rh/gcc-toolset-11/root/usr/lib/gcc/x86_64-redhat-linux/11/../../../../bin/ld:
> >
> > FYI I was testing with GCC 12.1.
> >
> > > /tmp/test-4eec0b.o: in function `main':
> > > test.c:(.text+0xe): undefined reference to `something'
> > >
> > > But we get back to the original problem with GCC:
> > >
> > > $ cc test.c
> > > test.c:10:1: error: expected ‘=’, ‘,’, ‘;’, ‘asm’ or ‘__attribute__’
> > > before ‘#pragma’
> > > 10 | extern int something SEC("whatever");
> > > | ^~~
> > > test.c:10:1: error: expected identifier or ‘(’ before ‘#pragma’
> > > test.c: In function ‘main’:
> > > test.c:14:16: error: ‘something’ undeclared (first use in this function)
> > > 14 | return something;
> > > | ^~~~~~~~~
> > >
> > >
> > > So the best way forward I can propose for you is this:
> >
> > Yeah, probably the best option for now.
> >
> > >
> > >
> > > #if __GNUC__ && !__clang__
> > >
> > > #define SEC(name) __attribute__((section(name), used))
> > >
> > > #else
> > >
> > > #define SEC(name) \
> > > _Pragma("GCC diagnostic push") \
> > > _Pragma("GCC diagnostic ignored \"-Wignored-attributes\"") \
> > > __attribute__((section(name), used)) \
> > > _Pragma("GCC diagnostic pop") \
> > >
> > > #endif
> > >
> > > extern int something SEC("whatever");
> > >
> > > int main()
> > > {
> > > return something;
> > > }
> > >
> > >
> > > With some comments explaining how broken GCC is w.r.t. _Pragma. And
> > > just live with compiler warning about used if used with externs.
> >
> > Yeah, do you want to spin a patch with that? I think you probably have a better
> > understanding of the issue at this point than I do.
>
> I'd appreciate it if you do that and test selftests/bpf compilation
> and execution with bpf-gcc (which I don't have locally). Our CI will
> take care of testing Clang compilation. Thanks!

Tested as best I can, I don't have full runtime execution testing working yet
with my cross compile environment but it does fix that build issue I was hitting
at least.

https://lore.kernel.org/bpf/[email protected]/

>
> >
> > >
> > >
> > > >
> > > > >
> > > > > >
> > > > > > >
> > > > > > > > /* Avoid 'linux/stddef.h' definition of '__always_inline'. */
> > > > > > > > #undef __always_inline
> > > > > > > > --
> > > > > > > > 2.25.1
> > > > > > > >