From: Namhyung Kim <namhyung@kernel.org> To: Jiri Slaby <jslaby@suse.cz> Cc: Jiri Olsa <jolsa@redhat.com>, Arnaldo Carvalho de Melo <acme@kernel.org>, linux-kernel <linux-kernel@vger.kernel.org>, Peter Zijlstra <peterz@infradead.org>, Ingo Molnar <mingo@redhat.com>, Mark Rutland <mark.rutland@arm.com>, Alexander Shishkin <alexander.shishkin@linux.intel.com>, linuxppc-dev@lists.ozlabs.org Subject: Re: [PATCH] perf tools: Resolve symbols against debug file first Date: Thu, 14 Jan 2021 13:54:06 +0900 [thread overview] Message-ID: <CAM9d7cgmUqLX+C1wDPe9qaxDh1tY4sVmLx2qZqey3CQSmZSo2Q@mail.gmail.com> (raw) In-Reply-To: <388a2e21-14ee-4609-84d0-c8824154c015@suse.cz> Hi both of Jiri, On Wed, Jan 13, 2021 at 8:43 PM Jiri Slaby <jslaby@suse.cz> wrote: > > On 13. 01. 21, 11:46, Jiri Olsa wrote: > > On Wed, Jan 13, 2021 at 09:01:28AM +0100, Jiri Slaby wrote: > >> With LTO, there are symbols like these: > >> /usr/lib/debug/usr/lib64/libantlr4-runtime.so.4.8-4.8-1.4.x86_64.debug > >> 10305: 0000000000955fa4 0 NOTYPE LOCAL DEFAULT 29 Predicate.cpp.2bc410e7 > >> > >> This comes from a runtime/debug split done by the standard way: > >> objcopy --only-keep-debug $runtime $debug > >> objcopy --add-gnu-debuglink=$debugfn -R .comment -R .GCC.command.line --strip-all $runtime > >> > >> perf currently cannot resolve such symbols (relicts of LTO), as section > >> 29 exists only in the debug file (29 is .debug_info). And perf resolves > >> symbols only against runtime file. This results in all symbols from such > >> a library being unresolved: > >> 0.38% main2 libantlr4-runtime.so.4.8 [.] 0x00000000000671e0 > >> > >> So try resolving against the debug file first. And only if it fails (the > >> section has NOBITS set), try runtime file. We can do this, as "objcopy > >> --only-keep-debug" per documentation preserves all sections, but clears > >> data of some of them (the runtime ones) and marks them as NOBITS. > >> > >> The correct result is now: > >> 0.38% main2 libantlr4-runtime.so.4.8 [.] antlr4::IntStream::~IntStream > >> > >> Note that these LTO symbols are properly skipped anyway as they belong > >> neither to *text* nor to *data* (is_label && !elf_sec__filter(&shdr, > >> secstrs) is true). > >> > >> Signed-off-by: Jiri Slaby <jslaby@suse.cz> > >> Cc: Peter Zijlstra <peterz@infradead.org> > >> Cc: Ingo Molnar <mingo@redhat.com> > >> Cc: Arnaldo Carvalho de Melo <acme@kernel.org> > >> Cc: Mark Rutland <mark.rutland@arm.com> > >> Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com> > >> Cc: Jiri Olsa <jolsa@redhat.com> > >> Cc: Namhyung Kim <namhyung@kernel.org> > >> --- > >> tools/perf/util/symbol-elf.c | 10 +++++++++- > >> 1 file changed, 9 insertions(+), 1 deletion(-) > >> > >> diff --git a/tools/perf/util/symbol-elf.c b/tools/perf/util/symbol-elf.c > >> index f3577f7d72fe..a31b716fa61c 100644 > >> --- a/tools/perf/util/symbol-elf.c > >> +++ b/tools/perf/util/symbol-elf.c > >> @@ -1226,12 +1226,20 @@ int dso__load_sym(struct dso *dso, struct map *map, struct symsrc *syms_ss, > >> if (sym.st_shndx == SHN_ABS) > >> continue; > >> > >> - sec = elf_getscn(runtime_ss->elf, sym.st_shndx); > >> + sec = elf_getscn(syms_ss->elf, sym.st_shndx); > >> if (!sec) > >> goto out_elf_end; > > > > we iterate symbols from syms_ss, so the fix seems to be correct > > to call elf_getscn on syms_ss, not on runtime_ss as we do now > > > > I'd think this worked only when runtime_ss == syms_ss > > No, because the headers are copied 1:1 from runtime_ss to syms_ss. And > runtime_ss is then stripped, so only .debug* sections are removed there. > (And syms_ss's are set as NOBITS.) > > We iterated .debug* sections in syms_ss and used runtime_ss section > _headers_ only to adjust symbols (sometimes). That worked. It seems PPC has an opd section only in the runtime_ss and that's why we use it for section headers. > > >> gelf_getshdr(sec, &shdr); > >> > >> + if (shdr.sh_type == SHT_NOBITS) { > >> + sec = elf_getscn(runtime_ss->elf, sym.st_shndx); > >> + if (!sec) > >> + goto out_elf_end; > >> + > >> + gelf_getshdr(sec, &shdr); > >> + } > > > > is that fallback necessary? the symbol is from syms_ss > > Provided the above, we don't need the section data here, only headers, > so the NOBITS test is superfluous and the fallback shouldn't be needed. > Let me test it. We need to talk to PPC folks like I said. Or maybe we can change the default ss depending on the arch. Thanks, Namhyung
WARNING: multiple messages have this Message-ID (diff)
From: Namhyung Kim <namhyung@kernel.org> To: Jiri Slaby <jslaby@suse.cz> Cc: Mark Rutland <mark.rutland@arm.com>, Peter Zijlstra <peterz@infradead.org>, linuxppc-dev@lists.ozlabs.org, linux-kernel <linux-kernel@vger.kernel.org>, Arnaldo Carvalho de Melo <acme@kernel.org>, Alexander Shishkin <alexander.shishkin@linux.intel.com>, Ingo Molnar <mingo@redhat.com>, Jiri Olsa <jolsa@redhat.com> Subject: Re: [PATCH] perf tools: Resolve symbols against debug file first Date: Thu, 14 Jan 2021 13:54:06 +0900 [thread overview] Message-ID: <CAM9d7cgmUqLX+C1wDPe9qaxDh1tY4sVmLx2qZqey3CQSmZSo2Q@mail.gmail.com> (raw) In-Reply-To: <388a2e21-14ee-4609-84d0-c8824154c015@suse.cz> Hi both of Jiri, On Wed, Jan 13, 2021 at 8:43 PM Jiri Slaby <jslaby@suse.cz> wrote: > > On 13. 01. 21, 11:46, Jiri Olsa wrote: > > On Wed, Jan 13, 2021 at 09:01:28AM +0100, Jiri Slaby wrote: > >> With LTO, there are symbols like these: > >> /usr/lib/debug/usr/lib64/libantlr4-runtime.so.4.8-4.8-1.4.x86_64.debug > >> 10305: 0000000000955fa4 0 NOTYPE LOCAL DEFAULT 29 Predicate.cpp.2bc410e7 > >> > >> This comes from a runtime/debug split done by the standard way: > >> objcopy --only-keep-debug $runtime $debug > >> objcopy --add-gnu-debuglink=$debugfn -R .comment -R .GCC.command.line --strip-all $runtime > >> > >> perf currently cannot resolve such symbols (relicts of LTO), as section > >> 29 exists only in the debug file (29 is .debug_info). And perf resolves > >> symbols only against runtime file. This results in all symbols from such > >> a library being unresolved: > >> 0.38% main2 libantlr4-runtime.so.4.8 [.] 0x00000000000671e0 > >> > >> So try resolving against the debug file first. And only if it fails (the > >> section has NOBITS set), try runtime file. We can do this, as "objcopy > >> --only-keep-debug" per documentation preserves all sections, but clears > >> data of some of them (the runtime ones) and marks them as NOBITS. > >> > >> The correct result is now: > >> 0.38% main2 libantlr4-runtime.so.4.8 [.] antlr4::IntStream::~IntStream > >> > >> Note that these LTO symbols are properly skipped anyway as they belong > >> neither to *text* nor to *data* (is_label && !elf_sec__filter(&shdr, > >> secstrs) is true). > >> > >> Signed-off-by: Jiri Slaby <jslaby@suse.cz> > >> Cc: Peter Zijlstra <peterz@infradead.org> > >> Cc: Ingo Molnar <mingo@redhat.com> > >> Cc: Arnaldo Carvalho de Melo <acme@kernel.org> > >> Cc: Mark Rutland <mark.rutland@arm.com> > >> Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com> > >> Cc: Jiri Olsa <jolsa@redhat.com> > >> Cc: Namhyung Kim <namhyung@kernel.org> > >> --- > >> tools/perf/util/symbol-elf.c | 10 +++++++++- > >> 1 file changed, 9 insertions(+), 1 deletion(-) > >> > >> diff --git a/tools/perf/util/symbol-elf.c b/tools/perf/util/symbol-elf.c > >> index f3577f7d72fe..a31b716fa61c 100644 > >> --- a/tools/perf/util/symbol-elf.c > >> +++ b/tools/perf/util/symbol-elf.c > >> @@ -1226,12 +1226,20 @@ int dso__load_sym(struct dso *dso, struct map *map, struct symsrc *syms_ss, > >> if (sym.st_shndx == SHN_ABS) > >> continue; > >> > >> - sec = elf_getscn(runtime_ss->elf, sym.st_shndx); > >> + sec = elf_getscn(syms_ss->elf, sym.st_shndx); > >> if (!sec) > >> goto out_elf_end; > > > > we iterate symbols from syms_ss, so the fix seems to be correct > > to call elf_getscn on syms_ss, not on runtime_ss as we do now > > > > I'd think this worked only when runtime_ss == syms_ss > > No, because the headers are copied 1:1 from runtime_ss to syms_ss. And > runtime_ss is then stripped, so only .debug* sections are removed there. > (And syms_ss's are set as NOBITS.) > > We iterated .debug* sections in syms_ss and used runtime_ss section > _headers_ only to adjust symbols (sometimes). That worked. It seems PPC has an opd section only in the runtime_ss and that's why we use it for section headers. > > >> gelf_getshdr(sec, &shdr); > >> > >> + if (shdr.sh_type == SHT_NOBITS) { > >> + sec = elf_getscn(runtime_ss->elf, sym.st_shndx); > >> + if (!sec) > >> + goto out_elf_end; > >> + > >> + gelf_getshdr(sec, &shdr); > >> + } > > > > is that fallback necessary? the symbol is from syms_ss > > Provided the above, we don't need the section data here, only headers, > so the NOBITS test is superfluous and the fallback shouldn't be needed. > Let me test it. We need to talk to PPC folks like I said. Or maybe we can change the default ss depending on the arch. Thanks, Namhyung
next prev parent reply other threads:[~2021-01-14 4:55 UTC|newest] Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top 2021-01-13 8:01 [PATCH] perf tools: Resolve symbols against debug file first Jiri Slaby 2021-01-13 10:46 ` Jiri Olsa 2021-01-13 11:43 ` Jiri Slaby 2021-01-14 4:54 ` Namhyung Kim [this message] 2021-01-14 4:54 ` Namhyung Kim 2021-01-14 11:17 ` Michael Ellerman 2021-01-14 11:17 ` Michael Ellerman 2021-01-15 7:37 ` Namhyung Kim 2021-01-15 7:37 ` Namhyung Kim 2021-01-28 10:43 ` Jiri Slaby 2021-02-03 4:28 ` Namhyung Kim 2021-02-03 19:32 ` Jiri Olsa
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=CAM9d7cgmUqLX+C1wDPe9qaxDh1tY4sVmLx2qZqey3CQSmZSo2Q@mail.gmail.com \ --to=namhyung@kernel.org \ --cc=acme@kernel.org \ --cc=alexander.shishkin@linux.intel.com \ --cc=jolsa@redhat.com \ --cc=jslaby@suse.cz \ --cc=linux-kernel@vger.kernel.org \ --cc=linuxppc-dev@lists.ozlabs.org \ --cc=mark.rutland@arm.com \ --cc=mingo@redhat.com \ --cc=peterz@infradead.org \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.