Request for review: 7110152 assert(size_in_words <= (julong)max_jint) failed: no overflow

[Vladimir Kozlov]

Bengt Rutisson wrote:
> 
> ...except that I just saw that CollectedHeap::array_allocate_nozero() 
> uses int.

Yes, this is what I tried to tell you :)

You also missed code in typeArrayKlass::allocate_permanent() and nozero code in 
typeArrayKlass::allocate_common(). May be also other places where 
CollectedHeap::array_allocate is called.

Could you also remove 'size' argument in post_allocation_install_obj_klass() 
which does not use it?

Thanks,
Vladimir

> 
> I'll fix that and update the webrev.
> 
> Bengt
> 
> On 2011-11-09 23:47, Bengt Rutisson wrote:
>>
>> Vladimir,
>>
>> Thanks for looking at this!
>>
>> On 2011-11-09 23:06, Vladimir Kozlov wrote:
>>> Why not change it in all CollectedHeap methods?
>>
>> The other methods in CollectedHeap that use int for size are handling 
>> objects. Those should not have been affected by my other change (since 
>> it only changed the maximum array length).
>>
>> I have not investigated whether or not it is possible to create an 
>> object (that is not an array) that will have a size larger than int. I 
>> assume that there is some limit to how many fields an object can have. 
>> But I don't know what that limit is and whether it will guarantee that 
>> the largest object fits into an int. It also seems much more unlikely 
>> that anyone would actually create such a large object.
>>
>> I'd like to only change this for arrays now since we have failures in 
>> the nightlies. If you think it is ok I could create a separate CR to 
>> investigate whether or not the int object sizes in CollectedHeap are 
>> safe.
>>
>> Bengt
>>
>>>
>>> Vladimir
>>>
>>> Bengt Rutisson wrote:
>>>>
>>>> Hi all,
>>>>
>>>> Could I have a couple of reviews for this small change?
>>>> http://cr.openjdk.java.net/~brutisso/7110152/webrev.01/
>>>>
>>>> The problem is that CollectedHeap::array_allocate() takes its size 
>>>> as an int instead of a size_t. This was exposed by the fix I made to 
>>>> 7102044 recently.
>>>>
>>>> That change allows arrays to be max_jint long on 64 bit systems. 
>>>> This is correct since we can handle this size in a 64 bit size_t 
>>>> even when we add the object header. However, it will overflow an int.
>>>>
>>>> Here is a small reproducer that will trigger the assert mentioned in 
>>>> 7110152 when it is run on a 64 bit VM:
>>>>
>>>> public class MaxJIntArray {
>>>>     public static void main(String[] args) {
>>>>         final int MAX_JINT = 2147483647;
>>>>         double[] a = new double[MAX_JINT];
>>>>         System.out.println("Allocated a[" + a.length + "]");
>>>>     }
>>>> }
>>>>
>>>> The fix is to remove the assert (since it is too strict) and 
>>>> changing CollectedHeap::array_allocate() to take a size_t instead of 
>>>> an int.
>>>>
>>>> CR:
>>>> 7110152 assert(size_in_words <= (julong)max_jint) failed: no overflow
>>>> http://monaco.sfbay.sun.com/detail.jsf?cr=7110152
>>>>
>>>> Bengt
>>
>