| Message ID | 20220606132741.3462925-1-james.hilliard1@gmail.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | BPF |
| Headers | show |
| Series | [1/1] libbpf: fix broken gcc pragma macros in bpf_helpers.h/bpf_tracing.h | expand |
| Context | Check | Description |
|---|---|---|
| netdev/tree_selection | success | Not a local patch |
| bpf/vmtest-bpf-next-PR | fail | PR summary |
| bpf/vmtest-bpf-next-VM_Test-3 | fail | Logs for Kernel LATEST on z15 with gcc |
| bpf/vmtest-bpf-next-VM_Test-1 | fail | Logs for Kernel LATEST on ubuntu-latest with gcc |
| bpf/vmtest-bpf-next-VM_Test-2 | fail | Logs for Kernel LATEST on ubuntu-latest with llvm-15 |
On Mon, Jun 6, 2022 at 6:28 AM James Hilliard <james.hilliard1@gmail.com> 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? _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. Let's first try to root cause this. > Signed-off-by: James Hilliard <james.hilliard1@gmail.com> > --- > 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 >
On Mon, Jun 6, 2022 at 12:02 PM Andrii Nakryiko <andrii.nakryiko@gmail.com> wrote: > > On Mon, Jun 6, 2022 at 6:28 AM James Hilliard <james.hilliard1@gmail.com> 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@gmail.com> > > --- > > 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 > >
On Mon, Jun 6, 2022 at 2:20 PM James Hilliard <james.hilliard1@gmail.com> wrote: > > On Mon, Jun 6, 2022 at 12:02 PM Andrii Nakryiko > <andrii.nakryiko@gmail.com> wrote: > > > > On Mon, Jun 6, 2022 at 6:28 AM James Hilliard <james.hilliard1@gmail.com> 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 It would be great to be able to repro it as part of selftests. Can you try gcc 12 with selftests/bpf and see if you get the same problem? > > > _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 If you are referring to DO_PRAGMA example? That example is done that way to do argument stringification, but not because _Pragma can't be used as is in macros. > > > 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 > I don't like the obscurity of the changes in this patch and don't see how it fundamentally changes anything. So I'd like to actually try to be able to repro it and see what other solutions there are before committing to this. I also suspect that it's only the SEC() macro that's problematic and we shouldn't touch any other macro at all. But again, I'd like to get a repro first. > > > > > Signed-off-by: James Hilliard <james.hilliard1@gmail.com> > > > --- > > > 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 > > >
On Tue, Jun 7, 2022 at 5:21 PM Andrii Nakryiko <andrii.nakryiko@gmail.com> wrote: > > On Mon, Jun 6, 2022 at 2:20 PM James Hilliard <james.hilliard1@gmail.com> wrote: > > > > On Mon, Jun 6, 2022 at 12:02 PM Andrii Nakryiko > > <andrii.nakryiko@gmail.com> wrote: > > > > > > On Mon, Jun 6, 2022 at 6:28 AM James Hilliard <james.hilliard1@gmail.com> 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 > > It would be great to be able to repro it as part of selftests. Can you > try gcc 12 with selftests/bpf and see if you get the same problem? > > > > > > _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 > > > If you are referring to DO_PRAGMA example? That example is done that > way to do argument stringification, but not because _Pragma can't be > used as is in macros. > > > > > > 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 > > > > I don't like the obscurity of the changes in this patch and don't see > how it fundamentally changes anything. So I'd like to actually try to > be able to repro it and see what other solutions there are before > committing to this. > > I also suspect that it's only the SEC() macro that's problematic and > we shouldn't touch any other macro at all. But again, I'd like to get > a repro first. Seems typeof was triggering the issue, replacing it with __typeof__ seems to fix it: https://lore.kernel.org/bpf/20220608064004.1493239-1-james.hilliard1@gmail.com/ Seems similar to this issue: https://patches.dpdk.org/project/dpdk/patch/2601191342CEEE43887BDE71AB977258213F3012@irsmsx105.ger.corp.intel.com/ Looks like gcc and llvm document there's some differences between __typeof__ and typeof with __typeof__ being the more portable variant: https://gcc.gnu.org/onlinedocs/gcc/Typeof.html https://clang.llvm.org/docs/UsersManual.html#differences-between-various-standard-modes > > > > > > > > Signed-off-by: James Hilliard <james.hilliard1@gmail.com> > > > > --- > > > > 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 > > > >
On Tue, Jun 7, 2022 at 5:21 PM Andrii Nakryiko <andrii.nakryiko@gmail.com> wrote: > > On Mon, Jun 6, 2022 at 2:20 PM James Hilliard <james.hilliard1@gmail.com> wrote: > > > > On Mon, Jun 6, 2022 at 12:02 PM Andrii Nakryiko > > <andrii.nakryiko@gmail.com> wrote: > > > > > > On Mon, Jun 6, 2022 at 6:28 AM James Hilliard <james.hilliard1@gmail.com> 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 > > It would be great to be able to repro it as part of selftests. Can you > try gcc 12 with selftests/bpf and see if you get the same problem? > > > > > > _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 > > > If you are referring to DO_PRAGMA example? That example is done that > way to do argument stringification, but not because _Pragma can't be > used as is in macros. > > > > > > 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 > > > > I don't like the obscurity of the changes in this patch and don't see > how it fundamentally changes anything. So I'd like to actually try to > be able to repro it and see what other solutions there are before > committing to this. > > I also suspect that it's only the SEC() macro that's problematic and > we shouldn't touch any other macro at all. But again, I'd like to get > a repro first. Ok, yeah looks like it's just the SEC() macro, resent with just that changed: https://lore.kernel.org/bpf/20220609062412.3950380-1-james.hilliard1@gmail.com/ Seems there's a separate issue with -std=c17 and typeof(): https://lore.kernel.org/bpf/20220609062829.293217-1-james.hilliard1@gmail.com/ > > > > > > > > Signed-off-by: James Hilliard <james.hilliard1@gmail.com> > > > > --- > > > > 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 > > > >
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)
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"; | ^~~ Signed-off-by: James Hilliard <james.hilliard1@gmail.com> --- tools/lib/bpf/bpf_helpers.h | 26 ++++++++++++++------------ tools/lib/bpf/bpf_tracing.h | 26 ++++++++++++++------------ 2 files changed, 28 insertions(+), 24 deletions(-)