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

[David Holmes]

Hi Bengt,

On 14/11/2011 12:14 AM, Bengt Rutisson wrote:
> I already sent this out for review on the GC alias. It started out as a
> pure GC change, but after some updates it has become a change in
> src/share/vm/oops/arrayOop.hpp, which deserves a broader alias (thanks
> David Holmes for pointing this out). So, I am resending this review
> request to the hotspot-dev alias.
>
> http://cr.openjdk.java.net/~brutisso/7110152/webrev.04/
>
> Vladimir and David Holmes have already looked at it. But more comments
> are of course welcome.
>
> The background is that when I pushed this:
> 7102044: G1: VM crashes with assert(old_end != new_end) failed: don't
> call this otherwise
> http://hg.openjdk.java.net/hsx/hotspot-gc/hotspot/rev/6fd81579526f
>
> I changed arrayOopDesc::max_array_length() to always return max_jint in
> 64 bit VMs. This was intentional and is correct as far as I can tell.
> But it turns out that a large part of the GC code passes object sizes
> around as int and not size_t. Thus, we might overflow the int when we
> add the object header.
>
> My proposed change now reverts back to reducing the maximum array length
> to be slightly less than max_jint.

Okay, so the original code would only return max_jint for the smaller 
element types (up to ints on 64-bit, and short/char on 32-bit). And 
otherwise calculated a size in words that allowed for adding back the 
header. The previous fix allowed max_jint to be returned for all types 
but didn't allow for adding back the header - which is now fixed (and 
only necessary because int is used in some places as you stated).

Anyway this fix seems okay to me as long as nothing else in the code 
tries to pass around an object size in bytes using an int.

> CR:
>
> 7110152 assert(size_in_words <= (julong)max_jint) failed: no overflow
> http://monaco.sfbay.sun.com/detail.jsf?cr=7110152

http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=7110152

Cheers,
David
-----

> Here is a reproducer that triggers the assert before my change but
> passes after:
>
> 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 + "]");
> }
> }
>
> Thanks,
> Bengt