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>
In-Reply-To: <20220609062412.3950380-1-james.hilliard1@gmail.com>
From:   Andrii Nakryiko <andrii.nakryiko@gmail.com>
Date:   Thu, 9 Jun 2022 11:13:08 -0700
Message-ID: <CAEf4BzbL8ivLH=HZDFTNyCTFjhWrWLcY3K34Ef+q4Pr+oDe_Gw@mail.gmail.com>
Subject: Re: [PATCH v3 1/1] libbpf: fix broken gcc SEC pragma macro
To:     James Hilliard <james.hilliard1@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 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

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