Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752059AbdFPGPE (ORCPT ); Fri, 16 Jun 2017 02:15:04 -0400 Received: from mx1.redhat.com ([209.132.183.28]:49456 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751098AbdFPGPC (ORCPT ); Fri, 16 Jun 2017 02:15:02 -0400 DMARC-Filter: OpenDMARC Filter v1.3.2 mx1.redhat.com 0E4CB61D2F Authentication-Results: ext-mx10.extmail.prod.ext.phx2.redhat.com; dmarc=none (p=none dis=none) header.from=redhat.com Authentication-Results: ext-mx10.extmail.prod.ext.phx2.redhat.com; spf=pass smtp.mailfrom=jan.kratochvil@redhat.com DKIM-Filter: OpenDKIM Filter v2.11.0 mx1.redhat.com 0E4CB61D2F Date: Fri, 16 Jun 2017 08:14:56 +0200 From: Jan Kratochvil To: Milian Wolff Cc: Linux-kernel@vger.kernel.org, linux-perf-users@vger.kernel.org, Arnaldo Carvalho de Melo , David Ahern , Namhyung Kim , Peter Zijlstra , Yao Jin , Jiri Olsa Subject: Re: perf report: fix off-by-one for non-activation frames Message-ID: <20170616061456.GA2472@host1.jankratochvil.net> References: <20170515150444.6841-1-milian.wolff@kdab.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20170515150444.6841-1-milian.wolff@kdab.com> User-Agent: Mutt/1.8.0 (2017-02-23) X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.39]); Fri, 16 Jun 2017 06:15:02 +0000 (UTC) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 1971 Lines: 48 On Mon, 15 May 2017 17:04:44 +0200, Milian Wolff wrote: commit 1982ad48fc82c284a5cc55697a012d3357e84d01 Author: Milian Wolff 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 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 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=, argv=) at ../../gdb/gdb.c:40 0x000055555565f6f6 <+54>: callq 0x5555558f17a0 => 0x000055555565f6fb <+59>: mov 0x18(%rsp),%rcx Line 40 of "../../gdb/gdb.c" starts at address 0x55555565f6f6 and ends at 0x55555565f6fb . Line 41 of "../../gdb/gdb.c" starts at address 0x55555565f6fb 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