From: Milian Wolff <milian.wolff@kdab.com>
To: Jan Kratochvil <jan.kratochvil@redhat.com>
Cc: Linux-kernel@vger.kernel.org, linux-perf-users@vger.kernel.org,
Arnaldo Carvalho de Melo <acme@redhat.com>,
David Ahern <dsahern@gmail.com>,
Namhyung Kim <namhyung@kernel.org>,
Peter Zijlstra <a.p.zijlstra@chello.nl>,
Yao Jin <yao.jin@linux.intel.com>, Jiri Olsa <jolsa@redhat.com>
Subject: Re: perf report: fix off-by-one for non-activation frames
Date: Fri, 16 Jun 2017 13:51:37 +0200 [thread overview]
Message-ID: <5576772.scvU8aRrkq@agathebauer> (raw)
In-Reply-To: <20170616061456.GA2472@host1.jankratochvil.net>
On Freitag, 16. Juni 2017 08:14:56 CEST Jan Kratochvil wrote:
> 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.
Excuse me, but I'm having trouble following you. The non-GDB backtraces you
are pasting do not show srcline information. So what exactly is broken? Can
you show me the differences a bit more clearly? Maybe paste the perf output
you get now and highlight what you'd expect instead? Best would be an
accompanying test case that I can use to improve the situation, if possible?
Thanks
--
Milian Wolff | milian.wolff@kdab.com | Senior Software Engineer
KDAB (Deutschland) GmbH&Co KG, a KDAB Group company
Tel: +49-30-521325470
KDAB - The Qt Experts
next prev parent reply other threads:[~2017-06-16 11:51 UTC|newest]
Thread overview: 13+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-05-15 15:04 [PATCH] perf report: fix off-by-one for non-activation frames Milian Wolff
2017-05-15 15:13 ` Milian Wolff
2017-05-16 1:57 ` Namhyung Kim
2017-05-16 9:00 ` Milian Wolff
2017-06-16 6:14 ` Jan Kratochvil
2017-06-16 11:51 ` Milian Wolff [this message]
2017-06-16 11:57 ` Jan Kratochvil
2017-06-16 19:54 ` Milian Wolff
2017-06-17 7:56 ` Namhyung Kim
2017-06-17 8:04 ` Jan Kratochvil
2017-06-17 11:13 ` Milian Wolff
2017-06-19 18:59 ` Arnaldo Carvalho de Melo
2017-07-04 7:59 ` Milian Wolff
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=5576772.scvU8aRrkq@agathebauer \
--to=milian.wolff@kdab.com \
--cc=Linux-kernel@vger.kernel.org \
--cc=a.p.zijlstra@chello.nl \
--cc=acme@redhat.com \
--cc=dsahern@gmail.com \
--cc=jan.kratochvil@redhat.com \
--cc=jolsa@redhat.com \
--cc=linux-perf-users@vger.kernel.org \
--cc=namhyung@kernel.org \
--cc=yao.jin@linux.intel.com \
/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: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).