[8u] RFR: 8151284: aarch32: on-stack replacement does not work properly when invoked from compiled

[Andrey Petushkov]

Hi Ed,

Absolutely you are right!  Indeed this problem is already fixed on our side and I was under assumption that it was part of Sergey’s cumulative patch. Apparently it’s not so please excuse me for not checking that. In my version r4 is used to persist the nmethod. I can regenerate the patch with this replacement, should I?

Thanks,
Andrey


> On 10 Mar 2016, at 15:20, Edward Nevill <edward.nevill at gmail.com> wrote:
> 
> On Fri, 2016-03-04 at 18:30 +0300, Andrey Petushkov wrote:
>> Hi All!
>> 
>> Below is a fix for the On Stack Replacement functionality. The bug is caused by borrowing code from aarch64 but using sp for both sp and esp purposes. As a result the correct value of sp gets lost after removing of the frame. The trivial fix is to save the value across the frame pop routine.
>> 
> 
> Hi Andrey,
> 
> Thanks for this. One thing confuses me about this code. Here is the code from templateTable_aarch32.cpp post your patch
> 
>      // We have the address of an on stack replacement routine in r0
>      // We need to prepare to execute the OSR method. First we must
>      // migrate the locals and monitors off of the stack.
> 
> <<<< LOOK HERE
>      __ str(r0, Address( __ pre(sp, -wordSize)));               // save the nmethod
> 
>      call_VM(noreg, CAST_FROM_FN_PTR(address, SharedRuntime::OSR_migration_begin));
> 
>      // r0 is OSR buffer, move it to expected parameter location
>      __ mov(j_rarg0, r0);
> 
>      // remove activation
>      // get sender sp
>      __ ldr(rscratch1,
>          Address(rfp, frame::interpreter_frame_sender_sp_offset * wordSize));
>      // remove frame anchor
>      __ leave();
>      __ mov(sp, rscratch1);
>      // Ensure compiled code always sees stack at proper alignment
>      __ align_stack();
> 
>      // and begin the OSR nmethod
> <<<<< LOOK HERE
>      __ ldr(r14, Address( __ post(sp, wordSize)));
>      __ ldr(rscratch1, Address(r14, nmethod::osr_entry_point_offset()));
>      __ b(rscratch1);
> 
> At the first point marked "LOOK HERE" it does
> 
>      __ str(r0, Address( __ pre(sp, -wordSize)));               // save the nmethod
> 
> to save the nmethod on the stack. It then modifies the stack pointer and the, at the second "LOOK HERE" it does
> 
>      __ ldr(r14, Address( __ post(sp, wordSize)));
> 
> This would seem to be broken? The aarch64 code uses r19 which is preserved around the call.
> 
> Is there a callee save register we can use to preserve the nmethod around the call?
> 
> Alternatively I think the ldr r14, ... should go before the leave().
> 
> But I have no way of testing this.
> 
> All the best,
> Ed.
> 
>