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

[Lindenmaier, Goetz]

Hi Stuart,

thanks for fixing this! Webrev with your changes:
http://cr.openjdk.java.net/~goetz/wr18/8201593-lenInAIOOB/05/

> There is the possibility of overwriting live values though, aren't
> there? The registers are saved by call_RT. Should I be concerned about
> deopt and debugging going wrong? Furthermore, won't there be issues in
> exception handlers?
As I understand, this just has to survive the far_call.
The call_RT in c1_Runtime then moves it into the 
proper argument registers.  This is just the handling of an 
exception, and in these few instructions no java code is
executed, no safepoint is passed, so this should be fine.

incremental diff:
iff -r 874f2b999ff6 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 26 15:55:18 2018 +0200
@@ -75,16 +75,16 @@
   }
 
   if (_index->is_cpu_register()) {
-    __ mov(rscratch1, _index->as_register());
+    __ mov(r22, _index->as_register());
   } else {
-    __ mov(rscratch1, _index->as_jint());
+    __ mov(r22, _index->as_jint());
   }
   Runtime1::StubID stub_id;
   if (_throw_index_out_of_bounds_exception) {
     stub_id = Runtime1::throw_index_exception_id;
   } else {
     assert(_array != NULL, "sanity");
-    __ mov(rscratch2, _array->as_pointer_register());
+    __ mov(r23, _array->as_pointer_register());
     stub_id = Runtime1::throw_range_check_failed_id;
   }
   __ far_call(RuntimeAddress(Runtime1::entry_for(stub_id)), NULL, rscratch2);
diff -r 874f2b999ff6 src/hotspot/cpu/aarch64/c1_Runtime1_aarch64.cpp
--- a/src/hotspot/cpu/aarch64/c1_Runtime1_aarch64.cpp	Mon Apr 16 15:17:20 2018 +0200
+++ b/src/hotspot/cpu/aarch64/c1_Runtime1_aarch64.cpp	Thu Apr 26 15:55:18 2018 +0200
@@ -327,7 +327,7 @@
 
 
 // target: the entry point of the method that creates and posts the exception oop
-// has_argument: true if the exception needs an argument (passed in rscratch1)
+// has_argument: true if the exception needs arguments (passed in r22 and r23)
 
 OopMapSet* Runtime1::generate_exception_throw(StubAssembler* sasm, address target, bool has_argument) {
   // make a frame and preserve the caller's caller-save registers
@@ -336,7 +336,7 @@
   if (!has_argument) {
     call_offset = __ call_RT(noreg, noreg, target);
   } else {
-    call_offset = __ call_RT(noreg, noreg, target, rscratch1, rscratch2);
+    call_offset = __ call_RT(noreg, noreg, target, r22, r23);
   }
   OopMapSet* oop_maps = new OopMapSet();
   oop_maps->add_gc_map(call_offset, oop_map);

Best regards,
 Goetz.


> -----Original Message-----
> From: Stuart Monteith [mailto:stuart.monteith at linaro.org]
> Sent: Donnerstag, 26. April 2018 12:52
> To: Andrew Haley <aph at redhat.com>
> Cc: Lindenmaier, Goetz <goetz.lindenmaier at sap.com>; hotspot-compiler-
> dev at openjdk.java.net; aarch64-port-dev at openjdk.java.net; hotspot-
> runtime-dev at openjdk.java.net; aarch32-port-dev at openjdk.java.net
> Subject: Re: RFR(M): 8201593: Print array length in
> ArrayIndexOutOfBoundsException.
> 
> Hi,
>   Using c_rarg1 and c_rarg2 instead of rscratch1 and overwriting
> rscratch2 causes a SIGSEGV.
> Using r22 and r23 instead, the test ran successfully.
> 
> In c1_CodeStubs_aarch64.cpp
> :
>  77   if (_index->is_cpu_register()) {
>  78     __ mov(r22, _index->as_register());
>  79   } else {
>  80     __ mov(r22, _index->as_jint());
>  81   }
>  82   Runtime1::StubID stub_id;
>  83   if (_throw_index_out_of_bounds_exception) {
>  84     stub_id = Runtime1::throw_index_exception_id;
>  85   } else {
>  86     assert(_array != NULL, "sanity");
>  87     __ mov(r23, _array->as_pointer_register());
>  88     stub_id = Runtime1::throw_range_check_failed_id;
>  89   }
> 
> in c1_Runtime_aarch64.cpp:
> 
>  336   if (!has_argument) {
>  337     call_offset = __ call_RT(noreg, noreg, target);
>  338   } else {
>  339     call_offset = __ call_RT(noreg, noreg, target, r22, r23);
>  340   }
> 
> There is the possibility of overwriting live values though, aren't
> there? The registers are saved by call_RT. Should I be concerned about
> deopt and debugging going wrong? Furthermore, won't there be issues in
> exception handlers?
> 
> BR,
>   Stuart
> 
> 
> On 25 April 2018 at 16:49, Stuart Monteith <stuart.monteith at linaro.org>
> wrote:
> > Indeed - and that is what I am seeing. Usually no parameters are being
> > called with this pattern, or rscratch1, with the temporary variable
> > being changed to use rscratch2 in such circumstances.
> > I'll try c_rarg1 and c_rarg2 - they should pass straight through,if I
> > interpret the code correcting.
> >
> > On 25 April 2018 at 16:26, Andrew Haley <aph at redhat.com> wrote:
> >> On 04/25/2018 04:00 PM, Stuart Monteith wrote:
> >>> I'm not quite sure to solve this yet - we'll need to use the stack in
> >>> some safe way.
> >>
> >> It's not a great idea to pass arguments in rscratch1 or rscratch2.  These
> >> registers are for use in macros and should be treated as volatile.  Given
> >> that you're throwing an exception, registers will be clobbered anyway.
> >>
> >> --
> >> Andrew Haley
> >> Java Platform Lead Engineer
> >> Red Hat UK Ltd. <https://www.redhat.com>
> >> EAC8 43EB D3EF DB98 CC77 2FAD A5CD 6035 332F A671