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

David Holmes david.holmes at oracle.com
Mon Apr 23 07:14:09 UTC 2018 

On 23/04/2018 5:08 PM, Lindenmaier, Goetz wrote:
> Hi,
> 
>> I still don't like the array type information - it's not relevant to the
>> error IMHO.
>>
>> That aside, for the array copy cases I'd go for something like:
>>
>> arraycopy src index -1 out of bounds for double[10]
>> arraycopy dest index -1 out of bounds for double[10]
>> arraycopy end src index 11 out of bounds for double[10]
>> arraycopy end dest index 11 out of bounds for double[10]
> It's a good idea to point out it's the end of the region to copy.
> But I would find "last src index" better to understand than
> "end src index":

Sure - end/last/final whatever you prefer.

> "last arraycopy src index 11 out of bounds for double[10]"

I'd prefer to see "arraycopy" always come first to show clearly these 
are specific to arraycopy. Even "arraycopy: ...".

>> arraycopy length -19 is negative
>>
>> In the final case the array lengths are not relevant.
> I think they might help in case the length computation depends on the
> array lengths, which is quite likely the case. But I'll leave it out.

Happy to let others give opinions.

David
-----

>>> I'll reply to the other points in a comprehensive mail when I know
>>> how to put the message.
> ... these all are not related to the wording of the message.
>>
>> I don't oppose additional useful information in exception messages, but
>> if it not directly relevant it just makes the error message harder to
>> decipher IMO.
> SAP JVM tries to give all information useful to track down the cause.
> In many situations the printed information might be superfluous, but
> in this one critical situation you will be very happy to have it.  SAP builds
> 'real' software on Java, not just 100 line jtreg tests ...
> But the message should stay clear. Therefore I also don't like the
> 1-sentence rule you sometimes mention. Several sentences can give
> structure to distinguish root cause information and additional information.
> Not relevant here currently, though.
> 
>> I also don't want to pay additional runtime costs just to
>> make an exception message a little bit clearer.
> Yes, especially it should not impose any cost on the case without exception.
> 
> Best regards,
>    Goetz.
> 
> 
>> Thanks,
>> David
>>
>>> 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.
>>>>>>>
>>>>>>>

More information about the aarch32-port-dev mailing list