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

Reply via email to