RFR: 8256843: [PPC64] runtime/logging/RedefineClasses.java fails with assert: registers not saved on stack

[Richard Reingruber]

On Wed, 9 Dec 2020 22:11:18 GMT, Martin Doerr <mdoerr at openjdk.org> wrote:

> Method handle logging is broken in fastdebug builds. Problem is that os::current_frame() doesn't return the right frame in fastdebug builds.
> Unfortunately, fixing os::current_frame() would break NMT stack traces (runtime/NMT/CheckForProperDetailStackTrace.java).

Hi Martin,

the fix is, as you commented too, a hack.

I think that (A) os::current_stack_pointer() is inlined into os::current_frame() with optimized builds (all except slowdebug) so for the slowdebug build (_NMT_NOINLINE_ is defined) the sender of the sender of topframe should be returned. For optimized builds just the sender of topframe should be returned (see implementation on s390).

I think furthermore that (B) the compilers on PPC64 don't use a tail call in NativeCallStack::NativeCallStack() so one more frame needs to be skipped.

I'd imagine a fix like the following.

diff --git a/src/hotspot/os_cpu/aix_ppc/os_aix_ppc.cpp b/src/hotspot/os_cpu/aix_ppc/os_aix_ppc.cpp
index 2eaf38d05cd..f6114062f71 100644
--- a/src/hotspot/os_cpu/aix_ppc/os_aix_ppc.cpp
+++ b/src/hotspot/os_cpu/aix_ppc/os_aix_ppc.cpp
@@ -164,8 +164,12 @@ frame os::current_frame() {
   frame topframe(csp, (address)0x8);
   // Return sender of sender of current topframe which hopefully
   // both have pc != NULL.
+#ifdef _NMT_NOINLINE_
   frame tmp = os::get_sender_for_C_frame(&topframe);
   return os::get_sender_for_C_frame(&tmp);
+#else
+  return os::get_sender_for_C_frame(&topframe);
+#endif
 }
 
 bool PosixSignals::pd_hotspot_signal_handler(int sig, siginfo_t* info,
diff --git a/src/hotspot/os_cpu/linux_ppc/os_linux_ppc.cpp b/src/hotspot/os_cpu/linux_ppc/os_linux_ppc.cpp
index 5d68c4b3407..18d845cc4f3 100644
--- a/src/hotspot/os_cpu/linux_ppc/os_linux_ppc.cpp
+++ b/src/hotspot/os_cpu/linux_ppc/os_linux_ppc.cpp
@@ -184,8 +184,12 @@ frame os::current_frame() {
   frame topframe(csp, (address)0x8);
   // Return sender of sender of current topframe which hopefully
   // both have pc != NULL.
+#ifdef _NMT_NOINLINE_
   frame tmp = os::get_sender_for_C_frame(&topframe);
   return os::get_sender_for_C_frame(&tmp);
+#else
+  return os::get_sender_for_C_frame(&topframe);
+#endif
 }
 
 bool PosixSignals::pd_hotspot_signal_handler(int sig, siginfo_t* info,
diff --git a/src/hotspot/share/utilities/nativeCallStack.cpp b/src/hotspot/share/utilities/nativeCallStack.cpp
index 1093a13eaec..45a29424fd7 100644
--- a/src/hotspot/share/utilities/nativeCallStack.cpp
+++ b/src/hotspot/share/utilities/nativeCallStack.cpp
@@ -36,7 +36,7 @@ NativeCallStack::NativeCallStack(int toSkip, bool fillStack) :
     // to call os::get_native_stack. A tail call is used if _NMT_NOINLINE_ is not defined
     // (which means this is not a slowdebug build), and we are on 64-bit (except Windows).
     // This is not necessarily a rule, but what has been obvserved to date.
-#if (defined(_NMT_NOINLINE_) || defined(_WINDOWS) || !defined(_LP64))
+#if (defined(_NMT_NOINLINE_) || defined(_WINDOWS) || !defined(_LP64) || defined(PPC64))
     // Not a tail call.
     toSkip++;
 #if (defined(_NMT_NOINLINE_) && defined(BSD) && defined(_LP64))

I tested it successfully on Linux and AIX with the fastdebug build (RedefineClasses.java with -XX:+Verbose).

What do you think?

Cheers, Richard.

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

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