Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1667437

Re: perf report: fix off-by-one for non-activation frames

From Jan Kratochvil <jan.kratochvil@redhat.com>
Newsgroups linux.kernel
Subject Re: perf report: fix off-by-one for non-activation frames
Date 2017-06-16 08:20 +0200
Message-ID <tSSLT-4KG-1@gated-at.bofh.it> (permalink)
References <tHpNg-1m3-21@gated-at.bofh.it>
Organization linux.* mail to news gateway

Show all headers | View raw


On Mon, 15 May 2017 17:04:44 +0200, Milian Wolff wrote:

commit 1982ad48fc82c284a5cc55697a012d3357e84d01
Author: Milian Wolff <milian.wolff@kdab.com>
Date:   Wed May 24 15:21:25 2017 +0900

> --- a/tools/perf/util/unwind-libdw.c
> +++ b/tools/perf/util/unwind-libdw.c
> @@ -168,12 +168,16 @@ frame_callback(Dwfl_Frame *state, void *arg)
...
> +	if (!isactivation)
> +		--pc;
> +

FYI I find it as a regression a bit:

perf-4.11.4-200.fc25.x86_64
                  30c563 gdb_main (/usr/libexec/gdb)
                   fae48 main (/usr/libexec/gdb)
   0x000055555564ee43 <+51>:    callq  0x55555585f340 <gdb_main(captured_main_args*)>
   0x000055555564ee48 <+56>:    mov    0x18(%rsp),%rcx

perf-4.12.0-0.rc5.git0.1.fc27.x86_64
                  39e32e gdb_main (/usr/libexec/gdb)
                  10b6fa main (/usr/libexec/gdb)
   0x000055555565f6f6 <+54>:    callq  0x5555558f17a0 <gdb_main(captured_main_args*)>
   0x000055555565f6fb <+59>:    mov    0x18(%rsp),%rcx

In backtraces it is correct to show the source line of the calling line - as
perf does now after your fix - but one still should report PC address of the
start of the next instruction.  At least this is what debuggers are used to
do:

#9  gdb_main (args=0x7fffffffe2e0) at ../../gdb/main.c:1257
#10 0x000055555565f6fb in main (argc=<optimized out>, argv=<optimized out>) at ../../gdb/gdb.c:40
   0x000055555565f6f6 <+54>:	callq  0x5555558f17a0 <gdb_main(captured_main_args*)>
=> 0x000055555565f6fb <+59>:	mov    0x18(%rsp),%rcx
Line 40 of "../../gdb/gdb.c" starts at address 0x55555565f6f6 <main(int, char**)+54> and ends at 0x55555565f6fb <main(int, char**)+59>.
Line 41 of "../../gdb/gdb.c" starts at address 0x55555565f6fb <main(int, char**)+59> and ends at 0x55555565f715.

You see "gdb.c:40" and 0x000055555565f6fb in the backtrace despite
0x55555565f6fb is already line 41.

This is also why elfutils reports separately PC and 'isactivation' flag.
Instead of just reporting decreased PC.


Jan

Back to linux.kernel | Previous | NextNext in thread | Find similar | Unroll thread


Thread

Re: perf report: fix off-by-one for non-activation frames Jan Kratochvil <jan.kratochvil@redhat.com> - 2017-06-16 08:20 +0200
  Re: perf report: fix off-by-one for non-activation frames Milian Wolff <milian.wolff@kdab.com> - 2017-06-16 14:00 +0200
    Re: perf report: fix off-by-one for non-activation frames Jan Kratochvil <jan.kratochvil@redhat.com> - 2017-06-16 14:00 +0200
      Re: perf report: fix off-by-one for non-activation frames Milian Wolff <milian.wolff@kdab.com> - 2017-06-16 22:00 +0200
        Re: perf report: fix off-by-one for non-activation frames Namhyung Kim <namhyung@kernel.org> - 2017-06-17 10:00 +0200
          Re: perf report: fix off-by-one for non-activation frames Jan Kratochvil <jan.kratochvil@redhat.com> - 2017-06-17 10:10 +0200
            Re: perf report: fix off-by-one for non-activation frames Milian Wolff <milian.wolff@kdab.com> - 2017-06-17 13:20 +0200
              Re: perf report: fix off-by-one for non-activation frames Arnaldo Carvalho de Melo <acme@redhat.com> - 2017-06-19 21:10 +0200

csiph-web