Script 'mail_helper' called by obssrc Hello community, here is the log from the commit of package perf for openSUSE:Factory checked in at 2021-03-02 12:36:13 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ Comparing /work/SRC/openSUSE:Factory/perf (Old) and /work/SRC/openSUSE:Factory/.perf.new.2378 (New) ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
Package is "perf" Tue Mar 2 12:36:13 2021 rev:57 rq:876132 version:MACRO Changes: -------- --- /work/SRC/openSUSE:Factory/perf/perf.changes 2020-11-23 10:38:11.585866746 +0100 +++ /work/SRC/openSUSE:Factory/.perf.new.2378/perf.changes 2021-03-02 14:20:38.333549922 +0100 @@ -1,0 +2,9 @@ +Tue Mar 2 06:05:56 UTC 2021 - Jiri Slaby <jsl...@suse.cz> + +- add: + * 0001-perf-annotate-Fix-jump-parsing-for-C-code.patch + (bsc#1182888) + * 0001-perf-symbols-Resolve-symbols-against-debug-file-firs.patch + (bsc#1180610) + +------------------------------------------------------------------- New: ---- 0001-perf-annotate-Fix-jump-parsing-for-C-code.patch 0001-perf-symbols-Resolve-symbols-against-debug-file-firs.patch ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ Other differences: ------------------ ++++++ perf.spec ++++++ --- /var/tmp/diff_new_pack.OG5kkm/_old 2021-03-02 14:20:38.741550309 +0100 +++ /var/tmp/diff_new_pack.OG5kkm/_new 2021-03-02 14:20:38.745550313 +0100 @@ -1,7 +1,7 @@ # # spec file for package perf # -# Copyright (c) 2020 SUSE LLC +# Copyright (c) 2021 SUSE LLC # # All modifications and additions to the file contributed by third parties # remain the property of their copyright owners, unless otherwise agreed @@ -36,6 +36,8 @@ License: GPL-2.0-only Group: Development/Tools/Debuggers URL: https://perf.wiki.kernel.org/ +Patch0: 0001-perf-annotate-Fix-jump-parsing-for-C-code.patch +Patch1: 0001-perf-symbols-Resolve-symbols-against-debug-file-firs.patch BuildRequires: OpenCSD-devel BuildRequires: audit-devel %ifnarch %{arm} ++++++ 0001-perf-annotate-Fix-jump-parsing-for-C-code.patch ++++++ From: =?UTF-8?q?Martin=20Li=C5=A1ka?= <mli...@suse.cz> Date: Thu, 11 Feb 2021 13:37:55 +0100 Subject: perf annotate: Fix jump parsing for C++ code. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Git-commit: 1f0e6edcd968ff19211245f7da6039e983aa51e5 Patch-mainline: 5.12-rc1 References: bsc#1182888 Considering the following testcase: int foo(int a, int b) { for (unsigned i = 0; i < 1000000000; i++) a += b; return a; } int main() { foo (3, 4); return 0; } 'perf annotate' displays: 86.52 ???40055e: ??? ja 40056c <foo(int, int)+0x26> 13.37 ???400560: mov -0x18(%rbp),%eax ???400563: add %eax,-0x14(%rbp) ???400566: addl $0x1,-0x4(%rbp) 0.11 ???40056a: ??? jmp 400557 <foo(int, int)+0x11> ???40056c: mov -0x14(%rbp),%eax ???40056f: pop %rbp and the 'ja 40056c' does not link to the location in the function. It's caused by fact that comma is wrongly parsed, it's part of function signature. With my patch I see: 86.52 ??? ?????????ja 26 13.37 ??? ??? mov -0x18(%rbp),%eax ??? ??? add %eax,-0x14(%rbp) ??? ??? addl $0x1,-0x4(%rbp) 0.11 ??? ?????? jmp 11 ???26:?????????mov -0x14(%rbp),%eax and 'o' output prints: 86.52 ???4005????????? ??? ja 40056c <foo(int, int)+0x26> 13.37 ???4005???0: mov -0x18(%rbp),%eax ???4005???3: add %eax,-0x14(%rbp) ???4005???6: addl $0x1,-0x4(%rbp) 0.11 ???4005???a: ??? jmp 400557 <foo(int, int)+0x11> ???4005????????? mov -0x14(%rbp),%eax On the contrary, compiling the very same file with gcc -x c, the parsing is fine because function arguments are not displayed: jmp 400543 <foo+0x1d> Committer testing: Before: $ cat cpp_args_annotate.c int foo(int a, int b) { for (unsigned i = 0; i < 1000000000; i++) a += b; return a; } int main() { foo (3, 4); return 0; } $ gcc --version |& head -1 gcc (GCC) 10.2.1 20201125 (Red Hat 10.2.1-9) $ gcc -g cpp_args_annotate.c -o cpp_args_annotate $ perf record ./cpp_args_annotate [ perf record: Woken up 2 times to write data ] [ perf record: Captured and wrote 0.275 MB perf.data (7188 samples) ] $ perf annotate --stdio2 foo Samples: 7K of event 'cycles:u', 4000 Hz, Event count (approx.): 7468429289, [percent: local period] foo() /home/acme/c/cpp_args_annotate Percent 0000000000401106 <foo>: foo(): int foo(int a, int b) { push %rbp mov %rsp,%rbp mov %edi,-0x14(%rbp) mov %esi,-0x18(%rbp) for (unsigned i = 0; i < 1000000000; i++) movl $0x0,-0x4(%rbp) ??? jmp 1d a += b; 13.45 13: mov -0x18(%rbp),%eax add %eax,-0x14(%rbp) for (unsigned i = 0; i < 1000000000; i++) addl $0x1,-0x4(%rbp) 0.09 1d: cmpl $0x3b9ac9ff,-0x4(%rbp) 86.46 ??? jbe 13 return a; mov -0x14(%rbp),%eax } pop %rbp ??? retq $ I.e. works for C, now lets switch to C++: $ g++ -g cpp_args_annotate.c -o cpp_args_annotate $ perf record ./cpp_args_annotate [ perf record: Woken up 1 times to write data ] [ perf record: Captured and wrote 0.268 MB perf.data (6976 samples) ] $ perf annotate --stdio2 foo Samples: 6K of event 'cycles:u', 4000 Hz, Event count (approx.): 7380681761, [percent: local period] foo() /home/acme/c/cpp_args_annotate Percent 0000000000401106 <foo(int, int)>: foo(int, int): int foo(int a, int b) { push %rbp mov %rsp,%rbp mov %edi,-0x14(%rbp) mov %esi,-0x18(%rbp) for (unsigned i = 0; i < 1000000000; i++) movl $0x0,-0x4(%rbp) cmpl $0x3b9ac9ff,-0x4(%rbp) 86.53 ??? ja 40112c <foo(int, int)+0x26> a += b; 13.32 mov -0x18(%rbp),%eax 0.00 add %eax,-0x14(%rbp) for (unsigned i = 0; i < 1000000000; i++) addl $0x1,-0x4(%rbp) 0.15 ??? jmp 401117 <foo(int, int)+0x11> return a; mov -0x14(%rbp),%eax } pop %rbp ??? retq $ Reproduced. Now with this patch: Reusing the C++ built binary, as we can see here: $ readelf -wi cpp_args_annotate | grep producer <c> DW_AT_producer : (indirect string, offset: 0x2e): GNU C++14 10.2.1 20201125 (Red Hat 10.2.1-9) -mtune=generic -march=x86-64 -g $ And furthermore: $ file cpp_args_annotate cpp_args_annotate: ELF 64-bit LSB executable, x86-64, version 1 (SYSV), dynamically linked, interpreter /lib64/ld-linux-x86-64.so.2, BuildID[sha1]=4fe3cab260204765605ec630d0dc7a7e93c361a9, for GNU/Linux 3.2.0, with debug_info, not stripped $ perf buildid-list -i cpp_args_annotate 4fe3cab260204765605ec630d0dc7a7e93c361a9 $ perf buildid-list | grep cpp_args_annotate 4fe3cab260204765605ec630d0dc7a7e93c361a9 /home/acme/c/cpp_args_annotate $ It now works: $ perf annotate --stdio2 foo Samples: 6K of event 'cycles:u', 4000 Hz, Event count (approx.): 7380681761, [percent: local period] foo() /home/acme/c/cpp_args_annotate Percent 0000000000401106 <foo(int, int)>: foo(int, int): int foo(int a, int b) { push %rbp mov %rsp,%rbp mov %edi,-0x14(%rbp) mov %esi,-0x18(%rbp) for (unsigned i = 0; i < 1000000000; i++) movl $0x0,-0x4(%rbp) 11: cmpl $0x3b9ac9ff,-0x4(%rbp) 86.53 ??? ja 26 a += b; 13.32 mov -0x18(%rbp),%eax 0.00 add %eax,-0x14(%rbp) for (unsigned i = 0; i < 1000000000; i++) addl $0x1,-0x4(%rbp) 0.15 ??? jmp 11 return a; 26: mov -0x14(%rbp),%eax } pop %rbp ??? retq $ Signed-off-by: Martin Li??ka <mli...@suse.cz> Tested-by: Arnaldo Carvalho de Melo <a...@redhat.com> Cc: Jiri Slaby <jsl...@suse.cz> Link: http://lore.kernel.org/lkml/13e1a405-edf9-e4c2-4327-a9b454353...@suse.cz Signed-off-by: Arnaldo Carvalho de Melo <a...@redhat.com> Signed-off-by: Jiri Slaby <jsl...@suse.cz> --- tools/perf/util/annotate.c | 8 ++++++++ tools/perf/util/annotate.h | 1 + 2 files changed, 9 insertions(+) diff --git a/tools/perf/util/annotate.c b/tools/perf/util/annotate.c index ce8c07bc8c56..e60841b86d27 100644 --- a/tools/perf/util/annotate.c +++ b/tools/perf/util/annotate.c @@ -321,12 +321,18 @@ bool ins__is_call(const struct ins *ins) /* * Prevents from matching commas in the comment section, e.g.: * ffff200008446e70: b.cs ffff2000084470f4 <generic_exec_single+0x314> // b.hs, b.nlast + * + * and skip comma as part of function arguments, e.g.: + * 1d8b4ac <linemap_lookup(line_maps const*, unsigned int)+0xcc> */ static inline const char *validate_comma(const char *c, struct ins_operands *ops) { if (ops->raw_comment && c > ops->raw_comment) return NULL; + if (ops->raw_func_start && c > ops->raw_func_start) + return NULL; + return c; } @@ -341,6 +347,8 @@ static int jump__parse(struct arch *arch, struct ins_operands *ops, struct map_s u64 start, end; ops->raw_comment = strchr(ops->raw, arch->objdump.comment_char); + ops->raw_func_start = strchr(ops->raw, '<'); + c = validate_comma(c, ops); /* diff --git a/tools/perf/util/annotate.h b/tools/perf/util/annotate.h index 0a0cd4f32175..096cdaf21b01 100644 --- a/tools/perf/util/annotate.h +++ b/tools/perf/util/annotate.h @@ -32,6 +32,7 @@ struct ins { struct ins_operands { char *raw; char *raw_comment; + char *raw_func_start; struct { char *raw; char *name; -- 2.30.1 ++++++ 0001-perf-symbols-Resolve-symbols-against-debug-file-firs.patch ++++++ From: Jiri Slaby <jsl...@suse.cz> Date: Wed, 17 Feb 2021 13:21:25 +0100 Subject: perf symbols: Resolve symbols against debug file first Git-commit: 6833e0b81aed44c0510aaf2eb72722ba1cf7ddbe Patch-mainline: 5.12-rc1 References: bsc#1180610 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 <jsl...@suse.cz> Acked-by: Namhyung Kim <namhy...@kernel.org> Cc: Alexander Shishkin <alexander.shish...@linux.intel.com> Cc: Jiri Olsa <jo...@redhat.com> Cc: Mark Rutland <mark.rutl...@arm.com> Cc: Peter Zijlstra <pet...@infradead.org> Link: http://lore.kernel.org/lkml/20210217122125.26416-1-jsl...@suse.cz Signed-off-by: Arnaldo Carvalho de Melo <a...@redhat.com> --- tools/perf/util/symbol-elf.c | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/tools/perf/util/symbol-elf.c b/tools/perf/util/symbol-elf.c index f3577f7d72fe..ecc05aa8399d 100644 --- a/tools/perf/util/symbol-elf.c +++ b/tools/perf/util/symbol-elf.c @@ -1226,12 +1226,26 @@ 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; gelf_getshdr(sec, &shdr); + /* + * We have to fallback to runtime when syms' section header has + * NOBITS set. NOBITS results in file offset (sh_offset) not + * being incremented. So sh_offset used below has different + * values for syms (invalid) and runtime (valid). + */ + 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); + } + if (is_label && !elf_sec__filter(&shdr, secstrs)) continue; -- 2.30.1