Re: [PATCH v3 1/3] perf dso: Use libbfd to read build_id and .gnu_debuglink section

From: Remi Bernon
Date: Fri Sep 04 2020 - 03:57:43 EST


On 2020-09-03 18:51, Arnaldo Carvalho de Melo wrote:
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@xxxxxxxxxxxxxxx>
Cc: Alexander Shishkin <alexander.shishkin@xxxxxxxxxxxxxxx>
Cc: Arnaldo Carvalho de Melo <acme@xxxxxxxxxx>
Cc: Ingo Molnar <mingo@xxxxxxxxxx>
Cc: Jiri Olsa <jolsa@xxxxxxxxxx>
Cc: Mark Rutland <mark.rutland@xxxxxxx>
Cc: Namhyung Kim <namhyung@xxxxxxxxxx>
Cc: Peter Zijlstra <peterz@xxxxxxxxxxxxx>
Cc: Jacek Caban <jacek@xxxxxxxxxxxxxxx>
---

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@five ~]$

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.


Thanks! I may have missed the first changeset you mention -- IIUC the one adding the feature check -- but it sounds good to me nonetheless.

Should I do anything?
--
Rémi Bernon <rbernon@xxxxxxxxxxxxxxx>