Re: [PATCH 4/4] perf probe: Add failure check when show variable range

[He Kuang <hekuang@huawei.com>]

hi, Masami

On 2015/7/1 19:00, Masami Hiramatsu wrote:
> On 2015/07/01 18:05, He Kuang wrote:
>> Change improper type size_t to diffptr_t to make consistent with libdw
>> and handle error code.
>>
>> Signed-off-by: He Kuang <hekuang@huawei.com>
>> ---
>>   tools/perf/util/dwarf-aux.c | 10 ++++++++--
>>   1 file changed, 8 insertions(+), 2 deletions(-)
>>
>> diff --git a/tools/perf/util/dwarf-aux.c b/tools/perf/util/dwarf-aux.c
>> index 2cb4c82..da96eb1 100644
>> --- a/tools/perf/util/dwarf-aux.c
>> +++ b/tools/perf/util/dwarf-aux.c
>> @@ -965,7 +965,7 @@ static int die_get_var_innermost_scope(Dwarf_Die *sp_die, Dwarf_Die *vr_die,
>>   {
>>   	Dwarf_Die *scopes;
>>   	int count;
>> -	size_t offset = 0;
>> +	ptrdiff_t offset = 0;
>>   	Dwarf_Addr base;
>>   	Dwarf_Addr start, end;
>>   	Dwarf_Addr entry;
>> @@ -991,6 +991,9 @@ static int die_get_var_innermost_scope(Dwarf_Die *sp_die, Dwarf_Die *vr_die,
>>   
>>   	while ((offset = dwarf_ranges(&scopes[1], offset, &base,
>>   				&start, &end)) > 0) {
>> +		if (offset < 0)
>> +			goto out;
> 
> Hmm, here offset always > 0, see the condition carefully.
> Perhaps, this should be placed at the right after the while loop.

Sorry for this silly mistake. Modified and resent with patch 3/4,
more info is added in commit message of that patch.

Thank you.

> 
>> +
>>   		start -= entry;
>>   		end -= entry;
>>   
>> @@ -1029,7 +1032,7 @@ int die_get_var_range(Dwarf_Die *sp_die, Dwarf_Die *vr_die, struct strbuf *buf)
>>   	Dwarf_Addr entry;
>>   	Dwarf_Op *op;
>>   	size_t nops;
>> -	size_t offset = 0;
>> +	ptrdiff_t offset = 0;
>>   	Dwarf_Attribute attr;
>>   	bool first = true;
>>   	const char *name;
>> @@ -1048,6 +1051,9 @@ int die_get_var_range(Dwarf_Die *sp_die, Dwarf_Die *vr_die, struct strbuf *buf)
>>   	while ((offset = dwarf_getlocations(
>>   				&attr, offset, &base,
>>   				&start, &end, &op, &nops)) > 0) {
>> +		if (offset < 0)
>> +			return offset;
> 
> Ditto.
> 
> Thanks,
> 
>> +
>>   		if (start == 0) {
>>   			/* Single Location Descriptions */
>>   			ret = die_get_var_innermost_scope(sp_die, vr_die, buf);
>>
> 
>