On 08/27/2018 12:37 PM, Namhyung Kim wrote: > Hello, > > On Thu, Aug 23, 2018 at 02:29:34PM +0200, Martin Liška wrote: >> The patch changes interpretation of: >> callq *0x8(%rbx) >> >> from: >> 0.26 │ → callq *8 >> to: >> 0.26 │ → callq *0x8(%rbx) >> >> in this can an address is followed by a register, thus >> one can't parse only address. > > Also there's a case with no offset like: callq *%rbx Yes. But this case is fine as strtoull returns 0 for that: 'If there were no digits at all, strtoul() stores the original value of nptr in *endptr (and returns 0).' So ops->target.addr is then 0 and it's fine. > > >> >> Signed-off-by: Martin Liška >> --- >> tools/perf/util/annotate.c | 10 ++++++++-- >> 1 file changed, 8 insertions(+), 2 deletions(-) >> >> > >> diff --git a/tools/perf/util/annotate.c b/tools/perf/util/annotate.c >> index e4268b948e0e..e32ead4744bd 100644 >> --- a/tools/perf/util/annotate.c >> +++ b/tools/perf/util/annotate.c >> @@ -246,8 +246,14 @@ static int call__parse(struct arch *arch, struct ins_operands *ops, struct map_s >> >> indirect_call: >> tok = strchr(endptr, '*'); >> - if (tok != NULL) >> - ops->target.addr = strtoull(tok + 1, NULL, 16); >> + if (tok != NULL) { >> + endptr++; >> + >> + /* Indirect call can use a non-rip register and offset: callq *0x8(%rbx). >> + * Do not parse such instruction. */ >> + if (strstr(endptr, "(%r") == NULL) >> + ops->target.addr = strtoull(endptr, NULL, 16); > > It seems too x86-specific, what about this? (not tested) It is, I'm fine with that. I've just tested that for the callq *0x8(%rbx) example. I'm sending patch for that version. Martin > > > indirect_call: > tok = strchr(endptr, '*'); > if (tok != NULL) { > endptr++; > if (!isdigit(*endptr)) > return 0; > > addr = strtoull(endptr, &endptr, 0); > if (*endptr != '(')) > ops->target.addr = addr; > > > Thanks, > Namhyung > > >> + } >> goto find_target; >> } >> >> >