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 for Android: free password hash cracker in your pocket
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20200903165132.GA3495158@kernel.org>
Date:   Thu, 3 Sep 2020 13:51:32 -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 Wed, Sep 02, 2020 at 09:25:19PM -0300, Arnaldo Carvalho de Melo escreveu:
> 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 ~]$

So, I have the cset at the end of this message in front of your series +
the following patch applied to your patch, the debuglink part seems ok
and can continue depending on just libbfd, having or not abfd->buildid.

- Arnaldo

diff --git a/tools/perf/util/symbol-elf.c b/tools/perf/util/symbol-elf.c
index f7432c4a4154a149..94a156df22d5a8d3 100644
--- a/tools/perf/util/symbol-elf.c
+++ b/tools/perf/util/symbol-elf.c
@@ -532,7 +532,7 @@ static int elf_read_build_id(Elf *elf, void *bf, size_t size)
 	return err;
 }
 
-#ifdef HAVE_LIBBFD_SUPPORT
+#ifdef HAVE_LIBBFD_BUILDID_SUPPORT
 
 int filename__read_build_id(const char *filename, void *bf, size_t size)
 {
@@ -560,7 +560,7 @@ int filename__read_build_id(const char *filename, void *bf, size_t size)
 	return err;
 }
 
-#else
+#else // HAVE_LIBBFD_BUILDID_SUPPORT
 
 int filename__read_build_id(const char *filename, void *bf, size_t size)
 {
@@ -589,7 +589,7 @@ int filename__read_build_id(const char *filename, void *bf, size_t size)
 	return err;
 }
 
-#endif
+#endif // HAVE_LIBBFD_BUILDID_SUPPORT
 
 int sysfs__read_build_id(const char *filename, void *build_id, size_t size)
 {

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ