lkml.org 
[lkml]   [2016]   [Oct]   [5]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH v7 5/6] perf annotate: Fix jump target outside of function address range
    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.

    - 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 <ravi.bangoria@linux.vnet.ibm.com>
    > ---
    > 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

    \
     
     \ /
      Last update: 2016-10-05 13:32    [W:5.153 / U:0.788 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site