changeset 10808:b58d341cef20

8209413, PR3797: AArch64: NPE in clhsdb jstack command Reviewed-by: adinn
author ngasson
date Thu, 07 May 2020 09:11:54 -0400
parents 8c532622ac0b
children 6deccb5af3b4
files agent/src/share/classes/sun/jvm/hotspot/runtime/aarch64/AARCH64CurrentFrameGuess.java agent/src/share/classes/sun/jvm/hotspot/runtime/aarch64/AARCH64Frame.java src/cpu/aarch64/vm/sharedRuntime_aarch64.cpp src/cpu/aarch64/vm/templateInterpreter_aarch64.cpp
diffstat 4 files changed, 37 insertions(+), 18 deletions(-) [+]
line wrap: on
line diff
--- a/agent/src/share/classes/sun/jvm/hotspot/runtime/aarch64/AARCH64CurrentFrameGuess.java	Wed Aug 12 19:59:24 2020 +0800
+++ b/agent/src/share/classes/sun/jvm/hotspot/runtime/aarch64/AARCH64CurrentFrameGuess.java	Thu May 07 09:11:54 2020 -0400
@@ -1,6 +1,6 @@
 /*
- * Copyright (c) 2003, 2006, Oracle and/or its affiliates. All rights reserved.
- * Copyright (c) 2015, Red Hat Inc.
+ * Copyright (c) 2003, 2019, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2015, 2019, Red Hat Inc.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -223,7 +223,13 @@
         }
       }
 
-      setValues(sp, fp, null);
+      // We found a PC in the frame anchor. Check that it's plausible, and
+      // if it is, use it.
+      if (vm.isJavaPCDbg(pc)) {
+        setValues(sp, fp, pc);
+      } else {
+        setValues(sp, fp, null);
+      }
 
       return true;
     }
--- a/agent/src/share/classes/sun/jvm/hotspot/runtime/aarch64/AARCH64Frame.java	Wed Aug 12 19:59:24 2020 +0800
+++ b/agent/src/share/classes/sun/jvm/hotspot/runtime/aarch64/AARCH64Frame.java	Thu May 07 09:11:54 2020 -0400
@@ -1,6 +1,6 @@
 /*
- * Copyright (c) 2001, 2012, Oracle and/or its affiliates. All rights reserved.
- * Copyright (c) 2015, Red Hat Inc.
+ * Copyright (c) 2001, 2019, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2015, 2019, Red Hat Inc.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -133,7 +133,15 @@
     this.raw_sp = raw_sp;
     this.raw_unextendedSP = raw_sp;
     this.raw_fp = raw_fp;
-    this.pc = raw_sp.getAddressAt(-1 * VM.getVM().getAddressSize());
+
+    // We cannot assume SP[-1] always contains a valid return PC (e.g. if
+    // the callee is a C/C++ compiled frame). If the PC is not known to
+    // Java then this.pc is null.
+    Address savedPC = raw_sp.getAddressAt(-1 * VM.getVM().getAddressSize());
+    if (VM.getVM().isJavaPCDbg(savedPC)) {
+      this.pc = savedPC;
+    }
+
     adjustUnextendedSP();
 
     // Frame must be fully constructed before this call
--- a/src/cpu/aarch64/vm/sharedRuntime_aarch64.cpp	Wed Aug 12 19:59:24 2020 +0800
+++ b/src/cpu/aarch64/vm/sharedRuntime_aarch64.cpp	Thu May 07 09:11:54 2020 -0400
@@ -1662,14 +1662,11 @@
   }
 
   // Change state to native (we save the return address in the thread, since it might not
-  // be pushed on the stack when we do a a stack traversal). It is enough that the pc()
-  // points into the right code segment. It does not have to be the correct return pc.
+  // be pushed on the stack when we do a stack traversal).
   // We use the same pc/oopMap repeatedly when we call out
 
-  intptr_t the_pc = (intptr_t) __ pc();
-  oop_maps->add_gc_map(the_pc - start, map);
-
-  __ set_last_Java_frame(sp, noreg, (address)the_pc, rscratch1);
+  Label native_return;
+  __ set_last_Java_frame(sp, noreg, native_return, rscratch1);
 
   Label dtrace_method_entry, dtrace_method_entry_done;
   {
@@ -1795,11 +1792,16 @@
       ShouldNotReachHere();
     }
     rt_call(masm, native_func,
-	    int_args + 2, // AArch64 passes up to 8 args in int registers
-	    float_args,   // and up to 8 float args
-	    return_type);
+            int_args + 2, // AArch64 passes up to 8 args in int registers
+            float_args,   // and up to 8 float args
+            return_type);
   }
 
+  __ bind(native_return);
+
+  intptr_t return_pc = (intptr_t) __ pc();
+  oop_maps->add_gc_map(return_pc - start, map);
+
   // Unpack native results.
   switch (ret_type) {
   case T_BOOLEAN: __ c2bool(r0);                     break;
--- a/src/cpu/aarch64/vm/templateInterpreter_aarch64.cpp	Wed Aug 12 19:59:24 2020 +0800
+++ b/src/cpu/aarch64/vm/templateInterpreter_aarch64.cpp	Thu May 07 09:11:54 2020 -0400
@@ -1069,9 +1069,11 @@
   // pass JNIEnv
   __ add(c_rarg0, rthread, in_bytes(JavaThread::jni_environment_offset()));
 
-  // It is enough that the pc() points into the right code
-  // segment. It does not have to be the correct return pc.
-  __ set_last_Java_frame(esp, rfp, (address)NULL, rscratch1);
+  // Set the last Java PC in the frame anchor to be the return address from
+  // the call to the native method: this will allow the debugger to
+  // generate an accurate stack trace.
+  Label native_return;
+  __ set_last_Java_frame(esp, rfp, native_return, rscratch1);
 
   // change thread state
 #ifdef ASSERT
@@ -1092,6 +1094,7 @@
 
   // Call the native method.
   __ blr(r10);
+  __ bind(native_return);
   __ maybe_isb();
   __ get_method(rmethod);
   // result potentially in r0 or v0