[aarch64-port-dev ] TestStackBangRbp hangs
Andrew Haley
aph at redhat.com
Mon Jan 13 03:55:10 PST 2014
On 01/08/2014 11:18 AM, Edward Nevill wrote:
> Hi,
>
> The following test program causes openjdk to hang indefinitely requiring a kill -9 to stop the process.
>
> To replicate
>
> - compile the test program
>
> javac TestStackBangRbp.java
>
> - run it as follows
>
> java -Xcomp TestStackBangRbp
>
> Note that the problem occurs for both client and server and also occurs on both builtin sim and on the RTSM model.
This was a real head-scratcher.
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
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:
// 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.
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.
The rest of this patch is tidying up as I want along and some
debugging improvements to the simulator environment that I had to make
in order to find the bug.
Andrew.
# HG changeset patch
# User aph
# Date 1389382344 0
# Node ID 277edfed6a723b4067f78a18544188261f540a35
# Parent 8ccf0e9d5070bda73a13e05f10fbd6b8e3154e8d
Properly restore frames when deoptimizing.
When removing a frame in the deoptimization handler, be sure to
restore LR anf FP.
Ensure compiled native methods begin with a NOP.
Notify simulator of deoptimization blobs.
diff -r 8ccf0e9d5070 -r 277edfed6a72 src/cpu/aarch64/vm/sharedRuntime_aarch64.cpp
--- a/src/cpu/aarch64/vm/sharedRuntime_aarch64.cpp Fri Jan 10 19:30:52 2014 +0000
+++ b/src/cpu/aarch64/vm/sharedRuntime_aarch64.cpp Fri Jan 10 19:32:24 2014 +0000
@@ -1542,12 +1542,14 @@
int vep_offset = ((intptr_t)__ pc()) - start;
- // The instruction at the verified entry point must be 5 bytes or longer
- // because it can be patched on the fly by make_non_entrant. The stack bang
- // instruction fits that requirement.
-
// Generate stack overflow check
+ // If we have to make this method not-entrant we'll overwrite its
+ // first instruction with a jump. For this action to be legal we
+ // must ensure that this first instruction is a B, BL, NOP, BKPT,
+ // SVC, HVC, or SMC. Make it a NOP.
+ __ nop();
+
if (UseStackBanging) {
__ bang_stack_with_offset(StackShadowPages*os::vm_page_size());
} else {
@@ -1993,7 +1995,7 @@
Label reguard;
Label reguard_done;
- __ ldrw(rscratch1, Address(rthread, JavaThread::stack_guard_state_offset()));
+ __ ldrb(rscratch1, Address(rthread, JavaThread::stack_guard_state_offset()));
__ cmpw(rscratch1, JavaThread::stack_guard_yellow_disabled);
__ br(Assembler::EQ, reguard);
__ bind(reguard_done);
@@ -2251,6 +2253,14 @@
OopMap* map = NULL;
OopMapSet *oop_maps = new OopMapSet();
+#ifdef BUILTIN_SIM
+ AArch64Simulator *simulator;
+ if (NotifySimulator) {
+ simulator = AArch64Simulator::get_current(UseSimulatorCache, DisableBCCheck);
+ simulator->notifyCompile(const_cast<char*>("SharedRuntime::deopt_blob"), __ pc());
+ }
+#endif
+
// -------------
// This code enters when returning to a de-optimized nmethod. A return
// address has been pushed on the the stack, and return values are in
@@ -2436,9 +2446,10 @@
// 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);
-
- // sp should be pointing at the return address to the caller (3)
+ __ ldp(rfp, lr, __ post(sp, 2 * wordSize));
+ // LR should now be the return address to the caller (3)
// Stack bang to make sure there's enough room for these interpreter frames.
if (UseStackBanging) {
@@ -2544,6 +2555,13 @@
_deopt_blob = DeoptimizationBlob::create(&buffer, oop_maps, 0, exception_offset, reexecute_offset, frame_size_in_words);
_deopt_blob->set_unpack_with_exception_in_tls_offset(exception_in_tls_offset);
+
+#ifdef BUILTIN_SIM
+ if (NotifySimulator) {
+ unsigned char *base = _deopt_blob->code_begin();
+ simulator->notifyRelocate(start, base - start);
+ }
+#endif
}
uint SharedRuntime::out_preserve_stack_slots() {
@@ -2559,6 +2577,14 @@
CodeBuffer buffer("uncommon_trap_blob", 2048, 1024);
MacroAssembler* masm = new MacroAssembler(&buffer);
+#ifdef BUILTIN_SIM
+ AArch64Simulator *simulator;
+ if (NotifySimulator) {
+ simulator = AArch64Simulator::get_current(UseSimulatorCache, DisableBCCheck);
+ simulator->notifyCompile(const_cast<char*>("SharedRuntime:uncommon_trap_blob"), __ pc());
+ }
+#endif
+
// TODO check various assumptions here
//
// call unimplemented to make sure we actually check this later
@@ -2631,9 +2657,10 @@
__ ldrw(r2, Address(r4,
Deoptimization::UnrollBlock::
size_of_deoptimized_frame_offset_in_bytes()));
+ __ sub(r2, r2, 2 * wordSize);
__ add(sp, sp, r2);
-
- // sp should now be pointing at the top of the caller (3) frame
+ __ ldp(rfp, lr, __ post(sp, 2 * wordSize));
+ // LR should now be the return address to the caller (3) frame
// Stack bang to make sure there's enough room for these interpreter frames.
if (UseStackBanging) {
@@ -2734,6 +2761,13 @@
_uncommon_trap_blob = UncommonTrapBlob::create(&buffer, oop_maps,
SimpleRuntimeFrame::framesize >> 1);
+
+#ifdef BUILTIN_SIM
+ if (NotifySimulator) {
+ unsigned char *base = _deopt_blob->code_begin();
+ simulator->notifyRelocate(start, base - start);
+ }
+#endif
}
#endif // COMPILER2
@@ -2744,7 +2778,7 @@
// and setup oopmap.
//
SafepointBlob* SharedRuntime::generate_handler_blob(address call_ptr, int poll_type) {
- ResourceMark rm;
+ ResourceMark rm;
OopMapSet *oop_maps = new OopMapSet();
OopMap* map;
diff -r 8ccf0e9d5070 -r 277edfed6a72 src/cpu/aarch64/vm/templateInterpreter_aarch64.cpp
--- a/src/cpu/aarch64/vm/templateInterpreter_aarch64.cpp Fri Jan 10 19:30:52 2014 +0000
+++ b/src/cpu/aarch64/vm/templateInterpreter_aarch64.cpp Fri Jan 10 19:32:24 2014 +0000
@@ -1016,11 +1016,10 @@
__ bind(no_oop);
}
-
{
Label no_reguard;
__ lea(rscratch1, Address(rthread, in_bytes(JavaThread::stack_guard_state_offset())));
- __ ldrw(rscratch1, Address(rscratch1));
+ __ ldrb(rscratch1, Address(rscratch1));
__ cmp(rscratch1, JavaThread::stack_guard_yellow_disabled);
__ br(Assembler::NE, no_reguard);
@@ -1029,11 +1028,9 @@
__ mov(rscratch2, CAST_FROM_FN_PTR(address, SharedRuntime::reguard_yellow_pages));
__ blrt(rscratch2, 0, 0, 0);
__ popa(); // XXX only restore smashed registers
- __ reinit_heapbase();
__ bind(no_reguard);
}
-
// The method register is junk from after the thread_in_native transition
// until here. Also can't call_VM until the bcp has been
// restored. Need bcp for throwing exception below so get it now.
More information about the aarch64-port-dev
mailing list