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

Stuart Monteith stuart.monteith at linaro.org
Fri Apr 27 14:36:38 UTC 2018


Hi,
   JTregs hasn't flagged any issues, so it should be ok.

Regarding the 32-bit arm code, in "void
RangeCheckStub::emit_code(LIR_Assembler* ce)" should:
   ce->verify_reserved_argument_area_size(1);
be
   ce->verify_reserved_argument_area_size(2);

so as to accommodate the array pointer you are pushing onto the stack?

I've not tested 32-bit arm.


BR,
    Stuart

On 26 April 2018 at 15:31, Stuart Monteith <stuart.monteith at linaro.org> wrote:
> Thanks, I'm happy with that.
>
> The registers have a clean path to call_RT - r22 and r23 aren't used
> inbetween. They are an arbitrary choice - c_rarg0 and c_rarg1 were
> always going to cause problems.  If _array->as_pointer_register()
> and/or _index->as_register() or _index->as_jint() were the registers
> we were using as parameters there would be trouble. However, with
> pd_last_allocatable_cpu_reg = 16, that shouldn't happen with r22/23,
> or indeed anything else in the range r17 to r28.
>
> I'm going to run all of JTRegs and seem what that produces now.
>
> BR,
>     Stuart
>
>
>
>
> On 26 April 2018 at 15:14, Lindenmaier, Goetz <goetz.lindenmaier at sap.com> wrote:
>> 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


More information about the hotspot-runtime-dev mailing list