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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <5576772.scvU8aRrkq@agathebauer>
Date:   Fri, 16 Jun 2017 13:51:37 +0200
From:   Milian Wolff <milian.wolff@...b.com>
To:     Jan Kratochvil <jan.kratochvil@...hat.com>
Cc:     Linux-kernel@...r.kernel.org, linux-perf-users@...r.kernel.org,
        Arnaldo Carvalho de Melo <acme@...hat.com>,
        David Ahern <dsahern@...il.com>,
        Namhyung Kim <namhyung@...nel.org>,
        Peter Zijlstra <a.p.zijlstra@...llo.nl>,
        Yao Jin <yao.jin@...ux.intel.com>, Jiri Olsa <jolsa@...hat.com>
Subject: Re: perf report: fix off-by-one for non-activation frames

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@...b.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@...b.com | Senior Software Engineer
KDAB (Deutschland) GmbH&Co KG, a KDAB Group company
Tel: +49-30-521325470
KDAB - The Qt Experts