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

[Martin Doerr]

On Thu, 10 Dec 2020 21:36:09 GMT, Richard Reingruber <rrich at openjdk.org> wrote:

>> 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.
>
>> 
>> 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

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

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