| Message ID | 20220703212551.1114923-3-andres@anarazel.de (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | BPF |
| Headers | show |
| Series | tools: fix compilation failure caused by init_disassemble_info API changes | expand |
| Context | Check | Description |
|---|---|---|
| netdev/tree_selection | success | Not a local patch |
| bpf/vmtest-bpf-next-VM_Test-1 | success | 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 |
| bpf/vmtest-bpf-next-PR | fail | PR summary |
| bpf/vmtest-bpf-next-VM_Test-3 | success | Logs for Kernel LATEST on z15 with gcc |
On 03/07/2022 22:25, Andres Freund wrote: > binutils changed the signature of init_disassemble_info(), which now causes > compilation failures for tools/{perf,bpf}, e.g. on debian unstable. > Relevant binutils commit: > https://sourceware.org/git/?p=binutils-gdb.git;a=commit;h=60a3da00bd5407f07 > > This commit introduces a wrapper for init_disassemble_info(), to avoid > spreading #ifdef DISASM_INIT_STYLED to a bunch of places. Subsequent > commits will use it to fix the build failures. > > It likely is worth adding a wrapper for disassember(), to avoid the already > existing DISASM_FOUR_ARGS_SIGNATURE ifdefery. > > Cc: Alexei Starovoitov <ast@kernel.org> > Cc: Arnaldo Carvalho de Melo <acme@redhat.com> > Cc: Sedat Dilek <sedat.dilek@gmail.com> > Cc: Quentin Monnet <quentin@isovalent.com> > Link: http://lore.kernel.org/lkml/20220622181918.ykrs5rsnmx3og4sv@alap3.anarazel.de > Signed-off-by: Andres Freund <andres@anarazel.de> > --- > tools/include/tools/dis-asm-compat.h | 53 ++++++++++++++++++++++++++++ > 1 file changed, 53 insertions(+) > create mode 100644 tools/include/tools/dis-asm-compat.h > > diff --git a/tools/include/tools/dis-asm-compat.h b/tools/include/tools/dis-asm-compat.h > new file mode 100644 > index 000000000000..d1d003ee3e2f > --- /dev/null > +++ b/tools/include/tools/dis-asm-compat.h > @@ -0,0 +1,53 @@ > +/* SPDX-License-Identifier: GPL-2.0 */ Any chance you could contribute this wrapper as dual-licenced (GPL-2.0-only OR BSD-2-Clause), for better compatibility with the rest of bpftool's code? The rest of the set looks good to me. Thanks a lot for this work! Quentin
Hi, On 2022-07-05 14:44:07 +0100, Quentin Monnet wrote: > > diff --git a/tools/include/tools/dis-asm-compat.h b/tools/include/tools/dis-asm-compat.h > > new file mode 100644 > > index 000000000000..d1d003ee3e2f > > --- /dev/null > > +++ b/tools/include/tools/dis-asm-compat.h > > @@ -0,0 +1,53 @@ > > +/* SPDX-License-Identifier: GPL-2.0 */ > > Any chance you could contribute this wrapper as dual-licenced > (GPL-2.0-only OR BSD-2-Clause), for better compatibility with the rest > of bpftool's code? Happy to do that from my end - however, right now it includes linux/compiler.h, which is GPL-2.0. I don't know what the policy around that is - is it just a statement about the licence of the header itself, or does it effectively include its dependencies? FWIW, linux/compiler.h is also included from bpftool. If preferrable, I can replace the linux/compiler.h include by just using __attribute__((__unused__)) directly or by using a (void) cast to avoid the unused-parameter pedantry. Greetings, Andres Freund
Hi, On 2022-07-15 12:39:27 -0700, Andres Freund wrote: > On 2022-07-05 14:44:07 +0100, Quentin Monnet wrote: > > > diff --git a/tools/include/tools/dis-asm-compat.h b/tools/include/tools/dis-asm-compat.h > > > new file mode 100644 > > > index 000000000000..d1d003ee3e2f > > > --- /dev/null > > > +++ b/tools/include/tools/dis-asm-compat.h > > > @@ -0,0 +1,53 @@ > > > +/* SPDX-License-Identifier: GPL-2.0 */ > > > > Any chance you could contribute this wrapper as dual-licenced > > (GPL-2.0-only OR BSD-2-Clause), for better compatibility with the rest > > of bpftool's code? > > Happy to do that from my end - however, right now it includes > linux/compiler.h, which is GPL-2.0. I don't know what the policy around that > is - is it just a statement about the licence of the header itself, or does it > effectively include its dependencies? FWIW, dis-asm.h itself is GPL-3-or-later. Greetings, Andres Freund
On 15/07/2022 20:39, Andres Freund wrote: > Hi, > > On 2022-07-05 14:44:07 +0100, Quentin Monnet wrote: >>> diff --git a/tools/include/tools/dis-asm-compat.h b/tools/include/tools/dis-asm-compat.h >>> new file mode 100644 >>> index 000000000000..d1d003ee3e2f >>> --- /dev/null >>> +++ b/tools/include/tools/dis-asm-compat.h >>> @@ -0,0 +1,53 @@ >>> +/* SPDX-License-Identifier: GPL-2.0 */ >> >> Any chance you could contribute this wrapper as dual-licenced >> (GPL-2.0-only OR BSD-2-Clause), for better compatibility with the rest >> of bpftool's code? > > Happy to do that from my end - however, right now it includes > linux/compiler.h, which is GPL-2.0. I don't know what the policy around that > is - is it just a statement about the licence of the header itself, or does it > effectively include its dependencies? My understanding is that programs using a GPL header need to be released as GPL, but I don't believe they have to be only GPL, the dual-license should cover the requirements. If someone wanted to redistribute the code from the new header dis-asm-compat.h as BSD only, they would probably have to get rid of the GPL-only dependencies though. But again, this is only my understanding, and “I am not a lawyer”. > > FWIW, linux/compiler.h is also included from bpftool. > > If preferrable, I can replace the linux/compiler.h include by just using > __attribute__((__unused__)) directly or by using a (void) cast to avoid the > unused-parameter pedantry. If compiler.h is just needed for the “unused” attribute, I wouldn't mind doing that. Thanks, Quentin
diff --git a/tools/include/tools/dis-asm-compat.h b/tools/include/tools/dis-asm-compat.h new file mode 100644 index 000000000000..d1d003ee3e2f --- /dev/null +++ b/tools/include/tools/dis-asm-compat.h @@ -0,0 +1,53 @@ +/* SPDX-License-Identifier: GPL-2.0 */ +#ifndef _TOOLS_DIS_ASM_COMPAT_H +#define _TOOLS_DIS_ASM_COMPAT_H + +#include <stdio.h> +#include <linux/compiler.h> +#include <dis-asm.h> + +/* define types for older binutils version, to centralize ifdef'ery a bit */ +#ifndef DISASM_INIT_STYLED +enum disassembler_style {DISASSEMBLER_STYLE_NOT_EMPTY}; +typedef int (*fprintf_styled_ftype) (void *, enum disassembler_style, const char*, ...); +#endif + +/* + * Trivial fprintf wrapper to be used as the fprintf_styled_func argument to + * init_disassemble_info_compat() when normal fprintf suffices. + */ +static inline int fprintf_styled(void *out, + enum disassembler_style style __maybe_unused, + const char *fmt, ...) +{ + va_list args; + int r; + + va_start(args, fmt); + r = vfprintf(out, fmt, args); + va_end(args); + + return r; +} + +/* + * Wrapper for init_disassemble_info() that hides version + * differences. Depending on binutils version and architecture either + * fprintf_func or fprintf_styled_func will be called. + */ +static inline void init_disassemble_info_compat(struct disassemble_info *info, + void *stream, + fprintf_ftype unstyled_func, + fprintf_styled_ftype styled_func __maybe_unused) +{ +#ifdef DISASM_INIT_STYLED + init_disassemble_info(info, stream, + unstyled_func, + styled_func); +#else + init_disassemble_info(info, stream, + unstyled_func); +#endif +} + +#endif /* _TOOLS_DIS_ASM_COMPAT_H */
binutils changed the signature of init_disassemble_info(), which now causes compilation failures for tools/{perf,bpf}, e.g. on debian unstable. Relevant binutils commit: https://sourceware.org/git/?p=binutils-gdb.git;a=commit;h=60a3da00bd5407f07 This commit introduces a wrapper for init_disassemble_info(), to avoid spreading #ifdef DISASM_INIT_STYLED to a bunch of places. Subsequent commits will use it to fix the build failures. It likely is worth adding a wrapper for disassember(), to avoid the already existing DISASM_FOUR_ARGS_SIGNATURE ifdefery. Cc: Alexei Starovoitov <ast@kernel.org> Cc: Arnaldo Carvalho de Melo <acme@redhat.com> Cc: Sedat Dilek <sedat.dilek@gmail.com> Cc: Quentin Monnet <quentin@isovalent.com> Link: http://lore.kernel.org/lkml/20220622181918.ykrs5rsnmx3og4sv@alap3.anarazel.de Signed-off-by: Andres Freund <andres@anarazel.de> --- tools/include/tools/dis-asm-compat.h | 53 ++++++++++++++++++++++++++++ 1 file changed, 53 insertions(+) create mode 100644 tools/include/tools/dis-asm-compat.h