Message-ID: <CAM9d7cgTrDEdAn=dv9ciRZfpMdYwdmDrAAvsYEYE=GssPS_aWw@mail.gmail.com>
Date: Wed, 31 Jul 2024 10:07:50 -0700
From: Namhyung Kim <namhyung@...nel.org>
To: Arnaldo Carvalho de Melo <acme@...nel.org>
Cc: Song Liu <song@...nel.org>, Adrian Hunter <adrian.hunter@...el.com>, 
	Ian Rogers <irogers@...gle.com>, Jiri Olsa <jolsa@...nel.org>, 
	Kan Liang <kan.liang@...ux.intel.com>, 
	Linux Kernel Mailing List <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 1/1] perf bpf: Move BPF disassembly routines to separate
 file to avoid clash with capstone bpf headers

On Wed, Jul 31, 2024 at 8:12 AM Arnaldo Carvalho de Melo
<acme@...nel.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@...el.com>
> Cc: Ian Rogers <irogers@...gle.com>
> Cc: Jiri Olsa <jolsa@...nel.org>
> Cc: Kan Liang <kan.liang@...ux.intel.com>
> Cc: Namhyung Kim <namhyung@...nel.org>
> Cc: Song Liu <song@...nel.org>
> Signed-off-by: Arnaldo Carvalho de Melo <acme@...hat.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
>

Powered by blists - more mailing lists