Re: [PATCH] Properly interpret indirect call in perf annotate.

[Martin LiÅka]

This is a multi-part message in MIME format.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 <mliska@xxxxxxx>
>> ---
>>  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;
>>  }
>>  
>>
>