perf bpf: Move BPF disassembly routines to separate file to avoid clash with capstone bpf headers

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

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 <[email protected]>

Cc Ian Rogers <[email protected]>

Cc Jiri Olsa <[email protected]>

Cc Kan Liang <[email protected]>

Cc Namhyung Kim <[email protected]>

Cc Song Liu <[email protected]>

Signed-off-by Arnaldo Carvalho de Melo <[email protected]>

---
 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

tools/perf/util/Build +0 -0

index 0f18fe81ef0b2a74..b24360c04aaea424 100644

tools/perf/util/Build +0 -0

 perf-util-y += env.o
 perf-util-y += event.o
 perf-util-y += evlist.o

tools/perf/util/disasm.c +1 -186

index 410e52cd9cfd0306..85fb0cfedf94554b 100644
@@ -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>
 

tools/perf/util/disasm_bpf.c new file +193 -0

new file mode 100644
index 0000000000000000..14994515beb341ce
@@ -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;
+}

tools/perf/util/disasm_bpf.h new file +12 -1

new file mode 100644
index 0000000000000000..2ecb19545388b114
@@ -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

On Wed, Jul 31, 2024 at 8:12 AM Arnaldo Carvalho de Melo <[email protected]> wrote:

Quoted 1 line

+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.

Quoted 3 lines · Namhyung Kim

On Wed, Jul 31, 2024 at 8:12 AM Arnaldo Carvalho de Melo <[email protected]> wrote:

[snip]

Quoted 4 lines

Quoted 1 line

+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.

Quoted 35 lines · 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 <[email protected]>

Cc Ian Rogers <[email protected]>

Cc Jiri Olsa <[email protected]>

Cc Kan Liang <[email protected]>

Cc Namhyung Kim <[email protected]>

Cc Song Liu <[email protected]>

Signed-off-by Arnaldo Carvalho de Melo <[email protected]>

---
 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

tools/perf/util/Build perf-util-y += copyfile.o · line 16

Quoted 6 lines

+++ b/tools/perf/util/Build

tools/perf/util/Build +1 -0

@@ -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.

Quoted 1 line · Namhyung Kim

On Wed, Jul 31, 2024 at 8:12 AM Arnaldo Carvalho de Melo

Quoted 2 lines

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.

Quoted 3 lines · Arnaldo Carvalho de Melo

Quoted 1 line · Namhyung Kim

On Wed, Jul 31, 2024 at 8:12 AM Arnaldo Carvalho de Melo

[snip]

Quoted 7 lines

Quoted 2 lines

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.

Quoted 2 lines

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.