RFR(M): 8201593: Print array length in ArrayIndexOutOfBoundsException.

Mon Apr 23 07:13:25 UTC 2018

Hi Simon, 

I chose hyphens because they are used in other AIOOB messages,
too.  But others (including me) seem not to like them either, so 
I'll leave them out.

Best regards,
  Goetz.

> -----Original Message-----
> From: Simon Nash [mailto:simon at cjnash.com]
> Sent: Freitag, 20. April 2018 17:17
> To: Lindenmaier, Goetz <goetz.lindenmaier at sap.com>
> Cc: David Holmes <david.holmes at oracle.com>; hotspot-runtime-
> dev at openjdk.java.net; hotspot-compiler-dev at openjdk.java.net; aarch64-
> port-dev at openjdk.java.net; aarch32-port-dev at openjdk.java.net; core-libs-
> dev Libs <core-libs-dev at openjdk.java.net>
> Subject: Re: RFR(M): 8201593: Print array length in
> ArrayIndexOutOfBoundsException.
> 
> Hi Goetz,
> It should be "out of bounds" without hyphens.
> 
> Simon
> 
> On 20/04/2018 11:03, Lindenmaier, Goetz wrote:
> > Hi David,
> >
> > What about this:
> >     java.lang.ArrayIndexOutOfBoundsException: Arraycopy source index -1
> out-of-bounds for double[10].
> >     java.lang.ArrayIndexOutOfBoundsException: Arraycopy target index 10
> out-of-bounds for object array[10].
> >     java.lang.ArrayIndexOutOfBoundsException: Negative length -19 in
> arraycopy from int[3] to int[9].
> >
> > I'll reply to the other points in a comprehensive mail when I know
> > how to put the message.
> >
> > Thanks,
> >   Goetz.
> >
> >
> >> -----Original Message-----
> >> From: David Holmes [mailto:david.holmes at oracle.com]
> >> Sent: Freitag, 20. April 2018 09:25
> >> To: Lindenmaier, Goetz <goetz.lindenmaier at sap.com>; hotspot-runtime-
> >> dev at openjdk.java.net; hotspot-compiler-dev at openjdk.java.net;
> aarch64-
> >> port-dev at openjdk.java.net; aarch32-port-dev at openjdk.java.net; core-
> libs-
> >> dev Libs <core-libs-dev at openjdk.java.net>
> >> Subject: Re: RFR(M): 8201593: Print array length in
> >> ArrayIndexOutOfBoundsException.
> >>
> >> Hi Goetz,
> >>
> >> This is not a file by file review ...
> >>
> >> On 19/04/2018 10:24 PM, Lindenmaier, Goetz wrote:
> >>> Hi,
> >>>
> >>> New webrev:
> >>> http://cr.openjdk.java.net/~goetz/wr18/8201593-lenInAIOOB/02/
> >>>
> >>> I admit my wording is not optimal.
> >> src/hotspot/share/oops/typeArrayKlass.cpp
> >>
> >> Sorry but this is still far too verbose for my tastes. The type of the
> >> array is not relevant. For the array copy its okay to indicate src or
> >> dst array. But the message should be clear and succinct not prose-like.
> >> Plus you have a lot of repetition in the ss.print statements when only
> >> one thing is changing.
> >>
> >> src/hotspot/cpu/x86/c1_CodeStubs_x86.cpp
> >>
> >> I'm not seeing why the throw_index_out_of_bounds_exception should
> be
> >> tied to whether or not you have an array reference. Certainly I hate
> >> seeing the array ref being used as an implicit bool!
> >>
> >>> It's because I extracted this change from a bigger one. Our message
> reads
> >> like this:
> >>> Object [] oa1 = new Object[10]
> >>> oa1[12]
> >>> "ArrayIndexOutOfBoundsException while trying to load from index 12 of
> an
> >> object array with length 10, loaded from local variable 'oa1'"
> >>> ... which seems not optimal, either. But it mentions the type (object),
> the
> >> operation (load, store etc ...) and the variable name.
> >>> Naming the array is quite detailed if it is in a complex expression (like
> >> returned from a call).
> >>> I'll contribute more of this if appreciated, this is a first step.
> >> I've never thought this complexity was warranted. We've had a few RFE's
> >> of this nature over the years and they have been closed (not necessarily
> >> by me I should point out!).
> >>
> >>> Printing "index N is outside range [0, length-1]" is problematic
> >>> for length '0'. I followed the proposal by Roger:
> >>> "Index -1 out-of-bounds for length 10."
> >> You can easily special-case length 0. But Roger's proposal for
> >> consistency with existing messages make sense - not withstanding
> >> Andrew's dislike for the hyphens.
> >>
> >>> I removed the change to ArrayIndexOutOfBoundsException.java.
> >> That's good.
> >>
> >>> I extended the test to cover the exception thrown in arraycopy better
> >> The test seems somewhat excessive, and I now see it is because of the
> >> more elaborate error messages you have at SAP. But with only the index
> >> and the array length of interest here the test can be considerably smaller.
> >>
> >> The creation tests for ArrayIndexOutOfBoundsException don't seem
> >> relevant in this context either. This looks more TCK like.
> >>
> >> David
> >> -----
> >>
> >>> and added the elementary type to the message text.  This probably
> >>> needs improvement in the text, too.  There are (currently) these cases:
> >>>
> >>> bject[]  oa1 = new Object[10];
> >>> Object[]  oa2 = new Object[5];
> >>> System.arraycopy(oa1, -17, oa2, 0, 5);
> >>> "while trying to copy from index -17 of an object array with length 10");
> >>> System.arraycopy(oa1, 2, oa2, -18, 5);
> >>> "while trying to copy to index -18 of an object array with length 5");
> >>> System.arraycopy(oa1, 2, oa2, 0, -19);
> >>> "while trying to copy a negative range -19 from an object array with
> length
> >> 10 to an object array with length 5");
> >>> System.arraycopy(oa1, 8, oa2, 0, 5);
> >>> "while trying to copy from index 13 of an object array with length 10");
> >>> System.arraycopy(oa1, 1, oa2, 0, 7);
> >>> "while trying to copy to index 7 of an object array with length 5");
> >>> double[]  ta1 = new double[10];
> >>> double[]  ta2 = new double[5];
> >>> System.arraycopy(ta1, -17, ta2, 0, 5);
> >>> "while trying to copy from index -17 of a doubl array with length 10");
> >>> System.arraycopy(ta1, 2, ta2, -18, 5);
> >>> "while trying to copy to index -18 of a double array with length 5");
> >>> System.arraycopy(ta1, 2, ta2, 0, -19);
> >>> "while trying to copy a negative range -19 from a double array with
> length
> >> 10 to a double array with length 5");
> >>> System.arraycopy(ta1, 8, ta2, 0, 5);
> >>> "while trying to copy from index 13 of a double array with length 10");
> >>> System.arraycopy(ta1, 1, ta2, 0, 7);
> >>> "while trying to copy to index 7 of a double array with length 5");
> >>>
> >>> Maybe it should say:
> >>> Arraycopy source index -1 out-of-bounds for double array of length 10.
> >>> Arraycopy target index 10 out-of-bounds for object array of length 10.
> >>> Negative range -19 when copying from an object array of length 10 to an
> >> object array of length 5.
> >>> Best regards,
> >>>    Goetz.
> >>>
> >>>> -----Original Message-----
> >>>> From: David Holmes [mailto:david.holmes at oracle.com]
> >>>> Sent: Mittwoch, 18. April 2018 10:55
> >>>> To: Lindenmaier, Goetz <goetz.lindenmaier at sap.com>; hotspot-
> runtime-
> >>>> dev at openjdk.java.net; hotspot-compiler-dev at openjdk.java.net;
> >> aarch64-
> >>>> port-dev at openjdk.java.net; aarch32-port-dev at openjdk.java.net;
> core-
> >> libs-
> >>>> dev Libs <core-libs-dev at openjdk.java.net>
> >>>> Subject: Re: RFR(M): 8201593: Print array length in
> >>>> ArrayIndexOutOfBoundsException.
> >>>>
> >>>> Adding core-libs-dev as you're changing
> >>>> java.lang.ArrayIndexOutOfBoundsException.
> >>>>
> >>>> I appreciate the intent here but I find the messages excessively
> >>>> verbose. The basic error is:
> >>>>
> >>>> index N is outside range [0, length-1]
> >>>>
> >>>> David
> >>>>
> >>>> On 18/04/2018 6:09 PM, Lindenmaier, Goetz wrote:
> >>>>> Hi,
> >>>>>
> >>>>> I would like to print a more verbose text on ArrayIndexOutOfBounds
> >>>> exception
> >>>>> that not only mentions the index, but also the length of the array
> >> accessed.
> >>>>> See the bug for documentation of the change of the message.
> >>>>> http://cr.openjdk.java.net/~goetz/wr18/8201593-lenInAIOOB/01/
> >>>>>
> >>>>> @aarch/arm people:
> >>>>> I edited the aarch/arm files. Could you please verify this is correct?
> >>>>> I can not build on these platforms.
> >>>>>
> >>>>> The code on all the other platforms is tested with all the jtreg and jck
> >> tests
> >>>> etc.
> >>>>> Best regards,
> >>>>>     Goetz.
> >>>>>
> >>>>>