Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752592AbcJJNhn (ORCPT ); Mon, 10 Oct 2016 09:37:43 -0400 Received: from mx0b-001b2d01.pphosted.com ([148.163.158.5]:45360 "EHLO mx0a-001b2d01.pphosted.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1752446AbcJJNhm (ORCPT ); Mon, 10 Oct 2016 09:37:42 -0400 Subject: Re: [PATCH v7 5/6] perf annotate: Fix jump target outside of function address range To: Arnaldo Carvalho de Melo References: <1474472876-2706-1-git-send-email-ravi.bangoria@linux.vnet.ibm.com> <1474472876-2706-6-git-send-email-ravi.bangoria@linux.vnet.ibm.com> <20161005113114.GU7143@kernel.org> Cc: linux-kernel@vger.kernel.org, kim.phillips@arm.com, linuxppc-dev@lists.ozlabs.org, peterz@infradead.org, mingo@redhat.com, alexander.shishkin@linux.intel.com, treeze.taeung@gmail.com, naveen.n.rao@linux.vnet.ibm.com, markus@trippelsdorf.de, namhyung@kernel.org, pawel.moll@arm.com, chris.ryder@arm.com, jolsa@kernel.org, mhiramat@kernel.org, Ravi Bangoria From: Ravi Bangoria Date: Mon, 10 Oct 2016 19:07:27 +0530 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.5.1 MIME-Version: 1.0 In-Reply-To: <20161005113114.GU7143@kernel.org> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit X-TM-AS-GCONF: 00 X-Content-Scanned: Fidelis XPS MAILER x-cbid: 16101013-0004-0000-0000-00001092AF15 X-IBM-SpamModules-Scores: X-IBM-SpamModules-Versions: BY=3.00005886; HX=3.00000240; KW=3.00000007; PH=3.00000004; SC=3.00000186; SDB=6.00766463; UDB=6.00366596; IPR=6.00542585; BA=6.00004799; NDR=6.00000001; ZLA=6.00000005; ZF=6.00000009; ZB=6.00000000; ZP=6.00000000; ZH=6.00000000; ZU=6.00000002; MB=3.00012934; XFM=3.00000011; UTC=2016-10-10 13:37:38 X-IBM-AV-DETECTION: SAVI=unused REMOTE=unused XFE=unused x-cbparentid: 16101013-0005-0000-0000-0000799B5D3C Message-Id: <57FB9997.4020002@linux.vnet.ibm.com> X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10432:,, definitions=2016-10-10_07:,, signatures=0 X-Proofpoint-Spam-Details: rule=outbound_notspam policy=outbound score=0 spamscore=0 suspectscore=2 malwarescore=0 phishscore=0 adultscore=0 bulkscore=0 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.0.1-1609300000 definitions=main-1610100229 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 5638 Lines: 161 On Wednesday 05 October 2016 05:01 PM, Arnaldo Carvalho de Melo wrote: > Em Wed, Sep 21, 2016 at 09:17:55PM +0530, Ravi Bangoria escreveu: >> If jump target is outside of function range, perf is not handling it >> correctly. Especially when target address is lesser than function start >> address, target offset will be negative. But, target address declared >> to be unsigned, converts negative number into 2's complement. See below >> example. Here target of 'jumpq' instruction at 34cf8 is 34ac0 which is >> lesser than function start address(34cf0). >> >> 34ac0 - 34cf0 = -0x230 = 0xfffffffffffffdd0 > This one looks ok, but isn't applying. This is applying fine for me on perf/core. Which branch are you trying? -Ravi > > - Arnaldo > >> Objdump output: >> >> 0000000000034cf0 <__sigaction>: >> __GI___sigaction(): >> 34cf0: lea -0x20(%rdi),%eax >> 34cf3: cmp -bashx1,%eax >> 34cf6: jbe 34d00 <__sigaction+0x10> >> 34cf8: jmpq 34ac0 <__GI___libc_sigaction> >> 34cfd: nopl (%rax) >> 34d00: mov 0x386161(%rip),%rax # 3bae68 <_DYNAMIC+0x2e8> >> 34d07: movl -bashx16,%fs:(%rax) >> 34d0e: mov -bashxffffffff,%eax >> 34d13: retq >> >> perf annotate before applying patch: >> >> __GI___sigaction /usr/lib64/libc-2.22.so >> lea -0x20(%rdi),%eax >> cmp -bashx1,%eax >> v jbe 10 >> v jmpq fffffffffffffdd0 >> nop >> 10: mov _DYNAMIC+0x2e8,%rax >> movl -bashx16,%fs:(%rax) >> mov -bashxffffffff,%eax >> retq >> >> perf annotate after applying patch: >> >> __GI___sigaction /usr/lib64/libc-2.22.so >> lea -0x20(%rdi),%eax >> cmp -bashx1,%eax >> v jbe 10 >> ^ jmpq 34ac0 <__GI___libc_sigaction> >> nop >> 10: mov _DYNAMIC+0x2e8,%rax >> movl -bashx16,%fs:(%rax) >> mov -bashxffffffff,%eax >> retq >> >> Signed-off-by: Ravi Bangoria >> --- >> Changes in v7: >> - No changes >> >> tools/perf/ui/browsers/annotate.c | 5 +++-- >> tools/perf/util/annotate.c | 14 +++++++++----- >> tools/perf/util/annotate.h | 5 +++-- >> 3 files changed, 15 insertions(+), 9 deletions(-) >> >> diff --git a/tools/perf/ui/browsers/annotate.c b/tools/perf/ui/browsers/annotate.c >> index 214a14a..2d04bdf 100644 >> --- a/tools/perf/ui/browsers/annotate.c >> +++ b/tools/perf/ui/browsers/annotate.c >> @@ -215,7 +215,7 @@ static void annotate_browser__write(struct ui_browser *browser, void *entry, int >> ui_browser__set_color(browser, color); >> if (dl->ins && dl->ins->ops->scnprintf) { >> if (ins__is_jump(dl->ins)) { >> - bool fwd = dl->ops.target.offset > (u64)dl->offset; >> + bool fwd = dl->ops.target.offset > dl->offset; >> >> ui_browser__write_graph(browser, fwd ? SLSMG_DARROW_CHAR : >> SLSMG_UARROW_CHAR); >> @@ -245,7 +245,8 @@ static bool disasm_line__is_valid_jump(struct disasm_line *dl, struct symbol *sy >> { >> if (!dl || !dl->ins || !ins__is_jump(dl->ins) >> || !disasm_line__has_offset(dl) >> - || dl->ops.target.offset >= symbol__size(sym)) >> + || dl->ops.target.offset < 0 >> + || dl->ops.target.offset >= (s64)symbol__size(sym)) >> return false; >> >> return true; >> diff --git a/tools/perf/util/annotate.c b/tools/perf/util/annotate.c >> index a9dbac1..fc44dd1 100644 >> --- a/tools/perf/util/annotate.c >> +++ b/tools/perf/util/annotate.c >> @@ -129,10 +129,12 @@ static int jump__parse(struct ins_operands *ops, struct map *map __maybe_unused) >> else >> ops->target.addr = strtoull(ops->raw, NULL, 16); >> >> - if (s++ != NULL) >> + if (s++ != NULL) { >> ops->target.offset = strtoull(s, NULL, 16); >> - else >> - ops->target.offset = UINT64_MAX; >> + ops->target.offset_avail = true; >> + } else { >> + ops->target.offset_avail = false; >> + } >> >> return 0; >> } >> @@ -140,7 +142,7 @@ static int jump__parse(struct ins_operands *ops, struct map *map __maybe_unused) >> static int jump__scnprintf(struct ins *ins, char *bf, size_t size, >> struct ins_operands *ops) >> { >> - if (!ops->target.addr) >> + if (!ops->target.addr || ops->target.offset < 0) >> return ins__raw_scnprintf(ins, bf, size, ops); >> >> return scnprintf(bf, size, "%-6.6s %" PRIx64, ins->name, ops->target.offset); >> @@ -1373,9 +1375,11 @@ static int symbol__parse_objdump_line(struct symbol *sym, struct map *map, >> if (dl == NULL) >> return -1; >> >> - if (dl->ops.target.offset == UINT64_MAX) >> + if (!disasm_line__has_offset(dl)) { >> dl->ops.target.offset = dl->ops.target.addr - >> map__rip_2objdump(map, sym->start); >> + dl->ops.target.offset_avail = true; >> + } >> >> /* kcore has no symbols, so add the call target name */ >> if (dl->ins && ins__is_call(dl->ins) && !dl->ops.target.name) { >> diff --git a/tools/perf/util/annotate.h b/tools/perf/util/annotate.h >> index 4400269..7ba3579 100644 >> --- a/tools/perf/util/annotate.h >> +++ b/tools/perf/util/annotate.h >> @@ -19,7 +19,8 @@ struct ins_operands { >> char *raw; >> char *name; >> u64 addr; >> - u64 offset; >> + s64 offset; >> + bool offset_avail; >> } target; >> union { >> struct { >> @@ -67,7 +68,7 @@ struct disasm_line { >> >> static inline bool disasm_line__has_offset(const struct disasm_line *dl) >> { >> - return dl->ops.target.offset != UINT64_MAX; >> + return dl->ops.target.offset_avail; >> } >> >> void disasm_line__free(struct disasm_line *dl); >> -- >> 2.5.5