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

[Lindenmaier, Goetz]

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.
> >> >
> >> >