init_disassemble_info() signature changes causes compile failures

init_disassemble_info() signature changes causes compile failures

[Andres Freund]

[-- Attachment #1: Type: text/plain, Size: 1190 bytes --]

Hi,

binutils changed the signature of init_disassemble_info(), which now causes
perf and bpftool to fail to compile (e.g. on debian unstable).

Relevant binutils commit: https://sourceware.org/git/?p=binutils-gdb.git;a=commitdiff;h=60a3da00bd5407f07d64dff82a4dae98230dfaac

util/annotate.c: In function ‘symbol__disassemble_bpf’:
util/annotate.c:1765:9: error: too few arguments to function ‘init_disassemble_info’
 1765 |         init_disassemble_info(&info, s,
      |         ^~~~~~~~~~~~~~~~~~~~~
In file included from util/annotate.c:1718:
/usr/include/dis-asm.h:472:13: note: declared here
  472 | extern void init_disassemble_info (struct disassemble_info *dinfo, void *stream,
      |             ^~~~~~~~~~~~~~~~~~~~~

with equivalent failures in

tools/bpf/bpf_jit_disasm.c
tools/bpf/bpftool/jit_disasm.c

The fix is easy enough, add a wrapper around fprintf() that conforms to the
new signature.

However I assume the necessary feature test and wrapper should only be added
once? I don't know the kernel stuff well enough to choose the right structure
here.

Attached is my local fix for perf. Obviously would need work to be a real
solution.

Greetings,

Andres Freund

[-- Attachment #2: perf-compile-bfd.diff --]
[-- Type: text/x-diff, Size: 923 bytes --]

diff --git i/tools/perf/util/annotate.c w/tools/perf/util/annotate.c
index 82cc396ef516..b0e364d235b4 100644
--- i/tools/perf/util/annotate.c
+++ w/tools/perf/util/annotate.c
@@ -1721,6 +1721,18 @@ static int dso__disassemble_filename(struct dso *dso, char *filename, size_t fil
 #include <bpf/libbpf.h>
 #include <linux/btf.h>
 
+static int fprintf_styled(void *, enum disassembler_style, const char* fmt, ...)
+{
+  va_list args;
+  int r;
+
+  va_start(args, fmt);
+  r = vprintf(fmt, args);
+  va_end(args);
+
+  return r;
+}
+
 static int symbol__disassemble_bpf(struct symbol *sym,
 				   struct annotate_args *args)
 {
@@ -1763,7 +1775,8 @@ static int symbol__disassemble_bpf(struct symbol *sym,
 		goto out;
 	}
 	init_disassemble_info(&info, s,
-			      (fprintf_ftype) fprintf);
+			      (fprintf_ftype) fprintf,
+			      fprintf_styled);
 
 	info.arch = bfd_get_arch(bfdf);
 	info.mach = bfd_get_mach(bfdf);

Re: init_disassemble_info() signature changes causes compile failures

[Quentin Monnet]

On Wed, 22 Jun 2022 at 19:19, Andres Freund <andres@anarazel.de> wrote:
>
> Hi,
>
> binutils changed the signature of init_disassemble_info(), which now causes
> perf and bpftool to fail to compile (e.g. on debian unstable).
>
> Relevant binutils commit: https://sourceware.org/git/?p=binutils-gdb.git;a=commitdiff;h=60a3da00bd5407f07d64dff82a4dae98230dfaac
>
> util/annotate.c: In function ‘symbol__disassemble_bpf’:
> util/annotate.c:1765:9: error: too few arguments to function ‘init_disassemble_info’
>  1765 |         init_disassemble_info(&info, s,
>       |         ^~~~~~~~~~~~~~~~~~~~~
> In file included from util/annotate.c:1718:
> /usr/include/dis-asm.h:472:13: note: declared here
>   472 | extern void init_disassemble_info (struct disassemble_info *dinfo, void *stream,
>       |             ^~~~~~~~~~~~~~~~~~~~~
>
> with equivalent failures in
>
> tools/bpf/bpf_jit_disasm.c
> tools/bpf/bpftool/jit_disasm.c

Hi Andres,

Too bad the libbfd API is changing again :/ But many thanks for
pinning down the relevant commit, and for the report!

> The fix is easy enough, add a wrapper around fprintf() that conforms to the
> new signature.
>
> However I assume the necessary feature test and wrapper should only be added
> once? I don't know the kernel stuff well enough to choose the right structure
> here.

We can probably find a common header for the wrapper under
tools/include/. One possibility could be a new header under
tools/include/tools/, like for libc_compat.h. Although personally I
don't mind too much about redefining the wrapper several times given
how short it is, and because maybe some tools could redefine it anyway
to use colour output in the future.

The feature test would better be shared, it would probably be similar
to what was done in the following commit to accommodate for a previous
change in libbfd:

    https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=fb982666e380c1632a74495b68b3c33a66e76430

> Attached is my local fix for perf. Obviously would need work to be a real
> solution.

Thanks a lot! Would you be willing to submit a patch for the feature
detection and wrapper?

Best regards,
Quentin

Re: init_disassemble_info() signature changes causes compile failures

[Andres Freund]

Hi,

On 2022-06-22 23:53:58 +0100, Quentin Monnet wrote:
> Too bad the libbfd API is changing again :/

Yea, not great. Particularly odd that
/* For compatibility with existing code.  */
#define INIT_DISASSEMBLE_INFO(INFO, STREAM, FPRINTF_FUNC, FPRINTF_STYLED_FUNC)  \

was changed. Leaving the "For compatibility with existing code." around,
despite obviously not providing compatibility...

CCed the author of that commit, maybe worth fixing?

Given that disassemble_set_printf() was added, it seems like it'd have been
easy to not change init_disassemble_info() / INIT_DISASSEMBLE_INFO() and
require disassemble_set_printf() to be called to get styled printf support.


> > The fix is easy enough, add a wrapper around fprintf() that conforms to the
> > new signature.
> >
> > However I assume the necessary feature test and wrapper should only be added
> > once? I don't know the kernel stuff well enough to choose the right structure
> > here.
> 
> We can probably find a common header for the wrapper under
> tools/include/. One possibility could be a new header under
> tools/include/tools/, like for libc_compat.h. Although personally I
> don't mind too much about redefining the wrapper several times given
> how short it is, and because maybe some tools could redefine it anyway
> to use colour output in the future.

I'm more bothered by duplicating the necessary ifdefery than duplicating the
short fprintf wrapper...


> The feature test would better be shared, it would probably be similar
> to what was done in the following commit to accommodate for a previous
> change in libbfd:
> 
>     https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=fb982666e380c1632a74495b68b3c33a66e76430

Ah, beautiful hand-rolled feature tests :)


> > Attached is my local fix for perf. Obviously would need work to be a real
> > solution.
> 
> Thanks a lot! Would you be willing to submit a patch for the feature
> detection and wrapper?

I'll give it a go, albeit probably not today.

Greetings,

Andres Freund

Re: init_disassemble_info() signature changes causes compile failures

[Andrew Burgess]

Andres Freund <andres@anarazel.de> writes:

> Hi,
>
> On 2022-06-22 23:53:58 +0100, Quentin Monnet wrote:
>> Too bad the libbfd API is changing again :/
>
> Yea, not great. Particularly odd that
> /* For compatibility with existing code.  */
> #define INIT_DISASSEMBLE_INFO(INFO, STREAM, FPRINTF_FUNC, FPRINTF_STYLED_FUNC)  \
>
> was changed. Leaving the "For compatibility with existing code." around,
> despite obviously not providing compatibility...
>
> CCed the author of that commit, maybe worth fixing?

Greetings!

First, massive appologies for breaking you existing code.  I wasn't
aware that the libopcodes disassembler was used by anything out of
tree.

> Given that disassemble_set_printf() was added, it seems like it'd have been
> easy to not change init_disassemble_info() / INIT_DISASSEMBLE_INFO() and
> require disassemble_set_printf() to be called to get styled printf support.

When I did this work I desperately wanted to maintain the original API
as much as possible.  But I couldn't figure out a way for this to work.

The problem is that we now have two classes of disassembler, those that
call the styled printf callback, and those that call the classic
non-styled printf.

When I originally did this work I did want to leave
INIT_DISASSEMBLE_INFO untouched, and provide a default styled printf
that would forward calls to the non-styled printf.

The problem I ran into is that the disassemble_info printf API is not
great.  If the API had passed the disassemble_info* as one of the
arguments then this would have been trivial.  But instead, we only get
passed a 'void *', which is one of the fields of disassemble_info.

As a result there's no easy way to forward calls from this imagined
default styled printf, to the actual, user supplied non-styled printf.

I did consider changing the 'void *' field from being the stream to
write to, to be the disassemble_info*, but then the non-styled printf
calls would need to be updated anyway, which would be a breaking change.

I also considered changing the 'void *' stream to be the
'disassemble_info*', then wrapping both styled and non-styled printf
calls.  This would allow me to provide a default for the styled printf,
and we can pull the required information from the 'disassemble_info*',
but the problem I have now is that the wrapper functions would be a
vararg function, and the user supplied printf functions are also vararg
functions.... and I didn't know how to forward the args from my wrapper
to the user supplied functions without changing the API of the user
supplied functions to take a va_list ... which again is an API breaking
change.

I'm aware that non of the above helps you at all, other than to say, I
didn't make this change without a little thought.  But, that doesn't
mean there isn't a better way this could have been done.  So, if you
have any suggestions, then I'm happy to give them a go.

Once again, sorry for the additional work this has created for you, and
if I can help at all to put this right, then please, do let me know.

Oh, and you're absolutely correct about the comment.  I should have just
deleted it.  Or really, I should have just deleted the macro entirely I
guess and forced everyone to call init_disassemble_info directly.  Bit
late for that now though!

Thanks,
Andrew


>> > The fix is easy enough, add a wrapper around fprintf() that conforms to the
>> > new signature.
>> >
>> > However I assume the necessary feature test and wrapper should only be added
>> > once? I don't know the kernel stuff well enough to choose the right structure
>> > here.
>> 
>> We can probably find a common header for the wrapper under
>> tools/include/. One possibility could be a new header under
>> tools/include/tools/, like for libc_compat.h. Although personally I
>> don't mind too much about redefining the wrapper several times given
>> how short it is, and because maybe some tools could redefine it anyway
>> to use colour output in the future.
>
> I'm more bothered by duplicating the necessary ifdefery than duplicating the
> short fprintf wrapper...
>
>
>> The feature test would better be shared, it would probably be similar
>> to what was done in the following commit to accommodate for a previous
>> change in libbfd:
>> 
>>     https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=fb982666e380c1632a74495b68b3c33a66e76430
>
> Ah, beautiful hand-rolled feature tests :)
>
>
>> > Attached is my local fix for perf. Obviously would need work to be a real
>> > solution.
>> 
>> Thanks a lot! Would you be willing to submit a patch for the feature
>> detection and wrapper?
>
> I'll give it a go, albeit probably not today.
>
> Greetings,
>
> Andres Freund

[PATCH v1 0/3] tools: fix compilation failure caused by init_disassemble_info API changes

[Andres Freund]

binutils changed the signature of init_disassemble_info(), which now causes
compilation failures for tools/{perf,bpf} on e.g. debian unstable. Relevant
binutils commit:
https://sourceware.org/git/?p=binutils-gdb.git;a=commit;h=60a3da00bd5407f07

I first fixed this without introducing the compat header, as suggested by
Quentin, but I thought the amount of repeated boilerplate was a bit too
much. So instead I introduced a compat header to wrap the API changes. Even
tools/bpf/bpftool/jit_disasm.c, which needs its own callbacks for json, imo
looks nicer this way.

I'm not regular contributor, so it very well might be my procedures are a
bit off...

I wasn't sure whether the split of the commits conforms with the kernel
habits should the changes to tools/bpf and tools/perf be split into
separate commits?

Nor was I sure if I the right [number of] people to CC?

WRT the feature test: Not sure what the point of the -DPACKAGE='"perf"' is,
nor why tools/perf/Makefile.config sets some LDFLAGS/CFLAGS that are also
in feature/Makefile and why -ldl isn't needed in the other places. But...

Andres Freund (3):
  tools build: add feature test for init_disassemble_info API changes
  tools: add dis-asm-compat.h to centralize handling of version
    differences
  tools: Use tools/dis-asm-compat.h to fix compilation errors with new
    binutils

To: bpf@vger.kernel.org
To: linux-kernel@vger.kernel.org
Cc: Quentin Monnet <quentin@isovalent.com>
Cc: Alexei Starovoitov <ast@kernel.org>
Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: Jiri Olsa <jolsa@kernel.org>

 tools/bpf/Makefile                            |  7 ++-
 tools/bpf/bpf_jit_disasm.c                    |  5 +-
 tools/bpf/bpftool/Makefile                    |  7 ++-
 tools/bpf/bpftool/jit_disasm.c                | 40 +++++++++++---
 tools/build/Makefile.feature                  |  4 +-
 tools/build/feature/Makefile                  |  4 ++
 tools/build/feature/test-all.c                |  4 ++
 .../feature/test-disassembler-init-styled.c   | 13 +++++
 tools/include/tools/dis-asm-compat.h          | 53 +++++++++++++++++++
 tools/perf/Makefile.config                    |  8 +++
 tools/perf/util/annotate.c                    |  7 +--
 11 files changed, 135 insertions(+), 17 deletions(-)
 create mode 100644 tools/build/feature/test-disassembler-init-styled.c
 create mode 100644 tools/include/tools/dis-asm-compat.h

-- 
2.35.1.677.gabf474a5dd

[PATCH v1 1/3] tools build: add feature test for init_disassemble_info API changes

[Andres Freund]

binutils changed the signature of init_disassemble_info(), which now causes
compilation failures for tools/{perf,bpf} on e.g. debian unstable. Relevant
binutils commit:
https://sourceware.org/git/?p=binutils-gdb.git;a=commit;h=60a3da00bd5407f07

This commit adds a feature test to detect the new signature and enables it
for tools using init_disassemble_info().  Subsequent commits will change
the code.

Cc: Quentin Monnet <quentin@isovalent.com>
Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: Jiri Olsa <jolsa@kernel.org>
Link: http://lore.kernel.org/lkml/20220622181918.ykrs5rsnmx3og4sv@alap3.anarazel.de
Signed-off-by: Andres Freund <andres@anarazel.de>
---
 tools/bpf/Makefile                                  |  7 +++++--
 tools/bpf/bpftool/Makefile                          |  7 +++++--
 tools/build/Makefile.feature                        |  4 +++-
 tools/build/feature/Makefile                        |  4 ++++
 tools/build/feature/test-all.c                      |  4 ++++
 tools/build/feature/test-disassembler-init-styled.c | 13 +++++++++++++
 tools/perf/Makefile.config                          |  8 ++++++++
 7 files changed, 42 insertions(+), 5 deletions(-)
 create mode 100644 tools/build/feature/test-disassembler-init-styled.c

diff --git a/tools/bpf/Makefile b/tools/bpf/Makefile
index b11cfc86a3d0..9c4e61c3a92b 100644
--- a/tools/bpf/Makefile
+++ b/tools/bpf/Makefile
@@ -34,8 +34,8 @@ else
 endif
 
 FEATURE_USER = .bpf
-FEATURE_TESTS = libbfd disassembler-four-args
-FEATURE_DISPLAY = libbfd disassembler-four-args
+FEATURE_TESTS = libbfd disassembler-four-args disassembler-init-styled
+FEATURE_DISPLAY = libbfd disassembler-four-args disassembler-init-styled
 
 check_feat := 1
 NON_CHECK_FEAT_TARGETS := clean bpftool_clean runqslower_clean resolve_btfids_clean
@@ -56,6 +56,9 @@ endif
 ifeq ($(feature-disassembler-four-args), 1)
 CFLAGS += -DDISASM_FOUR_ARGS_SIGNATURE
 endif
+ifeq ($(feature-disassembler-init-styled), 1)
+CFLAGS += -DDISASM_INIT_STYLED
+endif
 
 $(OUTPUT)%.yacc.c: $(srctree)/tools/bpf/%.y
 	$(QUIET_BISON)$(YACC) -o $@ -d $<
diff --git a/tools/bpf/bpftool/Makefile b/tools/bpf/bpftool/Makefile
index c6d2c77d0252..62195118d377 100644
--- a/tools/bpf/bpftool/Makefile
+++ b/tools/bpf/bpftool/Makefile
@@ -93,9 +93,9 @@ INSTALL ?= install
 RM ?= rm -f
 
 FEATURE_USER = .bpftool
-FEATURE_TESTS = libbfd disassembler-four-args zlib libcap \
+FEATURE_TESTS = libbfd disassembler-four-args disassembler-init-styled zlib libcap \
 	clang-bpf-co-re
-FEATURE_DISPLAY = libbfd disassembler-four-args zlib libcap \
+FEATURE_DISPLAY = libbfd disassembler-four-args disassembler-init-styled zlib libcap \
 	clang-bpf-co-re
 
 check_feat := 1
@@ -117,6 +117,9 @@ endif
 ifeq ($(feature-disassembler-four-args), 1)
 CFLAGS += -DDISASM_FOUR_ARGS_SIGNATURE
 endif
+ifeq ($(feature-disassembler-init-styled), 1)
+    CFLAGS += -DDISASM_INIT_STYLED
+endif
 
 LIBS = $(LIBBPF) -lelf -lz
 LIBS_BOOTSTRAP = $(LIBBPF_BOOTSTRAP) -lelf -lz
diff --git a/tools/build/Makefile.feature b/tools/build/Makefile.feature
index 888a0421d43b..339686b99a6e 100644
--- a/tools/build/Makefile.feature
+++ b/tools/build/Makefile.feature
@@ -70,6 +70,7 @@ FEATURE_TESTS_BASIC :=                  \
         libaio				\
         libzstd				\
         disassembler-four-args		\
+        disassembler-init-styled	\
         file-handle
 
 # FEATURE_TESTS_BASIC + FEATURE_TESTS_EXTRA is the complete list
@@ -135,7 +136,8 @@ FEATURE_DISPLAY ?=              \
          bpf			\
          libaio			\
          libzstd		\
-         disassembler-four-args
+         disassembler-four-args	\
+         disassembler-init-styled
 
 # Set FEATURE_CHECK_(C|LD)FLAGS-all for all FEATURE_TESTS features.
 # If in the future we need per-feature checks/flags for features not
diff --git a/tools/build/feature/Makefile b/tools/build/feature/Makefile
index 7c2a17e23c30..c3059739318a 100644
--- a/tools/build/feature/Makefile
+++ b/tools/build/feature/Makefile
@@ -18,6 +18,7 @@ FILES=                                          \
          test-libbfd.bin                        \
          test-libbfd-buildid.bin		\
          test-disassembler-four-args.bin        \
+         test-disassembler-init-styled.bin	\
          test-reallocarray.bin			\
          test-libbfd-liberty.bin                \
          test-libbfd-liberty-z.bin              \
@@ -248,6 +249,9 @@ $(OUTPUT)test-libbfd-buildid.bin:
 $(OUTPUT)test-disassembler-four-args.bin:
 	$(BUILD) -DPACKAGE='"perf"' -lbfd -lopcodes
 
+$(OUTPUT)test-disassembler-init-styled.bin:
+	$(BUILD) -DPACKAGE='"perf"' -lbfd -lopcodes
+
 $(OUTPUT)test-reallocarray.bin:
 	$(BUILD)
 
diff --git a/tools/build/feature/test-all.c b/tools/build/feature/test-all.c
index 5ffafb967b6e..957c02c7b163 100644
--- a/tools/build/feature/test-all.c
+++ b/tools/build/feature/test-all.c
@@ -166,6 +166,10 @@
 # include "test-disassembler-four-args.c"
 #undef main
 
+#define main main_test_disassembler_init_styled
+# include "test-disassembler-init-styled.c"
+#undef main
+
 #define main main_test_libzstd
 # include "test-libzstd.c"
 #undef main
diff --git a/tools/build/feature/test-disassembler-init-styled.c b/tools/build/feature/test-disassembler-init-styled.c
new file mode 100644
index 000000000000..f1ce0ec3bee9
--- /dev/null
+++ b/tools/build/feature/test-disassembler-init-styled.c
@@ -0,0 +1,13 @@
+// SPDX-License-Identifier: GPL-2.0
+#include <stdio.h>
+#include <dis-asm.h>
+
+int main(void)
+{
+	struct disassemble_info info;
+
+	init_disassemble_info(&info, stdout,
+			      NULL, NULL);
+
+	return 0;
+}
diff --git a/tools/perf/Makefile.config b/tools/perf/Makefile.config
index 73e0762092fe..ee417c321adb 100644
--- a/tools/perf/Makefile.config
+++ b/tools/perf/Makefile.config
@@ -298,6 +298,7 @@ FEATURE_CHECK_LDFLAGS-libpython := $(PYTHON_EMBED_LDOPTS)
 FEATURE_CHECK_LDFLAGS-libaio = -lrt
 
 FEATURE_CHECK_LDFLAGS-disassembler-four-args = -lbfd -lopcodes -ldl
+FEATURE_CHECK_LDFLAGS-disassembler-init-styled = -lbfd -lopcodes -ldl
 
 CORE_CFLAGS += -fno-omit-frame-pointer
 CORE_CFLAGS += -ggdb3
@@ -905,13 +906,16 @@ ifndef NO_LIBBFD
     ifeq ($(feature-libbfd-liberty), 1)
       EXTLIBS += -lbfd -lopcodes -liberty
       FEATURE_CHECK_LDFLAGS-disassembler-four-args += -liberty -ldl
+      FEATURE_CHECK_LDFLAGS-disassembler-init-styled += -liberty -ldl
     else
       ifeq ($(feature-libbfd-liberty-z), 1)
         EXTLIBS += -lbfd -lopcodes -liberty -lz
         FEATURE_CHECK_LDFLAGS-disassembler-four-args += -liberty -lz -ldl
+        FEATURE_CHECK_LDFLAGS-disassembler-init-styled += -liberty -lz -ldl
       endif
     endif
     $(call feature_check,disassembler-four-args)
+    $(call feature_check,disassembler-init-styled)
   endif
 
   ifeq ($(feature-libbfd-buildid), 1)
@@ -1025,6 +1029,10 @@ ifeq ($(feature-disassembler-four-args), 1)
     CFLAGS += -DDISASM_FOUR_ARGS_SIGNATURE
 endif
 
+ifeq ($(feature-disassembler-init-styled), 1)
+    CFLAGS += -DDISASM_INIT_STYLED
+endif
+
 ifeq (${IS_64_BIT}, 1)
   ifndef NO_PERF_READ_VDSO32
     $(call feature_check,compile-32)
-- 
2.35.1.677.gabf474a5dd

[PATCH v1 2/3] tools: add dis-asm-compat.h to centralize handling of version differences

[Andres Freund]

binutils changed the signature of init_disassemble_info(), which now causes
compilation failures for tools/{perf,bpf} on e.g. 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.

It likely is worth adding a wrapper for disassember(), to avoid the already
existing #ifdef DISASM_FOUR_ARGS_SIGNATURE.

Cc: Quentin Monnet <quentin@isovalent.com>
Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: Jiri Olsa <jolsa@kernel.org>
Cc: Alexei Starovoitov <ast@kernel.org>
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 */
+#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 */
-- 
2.35.1.677.gabf474a5dd

[PATCH v1 2/3] tools: introduce dis-asm.h wrapper to hide version differences

[Andres Freund]

binutils changed the signature of init_disassemble_info(), which now causes
perf and bpftool to fail to compile (e.g. on debian unstable). Relevant
binutils commit:
https://sourceware.org/git/?p=binutils-gdb.git;a=commit;h=60a3da00bd5407f07

This just wraps init_disassemble_info(), to avoid spreading #ifdef
DISASM_INIT_STYLED to a bunch of places. It likely is worth adding a
wrapper for disassember(), to avoid the already existing #ifdef
DISASM_FOUR_ARGS_SIGNATURE.

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..f44f7d9f053e
--- /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 <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, const char *fmt, ...)
+{
+	va_list args;
+	int r;
+
+	(void)style;
+
+	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 fprintf_func,
+						fprintf_styled_ftype fprintf_styled_func)
+{
+#ifdef DISASM_INIT_STYLED
+	init_disassemble_info(info, stream,
+			      fprintf_func,
+			      fprintf_styled_func);
+#else
+	(void)fprintf_styled_func;
+	init_disassemble_info(info, stream,
+			      fprintf_func);
+#endif
+}
+
+#endif /* _TOOLS_DIS_ASM_COMPAT_H */
-- 
2.35.1.677.gabf474a5dd

Re: [PATCH v1 2/3] tools: introduce dis-asm.h wrapper to hide version differences

[Andres Freund]

Hi,

On 2022-07-02 21:48:13 -0700, Andres Freund wrote:
> binutils changed the signature of init_disassemble_info(), which now causes
> perf and bpftool to fail to compile (e.g. on debian unstable). Relevant
> binutils commit:
> https://sourceware.org/git/?p=binutils-gdb.git;a=commit;h=60a3da00bd5407f07
> 
> This just wraps init_disassemble_info(), to avoid spreading #ifdef
> DISASM_INIT_STYLED to a bunch of places. It likely is worth adding a
> wrapper for disassember(), to avoid the already existing #ifdef
> DISASM_FOUR_ARGS_SIGNATURE.
> 
> Link: http://lore.kernel.org/lkml/20220622181918.ykrs5rsnmx3og4sv@alap3.anarazel.de
> Signed-off-by: Andres Freund <andres@anarazel.de>

Argh, please disregard this version of 0002 - it's an older version that I
somehow managed to send unintentionally. I don't use git send-email often...

Greetings,

Andres Freund

[PATCH v1 3/3] tools: Use tools/dis-asm-compat.h to fix compilation errors with new binutils

[Andres Freund]

binutils changed the signature of init_disassemble_info(), which now causes
compilation failures for tools/{perf,bpf} on e.g. debian unstable. Relevant
binutils commit:
https://sourceware.org/git/?p=binutils-gdb.git;a=commit;h=60a3da00bd5407f07

Use the previously introduced compat header to fix the build failures.

I verified that:
- perf can still disassemble bpf programs
- bpftool can still disassemble bpf programs, both plain in json
- bpf_jit_disasm still works - it does. Although it turns out that
  independent of these changes bpf_jit_enable = 2 currently is broken, see
  https://lore.kernel.org/20220703030210.pmjft7qc2eajzi6c@alap3.anarazel.de

Cc: Quentin Monnet <quentin@isovalent.com>
Cc: Alexei Starovoitov <ast@kernel.org>
Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: Jiri Olsa <jolsa@kernel.org>
Cc: Daniel Borkmann <daniel@iogearbox.net>
Link: http://lore.kernel.org/lkml/20220622181918.ykrs5rsnmx3og4sv@alap3.anarazel.de
Signed-off-by: Andres Freund <andres@anarazel.de>
---
 tools/bpf/bpf_jit_disasm.c     |  5 ++++-
 tools/bpf/bpftool/jit_disasm.c | 40 +++++++++++++++++++++++++++-------
 tools/perf/util/annotate.c     |  7 +++---
 3 files changed, 40 insertions(+), 12 deletions(-)

diff --git a/tools/bpf/bpf_jit_disasm.c b/tools/bpf/bpf_jit_disasm.c
index c8ae95804728..a90a5d110f92 100644
--- a/tools/bpf/bpf_jit_disasm.c
+++ b/tools/bpf/bpf_jit_disasm.c
@@ -28,6 +28,7 @@
 #include <sys/types.h>
 #include <sys/stat.h>
 #include <limits.h>
+#include <tools/dis-asm-compat.h>
 
 #define CMD_ACTION_SIZE_BUFFER		10
 #define CMD_ACTION_READ_ALL		3
@@ -64,7 +65,9 @@ static void get_asm_insns(uint8_t *image, size_t len, int opcodes)
 	assert(bfdf);
 	assert(bfd_check_format(bfdf, bfd_object));
 
-	init_disassemble_info(&info, stdout, (fprintf_ftype) fprintf);
+	init_disassemble_info_compat(&info, stdout,
+				     (fprintf_ftype) fprintf,
+				     fprintf_styled);
 	info.arch = bfd_get_arch(bfdf);
 	info.mach = bfd_get_mach(bfdf);
 	info.buffer = image;
diff --git a/tools/bpf/bpftool/jit_disasm.c b/tools/bpf/bpftool/jit_disasm.c
index 24734f2249d6..f0d5c5f0eb60 100644
--- a/tools/bpf/bpftool/jit_disasm.c
+++ b/tools/bpf/bpftool/jit_disasm.c
@@ -24,6 +24,7 @@
 #include <sys/stat.h>
 #include <limits.h>
 #include <bpf/libbpf.h>
+#include <tools/dis-asm-compat.h>
 
 #include "json_writer.h"
 #include "main.h"
@@ -39,15 +40,12 @@ static void get_exec_path(char *tpath, size_t size)
 }
 
 static int oper_count;
-static int fprintf_json(void *out, const char *fmt, ...)
+static int printf_json(void *out, const char *fmt, va_list ap)
 {
-	va_list ap;
 	char *s;
 	int err;
 
-	va_start(ap, fmt);
 	err = vasprintf(&s, fmt, ap);
-	va_end(ap);
 	if (err < 0)
 		return -1;
 
@@ -73,6 +71,30 @@ static int fprintf_json(void *out, const char *fmt, ...)
 	return 0;
 }
 
+static int fprintf_json(void *out, const char *fmt, ...)
+{
+	va_list ap;
+	int r;
+
+	va_start(ap, fmt);
+	r = printf_json(out, fmt, ap);
+	va_end(ap);
+
+	return r;
+}
+
+static int fprintf_json_styled(void *out, enum disassembler_style style, const char *fmt, ...)
+{
+	va_list ap;
+	int r;
+
+	va_start(ap, fmt);
+	r = printf_json(out, fmt, ap);
+	va_end(ap);
+
+	return r;
+}
+
 void disasm_print_insn(unsigned char *image, ssize_t len, int opcodes,
 		       const char *arch, const char *disassembler_options,
 		       const struct btf *btf,
@@ -99,11 +121,13 @@ void disasm_print_insn(unsigned char *image, ssize_t len, int opcodes,
 	assert(bfd_check_format(bfdf, bfd_object));
 
 	if (json_output)
-		init_disassemble_info(&info, stdout,
-				      (fprintf_ftype) fprintf_json);
+		init_disassemble_info_compat(&info, stdout,
+					     (fprintf_ftype) fprintf_json,
+					     fprintf_json_styled);
 	else
-		init_disassemble_info(&info, stdout,
-				      (fprintf_ftype) fprintf);
+		init_disassemble_info_compat(&info, stdout,
+					     (fprintf_ftype) fprintf,
+					     fprintf_styled);
 
 	/* Update architecture info for offload. */
 	if (arch) {
diff --git a/tools/perf/util/annotate.c b/tools/perf/util/annotate.c
index 82cc396ef516..daea1867381d 100644
--- a/tools/perf/util/annotate.c
+++ b/tools/perf/util/annotate.c
@@ -41,6 +41,7 @@
 #include <linux/string.h>
 #include <subcmd/parse-options.h>
 #include <subcmd/run-command.h>
+#include <tools/dis-asm-compat.h>
 
 /* FIXME: For the HE_COLORSET */
 #include "ui/browser.h"
@@ -1762,9 +1763,9 @@ static int symbol__disassemble_bpf(struct symbol *sym,
 		ret = errno;
 		goto out;
 	}
-	init_disassemble_info(&info, s,
-			      (fprintf_ftype) fprintf);
-
+	init_disassemble_info_compat(&info, s,
+				     (fprintf_ftype) fprintf,
+				     fprintf_styled);
 	info.arch = bfd_get_arch(bfdf);
 	info.mach = bfd_get_mach(bfdf);
 
-- 
2.35.1.677.gabf474a5dd

[PATCH v2 0/5] tools: fix compilation failure caused by init_disassemble_info API changes

[Andres Freund]

binutils changed the signature of init_disassemble_info(), which now causes
compilation failures for tools/{perf,bpf} on e.g. debian unstable. Relevant
binutils commit:
https://sourceware.org/git/?p=binutils-gdb.git;a=commit;h=60a3da00bd5407f07

I first fixed this without introducing the compat header, as suggested by
Quentin, but I thought the amount of repeated boilerplate was a bit too
much. So instead I introduced a compat header to wrap the API changes. Even
tools/bpf/bpftool/jit_disasm.c, which needs its own callbacks for json, imo
looks nicer this way.

I'm not regular contributor, so it very well might be my procedures are a
bit off...

I am not sure I added the right [number of] people to CC?

WRT the feature test: Not sure what the point of the -DPACKAGE='"perf"' is,
nor why tools/perf/Makefile.config sets some LDFLAGS/CFLAGS that are also
in feature/Makefile and why -ldl isn't needed in the other places. But...

V2:
- split patches further, so that tools/bpf and tools/perf part are entirely
  separate
- included a bit more information about tests I did in commit messages
- add a maybe_unused to fprintf_json_styled's style argument

Cc: Alexei Starovoitov <ast@kernel.org>
Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: Jiri Olsa <jolsa@kernel.org>
Cc: Sedat Dilek <sedat.dilek@gmail.com>
Cc: Quentin Monnet <quentin@isovalent.com>
To: bpf@vger.kernel.org
To: linux-kernel@vger.kernel.org
Link: https://lore.kernel.org/lkml/20220622181918.ykrs5rsnmx3og4sv@alap3.anarazel.de
Link: https://lore.kernel.org/lkml/CA+icZUVpr8ZeOKCj4zMMqbFT013KJz2T1csvXg+VSkdvJH1Ubw@mail.gmail.com

Andres Freund (5):
  tools build: add feature test for init_disassemble_info API changes
  tools include: add dis-asm-compat.h to handle version differences
  tools perf: Fix compilation error with new binutils
  tools bpf_jit_disasm: Fix compilation error with new binutils
  tools bpftool: Fix compilation error with new binutils

 tools/bpf/Makefile                            |  7 ++-
 tools/bpf/bpf_jit_disasm.c                    |  5 +-
 tools/bpf/bpftool/Makefile                    |  7 ++-
 tools/bpf/bpftool/jit_disasm.c                | 42 ++++++++++++---
 tools/build/Makefile.feature                  |  4 +-
 tools/build/feature/Makefile                  |  4 ++
 tools/build/feature/test-all.c                |  4 ++
 .../feature/test-disassembler-init-styled.c   | 13 +++++
 tools/include/tools/dis-asm-compat.h          | 53 +++++++++++++++++++
 tools/perf/Makefile.config                    |  8 +++
 tools/perf/util/annotate.c                    |  7 +--
 11 files changed, 137 insertions(+), 17 deletions(-)
 create mode 100644 tools/build/feature/test-disassembler-init-styled.c
 create mode 100644 tools/include/tools/dis-asm-compat.h

-- 
2.37.0.3.g30cc8d0f14

[PATCH v2 1/5] tools build: add feature test for init_disassemble_info API changes

[Andres Freund]

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 adds a feature test to detect the new signature.  Subsequent
commits will use it to fix the build failures.

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/build/Makefile.feature                        |  4 +++-
 tools/build/feature/Makefile                        |  4 ++++
 tools/build/feature/test-all.c                      |  4 ++++
 tools/build/feature/test-disassembler-init-styled.c | 13 +++++++++++++
 4 files changed, 24 insertions(+), 1 deletion(-)
 create mode 100644 tools/build/feature/test-disassembler-init-styled.c

diff --git a/tools/build/Makefile.feature b/tools/build/Makefile.feature
index 888a0421d43b..339686b99a6e 100644
--- a/tools/build/Makefile.feature
+++ b/tools/build/Makefile.feature
@@ -70,6 +70,7 @@ FEATURE_TESTS_BASIC :=                  \
         libaio				\
         libzstd				\
         disassembler-four-args		\
+        disassembler-init-styled	\
         file-handle
 
 # FEATURE_TESTS_BASIC + FEATURE_TESTS_EXTRA is the complete list
@@ -135,7 +136,8 @@ FEATURE_DISPLAY ?=              \
          bpf			\
          libaio			\
          libzstd		\
-         disassembler-four-args
+         disassembler-four-args	\
+         disassembler-init-styled
 
 # Set FEATURE_CHECK_(C|LD)FLAGS-all for all FEATURE_TESTS features.
 # If in the future we need per-feature checks/flags for features not
diff --git a/tools/build/feature/Makefile b/tools/build/feature/Makefile
index 7c2a17e23c30..c3059739318a 100644
--- a/tools/build/feature/Makefile
+++ b/tools/build/feature/Makefile
@@ -18,6 +18,7 @@ FILES=                                          \
          test-libbfd.bin                        \
          test-libbfd-buildid.bin		\
          test-disassembler-four-args.bin        \
+         test-disassembler-init-styled.bin	\
          test-reallocarray.bin			\
          test-libbfd-liberty.bin                \
          test-libbfd-liberty-z.bin              \
@@ -248,6 +249,9 @@ $(OUTPUT)test-libbfd-buildid.bin:
 $(OUTPUT)test-disassembler-four-args.bin:
 	$(BUILD) -DPACKAGE='"perf"' -lbfd -lopcodes
 
+$(OUTPUT)test-disassembler-init-styled.bin:
+	$(BUILD) -DPACKAGE='"perf"' -lbfd -lopcodes
+
 $(OUTPUT)test-reallocarray.bin:
 	$(BUILD)
 
diff --git a/tools/build/feature/test-all.c b/tools/build/feature/test-all.c
index 5ffafb967b6e..957c02c7b163 100644
--- a/tools/build/feature/test-all.c
+++ b/tools/build/feature/test-all.c
@@ -166,6 +166,10 @@
 # include "test-disassembler-four-args.c"
 #undef main
 
+#define main main_test_disassembler_init_styled
+# include "test-disassembler-init-styled.c"
+#undef main
+
 #define main main_test_libzstd
 # include "test-libzstd.c"
 #undef main
diff --git a/tools/build/feature/test-disassembler-init-styled.c b/tools/build/feature/test-disassembler-init-styled.c
new file mode 100644
index 000000000000..f1ce0ec3bee9
--- /dev/null
+++ b/tools/build/feature/test-disassembler-init-styled.c
@@ -0,0 +1,13 @@
+// SPDX-License-Identifier: GPL-2.0
+#include <stdio.h>
+#include <dis-asm.h>
+
+int main(void)
+{
+	struct disassemble_info info;
+
+	init_disassemble_info(&info, stdout,
+			      NULL, NULL);
+
+	return 0;
+}
-- 
2.37.0.3.g30cc8d0f14

[PATCH v2 2/5] tools include: add dis-asm-compat.h to handle version differences

[Andres Freund]

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 */
+#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 */
-- 
2.37.0.3.g30cc8d0f14

Re: [PATCH v2 2/5] tools include: add dis-asm-compat.h to handle version differences

[Quentin Monnet]

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

Re: [PATCH v2 2/5] tools include: add dis-asm-compat.h to handle version differences

[Andres Freund]

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

Re: [PATCH v2 2/5] tools include: add dis-asm-compat.h to handle version differences

[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

Re: [PATCH v2 2/5] tools include: add dis-asm-compat.h to handle version differences

[Quentin Monnet]

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

[PATCH v2 3/5] tools perf: Fix compilation error with new binutils

[Andres Freund]

binutils changed the signature of init_disassemble_info(), which now causes
compilation failures for tools/perf/util/annotate.c, e.g. on debian
unstable. Relevant binutils commit:
https://sourceware.org/git/?p=binutils-gdb.git;a=commit;h=60a3da00bd5407f07

Wire up the feature test and switch to init_disassemble_info_compat(),
which were introduced in prior commits, fixing the compilation failure.

I verified that perf can still disassemble bpf programs by using bpftrace
under load, recording a perf trace, and then annotating the bpf "function"
with and without the changes. With old binutils there's no change in output
before/after this patch. When comparing the output from old binutils (2.35)
to new bintuils with the patch (upstream snapshot) there are a few output
differences, but they are unrelated to this patch. An example hunk is:

     1.15 :   55:mov    %rbp,%rdx
     0.00 :   58:add    $0xfffffffffffffff8,%rdx
     0.00 :   5c:xor    %ecx,%ecx
-    1.03 :   5e:callq  0xffffffffe12aca3c
+    1.03 :   5e:call   0xffffffffe12aca3c
     0.00 :   63:xor    %eax,%eax
-    2.18 :   65:leaveq
-    2.82 :   66:retq
+    2.18 :   65:leave
+    2.82 :   66:ret

Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: Sedat Dilek <sedat.dilek@gmail.com>
Link: http://lore.kernel.org/lkml/20220622181918.ykrs5rsnmx3og4sv@alap3.anarazel.de
Signed-off-by: Andres Freund <andres@anarazel.de>
---
 tools/perf/Makefile.config | 8 ++++++++
 tools/perf/util/annotate.c | 7 ++++---
 2 files changed, 12 insertions(+), 3 deletions(-)

diff --git a/tools/perf/Makefile.config b/tools/perf/Makefile.config
index 73e0762092fe..ee417c321adb 100644
--- a/tools/perf/Makefile.config
+++ b/tools/perf/Makefile.config
@@ -298,6 +298,7 @@ FEATURE_CHECK_LDFLAGS-libpython := $(PYTHON_EMBED_LDOPTS)
 FEATURE_CHECK_LDFLAGS-libaio = -lrt
 
 FEATURE_CHECK_LDFLAGS-disassembler-four-args = -lbfd -lopcodes -ldl
+FEATURE_CHECK_LDFLAGS-disassembler-init-styled = -lbfd -lopcodes -ldl
 
 CORE_CFLAGS += -fno-omit-frame-pointer
 CORE_CFLAGS += -ggdb3
@@ -905,13 +906,16 @@ ifndef NO_LIBBFD
     ifeq ($(feature-libbfd-liberty), 1)
       EXTLIBS += -lbfd -lopcodes -liberty
       FEATURE_CHECK_LDFLAGS-disassembler-four-args += -liberty -ldl
+      FEATURE_CHECK_LDFLAGS-disassembler-init-styled += -liberty -ldl
     else
       ifeq ($(feature-libbfd-liberty-z), 1)
         EXTLIBS += -lbfd -lopcodes -liberty -lz
         FEATURE_CHECK_LDFLAGS-disassembler-four-args += -liberty -lz -ldl
+        FEATURE_CHECK_LDFLAGS-disassembler-init-styled += -liberty -lz -ldl
       endif
     endif
     $(call feature_check,disassembler-four-args)
+    $(call feature_check,disassembler-init-styled)
   endif
 
   ifeq ($(feature-libbfd-buildid), 1)
@@ -1025,6 +1029,10 @@ ifeq ($(feature-disassembler-four-args), 1)
     CFLAGS += -DDISASM_FOUR_ARGS_SIGNATURE
 endif
 
+ifeq ($(feature-disassembler-init-styled), 1)
+    CFLAGS += -DDISASM_INIT_STYLED
+endif
+
 ifeq (${IS_64_BIT}, 1)
   ifndef NO_PERF_READ_VDSO32
     $(call feature_check,compile-32)
diff --git a/tools/perf/util/annotate.c b/tools/perf/util/annotate.c
index 82cc396ef516..daea1867381d 100644
--- a/tools/perf/util/annotate.c
+++ b/tools/perf/util/annotate.c
@@ -41,6 +41,7 @@
 #include <linux/string.h>
 #include <subcmd/parse-options.h>
 #include <subcmd/run-command.h>
+#include <tools/dis-asm-compat.h>
 
 /* FIXME: For the HE_COLORSET */
 #include "ui/browser.h"
@@ -1762,9 +1763,9 @@ static int symbol__disassemble_bpf(struct symbol *sym,
 		ret = errno;
 		goto out;
 	}
-	init_disassemble_info(&info, s,
-			      (fprintf_ftype) fprintf);
-
+	init_disassemble_info_compat(&info, s,
+				     (fprintf_ftype) fprintf,
+				     fprintf_styled);
 	info.arch = bfd_get_arch(bfdf);
 	info.mach = bfd_get_mach(bfdf);
 
-- 
2.37.0.3.g30cc8d0f14

[PATCH v2 4/5] tools bpf_jit_disasm: Fix compilation error with new binutils

[Andres Freund]

binutils changed the signature of init_disassemble_info(), which now causes
compilation to fail for tools/bpf/bpf_jit_disasm.c, e.g. on debian
unstable. Relevant binutils commit:
https://sourceware.org/git/?p=binutils-gdb.git;a=commit;h=60a3da00bd5407f07

Wire up the feature test and switch to init_disassemble_info_compat(),
which were introduced in prior commits, fixing the compilation failure.

I verified that bpf_jit_disasm can still disassemble bpf programs, both
with the old and new dis-asm.h API. With old binutils there's no change in
output before/after this patch. When comparing the output from old
binutils (2.35) to new bintuils with the patch (upstream snapshot) there
are a few output differences, but they are unrelated to this patch. An
example hunk is:
   f4:	mov    %r14,%rsi
   f7:	mov    %r15,%rdx
   fa:	mov    $0x2a,%ecx
-  ff:	callq  0xffffffffea8c4988
+  ff:	call   0xffffffffea8c4988
  104:	test   %rax,%rax
  107:	jge    0x0000000000000110
  109:	xor    %eax,%eax
- 10b:	jmpq   0x0000000000000073
+ 10b:	jmp    0x0000000000000073
  110:	cmp    $0x16,%rax

However, I had to use an older kernel to generate the bpf_jit_enabled = 2
output, as that has been broken since 5.18 / 1022a5498f6f:
https://lore.kernel.org/20220703030210.pmjft7qc2eajzi6c@alap3.anarazel.de

Cc: Alexei Starovoitov <ast@kernel.org>
Cc: Daniel Borkmann <daniel@iogearbox.net>
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/bpf/Makefile         | 7 +++++--
 tools/bpf/bpf_jit_disasm.c | 5 ++++-
 2 files changed, 9 insertions(+), 3 deletions(-)

diff --git a/tools/bpf/Makefile b/tools/bpf/Makefile
index b11cfc86a3d0..9c4e61c3a92b 100644
--- a/tools/bpf/Makefile
+++ b/tools/bpf/Makefile
@@ -34,8 +34,8 @@ else
 endif
 
 FEATURE_USER = .bpf
-FEATURE_TESTS = libbfd disassembler-four-args
-FEATURE_DISPLAY = libbfd disassembler-four-args
+FEATURE_TESTS = libbfd disassembler-four-args disassembler-init-styled
+FEATURE_DISPLAY = libbfd disassembler-four-args disassembler-init-styled
 
 check_feat := 1
 NON_CHECK_FEAT_TARGETS := clean bpftool_clean runqslower_clean resolve_btfids_clean
@@ -56,6 +56,9 @@ endif
 ifeq ($(feature-disassembler-four-args), 1)
 CFLAGS += -DDISASM_FOUR_ARGS_SIGNATURE
 endif
+ifeq ($(feature-disassembler-init-styled), 1)
+CFLAGS += -DDISASM_INIT_STYLED
+endif
 
 $(OUTPUT)%.yacc.c: $(srctree)/tools/bpf/%.y
 	$(QUIET_BISON)$(YACC) -o $@ -d $<
diff --git a/tools/bpf/bpf_jit_disasm.c b/tools/bpf/bpf_jit_disasm.c
index c8ae95804728..a90a5d110f92 100644
--- a/tools/bpf/bpf_jit_disasm.c
+++ b/tools/bpf/bpf_jit_disasm.c
@@ -28,6 +28,7 @@
 #include <sys/types.h>
 #include <sys/stat.h>
 #include <limits.h>
+#include <tools/dis-asm-compat.h>
 
 #define CMD_ACTION_SIZE_BUFFER		10
 #define CMD_ACTION_READ_ALL		3
@@ -64,7 +65,9 @@ static void get_asm_insns(uint8_t *image, size_t len, int opcodes)
 	assert(bfdf);
 	assert(bfd_check_format(bfdf, bfd_object));
 
-	init_disassemble_info(&info, stdout, (fprintf_ftype) fprintf);
+	init_disassemble_info_compat(&info, stdout,
+				     (fprintf_ftype) fprintf,
+				     fprintf_styled);
 	info.arch = bfd_get_arch(bfdf);
 	info.mach = bfd_get_mach(bfdf);
 	info.buffer = image;
-- 
2.37.0.3.g30cc8d0f14

[PATCH v2 5/5] tools bpftool: Fix compilation error with new binutils

[Andres Freund]

binutils changed the signature of init_disassemble_info(), which now causes
compilation to fail for tools/bpf/bpftool/jit_disasm.c, e.g. on debian
unstable. Relevant binutils commit:
https://sourceware.org/git/?p=binutils-gdb.git;a=commit;h=60a3da00bd5407f07

Wire up the feature test and switch to init_disassemble_info_compat(),
which were introduced in prior commits, fixing the compilation failure.

I verified that bpftool can still disassemble bpf programs, both with an
old and new dis-asm.h API. There are no output changes for plain and json
formats. When comparing the output from old binutils (2.35)
to new bintuils with the patch (upstream snapshot) there are a few output
differences, but they are unrelated to this patch. An example hunk is:
   2f:	pop    %r14
   31:	pop    %r13
   33:	pop    %rbx
-  34:	leaveq
-  35:	retq
+  34:	leave
+  35:	ret

Cc: Alexei Starovoitov <ast@kernel.org>
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/bpf/bpftool/Makefile     |  7 ++++--
 tools/bpf/bpftool/jit_disasm.c | 42 +++++++++++++++++++++++++++-------
 2 files changed, 39 insertions(+), 10 deletions(-)

diff --git a/tools/bpf/bpftool/Makefile b/tools/bpf/bpftool/Makefile
index c6d2c77d0252..62195118d377 100644
--- a/tools/bpf/bpftool/Makefile
+++ b/tools/bpf/bpftool/Makefile
@@ -93,9 +93,9 @@ INSTALL ?= install
 RM ?= rm -f
 
 FEATURE_USER = .bpftool
-FEATURE_TESTS = libbfd disassembler-four-args zlib libcap \
+FEATURE_TESTS = libbfd disassembler-four-args disassembler-init-styled zlib libcap \
 	clang-bpf-co-re
-FEATURE_DISPLAY = libbfd disassembler-four-args zlib libcap \
+FEATURE_DISPLAY = libbfd disassembler-four-args disassembler-init-styled zlib libcap \
 	clang-bpf-co-re
 
 check_feat := 1
@@ -117,6 +117,9 @@ endif
 ifeq ($(feature-disassembler-four-args), 1)
 CFLAGS += -DDISASM_FOUR_ARGS_SIGNATURE
 endif
+ifeq ($(feature-disassembler-init-styled), 1)
+    CFLAGS += -DDISASM_INIT_STYLED
+endif
 
 LIBS = $(LIBBPF) -lelf -lz
 LIBS_BOOTSTRAP = $(LIBBPF_BOOTSTRAP) -lelf -lz
diff --git a/tools/bpf/bpftool/jit_disasm.c b/tools/bpf/bpftool/jit_disasm.c
index 24734f2249d6..aaf99a0168c9 100644
--- a/tools/bpf/bpftool/jit_disasm.c
+++ b/tools/bpf/bpftool/jit_disasm.c
@@ -24,6 +24,7 @@
 #include <sys/stat.h>
 #include <limits.h>
 #include <bpf/libbpf.h>
+#include <tools/dis-asm-compat.h>
 
 #include "json_writer.h"
 #include "main.h"
@@ -39,15 +40,12 @@ static void get_exec_path(char *tpath, size_t size)
 }
 
 static int oper_count;
-static int fprintf_json(void *out, const char *fmt, ...)
+static int printf_json(void *out, const char *fmt, va_list ap)
 {
-	va_list ap;
 	char *s;
 	int err;
 
-	va_start(ap, fmt);
 	err = vasprintf(&s, fmt, ap);
-	va_end(ap);
 	if (err < 0)
 		return -1;
 
@@ -73,6 +71,32 @@ static int fprintf_json(void *out, const char *fmt, ...)
 	return 0;
 }
 
+static int fprintf_json(void *out, const char *fmt, ...)
+{
+	va_list ap;
+	int r;
+
+	va_start(ap, fmt);
+	r = printf_json(out, fmt, ap);
+	va_end(ap);
+
+	return r;
+}
+
+static int fprintf_json_styled(void *out,
+			       enum disassembler_style style __maybe_unused,
+			       const char *fmt, ...)
+{
+	va_list ap;
+	int r;
+
+	va_start(ap, fmt);
+	r = printf_json(out, fmt, ap);
+	va_end(ap);
+
+	return r;
+}
+
 void disasm_print_insn(unsigned char *image, ssize_t len, int opcodes,
 		       const char *arch, const char *disassembler_options,
 		       const struct btf *btf,
@@ -99,11 +123,13 @@ void disasm_print_insn(unsigned char *image, ssize_t len, int opcodes,
 	assert(bfd_check_format(bfdf, bfd_object));
 
 	if (json_output)
-		init_disassemble_info(&info, stdout,
-				      (fprintf_ftype) fprintf_json);
+		init_disassemble_info_compat(&info, stdout,
+					     (fprintf_ftype) fprintf_json,
+					     fprintf_json_styled);
 	else
-		init_disassemble_info(&info, stdout,
-				      (fprintf_ftype) fprintf);
+		init_disassemble_info_compat(&info, stdout,
+					     (fprintf_ftype) fprintf,
+					     fprintf_styled);
 
 	/* Update architecture info for offload. */
 	if (arch) {
-- 
2.37.0.3.g30cc8d0f14

Re: [PATCH v2 0/5] tools: fix compilation failure caused by init_disassemble_info API changes

[Jiri Olsa]

On Sun, Jul 03, 2022 at 02:25:46PM -0700, Andres Freund wrote:
> binutils changed the signature of init_disassemble_info(), which now causes
> compilation failures for tools/{perf,bpf} on e.g. debian unstable. Relevant
> binutils commit:
> https://sourceware.org/git/?p=binutils-gdb.git;a=commit;h=60a3da00bd5407f07
> 
> I first fixed this without introducing the compat header, as suggested by
> Quentin, but I thought the amount of repeated boilerplate was a bit too
> much. So instead I introduced a compat header to wrap the API changes. Even
> tools/bpf/bpftool/jit_disasm.c, which needs its own callbacks for json, imo
> looks nicer this way.
> 
> I'm not regular contributor, so it very well might be my procedures are a
> bit off...
> 
> I am not sure I added the right [number of] people to CC?
> 
> WRT the feature test: Not sure what the point of the -DPACKAGE='"perf"' is,
> nor why tools/perf/Makefile.config sets some LDFLAGS/CFLAGS that are also
> in feature/Makefile and why -ldl isn't needed in the other places. But...
> 
> V2:
> - split patches further, so that tools/bpf and tools/perf part are entirely
>   separate
> - included a bit more information about tests I did in commit messages
> - add a maybe_unused to fprintf_json_styled's style argument
> 
> Cc: Alexei Starovoitov <ast@kernel.org>
> Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
> Cc: Jiri Olsa <jolsa@kernel.org>
> Cc: Sedat Dilek <sedat.dilek@gmail.com>
> Cc: Quentin Monnet <quentin@isovalent.com>
> To: bpf@vger.kernel.org
> To: linux-kernel@vger.kernel.org
> Link: https://lore.kernel.org/lkml/20220622181918.ykrs5rsnmx3og4sv@alap3.anarazel.de
> Link: https://lore.kernel.org/lkml/CA+icZUVpr8ZeOKCj4zMMqbFT013KJz2T1csvXg+VSkdvJH1Ubw@mail.gmail.com
> 
> Andres Freund (5):
>   tools build: add feature test for init_disassemble_info API changes
>   tools include: add dis-asm-compat.h to handle version differences
>   tools perf: Fix compilation error with new binutils
>   tools bpf_jit_disasm: Fix compilation error with new binutils
>   tools bpftool: Fix compilation error with new binutils

I think the disassembler checks should not be displayed by default,
with your change I can see all the time:

...        disassembler-four-args: [ on  ]
...      disassembler-init-styled: [ OFF ]


could you please squash something like below in? moving disassembler
checks out of sight and do manual detection

thanks,
jirka


---
diff --git a/tools/build/Makefile.feature b/tools/build/Makefile.feature
index 339686b99a6e..bce9a9b52b2c 100644
--- a/tools/build/Makefile.feature
+++ b/tools/build/Makefile.feature
@@ -69,8 +69,6 @@ FEATURE_TESTS_BASIC :=                  \
         setns				\
         libaio				\
         libzstd				\
-        disassembler-four-args		\
-        disassembler-init-styled	\
         file-handle
 
 # FEATURE_TESTS_BASIC + FEATURE_TESTS_EXTRA is the complete list
@@ -106,7 +104,9 @@ FEATURE_TESTS_EXTRA :=                  \
          libbpf-bpf_create_map		\
          libpfm4                        \
          libdebuginfod			\
-         clang-bpf-co-re
+         clang-bpf-co-re		\
+         disassembler-four-args		\
+         disassembler-init-styled
 
 
 FEATURE_TESTS ?= $(FEATURE_TESTS_BASIC)
@@ -135,9 +135,7 @@ FEATURE_DISPLAY ?=              \
          get_cpuid              \
          bpf			\
          libaio			\
-         libzstd		\
-         disassembler-four-args	\
-         disassembler-init-styled
+         libzstd
 
 # Set FEATURE_CHECK_(C|LD)FLAGS-all for all FEATURE_TESTS features.
 # If in the future we need per-feature checks/flags for features not
diff --git a/tools/perf/Makefile.config b/tools/perf/Makefile.config
index ee417c321adb..2aa0bad11f05 100644
--- a/tools/perf/Makefile.config
+++ b/tools/perf/Makefile.config
@@ -914,8 +914,6 @@ ifndef NO_LIBBFD
         FEATURE_CHECK_LDFLAGS-disassembler-init-styled += -liberty -lz -ldl
       endif
     endif
-    $(call feature_check,disassembler-four-args)
-    $(call feature_check,disassembler-init-styled)
   endif
 
   ifeq ($(feature-libbfd-buildid), 1)
@@ -1025,6 +1023,9 @@ ifdef HAVE_KVM_STAT_SUPPORT
     CFLAGS += -DHAVE_KVM_STAT_SUPPORT
 endif
 
+$(call feature_check,disassembler-four-args)
+$(call feature_check,disassembler-init-styled)
+
 ifeq ($(feature-disassembler-four-args), 1)
     CFLAGS += -DDISASM_FOUR_ARGS_SIGNATURE
 endif

Re: [PATCH v2 0/5] tools: fix compilation failure caused by init_disassemble_info API changes

[Andres Freund]

Hi,

On 2022-07-04 11:13:33 +0200, Jiri Olsa wrote:
> I think the disassembler checks should not be displayed by default,
> with your change I can see all the time:
> 
> ...        disassembler-four-args: [ on  ]
> ...      disassembler-init-styled: [ OFF ]
> 
> 
> could you please squash something like below in? moving disassembler
> checks out of sight and do manual detection

Makes sense - I was wondering why disassembler-four-args is displayed, but
though it better to mirror the existing behaviour. Does "hiding"
disassembler-four-args need to be its own set of commits?


> diff --git a/tools/perf/Makefile.config b/tools/perf/Makefile.config
> index ee417c321adb..2aa0bad11f05 100644
> --- a/tools/perf/Makefile.config
> +++ b/tools/perf/Makefile.config
> @@ -914,8 +914,6 @@ ifndef NO_LIBBFD
>          FEATURE_CHECK_LDFLAGS-disassembler-init-styled += -liberty -lz -ldl
>        endif
>      endif
> -    $(call feature_check,disassembler-four-args)
> -    $(call feature_check,disassembler-init-styled)
>    endif
>  
>    ifeq ($(feature-libbfd-buildid), 1)
> @@ -1025,6 +1023,9 @@ ifdef HAVE_KVM_STAT_SUPPORT
>      CFLAGS += -DHAVE_KVM_STAT_SUPPORT
>  endif
>  
> +$(call feature_check,disassembler-four-args)
> +$(call feature_check,disassembler-init-styled)
> +
>  ifeq ($(feature-disassembler-four-args), 1)
>      CFLAGS += -DDISASM_FOUR_ARGS_SIGNATURE
>  endif

This I don't understand - why do we want these to run under NO_LIBBFD etc?

Greetings,

Andres Freund

Re: [PATCH v2 0/5] tools: fix compilation failure caused by init_disassemble_info API changes

[Jiri Olsa]

On Mon, Jul 04, 2022 at 01:19:22PM -0700, Andres Freund wrote:
> Hi,
> 
> On 2022-07-04 11:13:33 +0200, Jiri Olsa wrote:
> > I think the disassembler checks should not be displayed by default,
> > with your change I can see all the time:
> > 
> > ...        disassembler-four-args: [ on  ]
> > ...      disassembler-init-styled: [ OFF ]
> > 
> > 
> > could you please squash something like below in? moving disassembler
> > checks out of sight and do manual detection
> 
> Makes sense - I was wondering why disassembler-four-args is displayed, but
> though it better to mirror the existing behaviour. Does "hiding"
> disassembler-four-args need to be its own set of commits?

I guess first hide the disassembler-four-args and add the new the same way

> 
> 
> > diff --git a/tools/perf/Makefile.config b/tools/perf/Makefile.config
> > index ee417c321adb..2aa0bad11f05 100644
> > --- a/tools/perf/Makefile.config
> > +++ b/tools/perf/Makefile.config
> > @@ -914,8 +914,6 @@ ifndef NO_LIBBFD
> >          FEATURE_CHECK_LDFLAGS-disassembler-init-styled += -liberty -lz -ldl
> >        endif
> >      endif
> > -    $(call feature_check,disassembler-four-args)
> > -    $(call feature_check,disassembler-init-styled)
> >    endif
> >  
> >    ifeq ($(feature-libbfd-buildid), 1)
> > @@ -1025,6 +1023,9 @@ ifdef HAVE_KVM_STAT_SUPPORT
> >      CFLAGS += -DHAVE_KVM_STAT_SUPPORT
> >  endif
> >  
> > +$(call feature_check,disassembler-four-args)
> > +$(call feature_check,disassembler-init-styled)
> > +
> >  ifeq ($(feature-disassembler-four-args), 1)
> >      CFLAGS += -DDISASM_FOUR_ARGS_SIGNATURE
> >  endif
> 
> This I don't understand - why do we want these to run under NO_LIBBFD etc?

when I was quickly testing that I did not have any of them detected
and got compile fail.. so I moved it to safe place ;-) it might be
placed in smarter place 

thanks,
jirka

> 
> Greetings,
> 
> Andres Freund

Re: [PATCH v2 0/5] tools: fix compilation failure caused by init_disassemble_info API changes

[Andres Freund]

Hi,

On 2022-07-05 00:12:37 +0200, Jiri Olsa wrote:
> On Mon, Jul 04, 2022 at 01:19:22PM -0700, Andres Freund wrote:
> > > diff --git a/tools/perf/Makefile.config b/tools/perf/Makefile.config
> > > index ee417c321adb..2aa0bad11f05 100644
> > > --- a/tools/perf/Makefile.config
> > > +++ b/tools/perf/Makefile.config
> > > @@ -914,8 +914,6 @@ ifndef NO_LIBBFD
> > >          FEATURE_CHECK_LDFLAGS-disassembler-init-styled += -liberty -lz -ldl
> > >        endif
> > >      endif
> > > -    $(call feature_check,disassembler-four-args)
> > > -    $(call feature_check,disassembler-init-styled)
> > >    endif
> > >
> > >    ifeq ($(feature-libbfd-buildid), 1)
> > > @@ -1025,6 +1023,9 @@ ifdef HAVE_KVM_STAT_SUPPORT
> > >      CFLAGS += -DHAVE_KVM_STAT_SUPPORT
> > >  endif
> > >
> > > +$(call feature_check,disassembler-four-args)
> > > +$(call feature_check,disassembler-init-styled)
> > > +
> > >  ifeq ($(feature-disassembler-four-args), 1)
> > >      CFLAGS += -DDISASM_FOUR_ARGS_SIGNATURE
> > >  endif
> >
> > This I don't understand - why do we want these to run under NO_LIBBFD etc?
>
> when I was quickly testing that I did not have any of them detected
> and got compile fail.. so I moved it to safe place ;-) it might be
> placed in smarter place

I think that's because you'd removed them from FEATURE_TESTS_BASIC in
Makefile.feature. In v3 I just sent out I just removed them from
FEATURE_DISPLAY, without any more "structural" changes in
tools/perf/Makefile.config. 

Greetings,

Andres Freund

Re: [PATCH v2 0/5] tools: fix compilation failure caused by init_disassemble_info API changes

[Sedat Dilek]

On Sun, Jul 3, 2022 at 11:25 PM Andres Freund <andres@anarazel.de> wrote:
>
> binutils changed the signature of init_disassemble_info(), which now causes
> compilation failures for tools/{perf,bpf} on e.g. debian unstable. Relevant
> binutils commit:
> https://sourceware.org/git/?p=binutils-gdb.git;a=commit;h=60a3da00bd5407f07
>

HI,

what was the base for this patchset?
I tried with Linux v5.19-rc5 and it doesn not apply cleanly.

Regards,
-Sedat-

> I first fixed this without introducing the compat header, as suggested by
> Quentin, but I thought the amount of repeated boilerplate was a bit too
> much. So instead I introduced a compat header to wrap the API changes. Even
> tools/bpf/bpftool/jit_disasm.c, which needs its own callbacks for json, imo
> looks nicer this way.
>
> I'm not regular contributor, so it very well might be my procedures are a
> bit off...
>
> I am not sure I added the right [number of] people to CC?
>
> WRT the feature test: Not sure what the point of the -DPACKAGE='"perf"' is,
> nor why tools/perf/Makefile.config sets some LDFLAGS/CFLAGS that are also
> in feature/Makefile and why -ldl isn't needed in the other places. But...
>
> V2:
> - split patches further, so that tools/bpf and tools/perf part are entirely
>   separate
> - included a bit more information about tests I did in commit messages
> - add a maybe_unused to fprintf_json_styled's style argument
>
> Cc: Alexei Starovoitov <ast@kernel.org>
> Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
> Cc: Jiri Olsa <jolsa@kernel.org>
> Cc: Sedat Dilek <sedat.dilek@gmail.com>
> Cc: Quentin Monnet <quentin@isovalent.com>
> To: bpf@vger.kernel.org
> To: linux-kernel@vger.kernel.org
> Link: https://lore.kernel.org/lkml/20220622181918.ykrs5rsnmx3og4sv@alap3.anarazel.de
> Link: https://lore.kernel.org/lkml/CA+icZUVpr8ZeOKCj4zMMqbFT013KJz2T1csvXg+VSkdvJH1Ubw@mail.gmail.com
>
> Andres Freund (5):
>   tools build: add feature test for init_disassemble_info API changes
>   tools include: add dis-asm-compat.h to handle version differences
>   tools perf: Fix compilation error with new binutils
>   tools bpf_jit_disasm: Fix compilation error with new binutils
>   tools bpftool: Fix compilation error with new binutils
>
>  tools/bpf/Makefile                            |  7 ++-
>  tools/bpf/bpf_jit_disasm.c                    |  5 +-
>  tools/bpf/bpftool/Makefile                    |  7 ++-
>  tools/bpf/bpftool/jit_disasm.c                | 42 ++++++++++++---
>  tools/build/Makefile.feature                  |  4 +-
>  tools/build/feature/Makefile                  |  4 ++
>  tools/build/feature/test-all.c                |  4 ++
>  .../feature/test-disassembler-init-styled.c   | 13 +++++
>  tools/include/tools/dis-asm-compat.h          | 53 +++++++++++++++++++
>  tools/perf/Makefile.config                    |  8 +++
>  tools/perf/util/annotate.c                    |  7 +--
>  11 files changed, 137 insertions(+), 17 deletions(-)
>  create mode 100644 tools/build/feature/test-disassembler-init-styled.c
>  create mode 100644 tools/include/tools/dis-asm-compat.h
>
> --
> 2.37.0.3.g30cc8d0f14
>

Re: [PATCH v2 0/5] tools: fix compilation failure caused by init_disassemble_info API changes

[Sedat Dilek]

On Sun, Jul 10, 2022 at 1:43 PM Sedat Dilek <sedat.dilek@gmail.com> wrote:
>
> On Sun, Jul 3, 2022 at 11:25 PM Andres Freund <andres@anarazel.de> wrote:
> >
> > binutils changed the signature of init_disassemble_info(), which now causes
> > compilation failures for tools/{perf,bpf} on e.g. debian unstable. Relevant
> > binutils commit:
> > https://sourceware.org/git/?p=binutils-gdb.git;a=commit;h=60a3da00bd5407f07
> >
>
> HI,
>
> what was the base for this patchset?
> I tried with Linux v5.19-rc5 and it doesn not apply cleanly.
>

More coffee.

$ egrep 'Auto-detecting|disassembler' make-log_perf-python3.10-install_bin.txt
15:Auto-detecting system features:
36:...        disassembler-four-args: [ on  ]
37:...      disassembler-init-styled: [ on  ]

Tested-by: Sedat Dilek <sedat.dilek@gmail.com> # LLVM-14 (x86-64)

-Sedat-

>
> > I first fixed this without introducing the compat header, as suggested by
> > Quentin, but I thought the amount of repeated boilerplate was a bit too
> > much. So instead I introduced a compat header to wrap the API changes. Even
> > tools/bpf/bpftool/jit_disasm.c, which needs its own callbacks for json, imo
> > looks nicer this way.
> >
> > I'm not regular contributor, so it very well might be my procedures are a
> > bit off...
> >
> > I am not sure I added the right [number of] people to CC?
> >
> > WRT the feature test: Not sure what the point of the -DPACKAGE='"perf"' is,
> > nor why tools/perf/Makefile.config sets some LDFLAGS/CFLAGS that are also
> > in feature/Makefile and why -ldl isn't needed in the other places. But...
> >
> > V2:
> > - split patches further, so that tools/bpf and tools/perf part are entirely
> >   separate
> > - included a bit more information about tests I did in commit messages
> > - add a maybe_unused to fprintf_json_styled's style argument
> >
> > Cc: Alexei Starovoitov <ast@kernel.org>
> > Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
> > Cc: Jiri Olsa <jolsa@kernel.org>
> > Cc: Sedat Dilek <sedat.dilek@gmail.com>
> > Cc: Quentin Monnet <quentin@isovalent.com>
> > To: bpf@vger.kernel.org
> > To: linux-kernel@vger.kernel.org
> > Link: https://lore.kernel.org/lkml/20220622181918.ykrs5rsnmx3og4sv@alap3.anarazel.de
> > Link: https://lore.kernel.org/lkml/CA+icZUVpr8ZeOKCj4zMMqbFT013KJz2T1csvXg+VSkdvJH1Ubw@mail.gmail.com
> >
> > Andres Freund (5):
> >   tools build: add feature test for init_disassemble_info API changes
> >   tools include: add dis-asm-compat.h to handle version differences
> >   tools perf: Fix compilation error with new binutils
> >   tools bpf_jit_disasm: Fix compilation error with new binutils
> >   tools bpftool: Fix compilation error with new binutils
> >
> >  tools/bpf/Makefile                            |  7 ++-
> >  tools/bpf/bpf_jit_disasm.c                    |  5 +-
> >  tools/bpf/bpftool/Makefile                    |  7 ++-
> >  tools/bpf/bpftool/jit_disasm.c                | 42 ++++++++++++---
> >  tools/build/Makefile.feature                  |  4 +-
> >  tools/build/feature/Makefile                  |  4 ++
> >  tools/build/feature/test-all.c                |  4 ++
> >  .../feature/test-disassembler-init-styled.c   | 13 +++++
> >  tools/include/tools/dis-asm-compat.h          | 53 +++++++++++++++++++
> >  tools/perf/Makefile.config                    |  8 +++
> >  tools/perf/util/annotate.c                    |  7 +--
> >  11 files changed, 137 insertions(+), 17 deletions(-)
> >  create mode 100644 tools/build/feature/test-disassembler-init-styled.c
> >  create mode 100644 tools/include/tools/dis-asm-compat.h
> >
> > --
> > 2.37.0.3.g30cc8d0f14
> >

Re: [PATCH v2 0/5] tools: fix compilation failure caused by init_disassemble_info API changes

[Sedat Dilek]

On Sun, Jul 3, 2022 at 11:25 PM Andres Freund <andres@anarazel.de> wrote:
>
> binutils changed the signature of init_disassemble_info(), which now causes
> compilation failures for tools/{perf,bpf} on e.g. debian unstable. Relevant
> binutils commit:
> https://sourceware.org/git/?p=binutils-gdb.git;a=commit;h=60a3da00bd5407f07
>
> I first fixed this without introducing the compat header, as suggested by
> Quentin, but I thought the amount of repeated boilerplate was a bit too
> much. So instead I introduced a compat header to wrap the API changes. Even
> tools/bpf/bpftool/jit_disasm.c, which needs its own callbacks for json, imo
> looks nicer this way.
>
> I'm not regular contributor, so it very well might be my procedures are a
> bit off...
>
> I am not sure I added the right [number of] people to CC?
>
> WRT the feature test: Not sure what the point of the -DPACKAGE='"perf"' is,
> nor why tools/perf/Makefile.config sets some LDFLAGS/CFLAGS that are also
> in feature/Makefile and why -ldl isn't needed in the other places. But...
>
> V2:
> - split patches further, so that tools/bpf and tools/perf part are entirely
>   separate
> - included a bit more information about tests I did in commit messages
> - add a maybe_unused to fprintf_json_styled's style argument
>

[ CC Ben ]

The Debian kernel-team has integrated your patchset v2.

In case you build without libbfd support there is [1].
So, feel free to take this for v3.

-Sedat-

[1] https://salsa.debian.org/kernel-team/linux/-/blob/sid/debian/patches/bugfix/all/tools-perf-fix-build-without-libbfd.patch

> Cc: Alexei Starovoitov <ast@kernel.org>
> Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
> Cc: Jiri Olsa <jolsa@kernel.org>
> Cc: Sedat Dilek <sedat.dilek@gmail.com>
> Cc: Quentin Monnet <quentin@isovalent.com>
> To: bpf@vger.kernel.org
> To: linux-kernel@vger.kernel.org
> Link: https://lore.kernel.org/lkml/20220622181918.ykrs5rsnmx3og4sv@alap3.anarazel.de
> Link: https://lore.kernel.org/lkml/CA+icZUVpr8ZeOKCj4zMMqbFT013KJz2T1csvXg+VSkdvJH1Ubw@mail.gmail.com
>
> Andres Freund (5):
>   tools build: add feature test for init_disassemble_info API changes
>   tools include: add dis-asm-compat.h to handle version differences
>   tools perf: Fix compilation error with new binutils
>   tools bpf_jit_disasm: Fix compilation error with new binutils
>   tools bpftool: Fix compilation error with new binutils
>
>  tools/bpf/Makefile                            |  7 ++-
>  tools/bpf/bpf_jit_disasm.c                    |  5 +-
>  tools/bpf/bpftool/Makefile                    |  7 ++-
>  tools/bpf/bpftool/jit_disasm.c                | 42 ++++++++++++---
>  tools/build/Makefile.feature                  |  4 +-
>  tools/build/feature/Makefile                  |  4 ++
>  tools/build/feature/test-all.c                |  4 ++
>  .../feature/test-disassembler-init-styled.c   | 13 +++++
>  tools/include/tools/dis-asm-compat.h          | 53 +++++++++++++++++++
>  tools/perf/Makefile.config                    |  8 +++
>  tools/perf/util/annotate.c                    |  7 +--
>  11 files changed, 137 insertions(+), 17 deletions(-)
>  create mode 100644 tools/build/feature/test-disassembler-init-styled.c
>  create mode 100644 tools/include/tools/dis-asm-compat.h
>
> --
> 2.37.0.3.g30cc8d0f14
>

Re: [PATCH v2 0/5] tools: fix compilation failure caused by init_disassemble_info API changes

[Ben Hutchings]

[-- Attachment #1: Type: text/plain, Size: 2034 bytes --]

On Thu, 2022-07-14 at 11:16 +0200, Sedat Dilek wrote:
> On Sun, Jul 3, 2022 at 11:25 PM Andres Freund <andres@anarazel.de> wrote:
> > 
> > binutils changed the signature of init_disassemble_info(), which now causes
> > compilation failures for tools/{perf,bpf} on e.g. debian unstable. Relevant
> > binutils commit:
> > https://sourceware.org/git/?p=binutils-gdb.git;a=commit;h=60a3da00bd5407f07
> > 
> > I first fixed this without introducing the compat header, as suggested by
> > Quentin, but I thought the amount of repeated boilerplate was a bit too
> > much. So instead I introduced a compat header to wrap the API changes. Even
> > tools/bpf/bpftool/jit_disasm.c, which needs its own callbacks for json, imo
> > looks nicer this way.
> > 
> > I'm not regular contributor, so it very well might be my procedures are a
> > bit off...
> > 
> > I am not sure I added the right [number of] people to CC?
> > 
> > WRT the feature test: Not sure what the point of the -DPACKAGE='"perf"' is,
> > nor why tools/perf/Makefile.config sets some LDFLAGS/CFLAGS that are also
> > in feature/Makefile and why -ldl isn't needed in the other places. But...
> > 
> > V2:
> > - split patches further, so that tools/bpf and tools/perf part are entirely
> >   separate
> > - included a bit more information about tests I did in commit messages
> > - add a maybe_unused to fprintf_json_styled's style argument
> > 
> 
> [ CC Ben ]
> 
> The Debian kernel-team has integrated your patchset v2.
> 
> In case you build without libbfd support there is [1].
> So, feel free to take this for v3.
> 
> -Sedat-
> 
> [1] https://salsa.debian.org/kernel-team/linux/-/blob/sid/debian/patches/bugfix/all/tools-perf-fix-build-without-libbfd.patch
[...]

Thanks, I meant to send that fix upstream but got distracted.  It
should really be folded into "tools perf: Fix compilation error with
new binutils".

Ben.
> 

-- 
Ben Hutchings
Always try to do things in chronological order;
it's less confusing that way.

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH v2 0/5] tools: fix compilation failure caused by init_disassemble_info API changes

[Andres Freund]

Hi,

On 2022-07-14 15:25:44 +0200, Ben Hutchings wrote:
> Thanks, I meant to send that fix upstream but got distracted.  It
> should really be folded into "tools perf: Fix compilation error with
> new binutils".

I'll try to send a new version out soon. I think the right process is to add
Signed-off-by: Ben Hutchings <benh@debian.org>
to the patch I squash it with?

Greetings,

Andres Freund

Re: [PATCH v2 0/5] tools: fix compilation failure caused by init_disassemble_info API changes

[Ben Hutchings]

[-- Attachment #1: Type: text/plain, Size: 578 bytes --]

On Fri, 2022-07-15 at 12:16 -0700, Andres Freund wrote:
> Hi,
> 
> On 2022-07-14 15:25:44 +0200, Ben Hutchings wrote:
> > Thanks, I meant to send that fix upstream but got distracted.  It
> > should really be folded into "tools perf: Fix compilation error with
> > new binutils".
> 
> I'll try to send a new version out soon. I think the right process is to add
> Signed-off-by: Ben Hutchings <benh@debian.org>
> to the patch I squash it with?

Yes, OK.

Ben.

-- 
Ben Hutchings
Unix is many things to many people,
but it's never been everything to anybody.

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH v2 0/5] tools: fix compilation failure caused by init_disassemble_info API changes

[Arnaldo Carvalho de Melo]

Em Fri, Jul 15, 2022 at 09:18:26PM +0200, Ben Hutchings escreveu:
> On Fri, 2022-07-15 at 12:16 -0700, Andres Freund wrote:
> > Hi,
> > 
> > On 2022-07-14 15:25:44 +0200, Ben Hutchings wrote:
> > > Thanks, I meant to send that fix upstream but got distracted.  It
> > > should really be folded into "tools perf: Fix compilation error with
> > > new binutils".
> > 
> > I'll try to send a new version out soon. I think the right process is to add
> > Signed-off-by: Ben Hutchings <benh@debian.org>
> > to the patch I squash it with?
> 
> Yes, OK.

Ok, so you agreed on this, I'm adding Ben's Signed-off-by tag then,

Thanks,

- Arnaldo

Re: [PATCH v2 0/5] tools: fix compilation failure caused by init_disassemble_info API changes

[Arnaldo Carvalho de Melo]

Em Fri, Jul 15, 2022 at 12:16:41PM -0700, Andres Freund escreveu:
> Hi,
> 
> On 2022-07-14 15:25:44 +0200, Ben Hutchings wrote:
> > Thanks, I meant to send that fix upstream but got distracted.  It
> > should really be folded into "tools perf: Fix compilation error with
> > new binutils".
> 
> I'll try to send a new version out soon. I think the right process is to add
> Signed-off-by: Ben Hutchings <benh@debian.org>
> to the patch I squash it with?

Hi,

	How is this going? Any new patch coming soon? :-)

- Arnaldo

Re: [PATCH v2 0/5] tools: fix compilation failure caused by init_disassemble_info API changes

[Andres Freund]

Hi,

On 2022-07-27 12:47:16 -0300, Arnaldo Carvalho de Melo wrote:
> Em Fri, Jul 15, 2022 at 12:16:41PM -0700, Andres Freund escreveu:
> > On 2022-07-14 15:25:44 +0200, Ben Hutchings wrote:
> > > Thanks, I meant to send that fix upstream but got distracted.  It
> > > should really be folded into "tools perf: Fix compilation error with
> > > new binutils".
> > 
> > I'll try to send a new version out soon. I think the right process is to add
> > Signed-off-by: Ben Hutchings <benh@debian.org>
> > to the patch I squash it with?
> 
> Hi,
> 
> 	How is this going? Any new patch coming soon? :-)

Sorry - had hoped to finish sending it out before my vacation (and then on the
flight, but wifi didn't work...). Now back, will work on it asap.

Greetings,

Andres Freund

[PATCH v3 0/8] tools: fix compilation failure caused by init_disassemble_info API changes

[Andres Freund]

binutils changed the signature of init_disassemble_info(), which now causes
compilation failures for tools/{perf,bpf} on e.g. debian unstable. Relevant
binutils commit:
https://sourceware.org/git/?p=binutils-gdb.git;a=commit;h=60a3da00bd5407f07

I first fixed this without introducing the compat header, as suggested by
Quentin, but I thought the amount of repeated boilerplate was a bit too
much. So instead I introduced a compat header to wrap the API changes. Even
tools/bpf/bpftool/jit_disasm.c, which needs its own callbacks for json, imo
looks nicer this way.

I'm not regular contributor, so it very well might be my procedures are a
bit off...

I am not sure I added the right [number of] people to CC?

WRT the feature test: Not sure what the point of the -DPACKAGE='"perf"' is,
nor why tools/perf/Makefile.config sets some LDFLAGS/CFLAGS that are also
in feature/Makefile and why -ldl isn't needed in the other places. But...

V2:
- split patches further, so that tools/bpf and tools/perf part are entirely
  separate
- included a bit more information about tests I did in commit messages
- add a maybe_unused to fprintf_json_styled's style argument

V3:
- don't include dis-asm-compat.h when building without libbfd
  (Ben Hutchings)
- don't include compiler.h in dis-asm-compat.h, use (void) casts instead,
  to avoid compiler.h include due to potential licensing conflict
- dual-license dis-asm-compat.h, for better compatibility with the rest of
  bpftool's code (suggested by Quentin Monnet)
- don't display feature-disassembler-init-styled test
  (suggested by Jiri Olsa)
- don't display feature-disassembler-four-args test, I split this for the
  different subsystems, but maybe that's overkill? (suggested by Jiri Olsa)

Cc: Alexei Starovoitov <ast@kernel.org>
Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: Jiri Olsa <jolsa@kernel.org>
Cc: Sedat Dilek <sedat.dilek@gmail.com>
Cc: Quentin Monnet <quentin@isovalent.com>
CC: Ben Hutchings <benh@debian.org>
Cc: bpf@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Link: https://lore.kernel.org/lkml/20220622181918.ykrs5rsnmx3og4sv@alap3.anarazel.de
Link: https://lore.kernel.org/lkml/CA+icZUVpr8ZeOKCj4zMMqbFT013KJz2T1csvXg+VSkdvJH1Ubw@mail.gmail.com

Andres Freund (8):
  tools build: Add feature test for init_disassemble_info API changes
  tools build: Don't display disassembler-four-args feature test
  tools include: add dis-asm-compat.h to handle version differences
  tools perf: Fix compilation error with new binutils
  tools bpf_jit_disasm: Fix compilation error with new binutils
  tools bpf_jit_disasm: Don't display disassembler-four-args feature
    test
  tools bpftool: Fix compilation error with new binutils
  tools bpftool: Don't display disassembler-four-args feature test

 tools/bpf/Makefile                            |  7 ++-
 tools/bpf/bpf_jit_disasm.c                    |  5 +-
 tools/bpf/bpftool/Makefile                    |  8 ++-
 tools/bpf/bpftool/jit_disasm.c                | 42 +++++++++++---
 tools/build/Makefile.feature                  |  4 +-
 tools/build/feature/Makefile                  |  4 ++
 tools/build/feature/test-all.c                |  4 ++
 .../feature/test-disassembler-init-styled.c   | 13 +++++
 tools/include/tools/dis-asm-compat.h          | 55 +++++++++++++++++++
 tools/perf/Makefile.config                    |  8 +++
 tools/perf/util/annotate.c                    |  7 ++-
 11 files changed, 138 insertions(+), 19 deletions(-)
 create mode 100644 tools/build/feature/test-disassembler-init-styled.c
 create mode 100644 tools/include/tools/dis-asm-compat.h

-- 
2.37.0.3.g30cc8d0f14

[PATCH v3 1/8] tools build: Add feature test for init_disassemble_info API changes

[Andres Freund]

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 adds a feature test to detect the new signature.  Subsequent
commits will use it to fix the build failures.

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/build/Makefile.feature                        |  1 +
 tools/build/feature/Makefile                        |  4 ++++
 tools/build/feature/test-all.c                      |  4 ++++
 tools/build/feature/test-disassembler-init-styled.c | 13 +++++++++++++
 4 files changed, 22 insertions(+)
 create mode 100644 tools/build/feature/test-disassembler-init-styled.c

diff --git a/tools/build/Makefile.feature b/tools/build/Makefile.feature
index 888a0421d43b..8f6578e4d324 100644
--- a/tools/build/Makefile.feature
+++ b/tools/build/Makefile.feature
@@ -70,6 +70,7 @@ FEATURE_TESTS_BASIC :=                  \
         libaio				\
         libzstd				\
         disassembler-four-args		\
+        disassembler-init-styled	\
         file-handle
 
 # FEATURE_TESTS_BASIC + FEATURE_TESTS_EXTRA is the complete list
diff --git a/tools/build/feature/Makefile b/tools/build/feature/Makefile
index 7c2a17e23c30..c3059739318a 100644
--- a/tools/build/feature/Makefile
+++ b/tools/build/feature/Makefile
@@ -18,6 +18,7 @@ FILES=                                          \
          test-libbfd.bin                        \
          test-libbfd-buildid.bin		\
          test-disassembler-four-args.bin        \
+         test-disassembler-init-styled.bin	\
          test-reallocarray.bin			\
          test-libbfd-liberty.bin                \
          test-libbfd-liberty-z.bin              \
@@ -248,6 +249,9 @@ $(OUTPUT)test-libbfd-buildid.bin:
 $(OUTPUT)test-disassembler-four-args.bin:
 	$(BUILD) -DPACKAGE='"perf"' -lbfd -lopcodes
 
+$(OUTPUT)test-disassembler-init-styled.bin:
+	$(BUILD) -DPACKAGE='"perf"' -lbfd -lopcodes
+
 $(OUTPUT)test-reallocarray.bin:
 	$(BUILD)
 
diff --git a/tools/build/feature/test-all.c b/tools/build/feature/test-all.c
index 5ffafb967b6e..957c02c7b163 100644
--- a/tools/build/feature/test-all.c
+++ b/tools/build/feature/test-all.c
@@ -166,6 +166,10 @@
 # include "test-disassembler-four-args.c"
 #undef main
 
+#define main main_test_disassembler_init_styled
+# include "test-disassembler-init-styled.c"
+#undef main
+
 #define main main_test_libzstd
 # include "test-libzstd.c"
 #undef main
diff --git a/tools/build/feature/test-disassembler-init-styled.c b/tools/build/feature/test-disassembler-init-styled.c
new file mode 100644
index 000000000000..f1ce0ec3bee9
--- /dev/null
+++ b/tools/build/feature/test-disassembler-init-styled.c
@@ -0,0 +1,13 @@
+// SPDX-License-Identifier: GPL-2.0
+#include <stdio.h>
+#include <dis-asm.h>
+
+int main(void)
+{
+	struct disassemble_info info;
+
+	init_disassemble_info(&info, stdout,
+			      NULL, NULL);
+
+	return 0;
+}
-- 
2.37.0.3.g30cc8d0f14

[PATCH v3 2/8] tools build: Don't display disassembler-four-args feature test

[Andres Freund]

The feature check does not seem important enough to display. Suggested by
Jiri Olsa.

Cc: Jiri Olsa <jolsa@kernel.org>
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/build/Makefile.feature | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/tools/build/Makefile.feature b/tools/build/Makefile.feature
index 8f6578e4d324..fc6ce0b2535a 100644
--- a/tools/build/Makefile.feature
+++ b/tools/build/Makefile.feature
@@ -135,8 +135,7 @@ FEATURE_DISPLAY ?=              \
          get_cpuid              \
          bpf			\
          libaio			\
-         libzstd		\
-         disassembler-four-args
+         libzstd
 
 # Set FEATURE_CHECK_(C|LD)FLAGS-all for all FEATURE_TESTS features.
 # If in the future we need per-feature checks/flags for features not
-- 
2.37.0.3.g30cc8d0f14

Re: [PATCH v3 2/8] tools build: Don't display disassembler-four-args feature test

[Arnaldo Carvalho de Melo]

Em Sun, Jul 31, 2022 at 06:38:28PM -0700, Andres Freund escreveu:
> The feature check does not seem important enough to display. Suggested by
> Jiri Olsa.

I turned the last paragraph into a:

Suggested-by: Jiri Olsa <jolsa@kernel.org>
 
> Cc: Jiri Olsa <jolsa@kernel.org>
> 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/build/Makefile.feature | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/tools/build/Makefile.feature b/tools/build/Makefile.feature
> index 8f6578e4d324..fc6ce0b2535a 100644
> --- a/tools/build/Makefile.feature
> +++ b/tools/build/Makefile.feature
> @@ -135,8 +135,7 @@ FEATURE_DISPLAY ?=              \
>           get_cpuid              \
>           bpf			\
>           libaio			\
> -         libzstd		\
> -         disassembler-four-args
> +         libzstd
>  
>  # Set FEATURE_CHECK_(C|LD)FLAGS-all for all FEATURE_TESTS features.
>  # If in the future we need per-feature checks/flags for features not
> -- 
> 2.37.0.3.g30cc8d0f14

-- 

- Arnaldo

[PATCH v3 3/8] tools include: add dis-asm-compat.h to handle version differences

[Andres Freund]

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>
Cc: Ben Hutchings <benh@debian.org>
Link: http://lore.kernel.org/lkml/20220622181918.ykrs5rsnmx3og4sv@alap3.anarazel.de
Signed-off-by: Andres Freund <andres@anarazel.de>
Signed-off-by: Ben Hutchings <benh@debian.org>
---
 tools/include/tools/dis-asm-compat.h | 55 ++++++++++++++++++++++++++++
 1 file changed, 55 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..70f331e23ed3
--- /dev/null
+++ b/tools/include/tools/dis-asm-compat.h
@@ -0,0 +1,55 @@
+/* SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause */
+#ifndef _TOOLS_DIS_ASM_COMPAT_H
+#define _TOOLS_DIS_ASM_COMPAT_H
+
+#include <stdio.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,
+				 const char *fmt, ...)
+{
+	va_list args;
+	int r;
+
+	(void)style;
+
+	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)
+{
+#ifdef DISASM_INIT_STYLED
+	init_disassemble_info(info, stream,
+			      unstyled_func,
+			      styled_func);
+#else
+	(void)styled_func;
+	init_disassemble_info(info, stream,
+			      unstyled_func);
+#endif
+}
+
+#endif /* _TOOLS_DIS_ASM_COMPAT_H */
-- 
2.37.0.3.g30cc8d0f14

Re: [PATCH v3 3/8] tools include: add dis-asm-compat.h to handle version differences

[Arnaldo Carvalho de Melo]

Em Sun, Jul 31, 2022 at 06:38:29PM -0700, Andres Freund escreveu:
> 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>
> Cc: Ben Hutchings <benh@debian.org>
> Link: http://lore.kernel.org/lkml/20220622181918.ykrs5rsnmx3og4sv@alap3.anarazel.de
> Signed-off-by: Andres Freund <andres@anarazel.de>
> Signed-off-by: Ben Hutchings <benh@debian.org>

So, who is the author of this patch? Ben? b4 complained about it:

NOTE: some trailers ignored due to from/email mismatches:
    ! Trailer: Signed-off-by: Ben Hutchings <benh@debian.org>
     Msg From: Andres Freund <andres@anarazel.de>
NOTE: Rerun with -S to apply them anyway

If it is Ben, then we would need a:

From: Ben Hutchings <benh@debian.org>

At the beginning of the patch, right?

- Arnaldo

> ---
>  tools/include/tools/dis-asm-compat.h | 55 ++++++++++++++++++++++++++++
>  1 file changed, 55 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..70f331e23ed3
> --- /dev/null
> +++ b/tools/include/tools/dis-asm-compat.h
> @@ -0,0 +1,55 @@
> +/* SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause */
> +#ifndef _TOOLS_DIS_ASM_COMPAT_H
> +#define _TOOLS_DIS_ASM_COMPAT_H
> +
> +#include <stdio.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,
> +				 const char *fmt, ...)
> +{
> +	va_list args;
> +	int r;
> +
> +	(void)style;
> +
> +	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)
> +{
> +#ifdef DISASM_INIT_STYLED
> +	init_disassemble_info(info, stream,
> +			      unstyled_func,
> +			      styled_func);
> +#else
> +	(void)styled_func;
> +	init_disassemble_info(info, stream,
> +			      unstyled_func);
> +#endif
> +}
> +
> +#endif /* _TOOLS_DIS_ASM_COMPAT_H */
> -- 
> 2.37.0.3.g30cc8d0f14

-- 

- Arnaldo

Re: [PATCH v3 3/8] tools include: add dis-asm-compat.h to handle version differences

[Andres Freund]

Hi,

On 2022-08-01 15:05:00 -0300, Arnaldo Carvalho de Melo wrote:
> Em Sun, Jul 31, 2022 at 06:38:29PM -0700, Andres Freund escreveu:
> > 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>
> > Cc: Ben Hutchings <benh@debian.org>
> > Link: http://lore.kernel.org/lkml/20220622181918.ykrs5rsnmx3og4sv@alap3.anarazel.de
> > Signed-off-by: Andres Freund <andres@anarazel.de>
> > Signed-off-by: Ben Hutchings <benh@debian.org>
> 
> So, who is the author of this patch? Ben? b4 complained about it:

I squashed a fixup of Ben into my patch (moving the include in annotate.c into
the HAVE_LIBBFD_SUPPORT section). I don't know what the proper procedure is
for that - I'd asked in
https://lore.kernel.org/20220715191641.go6xbmhic3kafcsc@awork3.anarazel.de


> NOTE: some trailers ignored due to from/email mismatches:
>     ! Trailer: Signed-off-by: Ben Hutchings <benh@debian.org>
>      Msg From: Andres Freund <andres@anarazel.de>
> NOTE: Rerun with -S to apply them anyway
> 
> If it is Ben, then we would need a:
> 
> From: Ben Hutchings <benh@debian.org>
> 
> At the beginning of the patch, right?

I don't know, I interact with the kernel processes too rarely...

Greetings,

Andres Freund

[PATCH v3 4/8] tools perf: Fix compilation error with new binutils

[Andres Freund]

binutils changed the signature of init_disassemble_info(), which now causes
compilation failures for tools/perf/util/annotate.c, e.g. on debian
unstable. Relevant binutils commit:
https://sourceware.org/git/?p=binutils-gdb.git;a=commit;h=60a3da00bd5407f07

Wire up the feature test and switch to init_disassemble_info_compat(),
which were introduced in prior commits, fixing the compilation failure.

I verified that perf can still disassemble bpf programs by using bpftrace
under load, recording a perf trace, and then annotating the bpf "function"
with and without the changes. With old binutils there's no change in output
before/after this patch. When comparing the output from old binutils (2.35)
to new bintuils with the patch (upstream snapshot) there are a few output
differences, but they are unrelated to this patch. An example hunk is:

     1.15 :   55:mov    %rbp,%rdx
     0.00 :   58:add    $0xfffffffffffffff8,%rdx
     0.00 :   5c:xor    %ecx,%ecx
-    1.03 :   5e:callq  0xffffffffe12aca3c
+    1.03 :   5e:call   0xffffffffe12aca3c
     0.00 :   63:xor    %eax,%eax
-    2.18 :   65:leaveq
-    2.82 :   66:retq
+    2.18 :   65:leave
+    2.82 :   66:ret

Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: Sedat Dilek <sedat.dilek@gmail.com>
Link: http://lore.kernel.org/lkml/20220622181918.ykrs5rsnmx3og4sv@alap3.anarazel.de
Signed-off-by: Andres Freund <andres@anarazel.de>
---
 tools/perf/Makefile.config | 8 ++++++++
 tools/perf/util/annotate.c | 7 ++++---
 2 files changed, 12 insertions(+), 3 deletions(-)

diff --git a/tools/perf/Makefile.config b/tools/perf/Makefile.config
index 73e0762092fe..ee417c321adb 100644
--- a/tools/perf/Makefile.config
+++ b/tools/perf/Makefile.config
@@ -298,6 +298,7 @@ FEATURE_CHECK_LDFLAGS-libpython := $(PYTHON_EMBED_LDOPTS)
 FEATURE_CHECK_LDFLAGS-libaio = -lrt
 
 FEATURE_CHECK_LDFLAGS-disassembler-four-args = -lbfd -lopcodes -ldl
+FEATURE_CHECK_LDFLAGS-disassembler-init-styled = -lbfd -lopcodes -ldl
 
 CORE_CFLAGS += -fno-omit-frame-pointer
 CORE_CFLAGS += -ggdb3
@@ -905,13 +906,16 @@ ifndef NO_LIBBFD
     ifeq ($(feature-libbfd-liberty), 1)
       EXTLIBS += -lbfd -lopcodes -liberty
       FEATURE_CHECK_LDFLAGS-disassembler-four-args += -liberty -ldl
+      FEATURE_CHECK_LDFLAGS-disassembler-init-styled += -liberty -ldl
     else
       ifeq ($(feature-libbfd-liberty-z), 1)
         EXTLIBS += -lbfd -lopcodes -liberty -lz
         FEATURE_CHECK_LDFLAGS-disassembler-four-args += -liberty -lz -ldl
+        FEATURE_CHECK_LDFLAGS-disassembler-init-styled += -liberty -lz -ldl
       endif
     endif
     $(call feature_check,disassembler-four-args)
+    $(call feature_check,disassembler-init-styled)
   endif
 
   ifeq ($(feature-libbfd-buildid), 1)
@@ -1025,6 +1029,10 @@ ifeq ($(feature-disassembler-four-args), 1)
     CFLAGS += -DDISASM_FOUR_ARGS_SIGNATURE
 endif
 
+ifeq ($(feature-disassembler-init-styled), 1)
+    CFLAGS += -DDISASM_INIT_STYLED
+endif
+
 ifeq (${IS_64_BIT}, 1)
   ifndef NO_PERF_READ_VDSO32
     $(call feature_check,compile-32)
diff --git a/tools/perf/util/annotate.c b/tools/perf/util/annotate.c
index 82cc396ef516..2c6a485c3de5 100644
--- a/tools/perf/util/annotate.c
+++ b/tools/perf/util/annotate.c
@@ -1720,6 +1720,7 @@ static int dso__disassemble_filename(struct dso *dso, char *filename, size_t fil
 #include <bpf/btf.h>
 #include <bpf/libbpf.h>
 #include <linux/btf.h>
+#include <tools/dis-asm-compat.h>
 
 static int symbol__disassemble_bpf(struct symbol *sym,
 				   struct annotate_args *args)
@@ -1762,9 +1763,9 @@ static int symbol__disassemble_bpf(struct symbol *sym,
 		ret = errno;
 		goto out;
 	}
-	init_disassemble_info(&info, s,
-			      (fprintf_ftype) fprintf);
-
+	init_disassemble_info_compat(&info, s,
+				     (fprintf_ftype) fprintf,
+				     fprintf_styled);
 	info.arch = bfd_get_arch(bfdf);
 	info.mach = bfd_get_mach(bfdf);
 
-- 
2.37.0.3.g30cc8d0f14

[PATCH v3 5/8] tools bpf_jit_disasm: Fix compilation error with new binutils

[Andres Freund]

binutils changed the signature of init_disassemble_info(), which now causes
compilation to fail for tools/bpf/bpf_jit_disasm.c, e.g. on debian
unstable. Relevant binutils commit:
https://sourceware.org/git/?p=binutils-gdb.git;a=commit;h=60a3da00bd5407f07

Wire up the feature test and switch to init_disassemble_info_compat(),
which were introduced in prior commits, fixing the compilation failure.

I verified that bpf_jit_disasm can still disassemble bpf programs, both
with the old and new dis-asm.h API. With old binutils there's no change in
output before/after this patch. When comparing the output from old
binutils (2.35) to new bintuils with the patch (upstream snapshot) there
are a few output differences, but they are unrelated to this patch. An
example hunk is:
   f4:	mov    %r14,%rsi
   f7:	mov    %r15,%rdx
   fa:	mov    $0x2a,%ecx
-  ff:	callq  0xffffffffea8c4988
+  ff:	call   0xffffffffea8c4988
  104:	test   %rax,%rax
  107:	jge    0x0000000000000110
  109:	xor    %eax,%eax
- 10b:	jmpq   0x0000000000000073
+ 10b:	jmp    0x0000000000000073
  110:	cmp    $0x16,%rax

However, I had to use an older kernel to generate the bpf_jit_enabled = 2
output, as that has been broken since 5.18 / 1022a5498f6f:
https://lore.kernel.org/20220703030210.pmjft7qc2eajzi6c@alap3.anarazel.de

Cc: Alexei Starovoitov <ast@kernel.org>
Cc: Daniel Borkmann <daniel@iogearbox.net>
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/bpf/Makefile         | 5 ++++-
 tools/bpf/bpf_jit_disasm.c | 5 ++++-
 2 files changed, 8 insertions(+), 2 deletions(-)

diff --git a/tools/bpf/Makefile b/tools/bpf/Makefile
index b11cfc86a3d0..664601ab1705 100644
--- a/tools/bpf/Makefile
+++ b/tools/bpf/Makefile
@@ -34,7 +34,7 @@ else
 endif
 
 FEATURE_USER = .bpf
-FEATURE_TESTS = libbfd disassembler-four-args
+FEATURE_TESTS = libbfd disassembler-four-args disassembler-init-styled
 FEATURE_DISPLAY = libbfd disassembler-four-args
 
 check_feat := 1
@@ -56,6 +56,9 @@ endif
 ifeq ($(feature-disassembler-four-args), 1)
 CFLAGS += -DDISASM_FOUR_ARGS_SIGNATURE
 endif
+ifeq ($(feature-disassembler-init-styled), 1)
+CFLAGS += -DDISASM_INIT_STYLED
+endif
 
 $(OUTPUT)%.yacc.c: $(srctree)/tools/bpf/%.y
 	$(QUIET_BISON)$(YACC) -o $@ -d $<
diff --git a/tools/bpf/bpf_jit_disasm.c b/tools/bpf/bpf_jit_disasm.c
index c8ae95804728..a90a5d110f92 100644
--- a/tools/bpf/bpf_jit_disasm.c
+++ b/tools/bpf/bpf_jit_disasm.c
@@ -28,6 +28,7 @@
 #include <sys/types.h>
 #include <sys/stat.h>
 #include <limits.h>
+#include <tools/dis-asm-compat.h>
 
 #define CMD_ACTION_SIZE_BUFFER		10
 #define CMD_ACTION_READ_ALL		3
@@ -64,7 +65,9 @@ static void get_asm_insns(uint8_t *image, size_t len, int opcodes)
 	assert(bfdf);
 	assert(bfd_check_format(bfdf, bfd_object));
 
-	init_disassemble_info(&info, stdout, (fprintf_ftype) fprintf);
+	init_disassemble_info_compat(&info, stdout,
+				     (fprintf_ftype) fprintf,
+				     fprintf_styled);
 	info.arch = bfd_get_arch(bfdf);
 	info.mach = bfd_get_mach(bfdf);
 	info.buffer = image;
-- 
2.37.0.3.g30cc8d0f14

[PATCH v3 6/8] tools bpf_jit_disasm: Don't display disassembler-four-args feature test

[Andres Freund]

The feature check does not seem important enough to display. Suggested by
Jiri Olsa.

Cc: Jiri Olsa <jolsa@kernel.org>
Cc: Alexei Starovoitov <ast@kernel.org>
Cc: Daniel Borkmann <daniel@iogearbox.net>
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/bpf/Makefile | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/bpf/Makefile b/tools/bpf/Makefile
index 664601ab1705..243b79f2b451 100644
--- a/tools/bpf/Makefile
+++ b/tools/bpf/Makefile
@@ -35,7 +35,7 @@ endif
 
 FEATURE_USER = .bpf
 FEATURE_TESTS = libbfd disassembler-four-args disassembler-init-styled
-FEATURE_DISPLAY = libbfd disassembler-four-args
+FEATURE_DISPLAY = libbfd
 
 check_feat := 1
 NON_CHECK_FEAT_TARGETS := clean bpftool_clean runqslower_clean resolve_btfids_clean
-- 
2.37.0.3.g30cc8d0f14

Re: [PATCH v3 6/8] tools bpf_jit_disasm: Don't display disassembler-four-args feature test

[Arnaldo Carvalho de Melo]

Em Sun, Jul 31, 2022 at 06:38:32PM -0700, Andres Freund escreveu:
> The feature check does not seem important enough to display. Suggested by
> Jiri Olsa.
> 
> Cc: Jiri Olsa <jolsa@kernel.org>
> Cc: Alexei Starovoitov <ast@kernel.org>
> Cc: Daniel Borkmann <daniel@iogearbox.net>
> 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/bpf/Makefile | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/tools/bpf/Makefile b/tools/bpf/Makefile
> index 664601ab1705..243b79f2b451 100644
> --- a/tools/bpf/Makefile
> +++ b/tools/bpf/Makefile
> @@ -35,7 +35,7 @@ endif
>  
>  FEATURE_USER = .bpf
>  FEATURE_TESTS = libbfd disassembler-four-args disassembler-init-styled
> -FEATURE_DISPLAY = libbfd disassembler-four-args
> +FEATURE_DISPLAY = libbfd
>  
>  check_feat := 1
>  NON_CHECK_FEAT_TARGETS := clean bpftool_clean runqslower_clean resolve_btfids_clean
> -- 
> 2.37.0.3.g30cc8d0f14

After this patch:

⬢[acme@toolbox perf]$ git log --oneline -7
cebe4f3a4a0af5bf (HEAD) tools bpf_jit_disasm: Don't display disassembler-four-args feature test
7f62593e5582cb27 tools bpf_jit_disasm: Fix compilation error with new binutils
ee4dc290ee5c09b7 tools perf: Fix compilation error with new binutils
335f8d183a609793 tools include: add dis-asm-compat.h to handle version differences
f2f95e8d0def9c5f tools build: Don't display disassembler-four-args feature test
ede0fece841bb743 tools build: Add feature test for init_disassemble_info API changes
00b32625982e0c79 perf test: Add ARM SPE system wide test
⬢[acme@toolbox perf]$

⬢[acme@toolbox perf]$ make -C tools/bpf/bpftool/ clean
make: Entering directory '/var/home/acme/git/perf/tools/bpf/bpftool'
  CLEAN   libbpf
  CLEAN   libbpf-bootstrap
  CLEAN   feature-detect
  CLEAN   bpftool
  CLEAN   core-gen
make: Leaving directory '/var/home/acme/git/perf/tools/bpf/bpftool'
⬢[acme@toolbox perf]$ make -C tools/bpf/bpftool/
make: Entering directory '/var/home/acme/git/perf/tools/bpf/bpftool'

Auto-detecting system features:
...                        libbfd: [ on  ]
...        disassembler-four-args: [ on  ]
...                          zlib: [ on  ]
...                        libcap: [ on  ]
...               clang-bpf-co-re: [ on  ]
<SNIP>

It is still there, we need the hunk below, that I folded into your patch, to
disable it, please ack :-)

- Arnaldo

diff --git a/tools/bpf/bpftool/Makefile b/tools/bpf/bpftool/Makefile
index c6d2c77d02524a37..a92fb4d312ec2363 100644
--- a/tools/bpf/bpftool/Makefile
+++ b/tools/bpf/bpftool/Makefile
@@ -95,7 +95,7 @@ RM ?= rm -f
 FEATURE_USER = .bpftool
 FEATURE_TESTS = libbfd disassembler-four-args zlib libcap \
 	clang-bpf-co-re
-FEATURE_DISPLAY = libbfd disassembler-four-args zlib libcap \
+FEATURE_DISPLAY = libbfd zlib libcap \
 	clang-bpf-co-re
 
 check_feat := 1

Re: [PATCH v3 6/8] tools bpf_jit_disasm: Don't display disassembler-four-args feature test

[Andres Freund]

Hi,

On 2022-08-01 15:27:22 -0300, Arnaldo Carvalho de Melo wrote:
> ⬢[acme@toolbox perf]$ git log --oneline -7
> cebe4f3a4a0af5bf (HEAD) tools bpf_jit_disasm: Don't display disassembler-four-args feature test
> 7f62593e5582cb27 tools bpf_jit_disasm: Fix compilation error with new binutils
> ee4dc290ee5c09b7 tools perf: Fix compilation error with new binutils
> 335f8d183a609793 tools include: add dis-asm-compat.h to handle version differences
> f2f95e8d0def9c5f tools build: Don't display disassembler-four-args feature test
> ede0fece841bb743 tools build: Add feature test for init_disassemble_info API changes
> 00b32625982e0c79 perf test: Add ARM SPE system wide test
> ⬢[acme@toolbox perf]$
> 
> ⬢[acme@toolbox perf]$ make -C tools/bpf/bpftool/ clean
> make: Entering directory '/var/home/acme/git/perf/tools/bpf/bpftool'
>   CLEAN   libbpf
>   CLEAN   libbpf-bootstrap
>   CLEAN   feature-detect
>   CLEAN   bpftool
>   CLEAN   core-gen
> make: Leaving directory '/var/home/acme/git/perf/tools/bpf/bpftool'
> ⬢[acme@toolbox perf]$ make -C tools/bpf/bpftool/
> make: Entering directory '/var/home/acme/git/perf/tools/bpf/bpftool'
> 
> Auto-detecting system features:
> ...                        libbfd: [ on  ]
> ...        disassembler-four-args: [ on  ]
> ...                          zlib: [ on  ]
> ...                        libcap: [ on  ]
> ...               clang-bpf-co-re: [ on  ]
> <SNIP>
> 
> It is still there, we need the hunk below, that I folded into your patch, to
> disable it, please ack :-)

This commit just removed disassembler-four-args display for bpf_jit_disasm,
not bpftool. That should be in a later commit.

Greetings,

Andres Freund

[PATCH v3 7/8] tools bpftool: Fix compilation error with new binutils

[Andres Freund]

binutils changed the signature of init_disassemble_info(), which now causes
compilation to fail for tools/bpf/bpftool/jit_disasm.c, e.g. on debian
unstable. Relevant binutils commit:
https://sourceware.org/git/?p=binutils-gdb.git;a=commit;h=60a3da00bd5407f07

Wire up the feature test and switch to init_disassemble_info_compat(),
which were introduced in prior commits, fixing the compilation failure.

I verified that bpftool can still disassemble bpf programs, both with an
old and new dis-asm.h API. There are no output changes for plain and json
formats. When comparing the output from old binutils (2.35)
to new bintuils with the patch (upstream snapshot) there are a few output
differences, but they are unrelated to this patch. An example hunk is:
   2f:	pop    %r14
   31:	pop    %r13
   33:	pop    %rbx
-  34:	leaveq
-  35:	retq
+  34:	leave
+  35:	ret

Cc: Alexei Starovoitov <ast@kernel.org>
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/bpf/bpftool/Makefile     |  5 +++-
 tools/bpf/bpftool/jit_disasm.c | 42 +++++++++++++++++++++++++++-------
 2 files changed, 38 insertions(+), 9 deletions(-)

diff --git a/tools/bpf/bpftool/Makefile b/tools/bpf/bpftool/Makefile
index c6d2c77d0252..436e671b2657 100644
--- a/tools/bpf/bpftool/Makefile
+++ b/tools/bpf/bpftool/Makefile
@@ -93,7 +93,7 @@ INSTALL ?= install
 RM ?= rm -f
 
 FEATURE_USER = .bpftool
-FEATURE_TESTS = libbfd disassembler-four-args zlib libcap \
+FEATURE_TESTS = libbfd disassembler-four-args disassembler-init-styled zlib libcap \
 	clang-bpf-co-re
 FEATURE_DISPLAY = libbfd disassembler-four-args zlib libcap \
 	clang-bpf-co-re
@@ -117,6 +117,9 @@ endif
 ifeq ($(feature-disassembler-four-args), 1)
 CFLAGS += -DDISASM_FOUR_ARGS_SIGNATURE
 endif
+ifeq ($(feature-disassembler-init-styled), 1)
+    CFLAGS += -DDISASM_INIT_STYLED
+endif
 
 LIBS = $(LIBBPF) -lelf -lz
 LIBS_BOOTSTRAP = $(LIBBPF_BOOTSTRAP) -lelf -lz
diff --git a/tools/bpf/bpftool/jit_disasm.c b/tools/bpf/bpftool/jit_disasm.c
index 24734f2249d6..aaf99a0168c9 100644
--- a/tools/bpf/bpftool/jit_disasm.c
+++ b/tools/bpf/bpftool/jit_disasm.c
@@ -24,6 +24,7 @@
 #include <sys/stat.h>
 #include <limits.h>
 #include <bpf/libbpf.h>
+#include <tools/dis-asm-compat.h>
 
 #include "json_writer.h"
 #include "main.h"
@@ -39,15 +40,12 @@ static void get_exec_path(char *tpath, size_t size)
 }
 
 static int oper_count;
-static int fprintf_json(void *out, const char *fmt, ...)
+static int printf_json(void *out, const char *fmt, va_list ap)
 {
-	va_list ap;
 	char *s;
 	int err;
 
-	va_start(ap, fmt);
 	err = vasprintf(&s, fmt, ap);
-	va_end(ap);
 	if (err < 0)
 		return -1;
 
@@ -73,6 +71,32 @@ static int fprintf_json(void *out, const char *fmt, ...)
 	return 0;
 }
 
+static int fprintf_json(void *out, const char *fmt, ...)
+{
+	va_list ap;
+	int r;
+
+	va_start(ap, fmt);
+	r = printf_json(out, fmt, ap);
+	va_end(ap);
+
+	return r;
+}
+
+static int fprintf_json_styled(void *out,
+			       enum disassembler_style style __maybe_unused,
+			       const char *fmt, ...)
+{
+	va_list ap;
+	int r;
+
+	va_start(ap, fmt);
+	r = printf_json(out, fmt, ap);
+	va_end(ap);
+
+	return r;
+}
+
 void disasm_print_insn(unsigned char *image, ssize_t len, int opcodes,
 		       const char *arch, const char *disassembler_options,
 		       const struct btf *btf,
@@ -99,11 +123,13 @@ void disasm_print_insn(unsigned char *image, ssize_t len, int opcodes,
 	assert(bfd_check_format(bfdf, bfd_object));
 
 	if (json_output)
-		init_disassemble_info(&info, stdout,
-				      (fprintf_ftype) fprintf_json);
+		init_disassemble_info_compat(&info, stdout,
+					     (fprintf_ftype) fprintf_json,
+					     fprintf_json_styled);
 	else
-		init_disassemble_info(&info, stdout,
-				      (fprintf_ftype) fprintf);
+		init_disassemble_info_compat(&info, stdout,
+					     (fprintf_ftype) fprintf,
+					     fprintf_styled);
 
 	/* Update architecture info for offload. */
 	if (arch) {
-- 
2.37.0.3.g30cc8d0f14

[PATCH v3 8/8] tools bpftool: Don't display disassembler-four-args feature test

[Andres Freund]

The feature check does not seem important enough to display. Requested by
Jiri Olsa.

Cc: Jiri Olsa <jolsa@kernel.org>
Cc: Alexei Starovoitov <ast@kernel.org>
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/bpf/bpftool/Makefile | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/tools/bpf/bpftool/Makefile b/tools/bpf/bpftool/Makefile
index 436e671b2657..517df016d54a 100644
--- a/tools/bpf/bpftool/Makefile
+++ b/tools/bpf/bpftool/Makefile
@@ -95,8 +95,7 @@ RM ?= rm -f
 FEATURE_USER = .bpftool
 FEATURE_TESTS = libbfd disassembler-four-args disassembler-init-styled zlib libcap \
 	clang-bpf-co-re
-FEATURE_DISPLAY = libbfd disassembler-four-args zlib libcap \
-	clang-bpf-co-re
+FEATURE_DISPLAY = libbfd zlib libcap clang-bpf-co-re
 
 check_feat := 1
 NON_CHECK_FEAT_TARGETS := clean uninstall doc doc-clean doc-install doc-uninstall
-- 
2.37.0.3.g30cc8d0f14

Re: [PATCH v3 8/8] tools bpftool: Don't display disassembler-four-args feature test

[Arnaldo Carvalho de Melo]

Em Sun, Jul 31, 2022 at 06:38:34PM -0700, Andres Freund escreveu:
> The feature check does not seem important enough to display. Requested by
> Jiri Olsa.

Sorry, I hadn't seen this one, removing my change.

- Arnaldo
 
> Cc: Jiri Olsa <jolsa@kernel.org>
> Cc: Alexei Starovoitov <ast@kernel.org>
> 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/bpf/bpftool/Makefile | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/tools/bpf/bpftool/Makefile b/tools/bpf/bpftool/Makefile
> index 436e671b2657..517df016d54a 100644
> --- a/tools/bpf/bpftool/Makefile
> +++ b/tools/bpf/bpftool/Makefile
> @@ -95,8 +95,7 @@ RM ?= rm -f
>  FEATURE_USER = .bpftool
>  FEATURE_TESTS = libbfd disassembler-four-args disassembler-init-styled zlib libcap \
>  	clang-bpf-co-re
> -FEATURE_DISPLAY = libbfd disassembler-four-args zlib libcap \
> -	clang-bpf-co-re
> +FEATURE_DISPLAY = libbfd zlib libcap clang-bpf-co-re
>  
>  check_feat := 1
>  NON_CHECK_FEAT_TARGETS := clean uninstall doc doc-clean doc-install doc-uninstall
> -- 
> 2.37.0.3.g30cc8d0f14

-- 

- Arnaldo

Re: [PATCH v3 0/8] tools: fix compilation failure caused by init_disassemble_info API changes

[Arnaldo Carvalho de Melo]

Em Sun, Jul 31, 2022 at 06:38:26PM -0700, Andres Freund escreveu:
> binutils changed the signature of init_disassemble_info(), which now causes
> compilation failures for tools/{perf,bpf} on e.g. debian unstable. Relevant
> binutils commit:
> https://sourceware.org/git/?p=binutils-gdb.git;a=commit;h=60a3da00bd5407f07
> 
> I first fixed this without introducing the compat header, as suggested by
> Quentin, but I thought the amount of repeated boilerplate was a bit too
> much. So instead I introduced a compat header to wrap the API changes. Even
> tools/bpf/bpftool/jit_disasm.c, which needs its own callbacks for json, imo
> looks nicer this way.
> 
> I'm not regular contributor, so it very well might be my procedures are a
> bit off...
> 
> I am not sure I added the right [number of] people to CC?

I think its ok
 
> WRT the feature test: Not sure what the point of the -DPACKAGE='"perf"' is,

I think its related to libbfd, and it comes from a long time ago, trying
to find the cset adding that...

> nor why tools/perf/Makefile.config sets some LDFLAGS/CFLAGS that are also
> in feature/Makefile and why -ldl isn't needed in the other places. But...
> 
> V2:
> - split patches further, so that tools/bpf and tools/perf part are entirely
>   separate

Cool, thanks, I'll process the first 4 patches, then at some point the
bpftool bits can be merged, alternatively I can process those as well if
the bpftool maintainers are ok with it.

I'll just wait a bit to see if Jiri and others have something to say.

- Arnaldo

> - included a bit more information about tests I did in commit messages
> - add a maybe_unused to fprintf_json_styled's style argument
> 
> V3:
> - don't include dis-asm-compat.h when building without libbfd
>   (Ben Hutchings)
> - don't include compiler.h in dis-asm-compat.h, use (void) casts instead,
>   to avoid compiler.h include due to potential licensing conflict
> - dual-license dis-asm-compat.h, for better compatibility with the rest of
>   bpftool's code (suggested by Quentin Monnet)
> - don't display feature-disassembler-init-styled test
>   (suggested by Jiri Olsa)
> - don't display feature-disassembler-four-args test, I split this for the
>   different subsystems, but maybe that's overkill? (suggested by Jiri Olsa)
> 
> Cc: Alexei Starovoitov <ast@kernel.org>
> Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
> Cc: Jiri Olsa <jolsa@kernel.org>
> Cc: Sedat Dilek <sedat.dilek@gmail.com>
> Cc: Quentin Monnet <quentin@isovalent.com>
> CC: Ben Hutchings <benh@debian.org>
> Cc: bpf@vger.kernel.org
> Cc: linux-kernel@vger.kernel.org
> Link: https://lore.kernel.org/lkml/20220622181918.ykrs5rsnmx3og4sv@alap3.anarazel.de
> Link: https://lore.kernel.org/lkml/CA+icZUVpr8ZeOKCj4zMMqbFT013KJz2T1csvXg+VSkdvJH1Ubw@mail.gmail.com
> 
> Andres Freund (8):
>   tools build: Add feature test for init_disassemble_info API changes
>   tools build: Don't display disassembler-four-args feature test
>   tools include: add dis-asm-compat.h to handle version differences
>   tools perf: Fix compilation error with new binutils
>   tools bpf_jit_disasm: Fix compilation error with new binutils
>   tools bpf_jit_disasm: Don't display disassembler-four-args feature
>     test
>   tools bpftool: Fix compilation error with new binutils
>   tools bpftool: Don't display disassembler-four-args feature test
> 
>  tools/bpf/Makefile                            |  7 ++-
>  tools/bpf/bpf_jit_disasm.c                    |  5 +-
>  tools/bpf/bpftool/Makefile                    |  8 ++-
>  tools/bpf/bpftool/jit_disasm.c                | 42 +++++++++++---
>  tools/build/Makefile.feature                  |  4 +-
>  tools/build/feature/Makefile                  |  4 ++
>  tools/build/feature/test-all.c                |  4 ++
>  .../feature/test-disassembler-init-styled.c   | 13 +++++
>  tools/include/tools/dis-asm-compat.h          | 55 +++++++++++++++++++
>  tools/perf/Makefile.config                    |  8 +++
>  tools/perf/util/annotate.c                    |  7 ++-
>  11 files changed, 138 insertions(+), 19 deletions(-)
>  create mode 100644 tools/build/feature/test-disassembler-init-styled.c
>  create mode 100644 tools/include/tools/dis-asm-compat.h
> 
> -- 
> 2.37.0.3.g30cc8d0f14

-- 

- Arnaldo

Re: [PATCH v3 0/8] tools: fix compilation failure caused by init_disassemble_info API changes

[Quentin Monnet]

On 01/08/2022 13:45, Arnaldo Carvalho de Melo wrote:
> Em Sun, Jul 31, 2022 at 06:38:26PM -0700, Andres Freund escreveu:
>> binutils changed the signature of init_disassemble_info(), which now causes
>> compilation failures for tools/{perf,bpf} on e.g. debian unstable. Relevant
>> binutils commit:
>> https://sourceware.org/git/?p=binutils-gdb.git;a=commit;h=60a3da00bd5407f07
>>
>> I first fixed this without introducing the compat header, as suggested by
>> Quentin, but I thought the amount of repeated boilerplate was a bit too
>> much. So instead I introduced a compat header to wrap the API changes. Even
>> tools/bpf/bpftool/jit_disasm.c, which needs its own callbacks for json, imo
>> looks nicer this way.
>>
>> I'm not regular contributor, so it very well might be my procedures are a
>> bit off...
>>
>> I am not sure I added the right [number of] people to CC?
> 
> I think its ok
>  
>> WRT the feature test: Not sure what the point of the -DPACKAGE='"perf"' is,
> 
> I think its related to libbfd, and it comes from a long time ago, trying
> to find the cset adding that...
> 
>> nor why tools/perf/Makefile.config sets some LDFLAGS/CFLAGS that are also
>> in feature/Makefile and why -ldl isn't needed in the other places. But...
>>
>> V2:
>> - split patches further, so that tools/bpf and tools/perf part are entirely
>>   separate
> 
> Cool, thanks, I'll process the first 4 patches, then at some point the
> bpftool bits can be merged, alternatively I can process those as well if
> the bpftool maintainers are ok with it.
> 
> I'll just wait a bit to see if Jiri and others have something to say.
> 
> - Arnaldo

Thanks for this work! For the series:

Acked-by: Quentin Monnet <quentin@isovalent.com>

For what it's worth, it would make sense to me that these patches remain
together (so, through Arnaldo's tree), given that both the perf and
bpftool parts depend on dis-asm-compat.h being available.

Quentin

Re: [PATCH v3 0/8] tools: fix compilation failure caused by init_disassemble_info API changes

[Arnaldo Carvalho de Melo]

Em Mon, Aug 01, 2022 at 04:15:19PM +0100, Quentin Monnet escreveu:
> On 01/08/2022 13:45, Arnaldo Carvalho de Melo wrote:
> > Em Sun, Jul 31, 2022 at 06:38:26PM -0700, Andres Freund escreveu:
> >> binutils changed the signature of init_disassemble_info(), which now causes
> >> compilation failures for tools/{perf,bpf} on e.g. debian unstable. Relevant
> >> binutils commit:
> >> https://sourceware.org/git/?p=binutils-gdb.git;a=commit;h=60a3da00bd5407f07
> >>
> >> I first fixed this without introducing the compat header, as suggested by
> >> Quentin, but I thought the amount of repeated boilerplate was a bit too
> >> much. So instead I introduced a compat header to wrap the API changes. Even
> >> tools/bpf/bpftool/jit_disasm.c, which needs its own callbacks for json, imo
> >> looks nicer this way.
> >>
> >> I'm not regular contributor, so it very well might be my procedures are a
> >> bit off...
> >>
> >> I am not sure I added the right [number of] people to CC?
> > 
> > I think its ok
> >  
> >> WRT the feature test: Not sure what the point of the -DPACKAGE='"perf"' is,
> > 
> > I think its related to libbfd, and it comes from a long time ago, trying
> > to find the cset adding that...
> > 
> >> nor why tools/perf/Makefile.config sets some LDFLAGS/CFLAGS that are also
> >> in feature/Makefile and why -ldl isn't needed in the other places. But...
> >>
> >> V2:
> >> - split patches further, so that tools/bpf and tools/perf part are entirely
> >>   separate
> > 
> > Cool, thanks, I'll process the first 4 patches, then at some point the
> > bpftool bits can be merged, alternatively I can process those as well if
> > the bpftool maintainers are ok with it.
> > 
> > I'll just wait a bit to see if Jiri and others have something to say.
> > 
> > - Arnaldo
> 
> Thanks for this work! For the series:
> 
> Acked-by: Quentin Monnet <quentin@isovalent.com>
> 
> For what it's worth, it would make sense to me that these patches remain
> together (so, through Arnaldo's tree), given that both the perf and
> bpftool parts depend on dis-asm-compat.h being available.

Ok, so I'm tentatively adding it to my local tree to do some tests, if
someone disagrees, please holler.

- Arnaldo

Re: [PATCH v3 0/8] tools: fix compilation failure caused by init_disassemble_info API changes

[Daniel Borkmann]

On 8/1/22 8:02 PM, Arnaldo Carvalho de Melo wrote:
> Em Mon, Aug 01, 2022 at 04:15:19PM +0100, Quentin Monnet escreveu:
>> On 01/08/2022 13:45, Arnaldo Carvalho de Melo wrote:
>>> Em Sun, Jul 31, 2022 at 06:38:26PM -0700, Andres Freund escreveu:
>>>> binutils changed the signature of init_disassemble_info(), which now causes
>>>> compilation failures for tools/{perf,bpf} on e.g. debian unstable. Relevant
>>>> binutils commit:
>>>> https://sourceware.org/git/?p=binutils-gdb.git;a=commit;h=60a3da00bd5407f07
>>>>
>>>> I first fixed this without introducing the compat header, as suggested by
>>>> Quentin, but I thought the amount of repeated boilerplate was a bit too
>>>> much. So instead I introduced a compat header to wrap the API changes. Even
>>>> tools/bpf/bpftool/jit_disasm.c, which needs its own callbacks for json, imo
>>>> looks nicer this way.
>>>>
>>>> I'm not regular contributor, so it very well might be my procedures are a
>>>> bit off...
>>>>
>>>> I am not sure I added the right [number of] people to CC?
>>>
>>> I think its ok
>>>   
>>>> WRT the feature test: Not sure what the point of the -DPACKAGE='"perf"' is,
>>>
>>> I think its related to libbfd, and it comes from a long time ago, trying
>>> to find the cset adding that...
>>>
>>>> nor why tools/perf/Makefile.config sets some LDFLAGS/CFLAGS that are also
>>>> in feature/Makefile and why -ldl isn't needed in the other places. But...
>>>>
>>>> V2:
>>>> - split patches further, so that tools/bpf and tools/perf part are entirely
>>>>    separate
>>>
>>> Cool, thanks, I'll process the first 4 patches, then at some point the
>>> bpftool bits can be merged, alternatively I can process those as well if
>>> the bpftool maintainers are ok with it.
>>>
>>> I'll just wait a bit to see if Jiri and others have something to say.
>>>
>>> - Arnaldo
>>
>> Thanks for this work! For the series:
>>
>> Acked-by: Quentin Monnet <quentin@isovalent.com>
>>
>> For what it's worth, it would make sense to me that these patches remain
>> together (so, through Arnaldo's tree), given that both the perf and
>> bpftool parts depend on dis-asm-compat.h being available.
> 
> Ok, so I'm tentatively adding it to my local tree to do some tests, if
> someone disagrees, please holler.

Ack, sgtm. Please route these fixes via your tree. Thanks Arnaldo!

Re: [PATCH v3 0/8] tools: fix compilation failure caused by init_disassemble_info API changes

[Jiri Olsa]

On Mon, Aug 01, 2022 at 09:45:06AM -0300, Arnaldo Carvalho de Melo wrote:
> Em Sun, Jul 31, 2022 at 06:38:26PM -0700, Andres Freund escreveu:
> > binutils changed the signature of init_disassemble_info(), which now causes
> > compilation failures for tools/{perf,bpf} on e.g. debian unstable. Relevant
> > binutils commit:
> > https://sourceware.org/git/?p=binutils-gdb.git;a=commit;h=60a3da00bd5407f07
> > 
> > I first fixed this without introducing the compat header, as suggested by
> > Quentin, but I thought the amount of repeated boilerplate was a bit too
> > much. So instead I introduced a compat header to wrap the API changes. Even
> > tools/bpf/bpftool/jit_disasm.c, which needs its own callbacks for json, imo
> > looks nicer this way.
> > 
> > I'm not regular contributor, so it very well might be my procedures are a
> > bit off...
> > 
> > I am not sure I added the right [number of] people to CC?
> 
> I think its ok
>  
> > WRT the feature test: Not sure what the point of the -DPACKAGE='"perf"' is,
> 
> I think its related to libbfd, and it comes from a long time ago, trying
> to find the cset adding that...
> 
> > nor why tools/perf/Makefile.config sets some LDFLAGS/CFLAGS that are also
> > in feature/Makefile and why -ldl isn't needed in the other places. But...
> > 
> > V2:
> > - split patches further, so that tools/bpf and tools/perf part are entirely
> >   separate
> 
> Cool, thanks, I'll process the first 4 patches, then at some point the
> bpftool bits can be merged, alternatively I can process those as well if
> the bpftool maintainers are ok with it.
> 
> I'll just wait a bit to see if Jiri and others have something to say.

looks good

Acked-by: Jiri Olsa <jolsa@kernel.org>

thanks,
jirka

> 
> - Arnaldo
> 
> > - included a bit more information about tests I did in commit messages
> > - add a maybe_unused to fprintf_json_styled's style argument
> > 
> > V3:
> > - don't include dis-asm-compat.h when building without libbfd
> >   (Ben Hutchings)
> > - don't include compiler.h in dis-asm-compat.h, use (void) casts instead,
> >   to avoid compiler.h include due to potential licensing conflict
> > - dual-license dis-asm-compat.h, for better compatibility with the rest of
> >   bpftool's code (suggested by Quentin Monnet)
> > - don't display feature-disassembler-init-styled test
> >   (suggested by Jiri Olsa)
> > - don't display feature-disassembler-four-args test, I split this for the
> >   different subsystems, but maybe that's overkill? (suggested by Jiri Olsa)
> > 
> > Cc: Alexei Starovoitov <ast@kernel.org>
> > Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
> > Cc: Jiri Olsa <jolsa@kernel.org>
> > Cc: Sedat Dilek <sedat.dilek@gmail.com>
> > Cc: Quentin Monnet <quentin@isovalent.com>
> > CC: Ben Hutchings <benh@debian.org>
> > Cc: bpf@vger.kernel.org
> > Cc: linux-kernel@vger.kernel.org
> > Link: https://lore.kernel.org/lkml/20220622181918.ykrs5rsnmx3og4sv@alap3.anarazel.de
> > Link: https://lore.kernel.org/lkml/CA+icZUVpr8ZeOKCj4zMMqbFT013KJz2T1csvXg+VSkdvJH1Ubw@mail.gmail.com
> > 
> > Andres Freund (8):
> >   tools build: Add feature test for init_disassemble_info API changes
> >   tools build: Don't display disassembler-four-args feature test
> >   tools include: add dis-asm-compat.h to handle version differences
> >   tools perf: Fix compilation error with new binutils
> >   tools bpf_jit_disasm: Fix compilation error with new binutils
> >   tools bpf_jit_disasm: Don't display disassembler-four-args feature
> >     test
> >   tools bpftool: Fix compilation error with new binutils
> >   tools bpftool: Don't display disassembler-four-args feature test
> > 
> >  tools/bpf/Makefile                            |  7 ++-
> >  tools/bpf/bpf_jit_disasm.c                    |  5 +-
> >  tools/bpf/bpftool/Makefile                    |  8 ++-
> >  tools/bpf/bpftool/jit_disasm.c                | 42 +++++++++++---
> >  tools/build/Makefile.feature                  |  4 +-
> >  tools/build/feature/Makefile                  |  4 ++
> >  tools/build/feature/test-all.c                |  4 ++
> >  .../feature/test-disassembler-init-styled.c   | 13 +++++
> >  tools/include/tools/dis-asm-compat.h          | 55 +++++++++++++++++++
> >  tools/perf/Makefile.config                    |  8 +++
> >  tools/perf/util/annotate.c                    |  7 ++-
> >  11 files changed, 138 insertions(+), 19 deletions(-)
> >  create mode 100644 tools/build/feature/test-disassembler-init-styled.c
> >  create mode 100644 tools/include/tools/dis-asm-compat.h
> > 
> > -- 
> > 2.37.0.3.g30cc8d0f14
> 
> -- 
> 
> - Arnaldo

Re: [PATCH v3 0/8] tools: fix compilation failure caused by init_disassemble_info API changes

[Sedat Dilek]

On Mon, Aug 1, 2022 at 3:38 AM Andres Freund <andres@anarazel.de> wrote:
>
> binutils changed the signature of init_disassemble_info(), which now causes
> compilation failures for tools/{perf,bpf} on e.g. debian unstable. Relevant
> binutils commit:
> https://sourceware.org/git/?p=binutils-gdb.git;a=commit;h=60a3da00bd5407f07
>
> I first fixed this without introducing the compat header, as suggested by
> Quentin, but I thought the amount of repeated boilerplate was a bit too
> much. So instead I introduced a compat header to wrap the API changes. Even
> tools/bpf/bpftool/jit_disasm.c, which needs its own callbacks for json, imo
> looks nicer this way.
>
> I'm not regular contributor, so it very well might be my procedures are a
> bit off...
>
> I am not sure I added the right [number of] people to CC?
>
> WRT the feature test: Not sure what the point of the -DPACKAGE='"perf"' is,
> nor why tools/perf/Makefile.config sets some LDFLAGS/CFLAGS that are also
> in feature/Makefile and why -ldl isn't needed in the other places. But...
>
> V2:
> - split patches further, so that tools/bpf and tools/perf part are entirely
>   separate
> - included a bit more information about tests I did in commit messages
> - add a maybe_unused to fprintf_json_styled's style argument
>
> V3:
> - don't include dis-asm-compat.h when building without libbfd
>   (Ben Hutchings)
> - don't include compiler.h in dis-asm-compat.h, use (void) casts instead,
>   to avoid compiler.h include due to potential licensing conflict
> - dual-license dis-asm-compat.h, for better compatibility with the rest of
>   bpftool's code (suggested by Quentin Monnet)
> - don't display feature-disassembler-init-styled test
>   (suggested by Jiri Olsa)
> - don't display feature-disassembler-four-args test, I split this for the
>   different subsystems, but maybe that's overkill? (suggested by Jiri Olsa)
>

Hi Andres,

Just made a quick test & run with some custom patchset and LLVM-15 RC1:

[ REPRODUCER ]

LLVM_MVER="15"

##LLVM_TOOLCHAIN_PATH="/usr/lib/llvm-${LLVM_MVER}/bin"
LLVM_TOOLCHAIN_PATH="/opt/llvm/bin"
if [ -d ${LLVM_TOOLCHAIN_PATH} ]; then
   export PATH="${LLVM_TOOLCHAIN_PATH}:${PATH}"
fi

PYTHON_VER="3.10"
MAKE="make"
MAKE_OPTS="V=1 -j1 HOSTCC=clang-$LLVM_MVER HOSTLD=ld.lld
HOSTAR=llvm-ar CC=clang-$LLVM_MVER LD=ld.lld AR=llvm-ar
STRIP=llvm-strip"

echo "LLVM MVER ........ $LLVM_MVER"
echo "Path settings .... $PATH"
echo "Python version ... $PYTHON_VER"
echo "make line ........ $MAKE $MAKE_OPTS"

LANG=C LC_ALL=C make -C tools/perf clean 2>&1 | tee ../make-log_perf-clean.txt

LANG=C LC_ALL=C $MAKE $MAKE_OPTS -C tools/perf
PYTHON=python${PYTHON_VER} install-bin 2>&1 | tee
../make-log_perf-install_bin_python${PYTHON_VER}_llvm${LLVM_MVER}.txt

Looks good.

$ ~/bin/perf -vv
perf version 5.19.0
                dwarf: [ on  ]  # HAVE_DWARF_SUPPORT
   dwarf_getlocations: [ on  ]  # HAVE_DWARF_GETLOCATIONS_SUPPORT
                glibc: [ on  ]  # HAVE_GLIBC_SUPPORT
        syscall_table: [ on  ]  # HAVE_SYSCALL_TABLE_SUPPORT
               libbfd: [ on  ]  # HAVE_LIBBFD_SUPPORT
           debuginfod: [ OFF ]  # HAVE_DEBUGINFOD_SUPPORT
               libelf: [ on  ]  # HAVE_LIBELF_SUPPORT
              libnuma: [ on  ]  # HAVE_LIBNUMA_SUPPORT
numa_num_possible_cpus: [ on  ]  # HAVE_LIBNUMA_SUPPORT
              libperl: [ on  ]  # HAVE_LIBPERL_SUPPORT
            libpython: [ on  ]  # HAVE_LIBPYTHON_SUPPORT
             libslang: [ on  ]  # HAVE_SLANG_SUPPORT
            libcrypto: [ on  ]  # HAVE_LIBCRYPTO_SUPPORT
            libunwind: [ on  ]  # HAVE_LIBUNWIND_SUPPORT
   libdw-dwarf-unwind: [ on  ]  # HAVE_DWARF_SUPPORT
                 zlib: [ on  ]  # HAVE_ZLIB_SUPPORT
                 lzma: [ on  ]  # HAVE_LZMA_SUPPORT
            get_cpuid: [ on  ]  # HAVE_AUXTRACE_SUPPORT
                  bpf: [ on  ]  # HAVE_LIBBPF_SUPPORT
                  aio: [ on  ]  # HAVE_AIO_SUPPORT
                 zstd: [ on  ]  # HAVE_ZSTD_SUPPORT
              libpfm4: [ OFF ]  # HAVE_LIBPFM

[ Test on Intel Sandybridge CPU ]

$ echo 0 | sudo tee /proc/sys/kernel/kptr_restrict
/proc/sys/kernel/perf_event_paranoid
0

$ ~/bin/perf test 10 93 94 95
10: PMU events                                                      :
10.1: PMU event table sanity                                        : Ok
10.2: PMU event map aliases                                         : Ok
10.3: Parsing of PMU event table metrics                            : Ok
10.4: Parsing of PMU event table metrics with fake PMUs             : Ok
93: perf all metricgroups test                                      : Ok
94: perf all metrics test                                           : Ok
95: perf all PMU test                                               : Ok

Feel free to add my:

Tested-by: Sedat Dilek <sedat.dilek@gmail.com> # LLVM v15.0.0-rc1 (x86-64)

Regards,
-Sedat-

> Cc: Alexei Starovoitov <ast@kernel.org>
> Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
> Cc: Jiri Olsa <jolsa@kernel.org>
> Cc: Sedat Dilek <sedat.dilek@gmail.com>
> Cc: Quentin Monnet <quentin@isovalent.com>
> CC: Ben Hutchings <benh@debian.org>
> Cc: bpf@vger.kernel.org
> Cc: linux-kernel@vger.kernel.org
> Link: https://lore.kernel.org/lkml/20220622181918.ykrs5rsnmx3og4sv@alap3.anarazel.de
> Link: https://lore.kernel.org/lkml/CA+icZUVpr8ZeOKCj4zMMqbFT013KJz2T1csvXg+VSkdvJH1Ubw@mail.gmail.com
>
> Andres Freund (8):
>   tools build: Add feature test for init_disassemble_info API changes
>   tools build: Don't display disassembler-four-args feature test
>   tools include: add dis-asm-compat.h to handle version differences
>   tools perf: Fix compilation error with new binutils
>   tools bpf_jit_disasm: Fix compilation error with new binutils
>   tools bpf_jit_disasm: Don't display disassembler-four-args feature
>     test
>   tools bpftool: Fix compilation error with new binutils
>   tools bpftool: Don't display disassembler-four-args feature test
>
>  tools/bpf/Makefile                            |  7 ++-
>  tools/bpf/bpf_jit_disasm.c                    |  5 +-
>  tools/bpf/bpftool/Makefile                    |  8 ++-
>  tools/bpf/bpftool/jit_disasm.c                | 42 +++++++++++---
>  tools/build/Makefile.feature                  |  4 +-
>  tools/build/feature/Makefile                  |  4 ++
>  tools/build/feature/test-all.c                |  4 ++
>  .../feature/test-disassembler-init-styled.c   | 13 +++++
>  tools/include/tools/dis-asm-compat.h          | 55 +++++++++++++++++++
>  tools/perf/Makefile.config                    |  8 +++
>  tools/perf/util/annotate.c                    |  7 ++-
>  11 files changed, 138 insertions(+), 19 deletions(-)
>  create mode 100644 tools/build/feature/test-disassembler-init-styled.c
>  create mode 100644 tools/include/tools/dis-asm-compat.h
>
> --
> 2.37.0.3.g30cc8d0f14
>