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

[Lindenmaier, Goetz]

Hi Thomas,

thanks for the review!

...
The err_msg was requested by David. It's also used in other exceptions.
So this should be fine, but obviously leaves room for improvement.

Best regards,
  Goetz.



> -----Original Message-----
> From: Thomas Stüfe [mailto:thomas.stuefe at gmail.com]
> Sent: Mittwoch, 30. Mai 2018 11:56
> To: Lindenmaier, Goetz <goetz.lindenmaier at sap.com>
> Cc: David Holmes <david.holmes at oracle.com>; hotspot-runtime-
> dev at openjdk.java.net
> Subject: Re: RFR (M): 8203881: Print errornous size in
> NegativeArraySizeException
> 
> Good for me too.
> 
> ..Thomas
> 
> On Wed 30. May 2018 at 11:14, Lindenmaier, Goetz
> <goetz.lindenmaier at sap.com <mailto:goetz.lindenmaier at sap.com> >
> 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.
> 
> 	New webrev, (only test changes):
> 	http://cr.openjdk.java.net/~goetz/wr18/8203881-exMsg-
> NegativeArraySize/03/
> 
> 	Best regards,
> 	  Goetz
> 
> 
> 	> -----Original Message-----
> 	> From: David Holmes [mailto:david.holmes at oracle.com
> <mailto:david.holmes at oracle.com> ]
> 	> Sent: Montag, 28. Mai 2018 15:18
> 	> To: Lindenmaier, Goetz <goetz.lindenmaier at sap.com
> <mailto:goetz.lindenmaier at sap.com> >; hotspot-runtime-
> 	> dev at openjdk.java.net <mailto: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.
> 	> >>>
>