RFR (M): 8203881: Print errornous size in NegativeArraySizeException

David Holmes david.holmes at oracle.com
Wed May 30 09:46:54 UTC 2018


Hi Goetz,

On 30/05/2018 7:14 PM, Lindenmaier, Goetz wrote:
> Hi David,
> 
> I checked the test, and added cases for multidimensional
> arrays where the first dimension is 0.  A test for arrayKlass.cpp,
> ArrayKlass::allocate_arrayArray() is missing, though.
> I don't think this is critical as the change is quite simple.

Does make me wonder what code you need to write to exercise it though ?? 
Maybe it's dead code :)

> New webrev, (only test changes):
> http://cr.openjdk.java.net/~goetz/wr18/8203881-exMsg-NegativeArraySize/03/

29  * @compile NegativeArraySizeExceptionTest.java
30  * @run main NegativeArraySizeExceptionTest

You don't need a separate @compile, the @run will compile the test file.

No need to see updated webrev.

Thanks,
David

> Best regards,
>    Goetz
> 
> 
>> -----Original Message-----
>> From: David Holmes [mailto:david.holmes at oracle.com]
>> Sent: Montag, 28. Mai 2018 15:18
>> To: Lindenmaier, Goetz <goetz.lindenmaier at sap.com>; hotspot-runtime-
>> dev at openjdk.java.net
>> Subject: Re: RFR (M): 8203881: Print errornous size in
>> NegativeArraySizeException
>>
>> Hi Goetz,
>>
>> On 28/05/2018 10:10 PM, Lindenmaier, Goetz wrote:
>>> Hi David,
>>>
>>> thanks for having a look!
>>>
>>>> First I have corrected the typo in the synopsis: errornous -> erroneous
>>> Thanks!
>>>
>>>> Second, can you not use the err_msg function instead of the explicit
>>>> ResourceMark + stringStream code?
>>> Yes, that's much better. New webrev:
>>> http://cr.openjdk.java.net/~goetz/wr18/8203881-exMsg-
>> NegativeArraySize/02
>>
>> The functional change seems fine.
>>
>> For the test have you checked that all 7 changes are hit by the
>> testcase? I'm curious as to what kinds of array allocations hit the
>> different bits of code. I confess I'm not sure what a typeArray is
>> versus an objArray. I only expected to see 4 cases here: reference or
>> primitive plus single-dim or multi. Plus the two reflection cases. That
>> leaves one I can't account for. :)
>>
>> Possibly overkill to test (implicit) interpreter plus C1 plus C2, given
>> there is no JIT specific code involved - at least I can't see any.
>>
>> Thanks,
>> David
>>
>>> Best regards,
>>>     Goetz.
>>>
>>>
>>>
>>>>
>>>> Thanks,
>>>> David
>>>>
>>>> On 28/05/2018 8:40 PM, Lindenmaier, Goetz wrote:
>>>>> Hi,
>>>>>
>>>>> This change adds printing the array size in case of negative array size
>>>> exception.
>>>>> Please review.
>>>>> http://cr.openjdk.java.net/~goetz/wr18/8203881-exMsg-
>>>> NegativeArraySize/01/
>>>>>
>>>>> Thanks,
>>>>>      Goetz.
>>>>>


More information about the hotspot-runtime-dev mailing list