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

[Richard Reingruber]

On Fri, 11 Dec 2020 10:18:38 GMT, Martin Doerr <mdoerr at openjdk.org> wrote:

>>> 
>>> I tested it successfully on Linux and AIX with the fastdebug build (RedefineClasses.java with -XX:+Verbose).
>> 
>> Tests succeeded also in slowdebug and release on Linux/PPC64 (-XX:+Verbose is not supported in release though).
>> 
>> test/hotspot/jtreg/runtime/NMT/CheckForProperDetailStackTrace.java failed in slowdebug on AIX. But that's no showstopper, is it?
>> My attempt to build also release on AIX failed because of an internal compiler error raised building libharfbuzz.
>
> Thanks for your proposal. I'll try it.
> 
>> test/hotspot/jtreg/runtime/NMT/CheckForProperDetailStackTrace.java failed in slowdebug on AIX. But that's no showstopper, is it?
> 
> If it fails without this change, it'd be no regression and hence ok.
> 
>> My attempt to build also release on AIX failed because of an internal compiler error raised building libharfbuzz.
> 
> That's a known xlc bug. Workaround:
> --- a/make/modules/java.desktop/lib/Awt2dLibraries.gmk
> +++ b/make/modules/java.desktop/lib/Awt2dLibraries.gmk
> @@ -475,6 +475,9 @@ else
>      #
> 
>    LIBHARFBUZZ_OPTIMIZATION := HIGH
> +  ifeq ($(call isTargetOs, aix), true)
> +    LIBHARFBUZZ_OPTIMIZATION := LOW
> +  endif
> 
>    LIBHARFBUZZ_CFLAGS += $(X_CFLAGS) -DLE_STANDALONE -DHEADLESS

Hi Martin

we can even do without _NMT_NOINLINE_.

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..83f8af3b751 100644
--- a/src/hotspot/os_cpu/aix_ppc/os_aix_ppc.cpp
+++ b/src/hotspot/os_cpu/aix_ppc/os_aix_ppc.cpp
@@ -159,13 +159,9 @@ frame os::get_sender_for_C_frame(frame* fr) {
 
 
 frame os::current_frame() {
-  intptr_t* csp = (intptr_t*) *((intptr_t*) os::current_stack_pointer());
-  // hack.
-  frame topframe(csp, (address)0x8);
-  // Return sender of sender of current topframe which hopefully
-  // both have pc != NULL.
-  frame tmp = os::get_sender_for_C_frame(&topframe);
-  return os::get_sender_for_C_frame(&tmp);
+  intptr_t* csp = *(intptr_t**) __builtin_frame_address(0);
+  frame topframe(csp, CAST_FROM_FN_PTR(address, os::current_frame));
+  return os::get_sender_for_C_frame(&topframe);
 }
 
 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..8e0af2acecc 100644
--- a/src/hotspot/os_cpu/linux_ppc/os_linux_ppc.cpp
+++ b/src/hotspot/os_cpu/linux_ppc/os_linux_ppc.cpp
@@ -179,13 +179,9 @@ frame os::get_sender_for_C_frame(frame* fr) {
 
 
 frame os::current_frame() {
-  intptr_t* csp = (intptr_t*) *((intptr_t*) os::current_stack_pointer());
-  // hack.
-  frame topframe(csp, (address)0x8);
-  // Return sender of sender of current topframe which hopefully
-  // both have pc != NULL.
-  frame tmp = os::get_sender_for_C_frame(&topframe);
-  return os::get_sender_for_C_frame(&tmp);
+  intptr_t* csp = *(intptr_t**) __builtin_frame_address(0);
+  frame topframe(csp, CAST_FROM_FN_PTR(address, os::current_frame));
+  return os::get_sender_for_C_frame(&topframe);
 }
 
 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))
diff --git a/test/hotspot/jtreg/runtime/logging/RedefineClasses.java b/test/hotspot/jtreg/runtime/logging/RedefineClasses.java

My manual tests succeed with this on aix and linuxppc64le, except for the slowdebug build on aix where test/hotspot/jtreg/runtime/NMT/CheckForProperDetailStackTrace.java fails with _and_ without the fix.

Currently I don't understand why the result of __builtin_frame_address(0) has to
be dereferenced? Looking at the disassembly of the slowdebug build it returns
the sp of os::current_frame(). The head revision does the same though.

(gdb) disass /s 0x7fffb71ed974,+64
Dump of assembler code from 0x7fffb71ed974 to 0x7fffb71ed9b4:

181	frame os::current_frame() {
   0x00007fffb71ed974 <os::current_frame()+0>:	addis   r2,r12,161
   0x00007fffb71ed978 <os::current_frame()+4>:	addi    r2,r2,-31604
   0x00007fffb71ed97c <os::current_frame()+8>:	mflr    r0
   0x00007fffb71ed980 <os::current_frame()+12>:	std     r0,16(r1)
   0x00007fffb71ed984 <os::current_frame()+16>:	std     r31,-8(r1)
   0x00007fffb71ed988 <os::current_frame()+20>:	stdu    r1,-128(r1)
   0x00007fffb71ed98c <os::current_frame()+24>:	mr      r31,r1
   0x00007fffb71ed990 <os::current_frame()+28>:	std     r3,40(r31)

182	  intptr_t* csp = *(intptr_t**) __builtin_frame_address(0);
   0x00007fffb71ed994 <os::current_frame()+32>:	mr      r9,r31
   0x00007fffb71ed998 <os::current_frame()+36>:	ld      r9,0(r9)
   0x00007fffb71ed99c <os::current_frame()+40>:	std     r9,56(r31)

183	  // hack.
184	  frame topframe(csp, CAST_FROM_FN_PTR(address, os::current_frame));
   0x00007fffb71ed9a0 <os::current_frame()+44>:	addi    r9,r31,64
   0x00007fffb71ed9a4 <os::current_frame()+48>:	addis   r5,r2,-161
   0x00007fffb71ed9a8 <os::current_frame()+52>:	addi    r5,r5,31604
   0x00007fffb71ed9ac <os::current_frame()+56>:	ld      r4,56(r31)
   0x00007fffb71ed9b0 <os::current_frame()+60>:	mr      r3,r9

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

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