[PATCH 1/1] perf bpf: Move BPF disassembly routines to separate file to avoid clash with capstone bpf headers

[PATCH 1/1] perf bpf: Move BPF disassembly routines to separate file to avoid clash with capstone bpf headers

[Arnaldo Carvalho de Melo]

There is a clash of the libbpf and capstone libraries, that ends up
with:

  In file included from /usr/include/capstone/capstone.h:325,
                   from util/disasm.c:1513:
  /usr/include/capstone/bpf.h:94:14: error: ‘bpf_insn’ defined as wrong kind of tag
     94 | typedef enum bpf_insn {

So far we're just trying to avoid this by not having both headers
included in the same .c or .h file, do it one more time by moving the
BPF diassembly routines from util/disasm.c to util/disasm_bpf.c.

This is only being hit when building with BUILD_NONDISTRO=1, i.e.
building with binutils-devel, that isn't the in the default build due to
a licencing clash. We need to reimplement what is now isolated in
util/disasm_bpf.c using some other library to have BPF annotation
feature that now only is available with BUILD_NONDISTRO=1.

Cc: Adrian Hunter <adrian.hunter@intel.com>
Cc: Ian Rogers <irogers@google.com>
Cc: Jiri Olsa <jolsa@kernel.org>
Cc: Kan Liang <kan.liang@linux.intel.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Song Liu <song@kernel.org>
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/Build        |   1 +
 tools/perf/util/disasm.c     | 187 +--------------------------------
 tools/perf/util/disasm_bpf.c | 193 +++++++++++++++++++++++++++++++++++
 tools/perf/util/disasm_bpf.h |  12 +++
 4 files changed, 207 insertions(+), 186 deletions(-)
 create mode 100644 tools/perf/util/disasm_bpf.c
 create mode 100644 tools/perf/util/disasm_bpf.h

diff --git a/tools/perf/util/Build b/tools/perf/util/Build
index 0f18fe81ef0b2a74..b24360c04aaea424 100644
--- a/tools/perf/util/Build
+++ b/tools/perf/util/Build
@@ -13,6 +13,7 @@ perf-util-y += copyfile.o
 perf-util-y += ctype.o
 perf-util-y += db-export.o
 perf-util-y += disasm.o
+perf-util-y += disasm_bpf.o
 perf-util-y += env.o
 perf-util-y += event.o
 perf-util-y += evlist.o
diff --git a/tools/perf/util/disasm.c b/tools/perf/util/disasm.c
index 410e52cd9cfd0306..85fb0cfedf94554b 100644
--- a/tools/perf/util/disasm.c
+++ b/tools/perf/util/disasm.c
@@ -16,6 +16,7 @@
 #include "build-id.h"
 #include "debug.h"
 #include "disasm.h"
+#include "disasm_bpf.h"
 #include "dso.h"
 #include "env.h"
 #include "evsel.h"
@@ -1323,192 +1324,6 @@ static int dso__disassemble_filename(struct dso *dso, char *filename, size_t fil
 	return 0;
 }
 
-#if defined(HAVE_LIBBFD_SUPPORT) && defined(HAVE_LIBBPF_SUPPORT)
-#define PACKAGE "perf"
-#include <bfd.h>
-#include <dis-asm.h>
-#include <bpf/bpf.h>
-#include <bpf/btf.h>
-#include <bpf/libbpf.h>
-#include <linux/btf.h>
-#include <tools/dis-asm-compat.h>
-
-#include "bpf-event.h"
-#include "bpf-utils.h"
-
-static int symbol__disassemble_bpf(struct symbol *sym,
-				   struct annotate_args *args)
-{
-	struct annotation *notes = symbol__annotation(sym);
-	struct bpf_prog_linfo *prog_linfo = NULL;
-	struct bpf_prog_info_node *info_node;
-	int len = sym->end - sym->start;
-	disassembler_ftype disassemble;
-	struct map *map = args->ms.map;
-	struct perf_bpil *info_linear;
-	struct disassemble_info info;
-	struct dso *dso = map__dso(map);
-	int pc = 0, count, sub_id;
-	struct btf *btf = NULL;
-	char tpath[PATH_MAX];
-	size_t buf_size;
-	int nr_skip = 0;
-	char *buf;
-	bfd *bfdf;
-	int ret;
-	FILE *s;
-
-	if (dso__binary_type(dso) != DSO_BINARY_TYPE__BPF_PROG_INFO)
-		return SYMBOL_ANNOTATE_ERRNO__BPF_INVALID_FILE;
-
-	pr_debug("%s: handling sym %s addr %" PRIx64 " len %" PRIx64 "\n", __func__,
-		  sym->name, sym->start, sym->end - sym->start);
-
-	memset(tpath, 0, sizeof(tpath));
-	perf_exe(tpath, sizeof(tpath));
-
-	bfdf = bfd_openr(tpath, NULL);
-	if (bfdf == NULL)
-		abort();
-
-	if (!bfd_check_format(bfdf, bfd_object))
-		abort();
-
-	s = open_memstream(&buf, &buf_size);
-	if (!s) {
-		ret = errno;
-		goto out;
-	}
-	init_disassemble_info_compat(&info, s,
-				     (fprintf_ftype) fprintf,
-				     fprintf_styled);
-	info.arch = bfd_get_arch(bfdf);
-	info.mach = bfd_get_mach(bfdf);
-
-	info_node = perf_env__find_bpf_prog_info(dso__bpf_prog(dso)->env,
-						 dso__bpf_prog(dso)->id);
-	if (!info_node) {
-		ret = SYMBOL_ANNOTATE_ERRNO__BPF_MISSING_BTF;
-		goto out;
-	}
-	info_linear = info_node->info_linear;
-	sub_id = dso__bpf_prog(dso)->sub_id;
-
-	info.buffer = (void *)(uintptr_t)(info_linear->info.jited_prog_insns);
-	info.buffer_length = info_linear->info.jited_prog_len;
-
-	if (info_linear->info.nr_line_info)
-		prog_linfo = bpf_prog_linfo__new(&info_linear->info);
-
-	if (info_linear->info.btf_id) {
-		struct btf_node *node;
-
-		node = perf_env__find_btf(dso__bpf_prog(dso)->env,
-					  info_linear->info.btf_id);
-		if (node)
-			btf = btf__new((__u8 *)(node->data),
-				       node->data_size);
-	}
-
-	disassemble_init_for_target(&info);
-
-#ifdef DISASM_FOUR_ARGS_SIGNATURE
-	disassemble = disassembler(info.arch,
-				   bfd_big_endian(bfdf),
-				   info.mach,
-				   bfdf);
-#else
-	disassemble = disassembler(bfdf);
-#endif
-	if (disassemble == NULL)
-		abort();
-
-	fflush(s);
-	do {
-		const struct bpf_line_info *linfo = NULL;
-		struct disasm_line *dl;
-		size_t prev_buf_size;
-		const char *srcline;
-		u64 addr;
-
-		addr = pc + ((u64 *)(uintptr_t)(info_linear->info.jited_ksyms))[sub_id];
-		count = disassemble(pc, &info);
-
-		if (prog_linfo)
-			linfo = bpf_prog_linfo__lfind_addr_func(prog_linfo,
-								addr, sub_id,
-								nr_skip);
-
-		if (linfo && btf) {
-			srcline = btf__name_by_offset(btf, linfo->line_off);
-			nr_skip++;
-		} else
-			srcline = NULL;
-
-		fprintf(s, "\n");
-		prev_buf_size = buf_size;
-		fflush(s);
-
-		if (!annotate_opts.hide_src_code && srcline) {
-			args->offset = -1;
-			args->line = strdup(srcline);
-			args->line_nr = 0;
-			args->fileloc = NULL;
-			args->ms.sym  = sym;
-			dl = disasm_line__new(args);
-			if (dl) {
-				annotation_line__add(&dl->al,
-						     &notes->src->source);
-			}
-		}
-
-		args->offset = pc;
-		args->line = buf + prev_buf_size;
-		args->line_nr = 0;
-		args->fileloc = NULL;
-		args->ms.sym  = sym;
-		dl = disasm_line__new(args);
-		if (dl)
-			annotation_line__add(&dl->al, &notes->src->source);
-
-		pc += count;
-	} while (count > 0 && pc < len);
-
-	ret = 0;
-out:
-	free(prog_linfo);
-	btf__free(btf);
-	fclose(s);
-	bfd_close(bfdf);
-	return ret;
-}
-#else // defined(HAVE_LIBBFD_SUPPORT) && defined(HAVE_LIBBPF_SUPPORT)
-static int symbol__disassemble_bpf(struct symbol *sym __maybe_unused,
-				   struct annotate_args *args __maybe_unused)
-{
-	return SYMBOL_ANNOTATE_ERRNO__NO_LIBOPCODES_FOR_BPF;
-}
-#endif // defined(HAVE_LIBBFD_SUPPORT) && defined(HAVE_LIBBPF_SUPPORT)
-
-static int
-symbol__disassemble_bpf_image(struct symbol *sym,
-			      struct annotate_args *args)
-{
-	struct annotation *notes = symbol__annotation(sym);
-	struct disasm_line *dl;
-
-	args->offset = -1;
-	args->line = strdup("to be implemented");
-	args->line_nr = 0;
-	args->fileloc = NULL;
-	dl = disasm_line__new(args);
-	if (dl)
-		annotation_line__add(&dl->al, &notes->src->source);
-
-	zfree(&args->line);
-	return 0;
-}
-
 #ifdef HAVE_LIBCAPSTONE_SUPPORT
 #include <capstone/capstone.h>
 
diff --git a/tools/perf/util/disasm_bpf.c b/tools/perf/util/disasm_bpf.c
new file mode 100644
index 0000000000000000..14994515beb341ce
--- /dev/null
+++ b/tools/perf/util/disasm_bpf.c
@@ -0,0 +1,193 @@
+// SPDX-License-Identifier: GPL-2.0-only
+
+#include "util/annotate.h"
+#include "util/disasm_bpf.h"
+#include "util/symbol.h"
+
+#if defined(HAVE_LIBBFD_SUPPORT) && defined(HAVE_LIBBPF_SUPPORT)
+#define PACKAGE "perf"
+#include <bfd.h>
+#include <bpf/bpf.h>
+#include <bpf/btf.h>
+#include <bpf/libbpf.h>
+#include <dis-asm.h>
+#include <errno.h>
+#include <linux/btf.h>
+#include <tools/dis-asm-compat.h>
+
+#include "util/bpf-event.h"
+#include "util/bpf-utils.h"
+#include "util/debug.h"
+#include "util/dso.h"
+#include "util/map.h"
+#include "util/env.h"
+#include "util/util.h"
+
+int symbol__disassemble_bpf(struct symbol *sym, struct annotate_args *args)
+{
+	struct annotation *notes = symbol__annotation(sym);
+	struct bpf_prog_linfo *prog_linfo = NULL;
+	struct bpf_prog_info_node *info_node;
+	int len = sym->end - sym->start;
+	disassembler_ftype disassemble;
+	struct map *map = args->ms.map;
+	struct perf_bpil *info_linear;
+	struct disassemble_info info;
+	struct dso *dso = map__dso(map);
+	int pc = 0, count, sub_id;
+	struct btf *btf = NULL;
+	char tpath[PATH_MAX];
+	size_t buf_size;
+	int nr_skip = 0;
+	char *buf;
+	bfd *bfdf;
+	int ret;
+	FILE *s;
+
+	if (dso__binary_type(dso) != DSO_BINARY_TYPE__BPF_PROG_INFO)
+		return SYMBOL_ANNOTATE_ERRNO__BPF_INVALID_FILE;
+
+	pr_debug("%s: handling sym %s addr %" PRIx64 " len %" PRIx64 "\n", __func__,
+		  sym->name, sym->start, sym->end - sym->start);
+
+	memset(tpath, 0, sizeof(tpath));
+	perf_exe(tpath, sizeof(tpath));
+
+	bfdf = bfd_openr(tpath, NULL);
+	if (bfdf == NULL)
+		abort();
+
+	if (!bfd_check_format(bfdf, bfd_object))
+		abort();
+
+	s = open_memstream(&buf, &buf_size);
+	if (!s) {
+		ret = errno;
+		goto out;
+	}
+	init_disassemble_info_compat(&info, s,
+				     (fprintf_ftype) fprintf,
+				     fprintf_styled);
+	info.arch = bfd_get_arch(bfdf);
+	info.mach = bfd_get_mach(bfdf);
+
+	info_node = perf_env__find_bpf_prog_info(dso__bpf_prog(dso)->env,
+						 dso__bpf_prog(dso)->id);
+	if (!info_node) {
+		ret = SYMBOL_ANNOTATE_ERRNO__BPF_MISSING_BTF;
+		goto out;
+	}
+	info_linear = info_node->info_linear;
+	sub_id = dso__bpf_prog(dso)->sub_id;
+
+	info.buffer = (void *)(uintptr_t)(info_linear->info.jited_prog_insns);
+	info.buffer_length = info_linear->info.jited_prog_len;
+
+	if (info_linear->info.nr_line_info)
+		prog_linfo = bpf_prog_linfo__new(&info_linear->info);
+
+	if (info_linear->info.btf_id) {
+		struct btf_node *node;
+
+		node = perf_env__find_btf(dso__bpf_prog(dso)->env,
+					  info_linear->info.btf_id);
+		if (node)
+			btf = btf__new((__u8 *)(node->data),
+				       node->data_size);
+	}
+
+	disassemble_init_for_target(&info);
+
+#ifdef DISASM_FOUR_ARGS_SIGNATURE
+	disassemble = disassembler(info.arch,
+				   bfd_big_endian(bfdf),
+				   info.mach,
+				   bfdf);
+#else
+	disassemble = disassembler(bfdf);
+#endif
+	if (disassemble == NULL)
+		abort();
+
+	fflush(s);
+	do {
+		const struct bpf_line_info *linfo = NULL;
+		struct disasm_line *dl;
+		size_t prev_buf_size;
+		const char *srcline;
+		u64 addr;
+
+		addr = pc + ((u64 *)(uintptr_t)(info_linear->info.jited_ksyms))[sub_id];
+		count = disassemble(pc, &info);
+
+		if (prog_linfo)
+			linfo = bpf_prog_linfo__lfind_addr_func(prog_linfo,
+								addr, sub_id,
+								nr_skip);
+
+		if (linfo && btf) {
+			srcline = btf__name_by_offset(btf, linfo->line_off);
+			nr_skip++;
+		} else
+			srcline = NULL;
+
+		fprintf(s, "\n");
+		prev_buf_size = buf_size;
+		fflush(s);
+
+		if (!annotate_opts.hide_src_code && srcline) {
+			args->offset = -1;
+			args->line = strdup(srcline);
+			args->line_nr = 0;
+			args->fileloc = NULL;
+			args->ms.sym  = sym;
+			dl = disasm_line__new(args);
+			if (dl) {
+				annotation_line__add(&dl->al,
+						     &notes->src->source);
+			}
+		}
+
+		args->offset = pc;
+		args->line = buf + prev_buf_size;
+		args->line_nr = 0;
+		args->fileloc = NULL;
+		args->ms.sym  = sym;
+		dl = disasm_line__new(args);
+		if (dl)
+			annotation_line__add(&dl->al, &notes->src->source);
+
+		pc += count;
+	} while (count > 0 && pc < len);
+
+	ret = 0;
+out:
+	free(prog_linfo);
+	btf__free(btf);
+	fclose(s);
+	bfd_close(bfdf);
+	return ret;
+}
+#else // defined(HAVE_LIBBFD_SUPPORT) && defined(HAVE_LIBBPF_SUPPORT)
+int symbol__disassemble_bpf(struct symbol *sym __maybe_unused, struct annotate_args *args __maybe_unused)
+{
+	return SYMBOL_ANNOTATE_ERRNO__NO_LIBOPCODES_FOR_BPF;
+}
+#endif // defined(HAVE_LIBBFD_SUPPORT) && defined(HAVE_LIBBPF_SUPPORT)
+
+int symbol__disassemble_bpf_image(struct symbol *sym, struct annotate_args *args)
+{
+	struct annotation *notes = symbol__annotation(sym);
+	struct disasm_line *dl;
+
+	args->offset = -1;
+	args->line = strdup("to be implemented");
+	args->line_nr = 0;
+	args->fileloc = NULL;
+	dl = disasm_line__new(args);
+	if (dl)
+		annotation_line__add(&dl->al, &notes->src->source);
+
+	zfree(&args->line);
+	return 0;
+}
diff --git a/tools/perf/util/disasm_bpf.h b/tools/perf/util/disasm_bpf.h
new file mode 100644
index 0000000000000000..2ecb19545388b114
--- /dev/null
+++ b/tools/perf/util/disasm_bpf.h
@@ -0,0 +1,12 @@
+// SPDX-License-Identifier: GPL-2.0-only
+
+#ifndef __PERF_DISASM_BPF_H
+#define __PERF_DISASM_BPF_H
+
+struct symbol;
+struct annotate_args;
+
+int symbol__disassemble_bpf(struct symbol *sym, struct annotate_args *args);
+int symbol__disassemble_bpf_image(struct symbol *sym, struct annotate_args *args);
+
+#endif /* __PERF_DISASM_BPF_H */
-- 
2.45.2

Re: [PATCH 1/1] perf bpf: Move BPF disassembly routines to separate file to avoid clash with capstone bpf headers

[Arnaldo Carvalho de Melo]

On Wed, Jul 31, 2024 at 12:12:10PM -0300, Arnaldo Carvalho de Melo wrote:
> There is a clash of the libbpf and capstone libraries, that ends up
> with:

So this is needed for building without BUILD_NONDISTRO=1, sry.


diff --git a/tools/perf/util/disasm_bpf.c b/tools/perf/util/disasm_bpf.c
index 14994515beb341ce..1fee71c79b624cc8 100644
--- a/tools/perf/util/disasm_bpf.c
+++ b/tools/perf/util/disasm_bpf.c
@@ -3,6 +3,8 @@
 #include "util/annotate.h"
 #include "util/disasm_bpf.h"
 #include "util/symbol.h"
+#include <linux/zalloc.h>
+#include <string.h>
 
 #if defined(HAVE_LIBBFD_SUPPORT) && defined(HAVE_LIBBPF_SUPPORT)
 #define PACKAGE "perf"

Re: [PATCH 1/1] perf bpf: Move BPF disassembly routines to separate file to avoid clash with capstone bpf headers

[Namhyung Kim]

On Wed, Jul 31, 2024 at 8:12 AM Arnaldo Carvalho de Melo
<acme@kernel.org> wrote:
>
> There is a clash of the libbpf and capstone libraries, that ends up
> with:
>
>   In file included from /usr/include/capstone/capstone.h:325,
>                    from util/disasm.c:1513:
>   /usr/include/capstone/bpf.h:94:14: error: ‘bpf_insn’ defined as wrong kind of tag
>      94 | typedef enum bpf_insn {
>
> So far we're just trying to avoid this by not having both headers
> included in the same .c or .h file, do it one more time by moving the
> BPF diassembly routines from util/disasm.c to util/disasm_bpf.c.
>
> This is only being hit when building with BUILD_NONDISTRO=1, i.e.
> building with binutils-devel, that isn't the in the default build due to
> a licencing clash. We need to reimplement what is now isolated in
> util/disasm_bpf.c using some other library to have BPF annotation
> feature that now only is available with BUILD_NONDISTRO=1.
>
> Cc: Adrian Hunter <adrian.hunter@intel.com>
> Cc: Ian Rogers <irogers@google.com>
> Cc: Jiri Olsa <jolsa@kernel.org>
> Cc: Kan Liang <kan.liang@linux.intel.com>
> Cc: Namhyung Kim <namhyung@kernel.org>
> Cc: Song Liu <song@kernel.org>
> Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
> ---
>  tools/perf/util/Build        |   1 +
>  tools/perf/util/disasm.c     | 187 +--------------------------------
>  tools/perf/util/disasm_bpf.c | 193 +++++++++++++++++++++++++++++++++++
>  tools/perf/util/disasm_bpf.h |  12 +++
>  4 files changed, 207 insertions(+), 186 deletions(-)
>  create mode 100644 tools/perf/util/disasm_bpf.c
>  create mode 100644 tools/perf/util/disasm_bpf.h
>
> diff --git a/tools/perf/util/Build b/tools/perf/util/Build
> index 0f18fe81ef0b2a74..b24360c04aaea424 100644
> --- a/tools/perf/util/Build
> +++ b/tools/perf/util/Build
> @@ -13,6 +13,7 @@ perf-util-y += copyfile.o
>  perf-util-y += ctype.o
>  perf-util-y += db-export.o
>  perf-util-y += disasm.o
> +perf-util-y += disasm_bpf.o

I think this can be gated by LIBBFD and LIBBPF config, but not sure
it can express the both requirements easily.

thanks,
Namhyung


>  perf-util-y += env.o
>  perf-util-y += event.o
>  perf-util-y += evlist.o
> diff --git a/tools/perf/util/disasm.c b/tools/perf/util/disasm.c
> index 410e52cd9cfd0306..85fb0cfedf94554b 100644
> --- a/tools/perf/util/disasm.c
> +++ b/tools/perf/util/disasm.c
> @@ -16,6 +16,7 @@
>  #include "build-id.h"
>  #include "debug.h"
>  #include "disasm.h"
> +#include "disasm_bpf.h"
>  #include "dso.h"
>  #include "env.h"
>  #include "evsel.h"
> @@ -1323,192 +1324,6 @@ static int dso__disassemble_filename(struct dso *dso, char *filename, size_t fil
>         return 0;
>  }
>
> -#if defined(HAVE_LIBBFD_SUPPORT) && defined(HAVE_LIBBPF_SUPPORT)
> -#define PACKAGE "perf"
> -#include <bfd.h>
> -#include <dis-asm.h>
> -#include <bpf/bpf.h>
> -#include <bpf/btf.h>
> -#include <bpf/libbpf.h>
> -#include <linux/btf.h>
> -#include <tools/dis-asm-compat.h>
> -
> -#include "bpf-event.h"
> -#include "bpf-utils.h"
> -
> -static int symbol__disassemble_bpf(struct symbol *sym,
> -                                  struct annotate_args *args)
> -{
> -       struct annotation *notes = symbol__annotation(sym);
> -       struct bpf_prog_linfo *prog_linfo = NULL;
> -       struct bpf_prog_info_node *info_node;
> -       int len = sym->end - sym->start;
> -       disassembler_ftype disassemble;
> -       struct map *map = args->ms.map;
> -       struct perf_bpil *info_linear;
> -       struct disassemble_info info;
> -       struct dso *dso = map__dso(map);
> -       int pc = 0, count, sub_id;
> -       struct btf *btf = NULL;
> -       char tpath[PATH_MAX];
> -       size_t buf_size;
> -       int nr_skip = 0;
> -       char *buf;
> -       bfd *bfdf;
> -       int ret;
> -       FILE *s;
> -
> -       if (dso__binary_type(dso) != DSO_BINARY_TYPE__BPF_PROG_INFO)
> -               return SYMBOL_ANNOTATE_ERRNO__BPF_INVALID_FILE;
> -
> -       pr_debug("%s: handling sym %s addr %" PRIx64 " len %" PRIx64 "\n", __func__,
> -                 sym->name, sym->start, sym->end - sym->start);
> -
> -       memset(tpath, 0, sizeof(tpath));
> -       perf_exe(tpath, sizeof(tpath));
> -
> -       bfdf = bfd_openr(tpath, NULL);
> -       if (bfdf == NULL)
> -               abort();
> -
> -       if (!bfd_check_format(bfdf, bfd_object))
> -               abort();
> -
> -       s = open_memstream(&buf, &buf_size);
> -       if (!s) {
> -               ret = errno;
> -               goto out;
> -       }
> -       init_disassemble_info_compat(&info, s,
> -                                    (fprintf_ftype) fprintf,
> -                                    fprintf_styled);
> -       info.arch = bfd_get_arch(bfdf);
> -       info.mach = bfd_get_mach(bfdf);
> -
> -       info_node = perf_env__find_bpf_prog_info(dso__bpf_prog(dso)->env,
> -                                                dso__bpf_prog(dso)->id);
> -       if (!info_node) {
> -               ret = SYMBOL_ANNOTATE_ERRNO__BPF_MISSING_BTF;
> -               goto out;
> -       }
> -       info_linear = info_node->info_linear;
> -       sub_id = dso__bpf_prog(dso)->sub_id;
> -
> -       info.buffer = (void *)(uintptr_t)(info_linear->info.jited_prog_insns);
> -       info.buffer_length = info_linear->info.jited_prog_len;
> -
> -       if (info_linear->info.nr_line_info)
> -               prog_linfo = bpf_prog_linfo__new(&info_linear->info);
> -
> -       if (info_linear->info.btf_id) {
> -               struct btf_node *node;
> -
> -               node = perf_env__find_btf(dso__bpf_prog(dso)->env,
> -                                         info_linear->info.btf_id);
> -               if (node)
> -                       btf = btf__new((__u8 *)(node->data),
> -                                      node->data_size);
> -       }
> -
> -       disassemble_init_for_target(&info);
> -
> -#ifdef DISASM_FOUR_ARGS_SIGNATURE
> -       disassemble = disassembler(info.arch,
> -                                  bfd_big_endian(bfdf),
> -                                  info.mach,
> -                                  bfdf);
> -#else
> -       disassemble = disassembler(bfdf);
> -#endif
> -       if (disassemble == NULL)
> -               abort();
> -
> -       fflush(s);
> -       do {
> -               const struct bpf_line_info *linfo = NULL;
> -               struct disasm_line *dl;
> -               size_t prev_buf_size;
> -               const char *srcline;
> -               u64 addr;
> -
> -               addr = pc + ((u64 *)(uintptr_t)(info_linear->info.jited_ksyms))[sub_id];
> -               count = disassemble(pc, &info);
> -
> -               if (prog_linfo)
> -                       linfo = bpf_prog_linfo__lfind_addr_func(prog_linfo,
> -                                                               addr, sub_id,
> -                                                               nr_skip);
> -
> -               if (linfo && btf) {
> -                       srcline = btf__name_by_offset(btf, linfo->line_off);
> -                       nr_skip++;
> -               } else
> -                       srcline = NULL;
> -
> -               fprintf(s, "\n");
> -               prev_buf_size = buf_size;
> -               fflush(s);
> -
> -               if (!annotate_opts.hide_src_code && srcline) {
> -                       args->offset = -1;
> -                       args->line = strdup(srcline);
> -                       args->line_nr = 0;
> -                       args->fileloc = NULL;
> -                       args->ms.sym  = sym;
> -                       dl = disasm_line__new(args);
> -                       if (dl) {
> -                               annotation_line__add(&dl->al,
> -                                                    &notes->src->source);
> -                       }
> -               }
> -
> -               args->offset = pc;
> -               args->line = buf + prev_buf_size;
> -               args->line_nr = 0;
> -               args->fileloc = NULL;
> -               args->ms.sym  = sym;
> -               dl = disasm_line__new(args);
> -               if (dl)
> -                       annotation_line__add(&dl->al, &notes->src->source);
> -
> -               pc += count;
> -       } while (count > 0 && pc < len);
> -
> -       ret = 0;
> -out:
> -       free(prog_linfo);
> -       btf__free(btf);
> -       fclose(s);
> -       bfd_close(bfdf);
> -       return ret;
> -}
> -#else // defined(HAVE_LIBBFD_SUPPORT) && defined(HAVE_LIBBPF_SUPPORT)
> -static int symbol__disassemble_bpf(struct symbol *sym __maybe_unused,
> -                                  struct annotate_args *args __maybe_unused)
> -{
> -       return SYMBOL_ANNOTATE_ERRNO__NO_LIBOPCODES_FOR_BPF;
> -}
> -#endif // defined(HAVE_LIBBFD_SUPPORT) && defined(HAVE_LIBBPF_SUPPORT)
> -
> -static int
> -symbol__disassemble_bpf_image(struct symbol *sym,
> -                             struct annotate_args *args)
> -{
> -       struct annotation *notes = symbol__annotation(sym);
> -       struct disasm_line *dl;
> -
> -       args->offset = -1;
> -       args->line = strdup("to be implemented");
> -       args->line_nr = 0;
> -       args->fileloc = NULL;
> -       dl = disasm_line__new(args);
> -       if (dl)
> -               annotation_line__add(&dl->al, &notes->src->source);
> -
> -       zfree(&args->line);
> -       return 0;
> -}
> -
>  #ifdef HAVE_LIBCAPSTONE_SUPPORT
>  #include <capstone/capstone.h>
>
> diff --git a/tools/perf/util/disasm_bpf.c b/tools/perf/util/disasm_bpf.c
> new file mode 100644
> index 0000000000000000..14994515beb341ce
> --- /dev/null
> +++ b/tools/perf/util/disasm_bpf.c
> @@ -0,0 +1,193 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +
> +#include "util/annotate.h"
> +#include "util/disasm_bpf.h"
> +#include "util/symbol.h"
> +
> +#if defined(HAVE_LIBBFD_SUPPORT) && defined(HAVE_LIBBPF_SUPPORT)
> +#define PACKAGE "perf"
> +#include <bfd.h>
> +#include <bpf/bpf.h>
> +#include <bpf/btf.h>
> +#include <bpf/libbpf.h>
> +#include <dis-asm.h>
> +#include <errno.h>
> +#include <linux/btf.h>
> +#include <tools/dis-asm-compat.h>
> +
> +#include "util/bpf-event.h"
> +#include "util/bpf-utils.h"
> +#include "util/debug.h"
> +#include "util/dso.h"
> +#include "util/map.h"
> +#include "util/env.h"
> +#include "util/util.h"
> +
> +int symbol__disassemble_bpf(struct symbol *sym, struct annotate_args *args)
> +{
> +       struct annotation *notes = symbol__annotation(sym);
> +       struct bpf_prog_linfo *prog_linfo = NULL;
> +       struct bpf_prog_info_node *info_node;
> +       int len = sym->end - sym->start;
> +       disassembler_ftype disassemble;
> +       struct map *map = args->ms.map;
> +       struct perf_bpil *info_linear;
> +       struct disassemble_info info;
> +       struct dso *dso = map__dso(map);
> +       int pc = 0, count, sub_id;
> +       struct btf *btf = NULL;
> +       char tpath[PATH_MAX];
> +       size_t buf_size;
> +       int nr_skip = 0;
> +       char *buf;
> +       bfd *bfdf;
> +       int ret;
> +       FILE *s;
> +
> +       if (dso__binary_type(dso) != DSO_BINARY_TYPE__BPF_PROG_INFO)
> +               return SYMBOL_ANNOTATE_ERRNO__BPF_INVALID_FILE;
> +
> +       pr_debug("%s: handling sym %s addr %" PRIx64 " len %" PRIx64 "\n", __func__,
> +                 sym->name, sym->start, sym->end - sym->start);
> +
> +       memset(tpath, 0, sizeof(tpath));
> +       perf_exe(tpath, sizeof(tpath));
> +
> +       bfdf = bfd_openr(tpath, NULL);
> +       if (bfdf == NULL)
> +               abort();
> +
> +       if (!bfd_check_format(bfdf, bfd_object))
> +               abort();
> +
> +       s = open_memstream(&buf, &buf_size);
> +       if (!s) {
> +               ret = errno;
> +               goto out;
> +       }
> +       init_disassemble_info_compat(&info, s,
> +                                    (fprintf_ftype) fprintf,
> +                                    fprintf_styled);
> +       info.arch = bfd_get_arch(bfdf);
> +       info.mach = bfd_get_mach(bfdf);
> +
> +       info_node = perf_env__find_bpf_prog_info(dso__bpf_prog(dso)->env,
> +                                                dso__bpf_prog(dso)->id);
> +       if (!info_node) {
> +               ret = SYMBOL_ANNOTATE_ERRNO__BPF_MISSING_BTF;
> +               goto out;
> +       }
> +       info_linear = info_node->info_linear;
> +       sub_id = dso__bpf_prog(dso)->sub_id;
> +
> +       info.buffer = (void *)(uintptr_t)(info_linear->info.jited_prog_insns);
> +       info.buffer_length = info_linear->info.jited_prog_len;
> +
> +       if (info_linear->info.nr_line_info)
> +               prog_linfo = bpf_prog_linfo__new(&info_linear->info);
> +
> +       if (info_linear->info.btf_id) {
> +               struct btf_node *node;
> +
> +               node = perf_env__find_btf(dso__bpf_prog(dso)->env,
> +                                         info_linear->info.btf_id);
> +               if (node)
> +                       btf = btf__new((__u8 *)(node->data),
> +                                      node->data_size);
> +       }
> +
> +       disassemble_init_for_target(&info);
> +
> +#ifdef DISASM_FOUR_ARGS_SIGNATURE
> +       disassemble = disassembler(info.arch,
> +                                  bfd_big_endian(bfdf),
> +                                  info.mach,
> +                                  bfdf);
> +#else
> +       disassemble = disassembler(bfdf);
> +#endif
> +       if (disassemble == NULL)
> +               abort();
> +
> +       fflush(s);
> +       do {
> +               const struct bpf_line_info *linfo = NULL;
> +               struct disasm_line *dl;
> +               size_t prev_buf_size;
> +               const char *srcline;
> +               u64 addr;
> +
> +               addr = pc + ((u64 *)(uintptr_t)(info_linear->info.jited_ksyms))[sub_id];
> +               count = disassemble(pc, &info);
> +
> +               if (prog_linfo)
> +                       linfo = bpf_prog_linfo__lfind_addr_func(prog_linfo,
> +                                                               addr, sub_id,
> +                                                               nr_skip);
> +
> +               if (linfo && btf) {
> +                       srcline = btf__name_by_offset(btf, linfo->line_off);
> +                       nr_skip++;
> +               } else
> +                       srcline = NULL;
> +
> +               fprintf(s, "\n");
> +               prev_buf_size = buf_size;
> +               fflush(s);
> +
> +               if (!annotate_opts.hide_src_code && srcline) {
> +                       args->offset = -1;
> +                       args->line = strdup(srcline);
> +                       args->line_nr = 0;
> +                       args->fileloc = NULL;
> +                       args->ms.sym  = sym;
> +                       dl = disasm_line__new(args);
> +                       if (dl) {
> +                               annotation_line__add(&dl->al,
> +                                                    &notes->src->source);
> +                       }
> +               }
> +
> +               args->offset = pc;
> +               args->line = buf + prev_buf_size;
> +               args->line_nr = 0;
> +               args->fileloc = NULL;
> +               args->ms.sym  = sym;
> +               dl = disasm_line__new(args);
> +               if (dl)
> +                       annotation_line__add(&dl->al, &notes->src->source);
> +
> +               pc += count;
> +       } while (count > 0 && pc < len);
> +
> +       ret = 0;
> +out:
> +       free(prog_linfo);
> +       btf__free(btf);
> +       fclose(s);
> +       bfd_close(bfdf);
> +       return ret;
> +}
> +#else // defined(HAVE_LIBBFD_SUPPORT) && defined(HAVE_LIBBPF_SUPPORT)
> +int symbol__disassemble_bpf(struct symbol *sym __maybe_unused, struct annotate_args *args __maybe_unused)
> +{
> +       return SYMBOL_ANNOTATE_ERRNO__NO_LIBOPCODES_FOR_BPF;
> +}
> +#endif // defined(HAVE_LIBBFD_SUPPORT) && defined(HAVE_LIBBPF_SUPPORT)
> +
> +int symbol__disassemble_bpf_image(struct symbol *sym, struct annotate_args *args)
> +{
> +       struct annotation *notes = symbol__annotation(sym);
> +       struct disasm_line *dl;
> +
> +       args->offset = -1;
> +       args->line = strdup("to be implemented");
> +       args->line_nr = 0;
> +       args->fileloc = NULL;
> +       dl = disasm_line__new(args);
> +       if (dl)
> +               annotation_line__add(&dl->al, &notes->src->source);
> +
> +       zfree(&args->line);
> +       return 0;
> +}
> diff --git a/tools/perf/util/disasm_bpf.h b/tools/perf/util/disasm_bpf.h
> new file mode 100644
> index 0000000000000000..2ecb19545388b114
> --- /dev/null
> +++ b/tools/perf/util/disasm_bpf.h
> @@ -0,0 +1,12 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +
> +#ifndef __PERF_DISASM_BPF_H
> +#define __PERF_DISASM_BPF_H
> +
> +struct symbol;
> +struct annotate_args;
> +
> +int symbol__disassemble_bpf(struct symbol *sym, struct annotate_args *args);
> +int symbol__disassemble_bpf_image(struct symbol *sym, struct annotate_args *args);
> +
> +#endif /* __PERF_DISASM_BPF_H */
> --
> 2.45.2
>

Re: [PATCH 1/1] perf bpf: Move BPF disassembly routines to separate file to avoid clash with capstone bpf headers

[Ian Rogers]

On Wed, Jul 31, 2024 at 10:08 AM Namhyung Kim <namhyung@kernel.org> wrote:
>
> On Wed, Jul 31, 2024 at 8:12 AM Arnaldo Carvalho de Melo
> <acme@kernel.org> wrote:
[snip]
> > +perf-util-y += disasm_bpf.o
>
> I think this can be gated by LIBBFD and LIBBPF config, but not sure
> it can express the both requirements easily.

Should we gate things on libbfd? Given we can't distribute a binary
linked against it, I support deleting all libbfd support. Fixes like
this show the pain in carrying it.

Thanks,
Ian

Re: [PATCH 1/1] perf bpf: Move BPF disassembly routines to separate file to avoid clash with capstone bpf headers

[Arnaldo Carvalho de Melo]

On Wed, Jul 31, 2024 at 10:35:12AM -0700, Ian Rogers wrote:
> On Wed, Jul 31, 2024 at 10:08 AM Namhyung Kim <namhyung@kernel.org> wrote:
> >
> > On Wed, Jul 31, 2024 at 8:12 AM Arnaldo Carvalho de Melo
> > <acme@kernel.org> wrote:
> [snip]
> > > +perf-util-y += disasm_bpf.o
> >
> > I think this can be gated by LIBBFD and LIBBPF config, but not sure
> > it can express the both requirements easily.
> 
> Should we gate things on libbfd? Given we can't distribute a binary
> linked against it, I support deleting all libbfd support. Fixes like
> this show the pain in carrying it.

I thought about it, but the problem at hand was that library A clashed
with library B for a namespace, so I fixed just that problem.

I agree that as soon as we reimplement the features that now are only
available with libbfd we should remove that code, now it is even more
isolated.

- Arnaldo

Re: [PATCH 1/1] perf bpf: Move BPF disassembly routines to separate file to avoid clash with capstone bpf headers

[Arnaldo Carvalho de Melo]

On Wed, Jul 31, 2024 at 10:07:50AM -0700, Namhyung Kim wrote:
> On Wed, Jul 31, 2024 at 8:12 AM Arnaldo Carvalho de Melo
> <acme@kernel.org> wrote:
> >
> > There is a clash of the libbpf and capstone libraries, that ends up
> > with:
> >
> >   In file included from /usr/include/capstone/capstone.h:325,
> >                    from util/disasm.c:1513:
> >   /usr/include/capstone/bpf.h:94:14: error: ‘bpf_insn’ defined as wrong kind of tag
> >      94 | typedef enum bpf_insn {
> >
> > So far we're just trying to avoid this by not having both headers
> > included in the same .c or .h file, do it one more time by moving the
> > BPF diassembly routines from util/disasm.c to util/disasm_bpf.c.
> >
> > This is only being hit when building with BUILD_NONDISTRO=1, i.e.
> > building with binutils-devel, that isn't the in the default build due to
> > a licencing clash. We need to reimplement what is now isolated in
> > util/disasm_bpf.c using some other library to have BPF annotation
> > feature that now only is available with BUILD_NONDISTRO=1.
> >
> > Cc: Adrian Hunter <adrian.hunter@intel.com>
> > Cc: Ian Rogers <irogers@google.com>
> > Cc: Jiri Olsa <jolsa@kernel.org>
> > Cc: Kan Liang <kan.liang@linux.intel.com>
> > Cc: Namhyung Kim <namhyung@kernel.org>
> > Cc: Song Liu <song@kernel.org>
> > Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
> > ---
> >  tools/perf/util/Build        |   1 +
> >  tools/perf/util/disasm.c     | 187 +--------------------------------
> >  tools/perf/util/disasm_bpf.c | 193 +++++++++++++++++++++++++++++++++++
> >  tools/perf/util/disasm_bpf.h |  12 +++
> >  4 files changed, 207 insertions(+), 186 deletions(-)
> >  create mode 100644 tools/perf/util/disasm_bpf.c
> >  create mode 100644 tools/perf/util/disasm_bpf.h
> >
> > +++ b/tools/perf/util/Build
> > @@ -13,6 +13,7 @@ perf-util-y += copyfile.o
> >  perf-util-y += ctype.o
> >  perf-util-y += db-export.o
> >  perf-util-y += disasm.o
> > +perf-util-y += disasm_bpf.o
> 
> I think this can be gated by LIBBFD and LIBBPF config, but not sure
> it can express the both requirements easily.

It is possible, but as discussed with Ian in another message, the
cheapest way to do it was, I think, like I did, in the end it is just a
stub returning an error when those libraries are not linked with perf.

When we manage to implement the disassembly of BPF using something other
than libbfd we can spend more time in this area, I think.

- Arnaldo

Re: [PATCH 1/1] perf bpf: Move BPF disassembly routines to separate file to avoid clash with capstone bpf headers

[Ian Rogers]

On Wed, Jul 31, 2024 at 11:51 AM Arnaldo Carvalho de Melo
<acme@kernel.org> wrote:
>
> On Wed, Jul 31, 2024 at 10:07:50AM -0700, Namhyung Kim wrote:
> > On Wed, Jul 31, 2024 at 8:12 AM Arnaldo Carvalho de Melo
[snip]
> > I think this can be gated by LIBBFD and LIBBPF config, but not sure
> > it can express the both requirements easily.
>
> It is possible, but as discussed with Ian in another message, the
> cheapest way to do it was, I think, like I did, in the end it is just a
> stub returning an error when those libraries are not linked with perf.
>
> When we manage to implement the disassembly of BPF using something other
> than libbfd we can spend more time in this area, I think.

Not following this, can you show an example perf command. There is a
BPF disassembler already in the kernel tree:
https://git.kernel.org/pub/scm/linux/kernel/git/perf/perf-tools-next.git/tree/kernel/bpf/disasm.c?h=perf-tools-next
and in use in bpftool that we use in the perf build.

Thanks,
Ian

Re: [PATCH 1/1] perf bpf: Move BPF disassembly routines to separate file to avoid clash with capstone bpf headers

[Arnaldo Carvalho de Melo]

On Thu, Aug 01, 2024 at 08:18:23AM -0700, Ian Rogers wrote:
> On Wed, Jul 31, 2024 at 11:51 AM Arnaldo Carvalho de Melo
> <acme@kernel.org> wrote:
> >
> > On Wed, Jul 31, 2024 at 10:07:50AM -0700, Namhyung Kim wrote:
> > > On Wed, Jul 31, 2024 at 8:12 AM Arnaldo Carvalho de Melo
> [snip]
> > > I think this can be gated by LIBBFD and LIBBPF config, but not sure
> > > it can express the both requirements easily.
> >
> > It is possible, but as discussed with Ian in another message, the
> > cheapest way to do it was, I think, like I did, in the end it is just a
> > stub returning an error when those libraries are not linked with perf.
> >
> > When we manage to implement the disassembly of BPF using something other
> > than libbfd we can spend more time in this area, I think.
> 
> Not following this, can you show an example perf command. There is a
> BPF disassembler already in the kernel tree:
> https://git.kernel.org/pub/scm/linux/kernel/git/perf/perf-tools-next.git/tree/kernel/bpf/disasm.c?h=perf-tools-next
> and in use in bpftool that we use in the perf build.

Well, looking at the files in this patch shows it, here, more precisely:


tools/perf/util/disasm_bpf.c

#else // defined(HAVE_LIBBFD_SUPPORT) && defined(HAVE_LIBBPF_SUPPORT)
int symbol__disassemble_bpf(struct symbol *sym __maybe_unused, struct annotate_args *args __maybe_unused)
{
        return SYMBOL_ANNOTATE_ERRNO__NO_LIBOPCODES_FOR_BPF;
}
#endif // defined(HAVE_LIBBFD_SUPPORT) && defined(HAVE_LIBBPF_SUPPORT)

- Arnaldo