[aarch64-port-dev ] TestStackBangRbp hangs

[Andrew Dinn]

On 13/01/14 11:55, Andrew Haley wrote:
> It turns out that there were three separate bugs.
> 
> This one in shared code:
> 
> http://mail.openjdk.java.net/pipermail/hotspot-compiler-dev/2014-January/012938.html

Yes, that makes sense.

> And two others in AArch64-specific code.  The problem was that when we
> returned from deoptimizing we were not properly removing the stack
> frame of the deoptimized method.  All we were doing was:
> 
>   // Pop deoptimized frame
>   __ ldrw(r2, Address(r5, Deoptimization::UnrollBlock::size_of_deoptimized_frame_offset_in_bytes()));
>   __ add(sp, sp, r2);
> 
> Clearly, this does not work.  We much also restore SP and LR.  We need
> to do this:

That doesn't make sense to me. Why do we need to restore SP and LR? They
get reset later (see below).

>   // Pop deoptimized frame
>   __ ldrw(r2, Address(r5, Deoptimization::UnrollBlock::size_of_deoptimized_frame_offset_in_bytes()));
>   __ sub(r2, r2, 2 * wordSize);
>   __ add(sp, sp, r2);
>   __ ldp(rfp, lr, __ post(sp, 2 * wordSize));
> 
> But -- amazingly -- there hardly ever was a failure caused by this
> fundamental mistake.

In the deopt case rfp and lr are patched up after the interpreter frames
have been pushed i.e. at line 2660:

  // Pick up the initial fp we should save
  __ ldr(rfp,
	 Address(r4, Deoptimization::UnrollBlock::initial_info_offset_in_bytes()));

and at line 2694:

  __ ldr(lr, Address(r2, 0));	  // save final return address
  // Re-push self-frame
  __ enter();			  // & old rfp & set new rfp

Also, if you do want to load rfp and lr asd above then would it not be
better to subtract r2 as is and then do the load pair with an immediate
offset of -2 * wordsize?

  // Pop deoptimized frame
  __ ldrw(r2, Address(r5,
Deoptimization::UnrollBlock::size_of_deoptimized_frame_offset_in_bytes()));
  __ add(sp, sp, r2);
  __ ldp(rfp, lr, Address(sp, -2 * wordSize));

> This same bug was present in SharedRuntime::generate_deopt_blob() and
> SharedRuntime::generate_uncommon_trap_blob().  I suppose the code was
> copied from one to another, or maybe it was a common programmer
> thinko.

In fact, they were all copied from the corresponding x86 code which also
does not bother to do anything with the frame pointer or return address.

regards,


Andrew Dinn
-----------