Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
MIME-Version: 1.0
References: <20220609062412.3950380-1-james.hilliard1@gmail.com> <CAEf4BzbL8ivLH=HZDFTNyCTFjhWrWLcY3K34Ef+q4Pr+oDe_Gw@mail.gmail.com>
In-Reply-To: <CAEf4BzbL8ivLH=HZDFTNyCTFjhWrWLcY3K34Ef+q4Pr+oDe_Gw@mail.gmail.com>
From:   James Hilliard <james.hilliard1@gmail.com>
Date:   Thu, 9 Jun 2022 17:27:20 -0600
Message-ID: <CADvTj4opMh978fMBV7cH89wbS1N_PK31AybZJ5NUacnp4kBeqg@mail.gmail.com>
Subject: Re: [PATCH v3 1/1] libbpf: fix broken gcc SEC pragma macro
To:     Andrii Nakryiko <andrii.nakryiko@gmail.com>
Cc:     bpf <bpf@vger.kernel.org>, Alexei Starovoitov <ast@kernel.org>,
        Daniel Borkmann <daniel@iogearbox.net>,
        Andrii Nakryiko <andrii@kernel.org>,
        Martin KaFai Lau <kafai@fb.com>,
        Song Liu <songliubraving@fb.com>, Yonghong Song <yhs@fb.com>,
        John Fastabend <john.fastabend@gmail.com>,
        KP Singh <kpsingh@kernel.org>,
        "open list:BPF (Safe dynamic programs and tools)" 
        <netdev@vger.kernel.org>, open list <linux-kernel@vger.kernel.org>
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

On Thu, Jun 9, 2022 at 12:13 PM Andrii Nakryiko
<andrii.nakryiko@gmail.com> wrote:
>
> On Wed, Jun 8, 2022 at 11:24 PM James Hilliard
> <james.hilliard1@gmail.com> 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 <james.hilliard1@gmail.com>
> > ---
> > 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
> >