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