[aarch64-port-dev ] Request review of fix for forward_exception_handler

[Andrew Dinn]

The following patch to StubGenerator::generate_forward_exception()
appears to be necessary to make exception forwarding work.

diff -r 0d89d051c190 src/cpu/aarch64/vm/stubGenerator_aarch64.cpp
--- a/src/cpu/aarch64/vm/stubGenerator_aarch64.cpp	Fri Aug 09 16:25:29
2013 +0100
+++ b/src/cpu/aarch64/vm/stubGenerator_aarch64.cpp	Fri Aug 09 16:26:08
2013 +0100
@@ -504,6 +504,8 @@
     __ call_VM_leaf(CAST_FROM_FN_PTR(address,

SharedRuntime::exception_handler_for_return_address),
                     rthread, c_rarg1);
+    // restore lr
+    __ mov(lr, r19);
     // setup r0 & r3 & clear pending exception
     __ mov(r3, r19);
     __ mov(r19, r0);

The reason for this is less than obvious so I'll explain where it goes
wrong under C2 -- n.b. I think the fact that we are entering this
routine is itself an error but I am not thereby convinced that the patch
is unnecesaary.

I am seeing an exception return from a call to native method
java/security/AccessController.doPrivileged(Ljava/security/PrivilegedExceptionAction;Ljava/security/AccessControlContext;)Ljava/lang/Object;
The native wrapper method checks for a pending exception when the native
code returns and punts to Stub::forward exception when it finds it is
not null. Before doing so it calls leave(). simcputrace shows it
executing this:

  0x00007fffed1708d0: mov	sp, xfp
  0x00007fffed1708d4: ldp	xfp, xlr, [sp],#16
  0x00007fffed1708d8: ldr	xscratch1, [xthread,#8]
  0x00007fffed1708dc: cbnz	xscratch1, 0x00007fffed1708e4
  0x00007fffed1708e4: b	Stub::forward exception  ;   {runtime_call}

So, when we enter the exception forwarding code we have no frame and the
return address into the caller of AccessController.doPrivileged is in
register lr.

The exception forwarder calls into the runtime to find the relevant
handler passing the thread and return address. It loads the exception
from the thread into r0, clearing the thread field and jumps to the
returned address

   0x00007fffed07a5a0: ldr	xscratch1, [xthread,#8]
  0x00007fffed07a5a4: cbnz	xscratch1, Stub::forward exception+120
0x0x7fffed07a618
  0x00007fffed07a618: mov	x1, xlr
  0x00007fffed07a61c: mov	x19, xlr
  0x00007fffed07a620: mov	x0, xthread
  0x00007fffed07a624: stp	xscratch1, xmethod, [sp,#-16]!
  0x00007fffed07a628: movz	xscratch1, #0xcc92
  0x00007fffed07a62c: movk	xscratch1, #0xf74b, lsl #16
  0x00007fffed07a630: movk	xscratch1, #0x7fff, lsl #32
  0x00007fffed07a634: brx86	xscratch1, #3, #0, #1
  0x00007fffed07a638: ldp	xscratch1, xmethod, [sp],#16
  0x00007fffed07a63c: mov	x3, x19
  0x00007fffed07a640: mov	x19, x0
  0x00007fffed07a644: ldr	x0, [xthread,#8]
  0x00007fffed07a648: str	xzr, [xthread,#8]
  0x00007fffed07a64c: cbnz	x0, Stub::forward exception+288 0x0x7fffed07a6c0
  0x00007fffed07a6c0: br	x19

n.b. 0x7ffff74bcc92 is
SharedRuntime::exception_handler_for_return_address(JavaThread*,
unsigned char*).

Note that the exception forward handler saves xlr in x19 as well as
passing it into the native call via x1 so it can restore it into r3
after the call -- the handler expects an oop in r0 and a return address
in r3. On x86 this pair of moves is not needed because the call to leave
has still left the return address on the stack. Instead the x86 code
pops the top of the stack into rdx after the branch into the runtime.

The important thing to note here is that on AArch64 the brx86 call
trashes lr, leaving it containing the address after the brx86 call.
Whereas on x86 the return address passed in rdx is for the caller of the
native wrapper. This is the thing I believe to be wrong. Her eis how it
continues to calamity.

In the case I am looking at exception_handler_for_return_address uses
the value passed in x1 to identify the correct caller continuation (it
is at the end of method
java/net/URLClassLoader.findClass(Ljava/lang/String;)Ljava/lang/Class;)

  0x00007fffed07a6c0: br	x19
[Exception Handler]
  0x00007fffed1b8750: b	0x00007fffed07f060      ;   {runtime_call}

As you can see it immediately punts to the opto handling method
OptoRuntime::handle_exception_Java:

  0x00007fffed07f060: stp	xfp, xlr, [sp,#-16]!
  0x00007fffed07f064: str	x0, [xthread,#704]
  0x00007fffed07f068: str	x3, [xthread,#712]
  0x00007fffed07f06c: adr	xscratch1, 0x00007fffed07f06c
  0x00007fffed07f070: str	xscratch1, [xthread,#512]
  0x00007fffed07f074: mov	xscratch1, sp
  0x00007fffed07f078: str	xscratch1, [xthread,#504]
  0x00007fffed07f07c: str	xfp, [xthread,#520]
  0x00007fffed07f080: mov	x0, xthread
  ;; 0x7FFFF74B3018
  0x00007fffed07f084: movz	xscratch1, #0x3018
  0x00007fffed07f088: movk	xscratch1, #0xf74b, lsl #16
  0x00007fffed07f08c: movk	xscratch1, #0x7fff, lsl #32
  0x00007fffed07f090: movk	xscratch1, #0x0, lsl #48
  0x00007fffed07f094: brx86	xscratch1, #1, #0, #1

OptoRuntime::handle_exception_Java sets up details of the current java
sp/fp/pc in the thread and then calls OptoRuntime::handle_exception_C.
Note that it creates a frame for itself at entry. Unfortunately, the
value in xlr is not the address in URLClassLoader.findClass where the
exception occurred but the address in forward_exception just following
the brx86. OptoRuntime::handle_exception_C looks 2 frames up to find
this address and asserts that it belongs to a nmethod. Only it doesn't!

So, my fix makes the exception forwarder code copy r19 back to xlr after
the brx86 call as well as copying it into r1.

I think this analysis is correct. It certainly allows the code to
continue further albeit only to the point where the uncaught exception
(an NPE) pops out of main. Can anyone see any problems with this?

regards,


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