Code review request: NMT: assertion failed: assert(rec->addr() + rec->size() <= cur->base()) failed: Can not overlap in memSnapshot.cpp

[Zhengyu Gu]

Coleen,

Thanks for reviewing.

On 11/5/2012 12:26 PM, Coleen Phillimore wrote:
>
> Zhengyu,
>
> *+          address committed_rgn_end = committed_rgn->addr() +*
> *!          _       committed_rgn->size(_);
>
> *
>
> Should "end" be addr() + size() -1 ?
>
I think we use this formula for end address.  For example in stack 
calculation code, etc. In range checking, the end address is excluded, 
however.

Thanks,

-Zhengyu


> This looks fine otherwise.   I like the renames that you did within 
> the code you changed.
>
> Thanks,
> Coleen
>
> On 11/2/2012 1:21 PM, Zhengyu Gu wrote:
>> Updated the webrev based on David's comment.  Overlapping stacks is 
>> tracked by 8001743.
>>
>> http://cr.openjdk.java.net/~zgu/8001591/webrev.01/
>>
>> Thanks,
>>
>> -Zhengyu
>>
>> On 10/31/2012 9:32 PM, David Holmes wrote:
>>> On 1/11/2012 2:12 AM, Zhengyu Gu wrote:
>>>> NMT did not allow overlapped commit on memory regions, which is
>>>> incorrect. Committing overlapped memory regions should be allowed, as
>>>> long as the regions are within a reserved region.
>>>
>>> So the overlapping stacks that were detected is perfectly valid?
>>>
>>>>
>>>> http://cr.openjdk.java.net/~zgu/8001591/webrev.00/
>>>
>>> The renaming from contains_xx to contain_xx is not correct. 
>>> "contains" is the correct form to use, and "overlaps" rather than 
>>> "overlap".
>>>
>>> Why the variable rename in 
>>> VMMemPointerIterator::add_reserved_region? Given it is initialized 
>>> from current() then 'cur' seems quite acceptable. Otherwise maybe it 
>>> is current() that has the wrong name? The renaming generates a lot 
>>> of noise in the webrev - it is hard to see the actual substantive 
>>> changes that were made.
>>>
>>> Cheers,
>>> David
>>>
>>>
>>>> Thanks,
>>>>
>>>> -Zhengyu