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

David Holmes david.holmes at oracle.com
Mon Apr 23 03:33:33 UTC 2018


On 20/04/2018 8:03 PM, 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 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]
arraycopy length -19 is negative

In the final case the array lengths are not relevant.

> I'll reply to the other points in a comprehensive mail when I know
> how to put 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. I also don't want to pay additional runtime costs just to 
make an exception message a little bit clearer.

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 hotspot-runtime-dev mailing list