| Message ID | 20210416202404.3443623-4-andrii@kernel.org |
|---|---|
| State | New |
| Headers | show |
| Series | BPF static linker: support externs | expand |
On 4/16/21 1:23 PM, Andrii Nakryiko wrote: > When used on externs SEC() macro will trigger compilation warning about > inapplicable `__attribute__((used))`. That's expected for extern declarations, > so suppress it with the corresponding _Pragma. > > Signed-off-by: Andrii Nakryiko <andrii@kernel.org> Ack with some comments below. Acked-by: Yonghong Song <yhs@fb.com> > --- > tools/lib/bpf/bpf_helpers.h | 11 +++++++++-- > 1 file changed, 9 insertions(+), 2 deletions(-) > > diff --git a/tools/lib/bpf/bpf_helpers.h b/tools/lib/bpf/bpf_helpers.h > index b904128626c2..75c7581b304c 100644 > --- a/tools/lib/bpf/bpf_helpers.h > +++ b/tools/lib/bpf/bpf_helpers.h > @@ -25,9 +25,16 @@ > /* > * Helper macro to place programs, maps, license in > * different sections in elf_bpf file. Section names > - * are interpreted by elf_bpf loader > + * are interpreted by libbpf depending on the context (BPF programs, BPF maps, > + * extern variables, etc). > + * To allow use of SEC() with externs (e.g., for extern .maps declarations), > + * make sure __attribute__((unused)) doesn't trigger compilation warning. > */ > -#define SEC(NAME) __attribute__((section(NAME), used)) > +#define SEC(name) \ > + _Pragma("GCC diagnostic push") \ > + _Pragma("GCC diagnostic ignored \"-Wignored-attributes\"") \ > + __attribute__((section(name), used)) \ > + _Pragma("GCC diagnostic pop") \ The 'used' attribute is mostly useful for static variable/functions since otherwise if not really used, the compiler could delete them freely. The 'used' attribute does not really have an impact on global variables regardless whether they are used or not in a particular compilation unit. We could drop 'used' here and selftests should still work. The only worry is that if people define something like static int _version SEC("version") = 1; static char _license[] SEC("license") = "GPL"; Removing 'used' may cause failure. Since we don't want to remove 'used', then adding _Pragma to silence the warning is a right thing to do here. > > /* Avoid 'linux/stddef.h' definition of '__always_inline'. */ > #undef __always_inline >
On Wed, Apr 21, 2021 at 8:48 PM Yonghong Song <yhs@fb.com> wrote: > > > > On 4/16/21 1:23 PM, Andrii Nakryiko wrote: > > When used on externs SEC() macro will trigger compilation warning about > > inapplicable `__attribute__((used))`. That's expected for extern declarations, > > so suppress it with the corresponding _Pragma. > > > > Signed-off-by: Andrii Nakryiko <andrii@kernel.org> > > Ack with some comments below. > Acked-by: Yonghong Song <yhs@fb.com> > > > --- > > tools/lib/bpf/bpf_helpers.h | 11 +++++++++-- > > 1 file changed, 9 insertions(+), 2 deletions(-) > > > > diff --git a/tools/lib/bpf/bpf_helpers.h b/tools/lib/bpf/bpf_helpers.h > > index b904128626c2..75c7581b304c 100644 > > --- a/tools/lib/bpf/bpf_helpers.h > > +++ b/tools/lib/bpf/bpf_helpers.h > > @@ -25,9 +25,16 @@ > > /* > > * Helper macro to place programs, maps, license in > > * different sections in elf_bpf file. Section names > > - * are interpreted by elf_bpf loader > > + * are interpreted by libbpf depending on the context (BPF programs, BPF maps, > > + * extern variables, etc). > > + * To allow use of SEC() with externs (e.g., for extern .maps declarations), > > + * make sure __attribute__((unused)) doesn't trigger compilation warning. > > */ > > -#define SEC(NAME) __attribute__((section(NAME), used)) > > +#define SEC(name) \ > > + _Pragma("GCC diagnostic push") \ > > + _Pragma("GCC diagnostic ignored \"-Wignored-attributes\"") \ > > + __attribute__((section(name), used)) \ > > + _Pragma("GCC diagnostic pop") \ > > The 'used' attribute is mostly useful for static variable/functions > since otherwise if not really used, the compiler could delete them > freely. The 'used' attribute does not really have an impact on > global variables regardless whether they are used or not in a particular > compilation unit. We could drop 'used' here and selftests should still > work. The only worry is that if people define something like > static int _version SEC("version") = 1; > static char _license[] SEC("license") = "GPL"; > Removing 'used' may cause failure. > > Since we don't want to remove 'used', then adding _Pragma to silence > the warning is a right thing to do here. Right, SEC() has become a universal macro used for functions, variables, and externs. I didn't want to introduce multiple variants, but also we can't break existing use cases. So pragma, luckily, allowed to support all cases. > > > > > /* Avoid 'linux/stddef.h' definition of '__always_inline'. */ > > #undef __always_inline > >
diff --git a/tools/lib/bpf/bpf_helpers.h b/tools/lib/bpf/bpf_helpers.h index b904128626c2..75c7581b304c 100644 --- a/tools/lib/bpf/bpf_helpers.h +++ b/tools/lib/bpf/bpf_helpers.h @@ -25,9 +25,16 @@ /* * Helper macro to place programs, maps, license in * different sections in elf_bpf file. Section names - * are interpreted by elf_bpf loader + * are interpreted by libbpf depending on the context (BPF programs, BPF maps, + * extern variables, etc). + * To allow use of SEC() with externs (e.g., for extern .maps declarations), + * make sure __attribute__((unused)) doesn't trigger compilation warning. */ -#define SEC(NAME) __attribute__((section(NAME), used)) +#define SEC(name) \ + _Pragma("GCC diagnostic push") \ + _Pragma("GCC diagnostic ignored \"-Wignored-attributes\"") \ + __attribute__((section(name), used)) \ + _Pragma("GCC diagnostic pop") \ /* Avoid 'linux/stddef.h' definition of '__always_inline'. */ #undef __always_inline
When used on externs SEC() macro will trigger compilation warning about inapplicable `__attribute__((used))`. That's expected for extern declarations, so suppress it with the corresponding _Pragma. Signed-off-by: Andrii Nakryiko <andrii@kernel.org> --- tools/lib/bpf/bpf_helpers.h | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-)