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

[Stuart Monteith]

Hello,
   The problem with C1 is in c1_CodeStubs_aarch64.cpp:
RangeCheckStub::emit_code .
There are two parameters being passed sometimes, with rscratch1
holding the index and rscratch2 holding a pointer to the array,
sometimes. The issue is there is a far call, and to construct the
pointer to jump to it is being passed rscratch2 to use as a temporary
register:

    __ mov(rscratch2, _array->as_pointer_register());

then...

  __ far_call(RuntimeAddress(Runtime1::entry_for(stub_id)), NULL, rscratch2);

By default rscratch1 is used as the temporary register. .

I'm not quite sure to solve this yet - we'll need to use the stack in
some safe way.

BR,
    Stuart



On 24 April 2018 at 13:27, Stuart Monteith <stuart.monteith at linaro.org> wrote:
> Hello Goetz,
>    It appears that it is just C1 that exhibits the problem - there has
> been a delay as I've been investigating a hang in Java JDK running
> some of my automation.
>
> I'll try and track down the issue. Otherwise, I do like the principle
> of the patch.
>
> BR,
>     Stuart
>
>
> On 24 April 2018 at 11:44, Lindenmaier, Goetz <goetz.lindenmaier at sap.com> wrote:
>> Hi Stuart,
>>
>> could you track down the remaining issue?
>>
>> Best regards,
>>   Goetz.
>>
>>> -----Original Message-----
>>> From: Stuart Monteith [mailto:stuart.monteith at linaro.org]
>>> Sent: Donnerstag, 19. April 2018 17:09
>>> To: Lindenmaier, Goetz <goetz.lindenmaier at sap.com>
>>> Cc: 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
>>> Subject: Re: RFR(M): 8201593: Print array length in
>>> ArrayIndexOutOfBoundsException.
>>>
>>> Hi,
>>>   I'm trying to work through what's going on here. With C1 not
>>> crashing, I'll check the behaviour of the interpreter and C2. I'm
>>> doing explicitly with -Xint, etc.  WIth -Xint I definitely get the
>>> error.
>>>
>>> You have the condition the wrong way round -
>>> _throw_index_out_of_bounds_exception is true when array is NULL.
>>>
>>>
>>> BR,
>>>    Stuart
>>>
>>>
>>> On 19 April 2018 at 15:36, Lindenmaier, Goetz <goetz.lindenmaier at sap.com>
>>> wrote:
>>> > Hi Stuart,
>>> >
>>> > thanks a lot for helping out here.
>>> > I added below patch to my latest webrev (replaced it in-place):
>>> > http://cr.openjdk.java.net/~goetz/wr18/8201593-lenInAIOOB/02/
>>> >
>>> > The remaining problem looks like the array argument does not
>>> > reach the method that prints the message properly.
>>> > Did I get a register wrong?
>>> > Did I use a wrong argument register, or does the untangling
>>> > of registers in overwrite something?
>>> > Is it correct to use mov and not movw in
>>> templateInterpreterGenerator_aarch64.cpp?
>>> > Is it safe to use r3 in templateTable_aarch64.cpp? (The "convention" here is
>>> stupid,
>>> > it seems to cause unnecessary register moves. But it's similar on all
>>> platforms.)
>>> >
>>> > The test runs several times, assuring it's C1, C2 and Xint.
>>> > This should help you to narrow down the problem.
>>> >
>>> > Best regards,
>>> >   Goetz.
>>> >
>>> >
>>> >
>>> >
>>> >
>>> >
>>> >
>>> >
>>> > diff -r 3fe33d48aa16 src/hotspot/cpu/aarch64/c1_CodeStubs_aarch64.cpp
>>> > --- a/src/hotspot/cpu/aarch64/c1_CodeStubs_aarch64.cpp  Mon Apr 16
>>> 15:17:20 2018 +0200
>>> > +++ b/src/hotspot/cpu/aarch64/c1_CodeStubs_aarch64.cpp  Thu Apr 19
>>> 16:06:23 2018 +0200
>>> > @@ -73,9 +73,10 @@
>>> >    } else {
>>> >      __ mov(rscratch1, _index->as_jint());
>>> >    }
>>> > -  __ mov(rscratch2, _array->as_pointer_register());
>>> >    Runtime1::StubID stub_id;
>>> >    if (_throw_index_out_of_bounds_exception) {
>>> > +    assert(_array != NULL, "sanity");
>>> > +    __ mov(rscratch2, _array->as_pointer_register());
>>> >      stub_id = Runtime1::throw_index_exception_id;
>>> >    } else {
>>> >      stub_id = Runtime1::throw_range_check_failed_id;
>>> >
>>> >
>>> >
>>> >
>>> >> -----Original Message-----
>>> >> From: Stuart Monteith [mailto:stuart.monteith at linaro.org]
>>> >> Sent: Donnerstag, 19. April 2018 14:26
>>> >> To: Lindenmaier, Goetz <goetz.lindenmaier at sap.com>
>>> >> Cc: 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
>>> >> Subject: Re: RFR(M): 8201593: Print array length in
>>> >> ArrayIndexOutOfBoundsException.
>>> >>
>>> >> Hello Goetz,
>>> >>    In c1_CodeStubs_aarch64.cpp, RangeCheckStub::emit_code you just
>>> >> need to guard the emission of rscratch2 - you don't use it in one of
>>> >> the cases, something like:
>>> >>
>>> >>      if (!_throw_index_out_of_bounds_exception) {
>>> >>         __ mov(rscratch2, _array->as_pointer_register());
>>> >>      }
>>> >>
>>> >>
>>> >> Once I can run the unit tests, I'm seeing failures in your unit test like:
>>> >>
>>> >> java.lang.AssertionError: expected [trying to access index -5 of an
>>> >> array with length 0] but found [trying to access index -5 of an array
>>> >> with length -1459548190]
>>> >>
>>> >>
>>> >> BR,
>>> >>     Stuart
>>> >>
>>> >>
>>> >> On 18 April 2018 at 09:09, Lindenmaier, Goetz
>>> <goetz.lindenmaier at sap.com>
>>> >> 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.
>>> >> >
>>> >> >