linux-kernel - Re: [PATCH v3 1/3] perf dso: Use libbfd to read build_id and .gnu

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20200903002519.GA3487700@kernel.org>
Date:   Wed, 2 Sep 2020 21:25:19 -0300
From:   Arnaldo Carvalho de Melo <acme@...nel.org>
To:     Remi Bernon <rbernon@...eweavers.com>
Cc:     linux-kernel@...r.kernel.org,
        Alexander Shishkin <alexander.shishkin@...ux.intel.com>,
        Ingo Molnar <mingo@...hat.com>, Jiri Olsa <jolsa@...hat.com>,
        Mark Rutland <mark.rutland@....com>,
        Namhyung Kim <namhyung@...nel.org>,
        Peter Zijlstra <peterz@...radead.org>,
        Jacek Caban <jacek@...eweavers.com>
Subject: Re: [PATCH v3 1/3] perf dso: Use libbfd to read build_id and
 .gnu_debuglink section

Em Fri, Aug 21, 2020 at 06:52:36PM +0200, Remi Bernon escreveu:
> Wine generates PE binaries for most of its modules and perf is unable
> to parse these files to get build_id or .gnu_debuglink section.
> 
> Using libbfd when available, instead of libelf, makes it possible to
> resolve debug file location regardless of the dso binary format.
> 
> Signed-off-by: Remi Bernon <rbernon@...eweavers.com>
> Cc: Alexander Shishkin <alexander.shishkin@...ux.intel.com>
> Cc: Arnaldo Carvalho de Melo <acme@...nel.org>
> Cc: Ingo Molnar <mingo@...hat.com>
> Cc: Jiri Olsa <jolsa@...hat.com>
> Cc: Mark Rutland <mark.rutland@....com>
> Cc: Namhyung Kim <namhyung@...nel.org>
> Cc: Peter Zijlstra <peterz@...radead.org>
> Cc: Jacek Caban <jacek@...eweavers.com>
> ---
> 
> v3: Rebase and small changes to PATCH 2/3 and and PATCH 3/3.
> 
>  tools/perf/util/symbol-elf.c | 80 ++++++++++++++++++++++++++++++++++--
>  1 file changed, 77 insertions(+), 3 deletions(-)
> 
> diff --git a/tools/perf/util/symbol-elf.c b/tools/perf/util/symbol-elf.c
> index 8cc4b0059fb0..f7432c4a4154 100644
> --- a/tools/perf/util/symbol-elf.c
> +++ b/tools/perf/util/symbol-elf.c
> @@ -50,6 +50,10 @@ typedef Elf64_Nhdr GElf_Nhdr;
>  #define DMGL_ANSI        (1 << 1)       /* Include const, volatile, etc */
>  #endif
>  
> +#ifdef HAVE_LIBBFD_SUPPORT

So, the feature test should also test for the buildid struct field, see
below:

> +#define PACKAGE 'perf'
> +#include <bfd.h>
> +#else
>  #ifdef HAVE_CPLUS_DEMANGLE_SUPPORT
>  extern char *cplus_demangle(const char *, int);
>  
> @@ -65,9 +69,7 @@ static inline char *bfd_demangle(void __maybe_unused *v,
>  {
>  	return NULL;
>  }
> -#else
> -#define PACKAGE 'perf'
> -#include <bfd.h>
> +#endif
>  #endif
>  #endif
>  
> @@ -530,6 +532,36 @@ static int elf_read_build_id(Elf *elf, void *bf, size_t size)
>  	return err;
>  }
>  
> +#ifdef HAVE_LIBBFD_SUPPORT
> +
> +int filename__read_build_id(const char *filename, void *bf, size_t size)
> +{
> +	int err = -1;
> +	bfd *abfd;
> +
> +	abfd = bfd_openr(filename, NULL);
> +	if (!abfd)
> +		return -1;
> +
> +	if (!bfd_check_format(abfd, bfd_object)) {
> +		pr_debug2("%s: cannot read %s bfd file.\n", __func__, filename);
> +		goto out_close;
> +	}
> +
> +	if (!abfd->build_id || abfd->build_id->size > size)
> +		goto out_close;

amazonlinux:1, centos:6, debian:8, mageia:5, oraclelinux:6, ubuntu:14.04
fail, its all old stuff, but adding a reference to abfd->build_id to the
feature test that ends up defining HAVE_LIBBFD_SUPPORT will solve that,
I'll do it tomorrow morning if you don't beat me to it.

util/symbol-elf.c: In function 'filename__read_build_id':
util/symbol-elf.c:551:11: error: 'bfd {aka struct bfd}' has no member named 'build_id'
  if (!abfd->build_id || abfd->build_id->size > size)
           ^~
util/symbol-elf.c:551:29: error: 'bfd {aka struct bfd}' has no member named 'build_id'
  if (!abfd->build_id || abfd->build_id->size > size)
                             ^~
util/symbol-elf.c:554:17: error: 'bfd {aka struct bfd}' has no member named 'build_id'
  memcpy(bf, abfd->build_id->data, abfd->build_id->size);
                 ^~
util/symbol-elf.c:554:39: error: 'bfd {aka struct bfd}' has no member named 'build_id'
  memcpy(bf, abfd->build_id->data, abfd->build_id->size);
                                       ^~
util/symbol-elf.c:555:18: error: 'bfd {aka struct bfd}' has no member named 'build_id'
  memset(bf + abfd->build_id->size, 0, size - abfd->build_id->size);
                  ^~
util/symbol-elf.c:555:50: error: 'bfd {aka struct bfd}' has no member named 'build_id'
  memset(bf + abfd->build_id->size, 0, size - abfd->build_id->size);
                                                  ^~
util/symbol-elf.c:556:12: error: 'bfd {aka struct bfd}' has no member named 'build_id'
  err = abfd->build_id->size;
            ^~
  CC       /tmp/build/perf/util/cap.o
make[4]: *** [/tmp/build/perf/util/symbol-elf.o] Error 1
make[4]: *** Waiting for unfinished jobs....
  LD       /tmp/build/perf/util/scripting-engines/perf-in.o
  LD       /tmp/build/perf/util/intel-pt-decoder/perf-in.o
make[3]: *** [util] Error 2
make[2]: *** [/tmp/build/perf/perf-in.o] Error 2
make[2]: *** Waiting for unfinished jobs....
make[1]: *** [sub-make] Error 2
make: *** [all] Error 2
make: Leaving directory `/git/linux/tools/perf'
+ exit 1
[perfbuilder@...e ~]$