| 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
| ||
|
Message-ID: <76cc6da5-bf1e-4de3-7b79-b9759dc6e5b9@codeweavers.com>
Date: Fri, 4 Sep 2020 09:57:33 +0200
From: Remi Bernon <rbernon@...eweavers.com>
To: Arnaldo Carvalho de Melo <acme@...nel.org>
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
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@...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.
>
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@...eweavers.com>
Powered by blists - more mailing lists