Re: [PATCH 1/1] libbpf: fix broken gcc pragma macros in bpf_helpers.h/bpf_tracing.h
From: James Hilliard
Date: Mon Jun 06 2022 - 17:20:45 EST
On Mon, Jun 6, 2022 at 12:02 PM Andrii Nakryiko
<andrii.nakryiko@xxxxxxxxx> wrote:
>
> On Mon, Jun 6, 2022 at 6:28 AM James Hilliard <james.hilliard1@xxxxxxxxx> wrote:
> >
> > It seems the gcc preprocessor breaks unless pragmas are wrapped
> > individually inside macros.
> >
> > 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";
> > | ^~~
> >
>
> We've been using this macro in this form for a while with no errors.
> How do you get these errors in the first place?
I was attempting to compile the systemd bpf programs using gcc 12.1.
https://github.com/systemd/systemd/tree/main/src/core/bpf
> _Pragma is supposed to
> be a full equivalent of #pragma specifically to be able to be used in
> macros, so these work-arounds shouldn't be necessary.
I did try and style this like the nested macro example here:
https://gcc.gnu.org/onlinedocs/gcc/Diagnostic-Pragmas.html
> Let's first try
> to root cause this.
I was looking around and it seems there's a bunch of gcc preprocessor
pragma issues in general, restyling this seemed to be the best option
at the moment since a lot looked to be unfixed:
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=53431
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=55578
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=89718
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=91669
>
> > Signed-off-by: James Hilliard <james.hilliard1@xxxxxxxxx>
> > ---
> > tools/lib/bpf/bpf_helpers.h | 26 ++++++++++++++------------
> > tools/lib/bpf/bpf_tracing.h | 26 ++++++++++++++------------
> > 2 files changed, 28 insertions(+), 24 deletions(-)
> >
> > diff --git a/tools/lib/bpf/bpf_helpers.h b/tools/lib/bpf/bpf_helpers.h
> > index fb04eaf367f1..6d159082727d 100644
> > --- a/tools/lib/bpf/bpf_helpers.h
> > +++ b/tools/lib/bpf/bpf_helpers.h
> > @@ -22,11 +22,13 @@
> > * To allow use of SEC() with externs (e.g., for extern .maps declarations),
> > * make sure __attribute__((unused)) doesn't trigger compilation warning.
> > */
> > +#define __gcc_helpers_pragma(x) _Pragma(#x)
> > +#define __gcc_helpers_diag_pragma(x) __gcc_helpers_pragma("GCC diagnostic " #x)
> > #define SEC(name) \
> > - _Pragma("GCC diagnostic push") \
> > - _Pragma("GCC diagnostic ignored \"-Wignored-attributes\"") \
> > + __gcc_helpers_diag_pragma(push) \
> > + __gcc_helpers_diag_pragma(ignored "-Wignored-attributes") \
> > __attribute__((section(name), used)) \
> > - _Pragma("GCC diagnostic pop") \
> > + __gcc_helpers_diag_pragma(pop)
> >
> > /* Avoid 'linux/stddef.h' definition of '__always_inline'. */
> > #undef __always_inline
> > @@ -215,10 +217,10 @@ enum libbpf_tristate {
> > static const char ___fmt[] = fmt; \
> > unsigned long long ___param[___bpf_narg(args)]; \
> > \
> > - _Pragma("GCC diagnostic push") \
> > - _Pragma("GCC diagnostic ignored \"-Wint-conversion\"") \
> > + __gcc_helpers_diag_pragma(push) \
> > + __gcc_helpers_diag_pragma(ignored "-Wint-conversion") \
> > ___bpf_fill(___param, args); \
> > - _Pragma("GCC diagnostic pop") \
> > + __gcc_helpers_diag_pragma(pop) \
> > \
> > bpf_seq_printf(seq, ___fmt, sizeof(___fmt), \
> > ___param, sizeof(___param)); \
> > @@ -233,10 +235,10 @@ enum libbpf_tristate {
> > static const char ___fmt[] = fmt; \
> > unsigned long long ___param[___bpf_narg(args)]; \
> > \
> > - _Pragma("GCC diagnostic push") \
> > - _Pragma("GCC diagnostic ignored \"-Wint-conversion\"") \
> > + __gcc_helpers_diag_pragma(push) \
> > + __gcc_helpers_diag_pragma(ignored "-Wint-conversion") \
> > ___bpf_fill(___param, args); \
> > - _Pragma("GCC diagnostic pop") \
> > + __gcc_helpers_diag_pragma(pop) \
> > \
> > bpf_snprintf(out, out_size, ___fmt, \
> > ___param, sizeof(___param)); \
> > @@ -264,10 +266,10 @@ enum libbpf_tristate {
> > static const char ___fmt[] = fmt; \
> > unsigned long long ___param[___bpf_narg(args)]; \
> > \
> > - _Pragma("GCC diagnostic push") \
> > - _Pragma("GCC diagnostic ignored \"-Wint-conversion\"") \
> > + __gcc_helpers_diag_pragma(push) \
> > + __gcc_helpers_diag_pragma(ignored "-Wint-conversion") \
> > ___bpf_fill(___param, args); \
> > - _Pragma("GCC diagnostic pop") \
> > + __gcc_helpers_diag_pragma(pop) \
> > \
> > bpf_trace_vprintk(___fmt, sizeof(___fmt), \
> > ___param, sizeof(___param)); \
> > diff --git a/tools/lib/bpf/bpf_tracing.h b/tools/lib/bpf/bpf_tracing.h
> > index 01ce121c302d..e08ffc290b3e 100644
> > --- a/tools/lib/bpf/bpf_tracing.h
> > +++ b/tools/lib/bpf/bpf_tracing.h
> > @@ -422,16 +422,18 @@ struct pt_regs;
> > * This is useful when using BPF helpers that expect original context
> > * as one of the parameters (e.g., for bpf_perf_event_output()).
> > */
> > +#define __gcc_tracing_pragma(x) _Pragma(#x)
> > +#define __gcc_tracing_diag_pragma(x) __gcc_tracing_pragma("GCC diagnostic " #x)
> > #define BPF_PROG(name, args...) \
> > name(unsigned long long *ctx); \
> > static __attribute__((always_inline)) typeof(name(0)) \
> > ____##name(unsigned long long *ctx, ##args); \
> > typeof(name(0)) name(unsigned long long *ctx) \
> > { \
> > - _Pragma("GCC diagnostic push") \
> > - _Pragma("GCC diagnostic ignored \"-Wint-conversion\"") \
> > + __gcc_tracing_diag_pragma(push) \
> > + __gcc_tracing_diag_pragma(ignored "-Wint-conversion") \
> > return ____##name(___bpf_ctx_cast(args)); \
> > - _Pragma("GCC diagnostic pop") \
> > + __gcc_tracing_diag_pragma(pop) \
> > } \
> > static __attribute__((always_inline)) typeof(name(0)) \
> > ____##name(unsigned long long *ctx, ##args)
> > @@ -462,10 +464,10 @@ static __attribute__((always_inline)) typeof(name(0)) \
> > ____##name(struct pt_regs *ctx, ##args); \
> > typeof(name(0)) name(struct pt_regs *ctx) \
> > { \
> > - _Pragma("GCC diagnostic push") \
> > - _Pragma("GCC diagnostic ignored \"-Wint-conversion\"") \
> > + __gcc_tracing_diag_pragma(push) \
> > + __gcc_tracing_diag_pragma(ignored "-Wint-conversion") \
> > return ____##name(___bpf_kprobe_args(args)); \
> > - _Pragma("GCC diagnostic pop") \
> > + __gcc_tracing_diag_pragma(pop) \
> > } \
> > static __attribute__((always_inline)) typeof(name(0)) \
> > ____##name(struct pt_regs *ctx, ##args)
> > @@ -486,10 +488,10 @@ static __attribute__((always_inline)) typeof(name(0)) \
> > ____##name(struct pt_regs *ctx, ##args); \
> > typeof(name(0)) name(struct pt_regs *ctx) \
> > { \
> > - _Pragma("GCC diagnostic push") \
> > - _Pragma("GCC diagnostic ignored \"-Wint-conversion\"") \
> > + __gcc_tracing_diag_pragma(push) \
> > + __gcc_tracing_diag_pragma(ignored "-Wint-conversion") \
> > return ____##name(___bpf_kretprobe_args(args)); \
> > - _Pragma("GCC diagnostic pop") \
> > + __gcc_tracing_diag_pragma(pop) \
> > } \
> > static __always_inline typeof(name(0)) ____##name(struct pt_regs *ctx, ##args)
> >
> > @@ -520,10 +522,10 @@ ____##name(struct pt_regs *ctx, ##args); \
> > typeof(name(0)) name(struct pt_regs *ctx) \
> > { \
> > struct pt_regs *regs = PT_REGS_SYSCALL_REGS(ctx); \
> > - _Pragma("GCC diagnostic push") \
> > - _Pragma("GCC diagnostic ignored \"-Wint-conversion\"") \
> > + __gcc_tracing_diag_pragma(push) \
> > + __gcc_tracing_diag_pragma(ignored "-Wint-conversion") \
> > return ____##name(___bpf_syscall_args(args)); \
> > - _Pragma("GCC diagnostic pop") \
> > + __gcc_tracing_diag_pragma(pop) \
> > } \
> > static __attribute__((always_inline)) typeof(name(0)) \
> > ____##name(struct pt_regs *ctx, ##args)
> > --
> > 2.25.1
> >