Re: [PATCH] perf tools: Resolve symbols against debug file first

3 messages, 2 authors, 2021-01-15 · open the first message on its own page

Re: [PATCH] perf tools: Resolve symbols against debug file first

From: Namhyung Kim <namhyung@kernel.org>
Date: 2021-01-14 04:55:24

Hi both of Jiri,

On Wed, Jan 13, 2021 at 8:43 PM Jiri Slaby [off-list ref] wrote:
On 13. 01. 21, 11:46, Jiri Olsa wrote:
quoted
On Wed, Jan 13, 2021 at 09:01:28AM +0100, Jiri Slaby wrote:
quoted
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 <redacted>
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 <redacted>
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.
quoted
quoted
             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

Re: [PATCH] perf tools: Resolve symbols against debug file first

From: Michael Ellerman <mpe@ellerman.id.au>
Date: 2021-01-14 11:18:29

Namhyung Kim [off-list ref] writes:
On Wed, Jan 13, 2021 at 8:43 PM Jiri Slaby [off-list ref] wrote:
quoted
On 13. 01. 21, 11:46, Jiri Olsa wrote:
quoted
On Wed, Jan 13, 2021 at 09:01:28AM +0100, Jiri Slaby wrote:
quoted
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
...
quoted
quoted
quoted
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.
At least on my system (Ubuntu 20.04.1) I see .opd in the debug file with
NOBITS set:

$ readelf -e vmlinux.debug | grep opd
  [37] .opd              NOBITS           c000000001c1f548  01202e14


But possibly that's not the case with older toolchains?

cheers

Re: [PATCH] perf tools: Resolve symbols against debug file first

From: Namhyung Kim <namhyung@kernel.org>
Date: 2021-01-15 07:38:44

Hello,

On Thu, Jan 14, 2021 at 8:17 PM Michael Ellerman [off-list ref] wrote:
Namhyung Kim [off-list ref] writes:
quoted
On Wed, Jan 13, 2021 at 8:43 PM Jiri Slaby [off-list ref] wrote:
quoted
On 13. 01. 21, 11:46, Jiri Olsa wrote:
quoted
On Wed, Jan 13, 2021 at 09:01:28AM +0100, Jiri Slaby wrote:
quoted
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
...
quoted
quoted
quoted
quoted
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.
At least on my system (Ubuntu 20.04.1) I see .opd in the debug file with
NOBITS set:

$ readelf -e vmlinux.debug | grep opd
  [37] .opd              NOBITS           c000000001c1f548  01202e14


But possibly that's not the case with older toolchains?
I was referring to this commit:

commit 261360b6e90a782f0a63d8f61a67683c376c88cf
Author: Cody P Schafer [off-list ref]
Date:   Fri Aug 10 15:23:01 2012 -0700

    perf symbols: Convert dso__load_syms to take 2 symsrc's

    To properly handle platforms with an opd section, both a runtime image
    (which contains the opd section but possibly lacks symbols) and a symbol
    image (which probably lacks an opd section but has symbols).

    The next patch ("perf symbol: use both runtime and debug images")
    adjusts the callsite in dso__load() to take advantage of being able to
    pass both runtime & debug images.

    Assumptions made here:

     - The opd section, if it exists in the runtime image, has headers in
       both the runtime image and the debug/syms image.

     - The index of the opd section (again, only if it exists in the runtime
       image) is the same in both the runtime and debug/symbols image.

    Both of these are true on RHEL, but it is unclear how accurate they are
    in general (on platforms with function descriptors in opd sections).

    Signed-off-by: Cody P Schafer [off-list ref]


Thanks,
Namhyung
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help