* [PATCH v6 0/7] perf: Cross arch annotate + few miscellaneous fixes
@ 2016-08-19 12:59 Ravi Bangoria
2016-08-19 12:59 ` [PATCH v6 1/7] perf: Define macro for normalized arch names Ravi Bangoria
` (7 more replies)
0 siblings, 8 replies; 19+ messages in thread
From: Ravi Bangoria @ 2016-08-19 12:59 UTC (permalink / raw)
To: linux-kernel, linuxppc-dev, acme
Cc: peterz, mingo, alexander.shishkin, treeze.taeung, naveen.n.rao,
markus, chris.ryder, pawel.moll, mhiramat, rmk+kernel, jolsa, mpe,
hemant, namhyung, Ravi Bangoria
Currently Perf annotate support code navigation (branches and calls)
only when run on the same architecture where perf.data was recorded.
But, for example, record on powerpc server and annotate on client's
x86 desktop is not supported.
This patchset enables cross arch annotate. Currently I've used x86
and arm instructions which are already available and added support
for powerpc.
Additionally this patch series also contains few other related fixes.
Patches are prepared on top of acme/perf/core and tested it with x86
and powerpc only.
Note for arm:
I don't have arm test machine. As suggested by Russell in one of the
review comment, I've copied all instructions from default table to
arm table. This way it want break tool on arm but cleanup is needed
for x86 specific instructions added in arm table.
Example:
Record on powerpc:
$ ./perf record -a
Report -> Annotate on x86:
$ ./perf report -i perf.data.powerpc --vmlinux vmlinux.powerpc
Changes in v6:
- Instead of adding only those instructions defined in #ifdef __arm__,
add all instructions from default table to arm table.
v5 link:
https://lkml.org/lkml/2016/8/19/35
Naveen N. Rao (1):
perf annotate: Add support for powerpc
Ravi Bangoria (6):
perf: Define macro for normalized arch names
perf annotate: Add cross arch annotate support
perf annotate: Do not ignore call instruction with indirect target
perf annotate: Show raw form for jump instruction with indirect target
perf annotate: Support jump instruction with target as second operand
perf annotate: Fix jump target outside of function address range
tools/perf/arch/common.c | 36 ++--
tools/perf/arch/common.h | 11 ++
tools/perf/builtin-top.c | 2 +-
tools/perf/ui/browsers/annotate.c | 8 +-
tools/perf/ui/gtk/annotate.c | 2 +-
tools/perf/util/annotate.c | 330 +++++++++++++++++++++++++++++++------
tools/perf/util/annotate.h | 10 +-
tools/perf/util/unwind-libunwind.c | 4 +-
8 files changed, 327 insertions(+), 76 deletions(-)
--
2.5.5
^ permalink raw reply [flat|nested] 19+ messages in thread* [PATCH v6 1/7] perf: Define macro for normalized arch names 2016-08-19 12:59 [PATCH v6 0/7] perf: Cross arch annotate + few miscellaneous fixes Ravi Bangoria @ 2016-08-19 12:59 ` Ravi Bangoria 2016-08-19 12:59 ` [PATCH v6 2/7] perf annotate: Add cross arch annotate support Ravi Bangoria ` (6 subsequent siblings) 7 siblings, 0 replies; 19+ messages in thread From: Ravi Bangoria @ 2016-08-19 12:59 UTC (permalink / raw) To: linux-kernel, linuxppc-dev, acme Cc: peterz, mingo, alexander.shishkin, treeze.taeung, naveen.n.rao, markus, chris.ryder, pawel.moll, mhiramat, rmk+kernel, jolsa, mpe, hemant, namhyung, Ravi Bangoria Define macro for each normalized arch name and use them instead of using arch name as string. Signed-off-by: Ravi Bangoria <ravi.bangoria@linux.vnet.ibm.com> --- Changes in v6: - No change tools/perf/arch/common.c | 36 ++++++++++++++++++------------------ tools/perf/arch/common.h | 11 +++++++++++ tools/perf/util/unwind-libunwind.c | 4 ++-- 3 files changed, 31 insertions(+), 20 deletions(-) diff --git a/tools/perf/arch/common.c b/tools/perf/arch/common.c index 886dd2a..f763666 100644 --- a/tools/perf/arch/common.c +++ b/tools/perf/arch/common.c @@ -123,25 +123,25 @@ static int lookup_triplets(const char *const *triplets, const char *name) const char *normalize_arch(char *arch) { if (!strcmp(arch, "x86_64")) - return "x86"; + return NORM_X86; if (arch[0] == 'i' && arch[2] == '8' && arch[3] == '6') - return "x86"; + return NORM_X86; if (!strcmp(arch, "sun4u") || !strncmp(arch, "sparc", 5)) - return "sparc"; + return NORM_SPARC; if (!strcmp(arch, "aarch64") || !strcmp(arch, "arm64")) - return "arm64"; + return NORM_ARM64; if (!strncmp(arch, "arm", 3) || !strcmp(arch, "sa110")) - return "arm"; + return NORM_ARM; if (!strncmp(arch, "s390", 4)) - return "s390"; + return NORM_S390; if (!strncmp(arch, "parisc", 6)) - return "parisc"; + return NORM_PARISC; if (!strncmp(arch, "powerpc", 7) || !strncmp(arch, "ppc", 3)) - return "powerpc"; + return NORM_POWERPC; if (!strncmp(arch, "mips", 4)) - return "mips"; + return NORM_MIPS; if (!strncmp(arch, "sh", 2) && isdigit(arch[2])) - return "sh"; + return NORM_SH; return arch; } @@ -181,21 +181,21 @@ static int perf_env__lookup_binutils_path(struct perf_env *env, zfree(&buf); } - if (!strcmp(arch, "arm")) + if (!strcmp(arch, NORM_ARM)) path_list = arm_triplets; - else if (!strcmp(arch, "arm64")) + else if (!strcmp(arch, NORM_ARM64)) path_list = arm64_triplets; - else if (!strcmp(arch, "powerpc")) + else if (!strcmp(arch, NORM_POWERPC)) path_list = powerpc_triplets; - else if (!strcmp(arch, "sh")) + else if (!strcmp(arch, NORM_SH)) path_list = sh_triplets; - else if (!strcmp(arch, "s390")) + else if (!strcmp(arch, NORM_S390)) path_list = s390_triplets; - else if (!strcmp(arch, "sparc")) + else if (!strcmp(arch, NORM_SPARC)) path_list = sparc_triplets; - else if (!strcmp(arch, "x86")) + else if (!strcmp(arch, NORM_X86)) path_list = x86_triplets; - else if (!strcmp(arch, "mips")) + else if (!strcmp(arch, NORM_MIPS)) path_list = mips_triplets; else { ui__error("binutils for %s not supported.\n", arch); diff --git a/tools/perf/arch/common.h b/tools/perf/arch/common.h index 6b01c73..14ca8ca 100644 --- a/tools/perf/arch/common.h +++ b/tools/perf/arch/common.h @@ -5,6 +5,17 @@ extern const char *objdump_path; +/* Macro for normalized arch names */ +#define NORM_X86 "x86" +#define NORM_SPARC "sparc" +#define NORM_ARM64 "arm64" +#define NORM_ARM "arm" +#define NORM_S390 "s390" +#define NORM_PARISC "parisc" +#define NORM_POWERPC "powerpc" +#define NORM_MIPS "mips" +#define NORM_SH "sh" + int perf_env__lookup_objdump(struct perf_env *env); const char *normalize_arch(char *arch); diff --git a/tools/perf/util/unwind-libunwind.c b/tools/perf/util/unwind-libunwind.c index 6d542a4..6199102 100644 --- a/tools/perf/util/unwind-libunwind.c +++ b/tools/perf/util/unwind-libunwind.c @@ -40,10 +40,10 @@ int unwind__prepare_access(struct thread *thread, struct map *map, arch = normalize_arch(thread->mg->machine->env->arch); - if (!strcmp(arch, "x86")) { + if (!strcmp(arch, NORM_X86)) { if (dso_type != DSO__TYPE_64BIT) ops = x86_32_unwind_libunwind_ops; - } else if (!strcmp(arch, "arm64") || !strcmp(arch, "arm")) { + } else if (!strcmp(arch, NORM_ARM64) || !strcmp(arch, NORM_ARM)) { if (dso_type == DSO__TYPE_64BIT) ops = arm64_unwind_libunwind_ops; } -- 2.5.5 ^ permalink raw reply related [flat|nested] 19+ messages in thread
* [PATCH v6 2/7] perf annotate: Add cross arch annotate support 2016-08-19 12:59 [PATCH v6 0/7] perf: Cross arch annotate + few miscellaneous fixes Ravi Bangoria 2016-08-19 12:59 ` [PATCH v6 1/7] perf: Define macro for normalized arch names Ravi Bangoria @ 2016-08-19 12:59 ` Ravi Bangoria 2016-08-22 23:01 ` Kim Phillips 2016-08-19 12:59 ` [PATCH v6 3/7] perf annotate: Add support for powerpc Ravi Bangoria ` (5 subsequent siblings) 7 siblings, 1 reply; 19+ messages in thread From: Ravi Bangoria @ 2016-08-19 12:59 UTC (permalink / raw) To: linux-kernel, linuxppc-dev, acme Cc: peterz, mingo, alexander.shishkin, treeze.taeung, naveen.n.rao, markus, chris.ryder, pawel.moll, mhiramat, rmk+kernel, jolsa, mpe, hemant, namhyung, Ravi Bangoria Change current data structures and function to enable cross arch annotate. Current perf implementation does not support cross arch annotate. To make it truly cross arch, instruction table of all arch should be present in perf binary. And use appropriate table based on arch where perf.data was recorded. Signed-off-by: Ravi Bangoria <ravi.bangoria@linux.vnet.ibm.com> --- Changes in v6: - Instead of adding only those instructions defined in #ifdef __arm__, add all instructions from default table to arm table. tools/perf/builtin-top.c | 2 +- tools/perf/ui/browsers/annotate.c | 3 +- tools/perf/ui/gtk/annotate.c | 2 +- tools/perf/util/annotate.c | 187 ++++++++++++++++++++++++++++++-------- tools/perf/util/annotate.h | 5 +- 5 files changed, 157 insertions(+), 42 deletions(-) diff --git a/tools/perf/builtin-top.c b/tools/perf/builtin-top.c index a3223aa..fdd4203 100644 --- a/tools/perf/builtin-top.c +++ b/tools/perf/builtin-top.c @@ -129,7 +129,7 @@ static int perf_top__parse_source(struct perf_top *top, struct hist_entry *he) return err; } - err = symbol__disassemble(sym, map, 0); + err = symbol__disassemble(sym, map, 0, NULL); if (err == 0) { out_assign: top->sym_filter_entry = he; diff --git a/tools/perf/ui/browsers/annotate.c b/tools/perf/ui/browsers/annotate.c index 2e2d100..21c5e10 100644 --- a/tools/perf/ui/browsers/annotate.c +++ b/tools/perf/ui/browsers/annotate.c @@ -1050,7 +1050,8 @@ int symbol__tui_annotate(struct symbol *sym, struct map *map, (nr_pcnt - 1); } - err = symbol__disassemble(sym, map, sizeof_bdl); + err = symbol__disassemble(sym, map, sizeof_bdl, + perf_evsel__env_arch(evsel)); if (err) { char msg[BUFSIZ]; symbol__strerror_disassemble(sym, map, err, msg, sizeof(msg)); diff --git a/tools/perf/ui/gtk/annotate.c b/tools/perf/ui/gtk/annotate.c index 42d3199..c127aba 100644 --- a/tools/perf/ui/gtk/annotate.c +++ b/tools/perf/ui/gtk/annotate.c @@ -167,7 +167,7 @@ static int symbol__gtk_annotate(struct symbol *sym, struct map *map, if (map->dso->annotate_warned) return -1; - err = symbol__disassemble(sym, map, 0); + err = symbol__disassemble(sym, map, 0, perf_evsel__env_arch(evsel)); if (err) { char msg[BUFSIZ]; symbol__strerror_disassemble(sym, map, err, msg, sizeof(msg)); diff --git a/tools/perf/util/annotate.c b/tools/perf/util/annotate.c index 25a9259..14a8808 100644 --- a/tools/perf/util/annotate.c +++ b/tools/perf/util/annotate.c @@ -20,12 +20,14 @@ #include <regex.h> #include <pthread.h> #include <linux/bitops.h> +#include <sys/utsname.h> +#include "../arch/common.h" const char *disassembler_style; const char *objdump_path; static regex_t file_lineno; -static struct ins *ins__find(const char *name); +static struct ins *ins__find(const char *name, const char *norm_arch); static int disasm_line__parse(char *line, char **namep, char **rawp); static void ins__delete(struct ins_operands *ops) @@ -53,7 +55,7 @@ int ins__scnprintf(struct ins *ins, char *bf, size_t size, return ins__raw_scnprintf(ins, bf, size, ops); } -static int call__parse(struct ins_operands *ops) +static int call__parse(struct ins_operands *ops, const char *norm_arch) { char *endptr, *tok, *name; @@ -65,10 +67,8 @@ static int call__parse(struct ins_operands *ops) name++; -#ifdef __arm__ - if (strchr(name, '+')) + if (!strcmp(norm_arch, NORM_ARM) && strchr(name, '+')) return -1; -#endif tok = strchr(name, '>'); if (tok == NULL) @@ -117,7 +117,8 @@ bool ins__is_call(const struct ins *ins) return ins->ops == &call_ops; } -static int jump__parse(struct ins_operands *ops) +static int jump__parse(struct ins_operands *ops, + const char *norm_arch __maybe_unused) { const char *s = strchr(ops->raw, '+'); @@ -172,7 +173,7 @@ static int comment__symbol(char *raw, char *comment, u64 *addrp, char **namep) return 0; } -static int lock__parse(struct ins_operands *ops) +static int lock__parse(struct ins_operands *ops, const char *norm_arch) { char *name; @@ -183,7 +184,7 @@ static int lock__parse(struct ins_operands *ops) if (disasm_line__parse(ops->raw, &name, &ops->locked.ops->raw) < 0) goto out_free_ops; - ops->locked.ins = ins__find(name); + ops->locked.ins = ins__find(name, norm_arch); free(name); if (ops->locked.ins == NULL) @@ -193,7 +194,7 @@ static int lock__parse(struct ins_operands *ops) return 0; if (ops->locked.ins->ops->parse && - ops->locked.ins->ops->parse(ops->locked.ops) < 0) + ops->locked.ins->ops->parse(ops->locked.ops, norm_arch) < 0) goto out_free_ops; return 0; @@ -236,7 +237,7 @@ static struct ins_ops lock_ops = { .scnprintf = lock__scnprintf, }; -static int mov__parse(struct ins_operands *ops) +static int mov__parse(struct ins_operands *ops, const char *norm_arch) { char *s = strchr(ops->raw, ','), *target, *comment, prev; @@ -251,11 +252,11 @@ static int mov__parse(struct ins_operands *ops) return -1; target = ++s; -#ifdef __arm__ - comment = strchr(s, ';'); -#else - comment = strchr(s, '#'); -#endif + + if (!strcmp(norm_arch, NORM_ARM)) + comment = strchr(s, ';'); + else + comment = strchr(s, '#'); if (comment != NULL) s = comment - 1; @@ -303,7 +304,8 @@ static struct ins_ops mov_ops = { .scnprintf = mov__scnprintf, }; -static int dec__parse(struct ins_operands *ops) +static int dec__parse(struct ins_operands *ops, + const char *norm_arch __maybe_unused) { char *target, *comment, *s, prev; @@ -363,14 +365,92 @@ bool ins__is_ret(const struct ins *ins) return ins->ops == &ret_ops; } -static struct ins instructions[] = { +static struct ins instructions_x86[] = { + { .name = "add", .ops = &mov_ops, }, + { .name = "addl", .ops = &mov_ops, }, + { .name = "addq", .ops = &mov_ops, }, + { .name = "addw", .ops = &mov_ops, }, + { .name = "and", .ops = &mov_ops, }, + { .name = "bts", .ops = &mov_ops, }, + { .name = "call", .ops = &call_ops, }, + { .name = "callq", .ops = &call_ops, }, + { .name = "cmp", .ops = &mov_ops, }, + { .name = "cmpb", .ops = &mov_ops, }, + { .name = "cmpl", .ops = &mov_ops, }, + { .name = "cmpq", .ops = &mov_ops, }, + { .name = "cmpw", .ops = &mov_ops, }, + { .name = "cmpxch", .ops = &mov_ops, }, + { .name = "dec", .ops = &dec_ops, }, + { .name = "decl", .ops = &dec_ops, }, + { .name = "imul", .ops = &mov_ops, }, + { .name = "inc", .ops = &dec_ops, }, + { .name = "incl", .ops = &dec_ops, }, + { .name = "ja", .ops = &jump_ops, }, + { .name = "jae", .ops = &jump_ops, }, + { .name = "jb", .ops = &jump_ops, }, + { .name = "jbe", .ops = &jump_ops, }, + { .name = "jc", .ops = &jump_ops, }, + { .name = "jcxz", .ops = &jump_ops, }, + { .name = "je", .ops = &jump_ops, }, + { .name = "jecxz", .ops = &jump_ops, }, + { .name = "jg", .ops = &jump_ops, }, + { .name = "jge", .ops = &jump_ops, }, + { .name = "jl", .ops = &jump_ops, }, + { .name = "jle", .ops = &jump_ops, }, + { .name = "jmp", .ops = &jump_ops, }, + { .name = "jmpq", .ops = &jump_ops, }, + { .name = "jna", .ops = &jump_ops, }, + { .name = "jnae", .ops = &jump_ops, }, + { .name = "jnb", .ops = &jump_ops, }, + { .name = "jnbe", .ops = &jump_ops, }, + { .name = "jnc", .ops = &jump_ops, }, + { .name = "jne", .ops = &jump_ops, }, + { .name = "jng", .ops = &jump_ops, }, + { .name = "jnge", .ops = &jump_ops, }, + { .name = "jnl", .ops = &jump_ops, }, + { .name = "jnle", .ops = &jump_ops, }, + { .name = "jno", .ops = &jump_ops, }, + { .name = "jnp", .ops = &jump_ops, }, + { .name = "jns", .ops = &jump_ops, }, + { .name = "jnz", .ops = &jump_ops, }, + { .name = "jo", .ops = &jump_ops, }, + { .name = "jp", .ops = &jump_ops, }, + { .name = "jpe", .ops = &jump_ops, }, + { .name = "jpo", .ops = &jump_ops, }, + { .name = "jrcxz", .ops = &jump_ops, }, + { .name = "js", .ops = &jump_ops, }, + { .name = "jz", .ops = &jump_ops, }, + { .name = "lea", .ops = &mov_ops, }, + { .name = "lock", .ops = &lock_ops, }, + { .name = "mov", .ops = &mov_ops, }, + { .name = "movb", .ops = &mov_ops, }, + { .name = "movdqa", .ops = &mov_ops, }, + { .name = "movl", .ops = &mov_ops, }, + { .name = "movq", .ops = &mov_ops, }, + { .name = "movslq", .ops = &mov_ops, }, + { .name = "movzbl", .ops = &mov_ops, }, + { .name = "movzwl", .ops = &mov_ops, }, + { .name = "nop", .ops = &nop_ops, }, + { .name = "nopl", .ops = &nop_ops, }, + { .name = "nopw", .ops = &nop_ops, }, + { .name = "or", .ops = &mov_ops, }, + { .name = "orl", .ops = &mov_ops, }, + { .name = "test", .ops = &mov_ops, }, + { .name = "testb", .ops = &mov_ops, }, + { .name = "testl", .ops = &mov_ops, }, + { .name = "xadd", .ops = &mov_ops, }, + { .name = "xbeginl", .ops = &jump_ops, }, + { .name = "xbeginq", .ops = &jump_ops, }, + { .name = "retq", .ops = &ret_ops, }, +}; + +static struct ins instructions_arm[] = { { .name = "add", .ops = &mov_ops, }, { .name = "addl", .ops = &mov_ops, }, { .name = "addq", .ops = &mov_ops, }, { .name = "addw", .ops = &mov_ops, }, { .name = "and", .ops = &mov_ops, }, -#ifdef __arm__ - { .name = "b", .ops = &jump_ops, }, // might also be a call + { .name = "b", .ops = &jump_ops, }, /* might also be a call */ { .name = "bcc", .ops = &jump_ops, }, { .name = "bcs", .ops = &jump_ops, }, { .name = "beq", .ops = &jump_ops, }, @@ -382,7 +462,6 @@ static struct ins instructions[] = { { .name = "blt", .ops = &jump_ops, }, { .name = "blx", .ops = &call_ops, }, { .name = "bne", .ops = &jump_ops, }, -#endif { .name = "bts", .ops = &mov_ops, }, { .name = "call", .ops = &call_ops, }, { .name = "callq", .ops = &call_ops, }, @@ -471,24 +550,48 @@ static int ins__cmp(const void *a, const void *b) return strcmp(ia->name, ib->name); } -static void ins__sort(void) +static void ins__sort(struct ins *instructions, int nmemb) { - const int nmemb = ARRAY_SIZE(instructions); - qsort(instructions, nmemb, sizeof(struct ins), ins__cmp); } -static struct ins *ins__find(const char *name) +static const char *annotate__norm_arch(char *arch) +{ + struct utsname uts; + + if (!arch) { /* Assume we are annotating locally. */ + if (uname(&uts) < 0) + return NULL; + arch = uts.machine; + } + return normalize_arch(arch); +} + +static struct ins *ins__find(const char *name, const char *norm_arch) { - const int nmemb = ARRAY_SIZE(instructions); static bool sorted; + struct ins *instructions; + int nmemb; if (!sorted) { - ins__sort(); + ins__sort(instructions_x86, ARRAY_SIZE(instructions_x86)); + ins__sort(instructions_arm, ARRAY_SIZE(instructions_arm)); sorted = true; } - return bsearch(name, instructions, nmemb, sizeof(struct ins), ins__key_cmp); + if (!strcmp(norm_arch, NORM_X86)) { + instructions = instructions_x86; + nmemb = ARRAY_SIZE(instructions_x86); + } else if (!strcmp(norm_arch, NORM_ARM)) { + instructions = instructions_arm; + nmemb = ARRAY_SIZE(instructions_arm); + } else { + pr_err("perf annotate not supported by %s arch\n", norm_arch); + return NULL; + } + + return bsearch(name, instructions, nmemb, sizeof(struct ins), + ins__key_cmp); } int symbol__annotate_init(struct map *map __maybe_unused, struct symbol *sym) @@ -715,17 +818,24 @@ int hist_entry__inc_addr_samples(struct hist_entry *he, int evidx, u64 ip) return symbol__inc_addr_samples(he->ms.sym, he->ms.map, evidx, ip); } -static void disasm_line__init_ins(struct disasm_line *dl) +static void disasm_line__init_ins(struct disasm_line *dl, char *arch) { - dl->ins = ins__find(dl->name); + const char *norm_arch = annotate__norm_arch(arch); + + if (!norm_arch) { + pr_err("Can not annotate. Could not determine architecture."); + return; + } + dl->ins = ins__find(dl->name, norm_arch); if (dl->ins == NULL) return; if (!dl->ins->ops) return; - if (dl->ins->ops->parse && dl->ins->ops->parse(&dl->ops) < 0) + if (dl->ins->ops->parse && + dl->ins->ops->parse(&dl->ops, norm_arch) < 0) dl->ins = NULL; } @@ -767,7 +877,8 @@ out_free_name: } static struct disasm_line *disasm_line__new(s64 offset, char *line, - size_t privsize, int line_nr) + size_t privsize, int line_nr, + char *arch) { struct disasm_line *dl = zalloc(sizeof(*dl) + privsize); @@ -782,7 +893,7 @@ static struct disasm_line *disasm_line__new(s64 offset, char *line, if (disasm_line__parse(dl->line, &dl->name, &dl->ops.raw) < 0) goto out_free_line; - disasm_line__init_ins(dl); + disasm_line__init_ins(dl, arch); } } @@ -1005,7 +1116,7 @@ static int disasm_line__print(struct disasm_line *dl, struct symbol *sym, u64 st */ static int symbol__parse_objdump_line(struct symbol *sym, struct map *map, FILE *file, size_t privsize, - int *line_nr) + int *line_nr, char *arch) { struct annotation *notes = symbol__annotation(sym); struct disasm_line *dl; @@ -1066,7 +1177,8 @@ static int symbol__parse_objdump_line(struct symbol *sym, struct map *map, parsed_line = tmp2 + 1; } - dl = disasm_line__new(offset, parsed_line, privsize, *line_nr); + dl = disasm_line__new(offset, parsed_line, privsize, + *line_nr, arch); free(line); (*line_nr)++; @@ -1197,7 +1309,8 @@ fallback: return 0; } -int symbol__disassemble(struct symbol *sym, struct map *map, size_t privsize) +int symbol__disassemble(struct symbol *sym, struct map *map, size_t privsize, + char *arch) { struct dso *dso = map->dso; char command[PATH_MAX * 2]; @@ -1313,7 +1426,7 @@ int symbol__disassemble(struct symbol *sym, struct map *map, size_t privsize) nline = 0; while (!feof(file)) { if (symbol__parse_objdump_line(sym, map, file, privsize, - &lineno) < 0) + &lineno, arch) < 0) break; nline++; } @@ -1710,7 +1823,7 @@ int symbol__tty_annotate(struct symbol *sym, struct map *map, struct rb_root source_line = RB_ROOT; u64 len; - if (symbol__disassemble(sym, map, 0) < 0) + if (symbol__disassemble(sym, map, 0, perf_evsel__env_arch(evsel)) < 0) return -1; len = symbol__size(sym); diff --git a/tools/perf/util/annotate.h b/tools/perf/util/annotate.h index f67ccb0..5cfad4e 100644 --- a/tools/perf/util/annotate.h +++ b/tools/perf/util/annotate.h @@ -36,7 +36,7 @@ struct ins_operands { struct ins_ops { void (*free)(struct ins_operands *ops); - int (*parse)(struct ins_operands *ops); + int (*parse)(struct ins_operands *ops, const char *norm_arch); int (*scnprintf)(struct ins *ins, char *bf, size_t size, struct ins_operands *ops); }; @@ -155,7 +155,8 @@ int hist_entry__inc_addr_samples(struct hist_entry *he, int evidx, u64 addr); int symbol__alloc_hist(struct symbol *sym); void symbol__annotate_zero_histograms(struct symbol *sym); -int symbol__disassemble(struct symbol *sym, struct map *map, size_t privsize); +int symbol__disassemble(struct symbol *sym, struct map *map, size_t privsize, + char *arch); enum symbol_disassemble_errno { SYMBOL_ANNOTATE_ERRNO__SUCCESS = 0, -- 2.5.5 ^ permalink raw reply related [flat|nested] 19+ messages in thread
* Re: [PATCH v6 2/7] perf annotate: Add cross arch annotate support 2016-08-19 12:59 ` [PATCH v6 2/7] perf annotate: Add cross arch annotate support Ravi Bangoria @ 2016-08-22 23:01 ` Kim Phillips 2016-08-23 2:17 ` Namhyung Kim 0 siblings, 1 reply; 19+ messages in thread From: Kim Phillips @ 2016-08-22 23:01 UTC (permalink / raw) To: Ravi Bangoria Cc: linux-kernel, linuxppc-dev, acme, peterz, mingo, alexander.shishkin, treeze.taeung, naveen.n.rao, markus, chris.ryder, pawel.moll, mhiramat, rmk+kernel, jolsa, mpe, hemant, namhyung On Fri, 19 Aug 2016 18:29:33 +0530 Ravi Bangoria <ravi.bangoria@linux.vnet.ibm.com> wrote: > Changes in v6: > - Instead of adding only those instructions defined in #ifdef __arm__, > add all instructions from default table to arm table. .. > +static struct ins instructions_arm[] = { > { .name = "add", .ops = &mov_ops, }, > { .name = "addl", .ops = &mov_ops, }, > { .name = "addq", .ops = &mov_ops, }, > { .name = "addw", .ops = &mov_ops, }, > { .name = "and", .ops = &mov_ops, }, > -#ifdef __arm__ > - { .name = "b", .ops = &jump_ops, }, // might also be a call > + { .name = "b", .ops = &jump_ops, }, /* might also be a call */ > { .name = "bcc", .ops = &jump_ops, }, > { .name = "bcs", .ops = &jump_ops, }, > { .name = "beq", .ops = &jump_ops, }, > @@ -382,7 +462,6 @@ static struct ins instructions[] = { > { .name = "blt", .ops = &jump_ops, }, > { .name = "blx", .ops = &call_ops, }, > { .name = "bne", .ops = &jump_ops, }, > -#endif > { .name = "bts", .ops = &mov_ops, }, > { .name = "call", .ops = &call_ops, }, > { .name = "callq", .ops = &call_ops, }, > @@ -471,24 +550,48 @@ static int ins__cmp(const void *a, const void *b) > return strcmp(ia->name, ib->name); > } Thanks, I've gone through the list and removed all not-ARM instructions, and added some missing ARM branch instructions: diff --git a/tools/perf/util/annotate.c b/tools/perf/util/annotate.c index b2c6cf3..9d686504 100644 --- a/tools/perf/util/annotate.c +++ b/tools/perf/util/annotate.c @@ -451,9 +451,6 @@ static struct ins instructions_x86[] = { static struct ins instructions_arm[] = { { .name = "add", .ops = &mov_ops, }, - { .name = "addl", .ops = &mov_ops, }, - { .name = "addq", .ops = &mov_ops, }, - { .name = "addw", .ops = &mov_ops, }, { .name = "and", .ops = &mov_ops, }, { .name = "b", .ops = &jump_ops, }, /* might also be a call */ { .name = "bcc", .ops = &jump_ops, }, @@ -463,81 +460,20 @@ static struct ins instructions_arm[] = { { .name = "bgt", .ops = &jump_ops, }, { .name = "bhi", .ops = &jump_ops, }, { .name = "bl", .ops = &call_ops, }, + { .name = "ble", .ops = &jump_ops, }, + { .name = "bleq", .ops = &call_ops, }, + { .name = "blne", .ops = &call_ops, }, { .name = "bls", .ops = &jump_ops, }, { .name = "blt", .ops = &jump_ops, }, { .name = "blx", .ops = &call_ops, }, + { .name = "blxne", .ops = &call_ops, }, + { .name = "bmi", .ops = &jump_ops, }, { .name = "bne", .ops = &jump_ops, }, - { .name = "bts", .ops = &mov_ops, }, - { .name = "call", .ops = &call_ops, }, - { .name = "callq", .ops = &call_ops, }, + { .name = "bpl", .ops = &jump_ops, }, { .name = "cmp", .ops = &mov_ops, }, - { .name = "cmpb", .ops = &mov_ops, }, - { .name = "cmpl", .ops = &mov_ops, }, - { .name = "cmpq", .ops = &mov_ops, }, - { .name = "cmpw", .ops = &mov_ops, }, - { .name = "cmpxch", .ops = &mov_ops, }, - { .name = "dec", .ops = &dec_ops, }, - { .name = "decl", .ops = &dec_ops, }, - { .name = "imul", .ops = &mov_ops, }, - { .name = "inc", .ops = &dec_ops, }, - { .name = "incl", .ops = &dec_ops, }, - { .name = "ja", .ops = &jump_ops, }, - { .name = "jae", .ops = &jump_ops, }, - { .name = "jb", .ops = &jump_ops, }, - { .name = "jbe", .ops = &jump_ops, }, - { .name = "jc", .ops = &jump_ops, }, - { .name = "jcxz", .ops = &jump_ops, }, - { .name = "je", .ops = &jump_ops, }, - { .name = "jecxz", .ops = &jump_ops, }, - { .name = "jg", .ops = &jump_ops, }, - { .name = "jge", .ops = &jump_ops, }, - { .name = "jl", .ops = &jump_ops, }, - { .name = "jle", .ops = &jump_ops, }, - { .name = "jmp", .ops = &jump_ops, }, - { .name = "jmpq", .ops = &jump_ops, }, - { .name = "jna", .ops = &jump_ops, }, - { .name = "jnae", .ops = &jump_ops, }, - { .name = "jnb", .ops = &jump_ops, }, - { .name = "jnbe", .ops = &jump_ops, }, - { .name = "jnc", .ops = &jump_ops, }, - { .name = "jne", .ops = &jump_ops, }, - { .name = "jng", .ops = &jump_ops, }, - { .name = "jnge", .ops = &jump_ops, }, - { .name = "jnl", .ops = &jump_ops, }, - { .name = "jnle", .ops = &jump_ops, }, - { .name = "jno", .ops = &jump_ops, }, - { .name = "jnp", .ops = &jump_ops, }, - { .name = "jns", .ops = &jump_ops, }, - { .name = "jnz", .ops = &jump_ops, }, - { .name = "jo", .ops = &jump_ops, }, - { .name = "jp", .ops = &jump_ops, }, - { .name = "jpe", .ops = &jump_ops, }, - { .name = "jpo", .ops = &jump_ops, }, - { .name = "jrcxz", .ops = &jump_ops, }, - { .name = "js", .ops = &jump_ops, }, - { .name = "jz", .ops = &jump_ops, }, - { .name = "lea", .ops = &mov_ops, }, - { .name = "lock", .ops = &lock_ops, }, { .name = "mov", .ops = &mov_ops, }, - { .name = "movb", .ops = &mov_ops, }, - { .name = "movdqa",.ops = &mov_ops, }, - { .name = "movl", .ops = &mov_ops, }, - { .name = "movq", .ops = &mov_ops, }, - { .name = "movslq", .ops = &mov_ops, }, - { .name = "movzbl", .ops = &mov_ops, }, - { .name = "movzwl", .ops = &mov_ops, }, { .name = "nop", .ops = &nop_ops, }, - { .name = "nopl", .ops = &nop_ops, }, - { .name = "nopw", .ops = &nop_ops, }, { .name = "or", .ops = &mov_ops, }, - { .name = "orl", .ops = &mov_ops, }, - { .name = "test", .ops = &mov_ops, }, - { .name = "testb", .ops = &mov_ops, }, - { .name = "testl", .ops = &mov_ops, }, - { .name = "xadd", .ops = &mov_ops, }, - { .name = "xbeginl", .ops = &jump_ops, }, - { .name = "xbeginq", .ops = &jump_ops, }, - { .name = "retq", .ops = &ret_ops, }, }; struct instructions_powerpc { Ideally ARM would - like this powerpc implementation - add a ins__find_arm() to handle its condition codes, but, for now, is it possible to merge this change into this series? I can post a follow-up patch if not. Thanks, Kim ^ permalink raw reply related [flat|nested] 19+ messages in thread
* Re: [PATCH v6 2/7] perf annotate: Add cross arch annotate support 2016-08-22 23:01 ` Kim Phillips @ 2016-08-23 2:17 ` Namhyung Kim 2016-08-23 20:36 ` Kim Phillips 0 siblings, 1 reply; 19+ messages in thread From: Namhyung Kim @ 2016-08-23 2:17 UTC (permalink / raw) To: Kim Phillips Cc: Ravi Bangoria, linux-kernel@vger.kernel.org, linuxppc-dev, Arnaldo Carvalho de Melo, Peter Zijlstra, Ingo Molnar, Alexander Shishkin, Taeung Song, Naveen N. Rao, markus, chris.ryder, Pawel Moll, Masami Hiramatsu, rmk+kernel, Jiri Olsa, Michael Ellerman, Hemant Kumar Hello, On Tue, Aug 23, 2016 at 8:01 AM, Kim Phillips <kim.phillips@arm.com> wrote: > On Fri, 19 Aug 2016 18:29:33 +0530 > Ravi Bangoria <ravi.bangoria@linux.vnet.ibm.com> wrote: > >> Changes in v6: >> - Instead of adding only those instructions defined in #ifdef __arm__, >> add all instructions from default table to arm table. > .. >> +static struct ins instructions_arm[] = { >> { .name = "add", .ops = &mov_ops, }, >> { .name = "addl", .ops = &mov_ops, }, >> { .name = "addq", .ops = &mov_ops, }, >> { .name = "addw", .ops = &mov_ops, }, >> { .name = "and", .ops = &mov_ops, }, >> -#ifdef __arm__ >> - { .name = "b", .ops = &jump_ops, }, // might also be a call >> + { .name = "b", .ops = &jump_ops, }, /* might also be a call */ >> { .name = "bcc", .ops = &jump_ops, }, >> { .name = "bcs", .ops = &jump_ops, }, >> { .name = "beq", .ops = &jump_ops, }, >> @@ -382,7 +462,6 @@ static struct ins instructions[] = { >> { .name = "blt", .ops = &jump_ops, }, >> { .name = "blx", .ops = &call_ops, }, >> { .name = "bne", .ops = &jump_ops, }, >> -#endif >> { .name = "bts", .ops = &mov_ops, }, >> { .name = "call", .ops = &call_ops, }, >> { .name = "callq", .ops = &call_ops, }, >> @@ -471,24 +550,48 @@ static int ins__cmp(const void *a, const void *b) >> return strcmp(ia->name, ib->name); >> } > > Thanks, I've gone through the list and removed all not-ARM > instructions, and added some missing ARM branch instructions: Can we use regex patterns instead? Thanks Namhyung > > diff --git a/tools/perf/util/annotate.c b/tools/perf/util/annotate.c > index b2c6cf3..9d686504 100644 > --- a/tools/perf/util/annotate.c > +++ b/tools/perf/util/annotate.c > @@ -451,9 +451,6 @@ static struct ins instructions_x86[] = { > > static struct ins instructions_arm[] = { > { .name = "add", .ops = &mov_ops, }, > - { .name = "addl", .ops = &mov_ops, }, > - { .name = "addq", .ops = &mov_ops, }, > - { .name = "addw", .ops = &mov_ops, }, > { .name = "and", .ops = &mov_ops, }, > { .name = "b", .ops = &jump_ops, }, /* might also be a call */ > { .name = "bcc", .ops = &jump_ops, }, > @@ -463,81 +460,20 @@ static struct ins instructions_arm[] = { > { .name = "bgt", .ops = &jump_ops, }, > { .name = "bhi", .ops = &jump_ops, }, > { .name = "bl", .ops = &call_ops, }, > + { .name = "ble", .ops = &jump_ops, }, > + { .name = "bleq", .ops = &call_ops, }, > + { .name = "blne", .ops = &call_ops, }, > { .name = "bls", .ops = &jump_ops, }, > { .name = "blt", .ops = &jump_ops, }, > { .name = "blx", .ops = &call_ops, }, > + { .name = "blxne", .ops = &call_ops, }, > + { .name = "bmi", .ops = &jump_ops, }, > { .name = "bne", .ops = &jump_ops, }, > - { .name = "bts", .ops = &mov_ops, }, > - { .name = "call", .ops = &call_ops, }, > - { .name = "callq", .ops = &call_ops, }, > + { .name = "bpl", .ops = &jump_ops, }, > { .name = "cmp", .ops = &mov_ops, }, > - { .name = "cmpb", .ops = &mov_ops, }, > - { .name = "cmpl", .ops = &mov_ops, }, > - { .name = "cmpq", .ops = &mov_ops, }, > - { .name = "cmpw", .ops = &mov_ops, }, > - { .name = "cmpxch", .ops = &mov_ops, }, > - { .name = "dec", .ops = &dec_ops, }, > - { .name = "decl", .ops = &dec_ops, }, > - { .name = "imul", .ops = &mov_ops, }, > - { .name = "inc", .ops = &dec_ops, }, > - { .name = "incl", .ops = &dec_ops, }, > - { .name = "ja", .ops = &jump_ops, }, > - { .name = "jae", .ops = &jump_ops, }, > - { .name = "jb", .ops = &jump_ops, }, > - { .name = "jbe", .ops = &jump_ops, }, > - { .name = "jc", .ops = &jump_ops, }, > - { .name = "jcxz", .ops = &jump_ops, }, > - { .name = "je", .ops = &jump_ops, }, > - { .name = "jecxz", .ops = &jump_ops, }, > - { .name = "jg", .ops = &jump_ops, }, > - { .name = "jge", .ops = &jump_ops, }, > - { .name = "jl", .ops = &jump_ops, }, > - { .name = "jle", .ops = &jump_ops, }, > - { .name = "jmp", .ops = &jump_ops, }, > - { .name = "jmpq", .ops = &jump_ops, }, > - { .name = "jna", .ops = &jump_ops, }, > - { .name = "jnae", .ops = &jump_ops, }, > - { .name = "jnb", .ops = &jump_ops, }, > - { .name = "jnbe", .ops = &jump_ops, }, > - { .name = "jnc", .ops = &jump_ops, }, > - { .name = "jne", .ops = &jump_ops, }, > - { .name = "jng", .ops = &jump_ops, }, > - { .name = "jnge", .ops = &jump_ops, }, > - { .name = "jnl", .ops = &jump_ops, }, > - { .name = "jnle", .ops = &jump_ops, }, > - { .name = "jno", .ops = &jump_ops, }, > - { .name = "jnp", .ops = &jump_ops, }, > - { .name = "jns", .ops = &jump_ops, }, > - { .name = "jnz", .ops = &jump_ops, }, > - { .name = "jo", .ops = &jump_ops, }, > - { .name = "jp", .ops = &jump_ops, }, > - { .name = "jpe", .ops = &jump_ops, }, > - { .name = "jpo", .ops = &jump_ops, }, > - { .name = "jrcxz", .ops = &jump_ops, }, > - { .name = "js", .ops = &jump_ops, }, > - { .name = "jz", .ops = &jump_ops, }, > - { .name = "lea", .ops = &mov_ops, }, > - { .name = "lock", .ops = &lock_ops, }, > { .name = "mov", .ops = &mov_ops, }, > - { .name = "movb", .ops = &mov_ops, }, > - { .name = "movdqa",.ops = &mov_ops, }, > - { .name = "movl", .ops = &mov_ops, }, > - { .name = "movq", .ops = &mov_ops, }, > - { .name = "movslq", .ops = &mov_ops, }, > - { .name = "movzbl", .ops = &mov_ops, }, > - { .name = "movzwl", .ops = &mov_ops, }, > { .name = "nop", .ops = &nop_ops, }, > - { .name = "nopl", .ops = &nop_ops, }, > - { .name = "nopw", .ops = &nop_ops, }, > { .name = "or", .ops = &mov_ops, }, > - { .name = "orl", .ops = &mov_ops, }, > - { .name = "test", .ops = &mov_ops, }, > - { .name = "testb", .ops = &mov_ops, }, > - { .name = "testl", .ops = &mov_ops, }, > - { .name = "xadd", .ops = &mov_ops, }, > - { .name = "xbeginl", .ops = &jump_ops, }, > - { .name = "xbeginq", .ops = &jump_ops, }, > - { .name = "retq", .ops = &ret_ops, }, > }; > > struct instructions_powerpc { > > Ideally ARM would - like this powerpc implementation - add a > ins__find_arm() to handle its condition codes, but, for now, is > it possible to merge this change into this series? I can post a > follow-up patch if not. > > Thanks, > > Kim ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v6 2/7] perf annotate: Add cross arch annotate support 2016-08-23 2:17 ` Namhyung Kim @ 2016-08-23 20:36 ` Kim Phillips 2016-08-26 6:21 ` Namhyung Kim 2016-08-26 7:26 ` Ravi Bangoria 0 siblings, 2 replies; 19+ messages in thread From: Kim Phillips @ 2016-08-23 20:36 UTC (permalink / raw) To: Namhyung Kim Cc: Ravi Bangoria, linux-kernel@vger.kernel.org, linuxppc-dev, Arnaldo Carvalho de Melo, Peter Zijlstra, Ingo Molnar, Alexander Shishkin, Taeung Song, Naveen N. Rao, markus, chris.ryder, Pawel Moll, Masami Hiramatsu, rmk+kernel, Jiri Olsa, Michael Ellerman, Hemant Kumar On Tue, 23 Aug 2016 11:17:16 +0900 Namhyung Kim <namhyung@kernel.org> wrote: > On Tue, Aug 23, 2016 at 8:01 AM, Kim Phillips <kim.phillips@arm.com> wrote: > > On Fri, 19 Aug 2016 18:29:33 +0530 > > Ravi Bangoria <ravi.bangoria@linux.vnet.ibm.com> wrote: > > > >> Changes in v6: > >> - Instead of adding only those instructions defined in #ifdef __arm__, > >> add all instructions from default table to arm table. > > Thanks, I've gone through the list and removed all not-ARM > > instructions, and added some missing ARM branch instructions: > > Can we use regex patterns instead? Yes, that helps prevent mistakes updating instruction lists - how does this look?: diff --git a/tools/perf/util/annotate.c b/tools/perf/util/annotate.c index b2c6cf3..52316f3 100644 --- a/tools/perf/util/annotate.c +++ b/tools/perf/util/annotate.c @@ -26,6 +26,7 @@ const char *disassembler_style; const char *objdump_path; static regex_t file_lineno; +static regex_t arm_call_insn, arm_jump_insn; static struct ins *ins__find(const char *name, const char *norm_arch); static int disasm_line__parse(char *line, char **namep, char **rawp); @@ -449,98 +450,7 @@ static struct ins instructions_x86[] = { { .name = "retq", .ops = &ret_ops, }, }; -static struct ins instructions_arm[] = { - { .name = "add", .ops = &mov_ops, }, - { .name = "addl", .ops = &mov_ops, }, - { .name = "addq", .ops = &mov_ops, }, - { .name = "addw", .ops = &mov_ops, }, - { .name = "and", .ops = &mov_ops, }, - { .name = "b", .ops = &jump_ops, }, /* might also be a call */ - { .name = "bcc", .ops = &jump_ops, }, - { .name = "bcs", .ops = &jump_ops, }, - { .name = "beq", .ops = &jump_ops, }, - { .name = "bge", .ops = &jump_ops, }, - { .name = "bgt", .ops = &jump_ops, }, - { .name = "bhi", .ops = &jump_ops, }, - { .name = "bl", .ops = &call_ops, }, - { .name = "bls", .ops = &jump_ops, }, - { .name = "blt", .ops = &jump_ops, }, - { .name = "blx", .ops = &call_ops, }, - { .name = "bne", .ops = &jump_ops, }, - { .name = "bts", .ops = &mov_ops, }, - { .name = "call", .ops = &call_ops, }, - { .name = "callq", .ops = &call_ops, }, - { .name = "cmp", .ops = &mov_ops, }, - { .name = "cmpb", .ops = &mov_ops, }, - { .name = "cmpl", .ops = &mov_ops, }, - { .name = "cmpq", .ops = &mov_ops, }, - { .name = "cmpw", .ops = &mov_ops, }, - { .name = "cmpxch", .ops = &mov_ops, }, - { .name = "dec", .ops = &dec_ops, }, - { .name = "decl", .ops = &dec_ops, }, - { .name = "imul", .ops = &mov_ops, }, - { .name = "inc", .ops = &dec_ops, }, - { .name = "incl", .ops = &dec_ops, }, - { .name = "ja", .ops = &jump_ops, }, - { .name = "jae", .ops = &jump_ops, }, - { .name = "jb", .ops = &jump_ops, }, - { .name = "jbe", .ops = &jump_ops, }, - { .name = "jc", .ops = &jump_ops, }, - { .name = "jcxz", .ops = &jump_ops, }, - { .name = "je", .ops = &jump_ops, }, - { .name = "jecxz", .ops = &jump_ops, }, - { .name = "jg", .ops = &jump_ops, }, - { .name = "jge", .ops = &jump_ops, }, - { .name = "jl", .ops = &jump_ops, }, - { .name = "jle", .ops = &jump_ops, }, - { .name = "jmp", .ops = &jump_ops, }, - { .name = "jmpq", .ops = &jump_ops, }, - { .name = "jna", .ops = &jump_ops, }, - { .name = "jnae", .ops = &jump_ops, }, - { .name = "jnb", .ops = &jump_ops, }, - { .name = "jnbe", .ops = &jump_ops, }, - { .name = "jnc", .ops = &jump_ops, }, - { .name = "jne", .ops = &jump_ops, }, - { .name = "jng", .ops = &jump_ops, }, - { .name = "jnge", .ops = &jump_ops, }, - { .name = "jnl", .ops = &jump_ops, }, - { .name = "jnle", .ops = &jump_ops, }, - { .name = "jno", .ops = &jump_ops, }, - { .name = "jnp", .ops = &jump_ops, }, - { .name = "jns", .ops = &jump_ops, }, - { .name = "jnz", .ops = &jump_ops, }, - { .name = "jo", .ops = &jump_ops, }, - { .name = "jp", .ops = &jump_ops, }, - { .name = "jpe", .ops = &jump_ops, }, - { .name = "jpo", .ops = &jump_ops, }, - { .name = "jrcxz", .ops = &jump_ops, }, - { .name = "js", .ops = &jump_ops, }, - { .name = "jz", .ops = &jump_ops, }, - { .name = "lea", .ops = &mov_ops, }, - { .name = "lock", .ops = &lock_ops, }, - { .name = "mov", .ops = &mov_ops, }, - { .name = "movb", .ops = &mov_ops, }, - { .name = "movdqa",.ops = &mov_ops, }, - { .name = "movl", .ops = &mov_ops, }, - { .name = "movq", .ops = &mov_ops, }, - { .name = "movslq", .ops = &mov_ops, }, - { .name = "movzbl", .ops = &mov_ops, }, - { .name = "movzwl", .ops = &mov_ops, }, - { .name = "nop", .ops = &nop_ops, }, - { .name = "nopl", .ops = &nop_ops, }, - { .name = "nopw", .ops = &nop_ops, }, - { .name = "or", .ops = &mov_ops, }, - { .name = "orl", .ops = &mov_ops, }, - { .name = "test", .ops = &mov_ops, }, - { .name = "testb", .ops = &mov_ops, }, - { .name = "testl", .ops = &mov_ops, }, - { .name = "xadd", .ops = &mov_ops, }, - { .name = "xbeginl", .ops = &jump_ops, }, - { .name = "xbeginq", .ops = &jump_ops, }, - { .name = "retq", .ops = &ret_ops, }, -}; - -struct instructions_powerpc { +struct instructions_arch { struct ins *ins; struct list_head list; }; @@ -560,41 +470,41 @@ static int ins__cmp(const void *a, const void *b) return strcmp(ia->name, ib->name); } -static struct ins *list_add__ins_powerpc(struct instructions_powerpc *head, +static struct ins *list_add__ins_arch(struct instructions_arch *head, const char *name, struct ins_ops *ops) { - struct instructions_powerpc *ins_powerpc; + struct instructions_arch *ins_arch; struct ins *ins; ins = zalloc(sizeof(struct ins)); if (!ins) return NULL; - ins_powerpc = zalloc(sizeof(struct instructions_powerpc)); - if (!ins_powerpc) + ins_arch = zalloc(sizeof(struct instructions_arch)); + if (!ins_arch) goto out_free_ins; ins->name = strdup(name); if (!ins->name) - goto out_free_ins_power; + goto out_free_ins_arch; ins->ops = ops; - ins_powerpc->ins = ins; - list_add_tail(&(ins_powerpc->list), &(head->list)); + ins_arch->ins = ins; + list_add_tail(&(ins_arch->list), &(head->list)); return ins; -out_free_ins_power: - zfree(&ins_powerpc); +out_free_ins_arch: + zfree(&ins_arch); out_free_ins: zfree(&ins); return NULL; } -static struct ins *list_search__ins_powerpc(struct instructions_powerpc *head, +static struct ins *list_search__ins_arch(struct instructions_arch *head, const char *name) { - struct instructions_powerpc *pos; + struct instructions_arch *pos; list_for_each_entry(pos, &head->list, list) { if (!strcmp(pos->ins->name, name)) @@ -608,7 +518,7 @@ static struct ins *ins__find_powerpc(const char *name) int i; struct ins *ins; struct ins_ops *ops; - static struct instructions_powerpc head; + static struct instructions_arch head; static bool list_initialized; /* @@ -629,7 +539,7 @@ static struct ins *ins__find_powerpc(const char *name) /* * Return if we already have object of 'struct ins' for this instruction */ - ins = list_search__ins_powerpc(&head, name); + ins = list_search__ins_arch(&head, name); if (ins) return ins; @@ -666,7 +576,44 @@ static struct ins *ins__find_powerpc(const char *name) * Add instruction to list so next time no need to * allocate memory for it. */ - return list_add__ins_powerpc(&head, name, ops); + return list_add__ins_arch(&head, name, ops); +} + +static struct ins *ins__find_arm(const char *name) +{ + struct ins *ins; + struct ins_ops *ops = &mov_ops; + static struct instructions_arch head; + static bool list_initialized; + regmatch_t match[2]; + int ret; + + if (!list_initialized) { + INIT_LIST_HEAD(&head.list); + list_initialized = true; + } + + /* + * Return if we already have object of 'struct ins' for this instruction + */ + ins = list_search__ins_arch(&head, name); + if (ins) + return ins; + + ret = regexec(&arm_call_insn, name, 2, match, 0); + if (!ret) { + ops = &call_ops; + } else { + ret = regexec(&arm_jump_insn, name, 2, match, 0); + if (!ret) + ops = &jump_ops; + } + + /* + * Add instruction to list so next time no need to + * allocate memory for it. + */ + return list_add__ins_arch(&head, name, ops); } static void ins__sort(struct ins *instructions, int nmemb) @@ -686,15 +633,26 @@ static const char *annotate__norm_arch(char *arch) return normalize_arch(arch); } +#define ARM_CONDS "(cc|cs|eq|ge|gt|hi|le|ls|lt|mi|ne|pl|vc|vs)" + static struct ins *ins__find(const char *name, const char *norm_arch) { static bool sorted; struct ins *instructions; - int nmemb; + int nmemb, ret; if (!sorted) { ins__sort(instructions_x86, ARRAY_SIZE(instructions_x86)); - ins__sort(instructions_arm, ARRAY_SIZE(instructions_arm)); + if (!strcmp(norm_arch, NORM_ARM)) { + ret = regcomp(&arm_call_insn, + "^blx?" ARM_CONDS "?$", REG_EXTENDED); + ret |= regcomp(&arm_jump_insn, + "^bx?" ARM_CONDS "?$", REG_EXTENDED); + if (ret) { + pr_err("regcomp failed\n"); + return NULL; + } + } sorted = true; } @@ -702,8 +660,7 @@ static struct ins *ins__find(const char *name, const char *norm_arch) instructions = instructions_x86; nmemb = ARRAY_SIZE(instructions_x86); } else if (!strcmp(norm_arch, NORM_ARM)) { - instructions = instructions_arm; - nmemb = ARRAY_SIZE(instructions_arm); + return ins__find_arm(name); } else if (!strcmp(norm_arch, NORM_POWERPC)) { return ins__find_powerpc(name); } else { Note that, for ARM, in order to match return instructions, 'lr' must be found to be the operand of the branch instruction, which is not contained in the 'name' variable passed to ins__find(). Kim ^ permalink raw reply related [flat|nested] 19+ messages in thread
* Re: [PATCH v6 2/7] perf annotate: Add cross arch annotate support 2016-08-23 20:36 ` Kim Phillips @ 2016-08-26 6:21 ` Namhyung Kim 2016-08-26 7:26 ` Ravi Bangoria 1 sibling, 0 replies; 19+ messages in thread From: Namhyung Kim @ 2016-08-26 6:21 UTC (permalink / raw) To: Kim Phillips Cc: Ravi Bangoria, linux-kernel@vger.kernel.org, linuxppc-dev, Arnaldo Carvalho de Melo, Peter Zijlstra, Ingo Molnar, Alexander Shishkin, Taeung Song, Naveen N. Rao, markus, chris.ryder, Pawel Moll, Masami Hiramatsu, rmk+kernel, Jiri Olsa, Michael Ellerman, Hemant Kumar Hi, On Tue, Aug 23, 2016 at 03:36:17PM -0500, Kim Phillips wrote: > On Tue, 23 Aug 2016 11:17:16 +0900 > Namhyung Kim <namhyung@kernel.org> wrote: > > > On Tue, Aug 23, 2016 at 8:01 AM, Kim Phillips <kim.phillips@arm.com> wrote: > > > On Fri, 19 Aug 2016 18:29:33 +0530 > > > Ravi Bangoria <ravi.bangoria@linux.vnet.ibm.com> wrote: > > > > > >> Changes in v6: > > >> - Instead of adding only those instructions defined in #ifdef __arm__, > > >> add all instructions from default table to arm table. > > > Thanks, I've gone through the list and removed all not-ARM > > > instructions, and added some missing ARM branch instructions: > > > > Can we use regex patterns instead? > > Yes, that helps prevent mistakes updating instruction lists - how does > this look?: Much better! Thanks, Namhyung > > diff --git a/tools/perf/util/annotate.c b/tools/perf/util/annotate.c > index b2c6cf3..52316f3 100644 > --- a/tools/perf/util/annotate.c > +++ b/tools/perf/util/annotate.c > @@ -26,6 +26,7 @@ > const char *disassembler_style; > const char *objdump_path; > static regex_t file_lineno; > +static regex_t arm_call_insn, arm_jump_insn; > > static struct ins *ins__find(const char *name, const char *norm_arch); > static int disasm_line__parse(char *line, char **namep, char **rawp); > @@ -449,98 +450,7 @@ static struct ins instructions_x86[] = { > { .name = "retq", .ops = &ret_ops, }, > }; > > -static struct ins instructions_arm[] = { > - { .name = "add", .ops = &mov_ops, }, > - { .name = "addl", .ops = &mov_ops, }, > - { .name = "addq", .ops = &mov_ops, }, > - { .name = "addw", .ops = &mov_ops, }, > - { .name = "and", .ops = &mov_ops, }, > - { .name = "b", .ops = &jump_ops, }, /* might also be a call */ > - { .name = "bcc", .ops = &jump_ops, }, > - { .name = "bcs", .ops = &jump_ops, }, > - { .name = "beq", .ops = &jump_ops, }, > - { .name = "bge", .ops = &jump_ops, }, > - { .name = "bgt", .ops = &jump_ops, }, > - { .name = "bhi", .ops = &jump_ops, }, > - { .name = "bl", .ops = &call_ops, }, > - { .name = "bls", .ops = &jump_ops, }, > - { .name = "blt", .ops = &jump_ops, }, > - { .name = "blx", .ops = &call_ops, }, > - { .name = "bne", .ops = &jump_ops, }, > - { .name = "bts", .ops = &mov_ops, }, > - { .name = "call", .ops = &call_ops, }, > - { .name = "callq", .ops = &call_ops, }, > - { .name = "cmp", .ops = &mov_ops, }, > - { .name = "cmpb", .ops = &mov_ops, }, > - { .name = "cmpl", .ops = &mov_ops, }, > - { .name = "cmpq", .ops = &mov_ops, }, > - { .name = "cmpw", .ops = &mov_ops, }, > - { .name = "cmpxch", .ops = &mov_ops, }, > - { .name = "dec", .ops = &dec_ops, }, > - { .name = "decl", .ops = &dec_ops, }, > - { .name = "imul", .ops = &mov_ops, }, > - { .name = "inc", .ops = &dec_ops, }, > - { .name = "incl", .ops = &dec_ops, }, > - { .name = "ja", .ops = &jump_ops, }, > - { .name = "jae", .ops = &jump_ops, }, > - { .name = "jb", .ops = &jump_ops, }, > - { .name = "jbe", .ops = &jump_ops, }, > - { .name = "jc", .ops = &jump_ops, }, > - { .name = "jcxz", .ops = &jump_ops, }, > - { .name = "je", .ops = &jump_ops, }, > - { .name = "jecxz", .ops = &jump_ops, }, > - { .name = "jg", .ops = &jump_ops, }, > - { .name = "jge", .ops = &jump_ops, }, > - { .name = "jl", .ops = &jump_ops, }, > - { .name = "jle", .ops = &jump_ops, }, > - { .name = "jmp", .ops = &jump_ops, }, > - { .name = "jmpq", .ops = &jump_ops, }, > - { .name = "jna", .ops = &jump_ops, }, > - { .name = "jnae", .ops = &jump_ops, }, > - { .name = "jnb", .ops = &jump_ops, }, > - { .name = "jnbe", .ops = &jump_ops, }, > - { .name = "jnc", .ops = &jump_ops, }, > - { .name = "jne", .ops = &jump_ops, }, > - { .name = "jng", .ops = &jump_ops, }, > - { .name = "jnge", .ops = &jump_ops, }, > - { .name = "jnl", .ops = &jump_ops, }, > - { .name = "jnle", .ops = &jump_ops, }, > - { .name = "jno", .ops = &jump_ops, }, > - { .name = "jnp", .ops = &jump_ops, }, > - { .name = "jns", .ops = &jump_ops, }, > - { .name = "jnz", .ops = &jump_ops, }, > - { .name = "jo", .ops = &jump_ops, }, > - { .name = "jp", .ops = &jump_ops, }, > - { .name = "jpe", .ops = &jump_ops, }, > - { .name = "jpo", .ops = &jump_ops, }, > - { .name = "jrcxz", .ops = &jump_ops, }, > - { .name = "js", .ops = &jump_ops, }, > - { .name = "jz", .ops = &jump_ops, }, > - { .name = "lea", .ops = &mov_ops, }, > - { .name = "lock", .ops = &lock_ops, }, > - { .name = "mov", .ops = &mov_ops, }, > - { .name = "movb", .ops = &mov_ops, }, > - { .name = "movdqa",.ops = &mov_ops, }, > - { .name = "movl", .ops = &mov_ops, }, > - { .name = "movq", .ops = &mov_ops, }, > - { .name = "movslq", .ops = &mov_ops, }, > - { .name = "movzbl", .ops = &mov_ops, }, > - { .name = "movzwl", .ops = &mov_ops, }, > - { .name = "nop", .ops = &nop_ops, }, > - { .name = "nopl", .ops = &nop_ops, }, > - { .name = "nopw", .ops = &nop_ops, }, > - { .name = "or", .ops = &mov_ops, }, > - { .name = "orl", .ops = &mov_ops, }, > - { .name = "test", .ops = &mov_ops, }, > - { .name = "testb", .ops = &mov_ops, }, > - { .name = "testl", .ops = &mov_ops, }, > - { .name = "xadd", .ops = &mov_ops, }, > - { .name = "xbeginl", .ops = &jump_ops, }, > - { .name = "xbeginq", .ops = &jump_ops, }, > - { .name = "retq", .ops = &ret_ops, }, > -}; > - > -struct instructions_powerpc { > +struct instructions_arch { > struct ins *ins; > struct list_head list; > }; > @@ -560,41 +470,41 @@ static int ins__cmp(const void *a, const void *b) > return strcmp(ia->name, ib->name); > } > > -static struct ins *list_add__ins_powerpc(struct instructions_powerpc *head, > +static struct ins *list_add__ins_arch(struct instructions_arch *head, > const char *name, struct ins_ops *ops) > { > - struct instructions_powerpc *ins_powerpc; > + struct instructions_arch *ins_arch; > struct ins *ins; > > ins = zalloc(sizeof(struct ins)); > if (!ins) > return NULL; > > - ins_powerpc = zalloc(sizeof(struct instructions_powerpc)); > - if (!ins_powerpc) > + ins_arch = zalloc(sizeof(struct instructions_arch)); > + if (!ins_arch) > goto out_free_ins; > > ins->name = strdup(name); > if (!ins->name) > - goto out_free_ins_power; > + goto out_free_ins_arch; > > ins->ops = ops; > - ins_powerpc->ins = ins; > - list_add_tail(&(ins_powerpc->list), &(head->list)); > + ins_arch->ins = ins; > + list_add_tail(&(ins_arch->list), &(head->list)); > > return ins; > > -out_free_ins_power: > - zfree(&ins_powerpc); > +out_free_ins_arch: > + zfree(&ins_arch); > out_free_ins: > zfree(&ins); > return NULL; > } > > -static struct ins *list_search__ins_powerpc(struct instructions_powerpc *head, > +static struct ins *list_search__ins_arch(struct instructions_arch *head, > const char *name) > { > - struct instructions_powerpc *pos; > + struct instructions_arch *pos; > > list_for_each_entry(pos, &head->list, list) { > if (!strcmp(pos->ins->name, name)) > @@ -608,7 +518,7 @@ static struct ins *ins__find_powerpc(const char *name) > int i; > struct ins *ins; > struct ins_ops *ops; > - static struct instructions_powerpc head; > + static struct instructions_arch head; > static bool list_initialized; > > /* > @@ -629,7 +539,7 @@ static struct ins *ins__find_powerpc(const char *name) > /* > * Return if we already have object of 'struct ins' for this instruction > */ > - ins = list_search__ins_powerpc(&head, name); > + ins = list_search__ins_arch(&head, name); > if (ins) > return ins; > > @@ -666,7 +576,44 @@ static struct ins *ins__find_powerpc(const char *name) > * Add instruction to list so next time no need to > * allocate memory for it. > */ > - return list_add__ins_powerpc(&head, name, ops); > + return list_add__ins_arch(&head, name, ops); > +} > + > +static struct ins *ins__find_arm(const char *name) > +{ > + struct ins *ins; > + struct ins_ops *ops = &mov_ops; > + static struct instructions_arch head; > + static bool list_initialized; > + regmatch_t match[2]; > + int ret; > + > + if (!list_initialized) { > + INIT_LIST_HEAD(&head.list); > + list_initialized = true; > + } > + > + /* > + * Return if we already have object of 'struct ins' for this instruction > + */ > + ins = list_search__ins_arch(&head, name); > + if (ins) > + return ins; > + > + ret = regexec(&arm_call_insn, name, 2, match, 0); > + if (!ret) { > + ops = &call_ops; > + } else { > + ret = regexec(&arm_jump_insn, name, 2, match, 0); > + if (!ret) > + ops = &jump_ops; > + } > + > + /* > + * Add instruction to list so next time no need to > + * allocate memory for it. > + */ > + return list_add__ins_arch(&head, name, ops); > } > > static void ins__sort(struct ins *instructions, int nmemb) > @@ -686,15 +633,26 @@ static const char *annotate__norm_arch(char *arch) > return normalize_arch(arch); > } > > +#define ARM_CONDS "(cc|cs|eq|ge|gt|hi|le|ls|lt|mi|ne|pl|vc|vs)" > + > static struct ins *ins__find(const char *name, const char *norm_arch) > { > static bool sorted; > struct ins *instructions; > - int nmemb; > + int nmemb, ret; > > if (!sorted) { > ins__sort(instructions_x86, ARRAY_SIZE(instructions_x86)); > - ins__sort(instructions_arm, ARRAY_SIZE(instructions_arm)); > + if (!strcmp(norm_arch, NORM_ARM)) { > + ret = regcomp(&arm_call_insn, > + "^blx?" ARM_CONDS "?$", REG_EXTENDED); > + ret |= regcomp(&arm_jump_insn, > + "^bx?" ARM_CONDS "?$", REG_EXTENDED); > + if (ret) { > + pr_err("regcomp failed\n"); > + return NULL; > + } > + } > sorted = true; > } > > @@ -702,8 +660,7 @@ static struct ins *ins__find(const char *name, const char *norm_arch) > instructions = instructions_x86; > nmemb = ARRAY_SIZE(instructions_x86); > } else if (!strcmp(norm_arch, NORM_ARM)) { > - instructions = instructions_arm; > - nmemb = ARRAY_SIZE(instructions_arm); > + return ins__find_arm(name); > } else if (!strcmp(norm_arch, NORM_POWERPC)) { > return ins__find_powerpc(name); > } else { > > Note that, for ARM, in order to match return instructions, 'lr' must be > found to be the operand of the branch instruction, which is not > contained in the 'name' variable passed to ins__find(). > > Kim ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v6 2/7] perf annotate: Add cross arch annotate support 2016-08-23 20:36 ` Kim Phillips 2016-08-26 6:21 ` Namhyung Kim @ 2016-08-26 7:26 ` Ravi Bangoria 2016-08-27 0:40 ` [PATCH] perf annotate: cross arch annotate support fixes for ARM Kim Phillips 1 sibling, 1 reply; 19+ messages in thread From: Ravi Bangoria @ 2016-08-26 7:26 UTC (permalink / raw) To: Kim Phillips Cc: Namhyung Kim, linux-kernel@vger.kernel.org, linuxppc-dev, Arnaldo Carvalho de Melo, Peter Zijlstra, Ingo Molnar, Alexander Shishkin, Taeung Song, Naveen N. Rao, markus, chris.ryder, Pawel Moll, Masami Hiramatsu, rmk+kernel, Jiri Olsa, Michael Ellerman, Hemant Kumar, Ravi Bangoria Hi Kim, I've tested your patch on x86 and powerpc and it looks fine to me. Can you please put your signed-off-by. Please add Act-by: Ravi Bangoria <ravi.bangoria@linux.vnet.ibm.com> as well. Regards, -Ravi On Wednesday 24 August 2016 02:06 AM, Kim Phillips wrote: > On Tue, 23 Aug 2016 11:17:16 +0900 > Namhyung Kim <namhyung@kernel.org> wrote: > >> On Tue, Aug 23, 2016 at 8:01 AM, Kim Phillips <kim.phillips@arm.com> wrote: >>> On Fri, 19 Aug 2016 18:29:33 +0530 >>> Ravi Bangoria <ravi.bangoria@linux.vnet.ibm.com> wrote: >>> >>>> Changes in v6: >>>> - Instead of adding only those instructions defined in #ifdef __arm__, >>>> add all instructions from default table to arm table. >>> Thanks, I've gone through the list and removed all not-ARM >>> instructions, and added some missing ARM branch instructions: >> Can we use regex patterns instead? > Yes, that helps prevent mistakes updating instruction lists - how does > this look?: > > diff --git a/tools/perf/util/annotate.c b/tools/perf/util/annotate.c > index b2c6cf3..52316f3 100644 > --- a/tools/perf/util/annotate.c > +++ b/tools/perf/util/annotate.c > @@ -26,6 +26,7 @@ > const char *disassembler_style; > const char *objdump_path; > static regex_t file_lineno; > +static regex_t arm_call_insn, arm_jump_insn; > > static struct ins *ins__find(const char *name, const char *norm_arch); > static int disasm_line__parse(char *line, char **namep, char **rawp); > @@ -449,98 +450,7 @@ static struct ins instructions_x86[] = { > { .name = "retq", .ops = &ret_ops, }, > }; > > -static struct ins instructions_arm[] = { > - { .name = "add", .ops = &mov_ops, }, > - { .name = "addl", .ops = &mov_ops, }, > - { .name = "addq", .ops = &mov_ops, }, > - { .name = "addw", .ops = &mov_ops, }, > - { .name = "and", .ops = &mov_ops, }, > - { .name = "b", .ops = &jump_ops, }, /* might also be a call */ > - { .name = "bcc", .ops = &jump_ops, }, > - { .name = "bcs", .ops = &jump_ops, }, > - { .name = "beq", .ops = &jump_ops, }, > - { .name = "bge", .ops = &jump_ops, }, > - { .name = "bgt", .ops = &jump_ops, }, > - { .name = "bhi", .ops = &jump_ops, }, > - { .name = "bl", .ops = &call_ops, }, > - { .name = "bls", .ops = &jump_ops, }, > - { .name = "blt", .ops = &jump_ops, }, > - { .name = "blx", .ops = &call_ops, }, > - { .name = "bne", .ops = &jump_ops, }, > - { .name = "bts", .ops = &mov_ops, }, > - { .name = "call", .ops = &call_ops, }, > - { .name = "callq", .ops = &call_ops, }, > - { .name = "cmp", .ops = &mov_ops, }, > - { .name = "cmpb", .ops = &mov_ops, }, > - { .name = "cmpl", .ops = &mov_ops, }, > - { .name = "cmpq", .ops = &mov_ops, }, > - { .name = "cmpw", .ops = &mov_ops, }, > - { .name = "cmpxch", .ops = &mov_ops, }, > - { .name = "dec", .ops = &dec_ops, }, > - { .name = "decl", .ops = &dec_ops, }, > - { .name = "imul", .ops = &mov_ops, }, > - { .name = "inc", .ops = &dec_ops, }, > - { .name = "incl", .ops = &dec_ops, }, > - { .name = "ja", .ops = &jump_ops, }, > - { .name = "jae", .ops = &jump_ops, }, > - { .name = "jb", .ops = &jump_ops, }, > - { .name = "jbe", .ops = &jump_ops, }, > - { .name = "jc", .ops = &jump_ops, }, > - { .name = "jcxz", .ops = &jump_ops, }, > - { .name = "je", .ops = &jump_ops, }, > - { .name = "jecxz", .ops = &jump_ops, }, > - { .name = "jg", .ops = &jump_ops, }, > - { .name = "jge", .ops = &jump_ops, }, > - { .name = "jl", .ops = &jump_ops, }, > - { .name = "jle", .ops = &jump_ops, }, > - { .name = "jmp", .ops = &jump_ops, }, > - { .name = "jmpq", .ops = &jump_ops, }, > - { .name = "jna", .ops = &jump_ops, }, > - { .name = "jnae", .ops = &jump_ops, }, > - { .name = "jnb", .ops = &jump_ops, }, > - { .name = "jnbe", .ops = &jump_ops, }, > - { .name = "jnc", .ops = &jump_ops, }, > - { .name = "jne", .ops = &jump_ops, }, > - { .name = "jng", .ops = &jump_ops, }, > - { .name = "jnge", .ops = &jump_ops, }, > - { .name = "jnl", .ops = &jump_ops, }, > - { .name = "jnle", .ops = &jump_ops, }, > - { .name = "jno", .ops = &jump_ops, }, > - { .name = "jnp", .ops = &jump_ops, }, > - { .name = "jns", .ops = &jump_ops, }, > - { .name = "jnz", .ops = &jump_ops, }, > - { .name = "jo", .ops = &jump_ops, }, > - { .name = "jp", .ops = &jump_ops, }, > - { .name = "jpe", .ops = &jump_ops, }, > - { .name = "jpo", .ops = &jump_ops, }, > - { .name = "jrcxz", .ops = &jump_ops, }, > - { .name = "js", .ops = &jump_ops, }, > - { .name = "jz", .ops = &jump_ops, }, > - { .name = "lea", .ops = &mov_ops, }, > - { .name = "lock", .ops = &lock_ops, }, > - { .name = "mov", .ops = &mov_ops, }, > - { .name = "movb", .ops = &mov_ops, }, > - { .name = "movdqa",.ops = &mov_ops, }, > - { .name = "movl", .ops = &mov_ops, }, > - { .name = "movq", .ops = &mov_ops, }, > - { .name = "movslq", .ops = &mov_ops, }, > - { .name = "movzbl", .ops = &mov_ops, }, > - { .name = "movzwl", .ops = &mov_ops, }, > - { .name = "nop", .ops = &nop_ops, }, > - { .name = "nopl", .ops = &nop_ops, }, > - { .name = "nopw", .ops = &nop_ops, }, > - { .name = "or", .ops = &mov_ops, }, > - { .name = "orl", .ops = &mov_ops, }, > - { .name = "test", .ops = &mov_ops, }, > - { .name = "testb", .ops = &mov_ops, }, > - { .name = "testl", .ops = &mov_ops, }, > - { .name = "xadd", .ops = &mov_ops, }, > - { .name = "xbeginl", .ops = &jump_ops, }, > - { .name = "xbeginq", .ops = &jump_ops, }, > - { .name = "retq", .ops = &ret_ops, }, > -}; > - > -struct instructions_powerpc { > +struct instructions_arch { > struct ins *ins; > struct list_head list; > }; > @@ -560,41 +470,41 @@ static int ins__cmp(const void *a, const void *b) > return strcmp(ia->name, ib->name); > } > > -static struct ins *list_add__ins_powerpc(struct instructions_powerpc *head, > +static struct ins *list_add__ins_arch(struct instructions_arch *head, > const char *name, struct ins_ops *ops) > { > - struct instructions_powerpc *ins_powerpc; > + struct instructions_arch *ins_arch; > struct ins *ins; > > ins = zalloc(sizeof(struct ins)); > if (!ins) > return NULL; > > - ins_powerpc = zalloc(sizeof(struct instructions_powerpc)); > - if (!ins_powerpc) > + ins_arch = zalloc(sizeof(struct instructions_arch)); > + if (!ins_arch) > goto out_free_ins; > > ins->name = strdup(name); > if (!ins->name) > - goto out_free_ins_power; > + goto out_free_ins_arch; > > ins->ops = ops; > - ins_powerpc->ins = ins; > - list_add_tail(&(ins_powerpc->list), &(head->list)); > + ins_arch->ins = ins; > + list_add_tail(&(ins_arch->list), &(head->list)); > > return ins; > > -out_free_ins_power: > - zfree(&ins_powerpc); > +out_free_ins_arch: > + zfree(&ins_arch); > out_free_ins: > zfree(&ins); > return NULL; > } > > -static struct ins *list_search__ins_powerpc(struct instructions_powerpc *head, > +static struct ins *list_search__ins_arch(struct instructions_arch *head, > const char *name) > { > - struct instructions_powerpc *pos; > + struct instructions_arch *pos; > > list_for_each_entry(pos, &head->list, list) { > if (!strcmp(pos->ins->name, name)) > @@ -608,7 +518,7 @@ static struct ins *ins__find_powerpc(const char *name) > int i; > struct ins *ins; > struct ins_ops *ops; > - static struct instructions_powerpc head; > + static struct instructions_arch head; > static bool list_initialized; > > /* > @@ -629,7 +539,7 @@ static struct ins *ins__find_powerpc(const char *name) > /* > * Return if we already have object of 'struct ins' for this instruction > */ > - ins = list_search__ins_powerpc(&head, name); > + ins = list_search__ins_arch(&head, name); > if (ins) > return ins; > > @@ -666,7 +576,44 @@ static struct ins *ins__find_powerpc(const char *name) > * Add instruction to list so next time no need to > * allocate memory for it. > */ > - return list_add__ins_powerpc(&head, name, ops); > + return list_add__ins_arch(&head, name, ops); > +} > + > +static struct ins *ins__find_arm(const char *name) > +{ > + struct ins *ins; > + struct ins_ops *ops = &mov_ops; > + static struct instructions_arch head; > + static bool list_initialized; > + regmatch_t match[2]; > + int ret; > + > + if (!list_initialized) { > + INIT_LIST_HEAD(&head.list); > + list_initialized = true; > + } > + > + /* > + * Return if we already have object of 'struct ins' for this instruction > + */ > + ins = list_search__ins_arch(&head, name); > + if (ins) > + return ins; > + > + ret = regexec(&arm_call_insn, name, 2, match, 0); > + if (!ret) { > + ops = &call_ops; > + } else { > + ret = regexec(&arm_jump_insn, name, 2, match, 0); > + if (!ret) > + ops = &jump_ops; > + } > + > + /* > + * Add instruction to list so next time no need to > + * allocate memory for it. > + */ > + return list_add__ins_arch(&head, name, ops); > } > > static void ins__sort(struct ins *instructions, int nmemb) > @@ -686,15 +633,26 @@ static const char *annotate__norm_arch(char *arch) > return normalize_arch(arch); > } > > +#define ARM_CONDS "(cc|cs|eq|ge|gt|hi|le|ls|lt|mi|ne|pl|vc|vs)" > + > static struct ins *ins__find(const char *name, const char *norm_arch) > { > static bool sorted; > struct ins *instructions; > - int nmemb; > + int nmemb, ret; > > if (!sorted) { > ins__sort(instructions_x86, ARRAY_SIZE(instructions_x86)); > - ins__sort(instructions_arm, ARRAY_SIZE(instructions_arm)); > + if (!strcmp(norm_arch, NORM_ARM)) { > + ret = regcomp(&arm_call_insn, > + "^blx?" ARM_CONDS "?$", REG_EXTENDED); > + ret |= regcomp(&arm_jump_insn, > + "^bx?" ARM_CONDS "?$", REG_EXTENDED); > + if (ret) { > + pr_err("regcomp failed\n"); > + return NULL; > + } > + } > sorted = true; > } > > @@ -702,8 +660,7 @@ static struct ins *ins__find(const char *name, const char *norm_arch) > instructions = instructions_x86; > nmemb = ARRAY_SIZE(instructions_x86); > } else if (!strcmp(norm_arch, NORM_ARM)) { > - instructions = instructions_arm; > - nmemb = ARRAY_SIZE(instructions_arm); > + return ins__find_arm(name); > } else if (!strcmp(norm_arch, NORM_POWERPC)) { > return ins__find_powerpc(name); > } else { > > Note that, for ARM, in order to match return instructions, 'lr' must be > found to be the operand of the branch instruction, which is not > contained in the 'name' variable passed to ins__find(). > > Kim > ^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH] perf annotate: cross arch annotate support fixes for ARM 2016-08-26 7:26 ` Ravi Bangoria @ 2016-08-27 0:40 ` Kim Phillips 0 siblings, 0 replies; 19+ messages in thread From: Kim Phillips @ 2016-08-27 0:40 UTC (permalink / raw) To: Ravi Bangoria Cc: Namhyung Kim, linux-kernel@vger.kernel.org, linuxppc-dev, Arnaldo Carvalho de Melo, Peter Zijlstra, Ingo Molnar, Alexander Shishkin, Taeung Song, Naveen N. Rao, markus, chris.ryder, Pawel Moll, Masami Hiramatsu, rmk+kernel, Jiri Olsa, Michael Ellerman, Hemant Kumar For ARM we remove the list that contains non-arm insns, and instead add more maintainable branch instruction regex logic. Signed-off-by: Kim Phillips <kim.phillips@arm.com> Acked-by: Ravi Bangoria <ravi.bangoria@linux.vnet.ibm.com> Cc: Namhyung Kim <namhyung@kernel.org> --- tools/perf/util/annotate.c | 177 +++++++++++++++++---------------------------- 1 file changed, 67 insertions(+), 110 deletions(-) diff --git a/tools/perf/util/annotate.c b/tools/perf/util/annotate.c index b2c6cf3..52316f3 100644 --- a/tools/perf/util/annotate.c +++ b/tools/perf/util/annotate.c @@ -26,6 +26,7 @@ const char *disassembler_style; const char *objdump_path; static regex_t file_lineno; +static regex_t arm_call_insn, arm_jump_insn; static struct ins *ins__find(const char *name, const char *norm_arch); static int disasm_line__parse(char *line, char **namep, char **rawp); @@ -449,98 +450,7 @@ static struct ins instructions_x86[] = { { .name = "retq", .ops = &ret_ops, }, }; -static struct ins instructions_arm[] = { - { .name = "add", .ops = &mov_ops, }, - { .name = "addl", .ops = &mov_ops, }, - { .name = "addq", .ops = &mov_ops, }, - { .name = "addw", .ops = &mov_ops, }, - { .name = "and", .ops = &mov_ops, }, - { .name = "b", .ops = &jump_ops, }, /* might also be a call */ - { .name = "bcc", .ops = &jump_ops, }, - { .name = "bcs", .ops = &jump_ops, }, - { .name = "beq", .ops = &jump_ops, }, - { .name = "bge", .ops = &jump_ops, }, - { .name = "bgt", .ops = &jump_ops, }, - { .name = "bhi", .ops = &jump_ops, }, - { .name = "bl", .ops = &call_ops, }, - { .name = "bls", .ops = &jump_ops, }, - { .name = "blt", .ops = &jump_ops, }, - { .name = "blx", .ops = &call_ops, }, - { .name = "bne", .ops = &jump_ops, }, - { .name = "bts", .ops = &mov_ops, }, - { .name = "call", .ops = &call_ops, }, - { .name = "callq", .ops = &call_ops, }, - { .name = "cmp", .ops = &mov_ops, }, - { .name = "cmpb", .ops = &mov_ops, }, - { .name = "cmpl", .ops = &mov_ops, }, - { .name = "cmpq", .ops = &mov_ops, }, - { .name = "cmpw", .ops = &mov_ops, }, - { .name = "cmpxch", .ops = &mov_ops, }, - { .name = "dec", .ops = &dec_ops, }, - { .name = "decl", .ops = &dec_ops, }, - { .name = "imul", .ops = &mov_ops, }, - { .name = "inc", .ops = &dec_ops, }, - { .name = "incl", .ops = &dec_ops, }, - { .name = "ja", .ops = &jump_ops, }, - { .name = "jae", .ops = &jump_ops, }, - { .name = "jb", .ops = &jump_ops, }, - { .name = "jbe", .ops = &jump_ops, }, - { .name = "jc", .ops = &jump_ops, }, - { .name = "jcxz", .ops = &jump_ops, }, - { .name = "je", .ops = &jump_ops, }, - { .name = "jecxz", .ops = &jump_ops, }, - { .name = "jg", .ops = &jump_ops, }, - { .name = "jge", .ops = &jump_ops, }, - { .name = "jl", .ops = &jump_ops, }, - { .name = "jle", .ops = &jump_ops, }, - { .name = "jmp", .ops = &jump_ops, }, - { .name = "jmpq", .ops = &jump_ops, }, - { .name = "jna", .ops = &jump_ops, }, - { .name = "jnae", .ops = &jump_ops, }, - { .name = "jnb", .ops = &jump_ops, }, - { .name = "jnbe", .ops = &jump_ops, }, - { .name = "jnc", .ops = &jump_ops, }, - { .name = "jne", .ops = &jump_ops, }, - { .name = "jng", .ops = &jump_ops, }, - { .name = "jnge", .ops = &jump_ops, }, - { .name = "jnl", .ops = &jump_ops, }, - { .name = "jnle", .ops = &jump_ops, }, - { .name = "jno", .ops = &jump_ops, }, - { .name = "jnp", .ops = &jump_ops, }, - { .name = "jns", .ops = &jump_ops, }, - { .name = "jnz", .ops = &jump_ops, }, - { .name = "jo", .ops = &jump_ops, }, - { .name = "jp", .ops = &jump_ops, }, - { .name = "jpe", .ops = &jump_ops, }, - { .name = "jpo", .ops = &jump_ops, }, - { .name = "jrcxz", .ops = &jump_ops, }, - { .name = "js", .ops = &jump_ops, }, - { .name = "jz", .ops = &jump_ops, }, - { .name = "lea", .ops = &mov_ops, }, - { .name = "lock", .ops = &lock_ops, }, - { .name = "mov", .ops = &mov_ops, }, - { .name = "movb", .ops = &mov_ops, }, - { .name = "movdqa",.ops = &mov_ops, }, - { .name = "movl", .ops = &mov_ops, }, - { .name = "movq", .ops = &mov_ops, }, - { .name = "movslq", .ops = &mov_ops, }, - { .name = "movzbl", .ops = &mov_ops, }, - { .name = "movzwl", .ops = &mov_ops, }, - { .name = "nop", .ops = &nop_ops, }, - { .name = "nopl", .ops = &nop_ops, }, - { .name = "nopw", .ops = &nop_ops, }, - { .name = "or", .ops = &mov_ops, }, - { .name = "orl", .ops = &mov_ops, }, - { .name = "test", .ops = &mov_ops, }, - { .name = "testb", .ops = &mov_ops, }, - { .name = "testl", .ops = &mov_ops, }, - { .name = "xadd", .ops = &mov_ops, }, - { .name = "xbeginl", .ops = &jump_ops, }, - { .name = "xbeginq", .ops = &jump_ops, }, - { .name = "retq", .ops = &ret_ops, }, -}; - -struct instructions_powerpc { +struct instructions_arch { struct ins *ins; struct list_head list; }; @@ -560,41 +470,41 @@ static int ins__cmp(const void *a, const void *b) return strcmp(ia->name, ib->name); } -static struct ins *list_add__ins_powerpc(struct instructions_powerpc *head, +static struct ins *list_add__ins_arch(struct instructions_arch *head, const char *name, struct ins_ops *ops) { - struct instructions_powerpc *ins_powerpc; + struct instructions_arch *ins_arch; struct ins *ins; ins = zalloc(sizeof(struct ins)); if (!ins) return NULL; - ins_powerpc = zalloc(sizeof(struct instructions_powerpc)); - if (!ins_powerpc) + ins_arch = zalloc(sizeof(struct instructions_arch)); + if (!ins_arch) goto out_free_ins; ins->name = strdup(name); if (!ins->name) - goto out_free_ins_power; + goto out_free_ins_arch; ins->ops = ops; - ins_powerpc->ins = ins; - list_add_tail(&(ins_powerpc->list), &(head->list)); + ins_arch->ins = ins; + list_add_tail(&(ins_arch->list), &(head->list)); return ins; -out_free_ins_power: - zfree(&ins_powerpc); +out_free_ins_arch: + zfree(&ins_arch); out_free_ins: zfree(&ins); return NULL; } -static struct ins *list_search__ins_powerpc(struct instructions_powerpc *head, +static struct ins *list_search__ins_arch(struct instructions_arch *head, const char *name) { - struct instructions_powerpc *pos; + struct instructions_arch *pos; list_for_each_entry(pos, &head->list, list) { if (!strcmp(pos->ins->name, name)) @@ -608,7 +518,7 @@ static struct ins *ins__find_powerpc(const char *name) int i; struct ins *ins; struct ins_ops *ops; - static struct instructions_powerpc head; + static struct instructions_arch head; static bool list_initialized; /* @@ -629,7 +539,7 @@ static struct ins *ins__find_powerpc(const char *name) /* * Return if we already have object of 'struct ins' for this instruction */ - ins = list_search__ins_powerpc(&head, name); + ins = list_search__ins_arch(&head, name); if (ins) return ins; @@ -666,7 +576,44 @@ static struct ins *ins__find_powerpc(const char *name) * Add instruction to list so next time no need to * allocate memory for it. */ - return list_add__ins_powerpc(&head, name, ops); + return list_add__ins_arch(&head, name, ops); +} + +static struct ins *ins__find_arm(const char *name) +{ + struct ins *ins; + struct ins_ops *ops = &mov_ops; + static struct instructions_arch head; + static bool list_initialized; + regmatch_t match[2]; + int ret; + + if (!list_initialized) { + INIT_LIST_HEAD(&head.list); + list_initialized = true; + } + + /* + * Return if we already have object of 'struct ins' for this instruction + */ + ins = list_search__ins_arch(&head, name); + if (ins) + return ins; + + ret = regexec(&arm_call_insn, name, 2, match, 0); + if (!ret) { + ops = &call_ops; + } else { + ret = regexec(&arm_jump_insn, name, 2, match, 0); + if (!ret) + ops = &jump_ops; + } + + /* + * Add instruction to list so next time no need to + * allocate memory for it. + */ + return list_add__ins_arch(&head, name, ops); } static void ins__sort(struct ins *instructions, int nmemb) @@ -686,15 +633,26 @@ static const char *annotate__norm_arch(char *arch) return normalize_arch(arch); } +#define ARM_CONDS "(cc|cs|eq|ge|gt|hi|le|ls|lt|mi|ne|pl|vc|vs)" + static struct ins *ins__find(const char *name, const char *norm_arch) { static bool sorted; struct ins *instructions; - int nmemb; + int nmemb, ret; if (!sorted) { ins__sort(instructions_x86, ARRAY_SIZE(instructions_x86)); - ins__sort(instructions_arm, ARRAY_SIZE(instructions_arm)); + if (!strcmp(norm_arch, NORM_ARM)) { + ret = regcomp(&arm_call_insn, + "^blx?" ARM_CONDS "?$", REG_EXTENDED); + ret |= regcomp(&arm_jump_insn, + "^bx?" ARM_CONDS "?$", REG_EXTENDED); + if (ret) { + pr_err("regcomp failed\n"); + return NULL; + } + } sorted = true; } @@ -702,8 +660,7 @@ static struct ins *ins__find(const char *name, const char *norm_arch) instructions = instructions_x86; nmemb = ARRAY_SIZE(instructions_x86); } else if (!strcmp(norm_arch, NORM_ARM)) { - instructions = instructions_arm; - nmemb = ARRAY_SIZE(instructions_arm); + return ins__find_arm(name); } else if (!strcmp(norm_arch, NORM_POWERPC)) { return ins__find_powerpc(name); } else { -- 2.9.3 ^ permalink raw reply related [flat|nested] 19+ messages in thread
* [PATCH v6 3/7] perf annotate: Add support for powerpc 2016-08-19 12:59 [PATCH v6 0/7] perf: Cross arch annotate + few miscellaneous fixes Ravi Bangoria 2016-08-19 12:59 ` [PATCH v6 1/7] perf: Define macro for normalized arch names Ravi Bangoria 2016-08-19 12:59 ` [PATCH v6 2/7] perf annotate: Add cross arch annotate support Ravi Bangoria @ 2016-08-19 12:59 ` Ravi Bangoria 2016-09-19 15:26 ` Arnaldo Carvalho de Melo 2016-08-19 12:59 ` [PATCH v6 4/7] perf annotate: Do not ignore call instruction with indirect target Ravi Bangoria ` (4 subsequent siblings) 7 siblings, 1 reply; 19+ messages in thread From: Ravi Bangoria @ 2016-08-19 12:59 UTC (permalink / raw) To: linux-kernel, linuxppc-dev, acme Cc: peterz, mingo, alexander.shishkin, treeze.taeung, naveen.n.rao, markus, chris.ryder, pawel.moll, mhiramat, rmk+kernel, jolsa, mpe, hemant, namhyung, Ravi Bangoria From: "Naveen N. Rao" <naveen.n.rao@linux.vnet.ibm.com> Current perf can disassemble annotated function but it does not have parsing logic for powerpc instructions. So all navigation options are not available for powerpc. Apart from that, Powerpc has long list of branch instructions and hardcoding them in table appears to be error-prone. So, add function to find instruction instead of creating table. This function dynamically create table (list of 'struct ins'), and instead of creating object every time, first check if list already contain object for that instruction. Signed-off-by: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com> Signed-off-by: Ravi Bangoria <ravi.bangoria@linux.vnet.ibm.com> --- Changes in v6: - No change tools/perf/util/annotate.c | 116 +++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 116 insertions(+) diff --git a/tools/perf/util/annotate.c b/tools/perf/util/annotate.c index 14a8808..ea07588 100644 --- a/tools/perf/util/annotate.c +++ b/tools/perf/util/annotate.c @@ -535,6 +535,11 @@ static struct ins instructions_arm[] = { { .name = "retq", .ops = &ret_ops, }, }; +struct instructions_powerpc { + struct ins *ins; + struct list_head list; +}; + static int ins__key_cmp(const void *name, const void *insp) { const struct ins *ins = insp; @@ -550,6 +555,115 @@ static int ins__cmp(const void *a, const void *b) return strcmp(ia->name, ib->name); } +static struct ins *list_add__ins_powerpc(struct instructions_powerpc *head, + const char *name, struct ins_ops *ops) +{ + struct instructions_powerpc *ins_powerpc; + struct ins *ins; + + ins = zalloc(sizeof(struct ins)); + if (!ins) + return NULL; + + ins_powerpc = zalloc(sizeof(struct instructions_powerpc)); + if (!ins_powerpc) + goto out_free_ins; + + ins->name = strdup(name); + if (!ins->name) + goto out_free_ins_power; + + ins->ops = ops; + ins_powerpc->ins = ins; + list_add_tail(&(ins_powerpc->list), &(head->list)); + + return ins; + +out_free_ins_power: + zfree(&ins_powerpc); +out_free_ins: + zfree(&ins); + return NULL; +} + +static struct ins *list_search__ins_powerpc(struct instructions_powerpc *head, + const char *name) +{ + struct instructions_powerpc *pos; + + list_for_each_entry(pos, &head->list, list) { + if (!strcmp(pos->ins->name, name)) + return pos->ins; + } + return NULL; +} + +static struct ins *ins__find_powerpc(const char *name) +{ + int i; + struct ins *ins; + struct ins_ops *ops; + static struct instructions_powerpc head; + static bool list_initialized; + + /* + * - Interested only if instruction starts with 'b'. + * - Few start with 'b', but aren't branch instructions. + */ + if (name[0] != 'b' || + !strncmp(name, "bcd", 3) || + !strncmp(name, "brinc", 5) || + !strncmp(name, "bper", 4)) + return NULL; + + if (!list_initialized) { + INIT_LIST_HEAD(&head.list); + list_initialized = true; + } + + /* + * Return if we already have object of 'struct ins' for this instruction + */ + ins = list_search__ins_powerpc(&head, name); + if (ins) + return ins; + + ops = &jump_ops; + + i = strlen(name) - 1; + if (i < 0) + return NULL; + + /* ignore optional hints at the end of the instructions */ + if (name[i] == '+' || name[i] == '-') + i--; + + if (name[i] == 'l' || (name[i] == 'a' && name[i-1] == 'l')) { + /* + * if the instruction ends up with 'l' or 'la', then + * those are considered 'calls' since they update LR. + * ... except for 'bnl' which is branch if not less than + * and the absolute form of the same. + */ + if (strcmp(name, "bnl") && strcmp(name, "bnl+") && + strcmp(name, "bnl-") && strcmp(name, "bnla") && + strcmp(name, "bnla+") && strcmp(name, "bnla-")) + ops = &call_ops; + } + if (name[i] == 'r' && name[i-1] == 'l') + /* + * instructions ending with 'lr' are considered to be + * return instructions + */ + ops = &ret_ops; + + /* + * Add instruction to list so next time no need to + * allocate memory for it. + */ + return list_add__ins_powerpc(&head, name, ops); +} + static void ins__sort(struct ins *instructions, int nmemb) { qsort(instructions, nmemb, sizeof(struct ins), ins__cmp); @@ -585,6 +699,8 @@ static struct ins *ins__find(const char *name, const char *norm_arch) } else if (!strcmp(norm_arch, NORM_ARM)) { instructions = instructions_arm; nmemb = ARRAY_SIZE(instructions_arm); + } else if (!strcmp(norm_arch, NORM_POWERPC)) { + return ins__find_powerpc(name); } else { pr_err("perf annotate not supported by %s arch\n", norm_arch); return NULL; -- 2.5.5 ^ permalink raw reply related [flat|nested] 19+ messages in thread
* Re: [PATCH v6 3/7] perf annotate: Add support for powerpc 2016-08-19 12:59 ` [PATCH v6 3/7] perf annotate: Add support for powerpc Ravi Bangoria @ 2016-09-19 15:26 ` Arnaldo Carvalho de Melo 0 siblings, 0 replies; 19+ messages in thread From: Arnaldo Carvalho de Melo @ 2016-09-19 15:26 UTC (permalink / raw) To: Ravi Bangoria Cc: linux-kernel, linuxppc-dev, peterz, mingo, alexander.shishkin, treeze.taeung, naveen.n.rao, markus, chris.ryder, pawel.moll, mhiramat, rmk+kernel, jolsa, mpe, hemant, namhyung Em Fri, Aug 19, 2016 at 06:29:34PM +0530, Ravi Bangoria escreveu: > From: "Naveen N. Rao" <naveen.n.rao@linux.vnet.ibm.com> > > +static struct ins *ins__find_powerpc(const char *name) > +{ > + int i; > + struct ins *ins; > + struct ins_ops *ops; > + static struct instructions_powerpc head; > + static bool list_initialized; > + > + /* > + * - Interested only if instruction starts with 'b'. > + * - Few start with 'b', but aren't branch instructions. > + */ > + if (name[0] != 'b' || > + !strncmp(name, "bcd", 3) || > + !strncmp(name, "brinc", 5) || > + !strncmp(name, "bper", 4)) > + return NULL; > + > + if (!list_initialized) { > + INIT_LIST_HEAD(&head.list); > + list_initialized = true; > + } Why not ditch list_initialized and instead just declare the list as: static struct instructions_powerpc head = { .list = LIST_HEAD_INIT(head.list), } Just like the kernel sources do? See for instance: net/core/net_namespace.c struct net init_net = { .dev_base_head = LIST_HEAD_INIT(init_net.dev_base_head), }; > + > + /* > + * Return if we already have object of 'struct ins' for this instruction > + */ > + ins = list_search__ins_powerpc(&head, name); > + if (ins) > + return ins; > + > + ops = &jump_ops; > + > + i = strlen(name) - 1; > + if (i < 0) > + return NULL; > + > + /* ignore optional hints at the end of the instructions */ > + if (name[i] == '+' || name[i] == '-') > + i--; > + > + if (name[i] == 'l' || (name[i] == 'a' && name[i-1] == 'l')) { > + /* > + * if the instruction ends up with 'l' or 'la', then > + * those are considered 'calls' since they update LR. > + * ... except for 'bnl' which is branch if not less than > + * and the absolute form of the same. > + */ > + if (strcmp(name, "bnl") && strcmp(name, "bnl+") && > + strcmp(name, "bnl-") && strcmp(name, "bnla") && > + strcmp(name, "bnla+") && strcmp(name, "bnla-")) > + ops = &call_ops; > + } > + if (name[i] == 'r' && name[i-1] == 'l') > + /* > + * instructions ending with 'lr' are considered to be > + * return instructions > + */ > + ops = &ret_ops; > + > + /* > + * Add instruction to list so next time no need to > + * allocate memory for it. > + */ > + return list_add__ins_powerpc(&head, name, ops); > +} > + > static void ins__sort(struct ins *instructions, int nmemb) > { > qsort(instructions, nmemb, sizeof(struct ins), ins__cmp); > @@ -585,6 +699,8 @@ static struct ins *ins__find(const char *name, const char *norm_arch) > } else if (!strcmp(norm_arch, NORM_ARM)) { > instructions = instructions_arm; > nmemb = ARRAY_SIZE(instructions_arm); > + } else if (!strcmp(norm_arch, NORM_POWERPC)) { > + return ins__find_powerpc(name); > } else { > pr_err("perf annotate not supported by %s arch\n", norm_arch); > return NULL; > -- > 2.5.5 ^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH v6 4/7] perf annotate: Do not ignore call instruction with indirect target 2016-08-19 12:59 [PATCH v6 0/7] perf: Cross arch annotate + few miscellaneous fixes Ravi Bangoria ` (2 preceding siblings ...) 2016-08-19 12:59 ` [PATCH v6 3/7] perf annotate: Add support for powerpc Ravi Bangoria @ 2016-08-19 12:59 ` Ravi Bangoria 2016-09-19 15:44 ` Arnaldo Carvalho de Melo 2016-08-19 12:59 ` [PATCH v6 5/7] perf annotate: Show raw form for jump " Ravi Bangoria ` (3 subsequent siblings) 7 siblings, 1 reply; 19+ messages in thread From: Ravi Bangoria @ 2016-08-19 12:59 UTC (permalink / raw) To: linux-kernel, linuxppc-dev, acme Cc: peterz, mingo, alexander.shishkin, treeze.taeung, naveen.n.rao, markus, chris.ryder, pawel.moll, mhiramat, rmk+kernel, jolsa, mpe, hemant, namhyung, Ravi Bangoria Do not ignore call instruction with indirect target when its already identified as a call. This is an extension of commit e8ea1561952b ("perf annotate: Use raw form for register indirect call instructions") to generalize annotation for all instructions with indirect calls. This is needed for certain powerpc call instructions that use address in a register (such as bctrl, btarl, ...). Apart from that, when kcore is used to disassemble function, all call instructions were ignored. This patch will fix it as a side effect by not ignoring them. For example, Before (with kcore): mov %r13,%rdi callq 0xffffffff811a7e70 ^ jmpq 64 mov %gs:0x7ef41a6e(%rip),%al After (with kcore): mov %r13,%rdi > callq 0xffffffff811a7e70 ^ jmpq 64 mov %gs:0x7ef41a6e(%rip),%al Suggested-by: Michael Ellerman <mpe@ellerman.id.au> [Suggested about 'bctrl' instruction] Signed-off-by: Ravi Bangoria <ravi.bangoria@linux.vnet.ibm.com> --- Changes in v6: - No change tools/perf/util/annotate.c | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/tools/perf/util/annotate.c b/tools/perf/util/annotate.c index ea07588..a05423b 100644 --- a/tools/perf/util/annotate.c +++ b/tools/perf/util/annotate.c @@ -81,16 +81,12 @@ static int call__parse(struct ins_operands *ops, const char *norm_arch) return ops->target.name == NULL ? -1 : 0; indirect_call: - tok = strchr(endptr, '('); - if (tok != NULL) { + tok = strchr(endptr, '*'); + if (tok == NULL) { ops->target.addr = 0; return 0; } - tok = strchr(endptr, '*'); - if (tok == NULL) - return -1; - ops->target.addr = strtoull(tok + 1, NULL, 16); return 0; } -- 2.5.5 ^ permalink raw reply related [flat|nested] 19+ messages in thread
* Re: [PATCH v6 4/7] perf annotate: Do not ignore call instruction with indirect target 2016-08-19 12:59 ` [PATCH v6 4/7] perf annotate: Do not ignore call instruction with indirect target Ravi Bangoria @ 2016-09-19 15:44 ` Arnaldo Carvalho de Melo 2016-09-20 14:35 ` Ravi Bangoria 0 siblings, 1 reply; 19+ messages in thread From: Arnaldo Carvalho de Melo @ 2016-09-19 15:44 UTC (permalink / raw) To: Ravi Bangoria Cc: linux-kernel, linuxppc-dev, peterz, mingo, alexander.shishkin, treeze.taeung, naveen.n.rao, markus, chris.ryder, pawel.moll, mhiramat, rmk+kernel, jolsa, mpe, hemant, namhyung Em Fri, Aug 19, 2016 at 06:29:35PM +0530, Ravi Bangoria escreveu: > Do not ignore call instruction with indirect target when its already > identified as a call. This is an extension of commit e8ea1561952b > ("perf annotate: Use raw form for register indirect call instructions") > to generalize annotation for all instructions with indirect calls. > > This is needed for certain powerpc call instructions that use address > in a register (such as bctrl, btarl, ...). > > Apart from that, when kcore is used to disassemble function, all call > instructions were ignored. This patch will fix it as a side effect by > not ignoring them. For example, > > Before (with kcore): > mov %r13,%rdi > callq 0xffffffff811a7e70 > ^ jmpq 64 > mov %gs:0x7ef41a6e(%rip),%al > > After (with kcore): > mov %r13,%rdi > > callq 0xffffffff811a7e70 > ^ jmpq 64 > mov %gs:0x7ef41a6e(%rip),%al Ok, makes sense, but then now I have the -> and can't press enter to go to that function, in fact for the case I'm using as a test, the vsnprintf kernel function, I get: │ 56: test %al,%al ▒ │ ↓ je 81 ▒ │ lea -0x38(%rbp),%rsi ▒ │ mov %r15,%rdi ▒ │ → callq 0xffffffff993e3230 That 0xffffffff993e3230 should've been resolved to: [root@jouet ~]# grep ffffffff993e3230 /proc/kallsyms ffffffff993e3230 t format_decode Trying to investigate why it doesn't... - Arnaldo > Suggested-by: Michael Ellerman <mpe@ellerman.id.au> > [Suggested about 'bctrl' instruction] > Signed-off-by: Ravi Bangoria <ravi.bangoria@linux.vnet.ibm.com> > --- > Changes in v6: > - No change > > tools/perf/util/annotate.c | 8 ++------ > 1 file changed, 2 insertions(+), 6 deletions(-) > > diff --git a/tools/perf/util/annotate.c b/tools/perf/util/annotate.c > index ea07588..a05423b 100644 > --- a/tools/perf/util/annotate.c > +++ b/tools/perf/util/annotate.c > @@ -81,16 +81,12 @@ static int call__parse(struct ins_operands *ops, const char *norm_arch) > return ops->target.name == NULL ? -1 : 0; > > indirect_call: > - tok = strchr(endptr, '('); > - if (tok != NULL) { > + tok = strchr(endptr, '*'); > + if (tok == NULL) { > ops->target.addr = 0; > return 0; > } > > - tok = strchr(endptr, '*'); > - if (tok == NULL) > - return -1; > - > ops->target.addr = strtoull(tok + 1, NULL, 16); > return 0; > } > -- > 2.5.5 ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v6 4/7] perf annotate: Do not ignore call instruction with indirect target 2016-09-19 15:44 ` Arnaldo Carvalho de Melo @ 2016-09-20 14:35 ` Ravi Bangoria 2016-09-20 14:56 ` Arnaldo Carvalho de Melo 0 siblings, 1 reply; 19+ messages in thread From: Ravi Bangoria @ 2016-09-20 14:35 UTC (permalink / raw) To: Arnaldo Carvalho de Melo Cc: linux-kernel, linuxppc-dev, peterz, mingo, alexander.shishkin, treeze.taeung, naveen.n.rao, markus, chris.ryder, pawel.moll, mhiramat, rmk+kernel, jolsa, mpe, hemant, namhyung, Ravi Bangoria Hi Arnaldo, On Monday 19 September 2016 09:14 PM, Arnaldo Carvalho de Melo wrote: > Em Fri, Aug 19, 2016 at 06:29:35PM +0530, Ravi Bangoria escreveu: >> Do not ignore call instruction with indirect target when its already >> identified as a call. This is an extension of commit e8ea1561952b >> ("perf annotate: Use raw form for register indirect call instructions") >> to generalize annotation for all instructions with indirect calls. >> >> This is needed for certain powerpc call instructions that use address >> in a register (such as bctrl, btarl, ...). >> >> Apart from that, when kcore is used to disassemble function, all call >> instructions were ignored. This patch will fix it as a side effect by >> not ignoring them. For example, >> >> Before (with kcore): >> mov %r13,%rdi >> callq 0xffffffff811a7e70 >> ^ jmpq 64 >> mov %gs:0x7ef41a6e(%rip),%al >> >> After (with kcore): >> mov %r13,%rdi >> > callq 0xffffffff811a7e70 >> ^ jmpq 64 >> mov %gs:0x7ef41a6e(%rip),%al > Ok, makes sense, but then now I have the -> and can't press enter to go > to that function, in fact for the case I'm using as a test, the > vsnprintf kernel function, I get: > > │ 56: test %al,%al ▒ > │ ↓ je 81 ▒ > │ lea -0x38(%rbp),%rsi ▒ > │ mov %r15,%rdi ▒ > │ → callq 0xffffffff993e3230 > > That 0xffffffff993e3230 should've been resolved to: > > [root@jouet ~]# grep ffffffff993e3230 /proc/kallsyms > ffffffff993e3230 t format_decode > > Trying to investigate why it doesn't... I investigated this. If this example is with kcore, then it's expected. Because, perf annotate does not inspect kallsyms when it can't find symbol name from disassembly itself. For example, disassembly of finish_task_switch, with kcore: ffffffff810cf1b0: mov $0x1,%esi ffffffff810cf1b5: mov $0x4,%edi ffffffff810cf1ba: callq 0xffffffff811aced0 ffffffff810cf1bf: andb $0xfb,0x4c4(%rbx) ffffffff810cf1c6: jmpq 0xffffffff810cf0e9 ffffffff810cf1cb: mov %rbx,%rsi ffffffff810cf1ce: mov %r13,%rdi ffffffff810cf1d1: callq 0xffffffff811a7e70 ffffffff810cf1d6: jmpq 0xffffffff810cf0e4 with debuginfo: ffffffff810cf1b0: mov $0x1,%esi ffffffff810cf1b5: mov $0x4,%edi ffffffff810cf1ba: callq ffffffff811aced0 <___perf_sw_event> ffffffff810cf1bf: andb $0xfb,0x4c4(%rbx) ffffffff810cf1c6: jmpq ffffffff810cf0e9 <finish_task_switch+0x69> ffffffff810cf1cb: mov %rbx,%rsi ffffffff810cf1ce: mov %r13,%rdi ffffffff810cf1d1: callq ffffffff811a7e70 <__perf_event_task_sched_in> ffffffff810cf1d6: jmpq ffffffff810cf0e4 <finish_task_switch+0x64> call__parse tries to find symbol from angle brackets which is not present in case of kcore. -Ravi > - Arnaldo > >> Suggested-by: Michael Ellerman <mpe@ellerman.id.au> >> [Suggested about 'bctrl' instruction] >> Signed-off-by: Ravi Bangoria <ravi.bangoria@linux.vnet.ibm.com> >> --- >> Changes in v6: >> - No change >> >> tools/perf/util/annotate.c | 8 ++------ >> 1 file changed, 2 insertions(+), 6 deletions(-) >> >> diff --git a/tools/perf/util/annotate.c b/tools/perf/util/annotate.c >> index ea07588..a05423b 100644 >> --- a/tools/perf/util/annotate.c >> +++ b/tools/perf/util/annotate.c >> @@ -81,16 +81,12 @@ static int call__parse(struct ins_operands *ops, const char *norm_arch) >> return ops->target.name == NULL ? -1 : 0; >> >> indirect_call: >> - tok = strchr(endptr, '('); >> - if (tok != NULL) { >> + tok = strchr(endptr, '*'); >> + if (tok == NULL) { >> ops->target.addr = 0; >> return 0; >> } >> >> - tok = strchr(endptr, '*'); >> - if (tok == NULL) >> - return -1; >> - >> ops->target.addr = strtoull(tok + 1, NULL, 16); >> return 0; >> } >> -- >> 2.5.5 ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v6 4/7] perf annotate: Do not ignore call instruction with indirect target 2016-09-20 14:35 ` Ravi Bangoria @ 2016-09-20 14:56 ` Arnaldo Carvalho de Melo 0 siblings, 0 replies; 19+ messages in thread From: Arnaldo Carvalho de Melo @ 2016-09-20 14:56 UTC (permalink / raw) To: Ravi Bangoria Cc: linux-kernel, linuxppc-dev, peterz, mingo, alexander.shishkin, treeze.taeung, naveen.n.rao, markus, chris.ryder, pawel.moll, mhiramat, rmk+kernel, jolsa, mpe, hemant, namhyung Em Tue, Sep 20, 2016 at 08:05:03PM +0530, Ravi Bangoria escreveu: > On Monday 19 September 2016 09:14 PM, Arnaldo Carvalho de Melo wrote: > > Em Fri, Aug 19, 2016 at 06:29:35PM +0530, Ravi Bangoria escreveu: > >> Do not ignore call instruction with indirect target when its already > >> identified as a call. This is an extension of commit e8ea1561952b > >> ("perf annotate: Use raw form for register indirect call instructions") > >> to generalize annotation for all instructions with indirect calls. > >> This is needed for certain powerpc call instructions that use address > >> in a register (such as bctrl, btarl, ...). > >> > >> Apart from that, when kcore is used to disassemble function, all call > >> instructions were ignored. This patch will fix it as a side effect by > >> not ignoring them. For example, > >> > >> Before (with kcore): > >> mov %r13,%rdi > >> callq 0xffffffff811a7e70 > >> ^ jmpq 64 > >> mov %gs:0x7ef41a6e(%rip),%al > >> > >> After (with kcore): > >> mov %r13,%rdi > >> > callq 0xffffffff811a7e70 > >> ^ jmpq 64 > >> mov %gs:0x7ef41a6e(%rip),%al > > Ok, makes sense, but then now I have the -> and can't press enter to go > > to that function, in fact for the case I'm using as a test, the > > vsnprintf kernel function, I get: > > > > │ 56: test %al,%al > > │ ↓ je 81 > > │ lea -0x38(%rbp),%rsi > > │ mov %r15,%rdi > > │ → callq 0xffffffff993e3230 > > > > That 0xffffffff993e3230 should've been resolved to: > > > > [root@jouet ~]# grep ffffffff993e3230 /proc/kallsyms > > ffffffff993e3230 t format_decode > > > > Trying to investigate why it doesn't... > > I investigated this. > > If this example is with kcore, then it's expected. Because, perf annotate does > not inspect kallsyms when it can't find symbol name from disassembly itself. > > For example, disassembly of finish_task_switch, Yeah, that was the case, up to yesterday, please take a look at my perf/core branch, specifically this patch: https://git.kernel.org/cgit/linux/kernel/git/acme/linux.git/commit/?id=4de86ebeb433aa59764a13ce851cba08d747e0e1 I'll do that for other instructions as well, like with 'mov $0xabcdef,%rax' that will try to resolve that address to a variable. Will try to continue processing your (and others) patches first tho :-) - Arnaldo > with kcore: > > ffffffff810cf1b0: mov $0x1,%esi > ffffffff810cf1b5: mov $0x4,%edi > ffffffff810cf1ba: callq 0xffffffff811aced0 > ffffffff810cf1bf: andb $0xfb,0x4c4(%rbx) > ffffffff810cf1c6: jmpq 0xffffffff810cf0e9 > ffffffff810cf1cb: mov %rbx,%rsi > ffffffff810cf1ce: mov %r13,%rdi > ffffffff810cf1d1: callq 0xffffffff811a7e70 > ffffffff810cf1d6: jmpq 0xffffffff810cf0e4 > > with debuginfo: > ffffffff810cf1b0: mov $0x1,%esi > ffffffff810cf1b5: mov $0x4,%edi > ffffffff810cf1ba: callq ffffffff811aced0 <___perf_sw_event> > ffffffff810cf1bf: andb $0xfb,0x4c4(%rbx) > ffffffff810cf1c6: jmpq ffffffff810cf0e9 <finish_task_switch+0x69> > ffffffff810cf1cb: mov %rbx,%rsi > ffffffff810cf1ce: mov %r13,%rdi > ffffffff810cf1d1: callq ffffffff811a7e70 <__perf_event_task_sched_in> > ffffffff810cf1d6: jmpq ffffffff810cf0e4 <finish_task_switch+0x64> > > call__parse tries to find symbol from angle brackets which is not present > in case of kcore. > > -Ravi > > > > - Arnaldo > > > >> Suggested-by: Michael Ellerman <mpe@ellerman.id.au> > >> [Suggested about 'bctrl' instruction] > >> Signed-off-by: Ravi Bangoria <ravi.bangoria@linux.vnet.ibm.com> > >> --- > >> Changes in v6: > >> - No change > >> > >> tools/perf/util/annotate.c | 8 ++------ > >> 1 file changed, 2 insertions(+), 6 deletions(-) > >> > >> diff --git a/tools/perf/util/annotate.c b/tools/perf/util/annotate.c > >> index ea07588..a05423b 100644 > >> --- a/tools/perf/util/annotate.c > >> +++ b/tools/perf/util/annotate.c > >> @@ -81,16 +81,12 @@ static int call__parse(struct ins_operands *ops, const char *norm_arch) > >> return ops->target.name == NULL ? -1 : 0; > >> > >> indirect_call: > >> - tok = strchr(endptr, '('); > >> - if (tok != NULL) { > >> + tok = strchr(endptr, '*'); > >> + if (tok == NULL) { > >> ops->target.addr = 0; > >> return 0; > >> } > >> > >> - tok = strchr(endptr, '*'); > >> - if (tok == NULL) > >> - return -1; > >> - > >> ops->target.addr = strtoull(tok + 1, NULL, 16); > >> return 0; > >> } > >> -- > >> 2.5.5 ^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH v6 5/7] perf annotate: Show raw form for jump instruction with indirect target 2016-08-19 12:59 [PATCH v6 0/7] perf: Cross arch annotate + few miscellaneous fixes Ravi Bangoria ` (3 preceding siblings ...) 2016-08-19 12:59 ` [PATCH v6 4/7] perf annotate: Do not ignore call instruction with indirect target Ravi Bangoria @ 2016-08-19 12:59 ` Ravi Bangoria 2016-08-19 12:59 ` [PATCH v6 6/7] perf annotate: Support jump instruction with target as second operand Ravi Bangoria ` (2 subsequent siblings) 7 siblings, 0 replies; 19+ messages in thread From: Ravi Bangoria @ 2016-08-19 12:59 UTC (permalink / raw) To: linux-kernel, linuxppc-dev, acme Cc: peterz, mingo, alexander.shishkin, treeze.taeung, naveen.n.rao, markus, chris.ryder, pawel.moll, mhiramat, rmk+kernel, jolsa, mpe, hemant, namhyung, Ravi Bangoria For jump instructions that does not include target address as direct operand, use raw value for that. This is needed for certain powerpc jump instructions that use target address in a register (such as bctr, btar, ...). Suggested-by: Michael Ellerman <mpe@ellerman.id.au> Signed-off-by: Ravi Bangoria <ravi.bangoria@linux.vnet.ibm.com> --- Changes in v6: - No changes tools/perf/util/annotate.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/tools/perf/util/annotate.c b/tools/perf/util/annotate.c index a05423b..7ecb1b8 100644 --- a/tools/perf/util/annotate.c +++ b/tools/perf/util/annotate.c @@ -131,6 +131,9 @@ static int jump__parse(struct ins_operands *ops, static int jump__scnprintf(struct ins *ins, char *bf, size_t size, struct ins_operands *ops) { + if (!ops->target.addr) + return ins__raw_scnprintf(ins, bf, size, ops); + return scnprintf(bf, size, "%-6.6s %" PRIx64, ins->name, ops->target.offset); } -- 2.5.5 ^ permalink raw reply related [flat|nested] 19+ messages in thread
* [PATCH v6 6/7] perf annotate: Support jump instruction with target as second operand 2016-08-19 12:59 [PATCH v6 0/7] perf: Cross arch annotate + few miscellaneous fixes Ravi Bangoria ` (4 preceding siblings ...) 2016-08-19 12:59 ` [PATCH v6 5/7] perf annotate: Show raw form for jump " Ravi Bangoria @ 2016-08-19 12:59 ` Ravi Bangoria 2016-08-19 12:59 ` [PATCH v6 7/7] perf annotate: Fix jump target outside of function address range Ravi Bangoria 2016-09-07 15:39 ` [PATCH v6 0/7] perf: Cross arch annotate + few miscellaneous fixes Ravi Bangoria 7 siblings, 0 replies; 19+ messages in thread From: Ravi Bangoria @ 2016-08-19 12:59 UTC (permalink / raw) To: linux-kernel, linuxppc-dev, acme Cc: peterz, mingo, alexander.shishkin, treeze.taeung, naveen.n.rao, markus, chris.ryder, pawel.moll, mhiramat, rmk+kernel, jolsa, mpe, hemant, namhyung, Ravi Bangoria Current perf is not able to parse jump instruction when second operand contains target address. Arch like powerpc has such instructions. For example, 'beq cr7,10173e60'. Signed-off-by: Ravi Bangoria <ravi.bangoria@linux.vnet.ibm.com> --- Changes in v6: - No changes tools/perf/util/annotate.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/tools/perf/util/annotate.c b/tools/perf/util/annotate.c index 7ecb1b8..73c4f48 100644 --- a/tools/perf/util/annotate.c +++ b/tools/perf/util/annotate.c @@ -117,8 +117,12 @@ static int jump__parse(struct ins_operands *ops, const char *norm_arch __maybe_unused) { const char *s = strchr(ops->raw, '+'); + const char *c = strchr(ops->raw, ','); - ops->target.addr = strtoull(ops->raw, NULL, 16); + if (c++ != NULL) + ops->target.addr = strtoull(c, NULL, 16); + else + ops->target.addr = strtoull(ops->raw, NULL, 16); if (s++ != NULL) ops->target.offset = strtoull(s, NULL, 16); -- 2.5.5 ^ permalink raw reply related [flat|nested] 19+ messages in thread
* [PATCH v6 7/7] perf annotate: Fix jump target outside of function address range 2016-08-19 12:59 [PATCH v6 0/7] perf: Cross arch annotate + few miscellaneous fixes Ravi Bangoria ` (5 preceding siblings ...) 2016-08-19 12:59 ` [PATCH v6 6/7] perf annotate: Support jump instruction with target as second operand Ravi Bangoria @ 2016-08-19 12:59 ` Ravi Bangoria 2016-09-07 15:39 ` [PATCH v6 0/7] perf: Cross arch annotate + few miscellaneous fixes Ravi Bangoria 7 siblings, 0 replies; 19+ messages in thread From: Ravi Bangoria @ 2016-08-19 12:59 UTC (permalink / raw) To: linux-kernel, linuxppc-dev, acme Cc: peterz, mingo, alexander.shishkin, treeze.taeung, naveen.n.rao, markus, chris.ryder, pawel.moll, mhiramat, rmk+kernel, jolsa, mpe, hemant, namhyung, Ravi Bangoria 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 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 v6: - 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 21c5e10..c13df5b 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 73c4f48..9409d54 100644 --- a/tools/perf/util/annotate.c +++ b/tools/perf/util/annotate.c @@ -124,10 +124,12 @@ static int jump__parse(struct ins_operands *ops, 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; } @@ -135,7 +137,7 @@ static int jump__parse(struct ins_operands *ops, 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); @@ -1304,9 +1306,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 5cfad4e..5787ed8 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 ^ permalink raw reply related [flat|nested] 19+ messages in thread
* Re: [PATCH v6 0/7] perf: Cross arch annotate + few miscellaneous fixes 2016-08-19 12:59 [PATCH v6 0/7] perf: Cross arch annotate + few miscellaneous fixes Ravi Bangoria ` (6 preceding siblings ...) 2016-08-19 12:59 ` [PATCH v6 7/7] perf annotate: Fix jump target outside of function address range Ravi Bangoria @ 2016-09-07 15:39 ` Ravi Bangoria 7 siblings, 0 replies; 19+ messages in thread From: Ravi Bangoria @ 2016-09-07 15:39 UTC (permalink / raw) To: linux-kernel, linuxppc-dev, acme Cc: peterz, mingo, alexander.shishkin, treeze.taeung, naveen.n.rao, markus, chris.ryder, pawel.moll, mhiramat, rmk+kernel, jolsa, mpe, hemant, namhyung, Ravi Bangoria Hello, Any update on this? -Ravi On Friday 19 August 2016 06:29 PM, Ravi Bangoria wrote: > Currently Perf annotate support code navigation (branches and calls) > only when run on the same architecture where perf.data was recorded. > But, for example, record on powerpc server and annotate on client's > x86 desktop is not supported. > > This patchset enables cross arch annotate. Currently I've used x86 > and arm instructions which are already available and added support > for powerpc. > > Additionally this patch series also contains few other related fixes. > > Patches are prepared on top of acme/perf/core and tested it with x86 > and powerpc only. > > Note for arm: > I don't have arm test machine. As suggested by Russell in one of the > review comment, I've copied all instructions from default table to > arm table. This way it want break tool on arm but cleanup is needed > for x86 specific instructions added in arm table. > > Example: > > Record on powerpc: > $ ./perf record -a > > Report -> Annotate on x86: > $ ./perf report -i perf.data.powerpc --vmlinux vmlinux.powerpc > > Changes in v6: > - Instead of adding only those instructions defined in #ifdef __arm__, > add all instructions from default table to arm table. > > v5 link: > https://lkml.org/lkml/2016/8/19/35 > > Naveen N. Rao (1): > perf annotate: Add support for powerpc > > Ravi Bangoria (6): > perf: Define macro for normalized arch names > perf annotate: Add cross arch annotate support > perf annotate: Do not ignore call instruction with indirect target > perf annotate: Show raw form for jump instruction with indirect target > perf annotate: Support jump instruction with target as second operand > perf annotate: Fix jump target outside of function address range > > tools/perf/arch/common.c | 36 ++-- > tools/perf/arch/common.h | 11 ++ > tools/perf/builtin-top.c | 2 +- > tools/perf/ui/browsers/annotate.c | 8 +- > tools/perf/ui/gtk/annotate.c | 2 +- > tools/perf/util/annotate.c | 330 +++++++++++++++++++++++++++++++------ > tools/perf/util/annotate.h | 10 +- > tools/perf/util/unwind-libunwind.c | 4 +- > 8 files changed, 327 insertions(+), 76 deletions(-) > ^ permalink raw reply [flat|nested] 19+ messages in thread
end of thread, other threads:[~2016-09-20 14:56 UTC | newest] Thread overview: 19+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2016-08-19 12:59 [PATCH v6 0/7] perf: Cross arch annotate + few miscellaneous fixes Ravi Bangoria 2016-08-19 12:59 ` [PATCH v6 1/7] perf: Define macro for normalized arch names Ravi Bangoria 2016-08-19 12:59 ` [PATCH v6 2/7] perf annotate: Add cross arch annotate support Ravi Bangoria 2016-08-22 23:01 ` Kim Phillips 2016-08-23 2:17 ` Namhyung Kim 2016-08-23 20:36 ` Kim Phillips 2016-08-26 6:21 ` Namhyung Kim 2016-08-26 7:26 ` Ravi Bangoria 2016-08-27 0:40 ` [PATCH] perf annotate: cross arch annotate support fixes for ARM Kim Phillips 2016-08-19 12:59 ` [PATCH v6 3/7] perf annotate: Add support for powerpc Ravi Bangoria 2016-09-19 15:26 ` Arnaldo Carvalho de Melo 2016-08-19 12:59 ` [PATCH v6 4/7] perf annotate: Do not ignore call instruction with indirect target Ravi Bangoria 2016-09-19 15:44 ` Arnaldo Carvalho de Melo 2016-09-20 14:35 ` Ravi Bangoria 2016-09-20 14:56 ` Arnaldo Carvalho de Melo 2016-08-19 12:59 ` [PATCH v6 5/7] perf annotate: Show raw form for jump " Ravi Bangoria 2016-08-19 12:59 ` [PATCH v6 6/7] perf annotate: Support jump instruction with target as second operand Ravi Bangoria 2016-08-19 12:59 ` [PATCH v6 7/7] perf annotate: Fix jump target outside of function address range Ravi Bangoria 2016-09-07 15:39 ` [PATCH v6 0/7] perf: Cross arch annotate + few miscellaneous fixes Ravi Bangoria
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).