RFR: 8277948: AArch64: Print the correct native stack if -XX:+PreserveFramePointer when crash

[Andrew Haley]

On Fri, 7 Jan 2022 01:43:30 GMT, Denghui Dong <ddong at openjdk.org> wrote:

> > I've had a good look at this - in fact spent all morning on it - and this is the wrong fix.
> > For example, it breaks the `pfl()` function in the test case. `pfl()` isn't called from anywhere in the JDK, but it is one of our essential debugging tools. If you're interested in pursuing this further I could explain what else to try, but I don't have any time to spend on this myself. Sorry.
> 
> Thanks for the comment. It would be nice if you could give me some other way that helps fix the problem.

OK. The following changes cause `dtrace_object_alloc()` to call `pfl()`. This should print the entire stack. (You can also clone https://github.com/theRealAph/jdk , branch `pull/6597` for the same code. With your patch included and `PreserveFramePointer` enabled, `pfl()` crashes. So it seems like your patch fixes one thing, but breaks other uses of stack walking.


diff --git a/src/hotspot/cpu/aarch64/c1_MacroAssembler_aarch64.cpp b/src/hotspot/cpu/aarch64/c1_MacroAssembler_aarch64.cpp
index 661fad89e47..3fa80da73f7 100644
--- a/src/hotspot/cpu/aarch64/c1_MacroAssembler_aarch64.cpp
+++ b/src/hotspot/cpu/aarch64/c1_MacroAssembler_aarch64.cpp
@@ -237,7 +237,9 @@ void C1_MacroAssembler::initialize_object(Register obj, Register klass, Register
 
   if (CURRENT_ENV->dtrace_alloc_probes()) {
     assert(obj == r0, "must be");
+    set_last_Java_frame(sp, rfp, (address)pc(), rscratch1);
     far_call(RuntimeAddress(Runtime1::entry_for(Runtime1::dtrace_object_alloc_id)));
+    reset_last_Java_frame(true);
   }
 
   verify_oop(obj);
@@ -270,7 +272,9 @@ void C1_MacroAssembler::allocate_array(Register obj, Register len, Register t1,
 
   if (CURRENT_ENV->dtrace_alloc_probes()) {
     assert(obj == r0, "must be");
+    set_last_Java_frame(sp, rfp, (address)pc(), rscratch1);
     far_call(RuntimeAddress(Runtime1::entry_for(Runtime1::dtrace_object_alloc_id)));
+    reset_last_Java_frame(true);
   }
 
   verify_oop(obj);
diff --git a/src/hotspot/cpu/aarch64/c1_Runtime1_aarch64.cpp b/src/hotspot/cpu/aarch64/c1_Runtime1_aarch64.cpp
index 005f739f0aa..b1da03398cf 100644
--- a/src/hotspot/cpu/aarch64/c1_Runtime1_aarch64.cpp
+++ b/src/hotspot/cpu/aarch64/c1_Runtime1_aarch64.cpp
@@ -1091,7 +1091,9 @@ OopMapSet* Runtime1::generate_code_for(StubID id, StubAssembler* sasm) {
         StubFrame f(sasm, "dtrace_object_alloc", dont_gc_arguments);
         save_live_registers(sasm);
 
+        __ set_last_Java_frame(sp, rfp, (address)(__ pc()), rscratch1);
         __ call_VM_leaf(CAST_FROM_FN_PTR(address, static_cast<int (*)(oopDesc*)>(SharedRuntime::dtrace_object_alloc)), c_rarg0);
+        __ reset_last_Java_frame(true);
 
         restore_live_registers(sasm);
       }
diff --git a/src/hotspot/share/runtime/sharedRuntime.cpp b/src/hotspot/share/runtime/sharedRuntime.cpp
index a5de65ea5ab..5e09a1de120 100644
--- a/src/hotspot/share/runtime/sharedRuntime.cpp
+++ b/src/hotspot/share/runtime/sharedRuntime.cpp
@@ -996,12 +996,16 @@ jlong SharedRuntime::get_java_tid(Thread* thread) {
   return 0;
 }
 
+extern "C" void pfl();
+
 /**
  * This function ought to be a void function, but cannot be because
  * it gets turned into a tail-call on sparc, which runs into dtrace bug
  * 6254741.  Once that is fixed we can remove the dummy return value.
  */
 int SharedRuntime::dtrace_object_alloc(oopDesc* o) {
+  pfl();
+  *(int*)0 = 1;
   return dtrace_object_alloc(Thread::current(), o, o->size());
 }

-------------

PR: https://git.openjdk.java.net/jdk/pull/6597