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

[Lutz Schmidt]

On Tue, 15 Dec 2020 15:34:33 GMT, Richard Reingruber <rrich at openjdk.org> wrote:

> // Continuation of https://github.com/openjdk/jdk/pull/1724
> // I'm taking over from @TheRealMDoerr who's busy with 11u downports
> 
> Please help review this small PPC64 fix.
> It replaces the call os::current_stack_pointer() in os::current_frame()
> with __builtin_frame_address(0) which is available also with xlC used by the AIX
> build. With this os::current_frame() is agnostic to C++ compiler inlining.
> 
> Furthermore the fix changes os::current_frame() to return the correct
> frame. Without the fix the returned frame is one frame to high which triggered
> the assertion in trace_method_handle_stub().
> This change uncovers an issue with NativeCallStack::NativeCallStack(int toSkip, bool fillStack)
> on PPC64. There the call os::get_native_stack() is not compiled
> to a tail call therefore the frame of the NativeCallStack constructor needs to
> be skipped on PPC64 too as on other platforms.
> 
> Without this
> 
> make run-test TEST=test/hotspot/jtreg/runtime/NMT/CheckForProperDetailStackTrace.java
> 
> fails.
> 
> Furthermore the inline assembler from os::current_stack_pointer() is
> removed. Instead __builtin_frame_address(0) is used again.
> 
> The fix passed our CI testing: JCK and JTREG, also in Xcomp mode, SPECjvm2008, SPECjbb2015, Renaissance Suite,
> SAP specific tests with fastdebug and release builds on all platforms

Changes look good. Thanks for fixing.

Side note (not relevant for approval): Wouldn't it be nice to use the builtin just once and call os::current_stack_pointer() in os::current_frame()?

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

Marked as reviewed by lucy (Reviewer).

PR: https://git.openjdk.java.net/jdk16/pull/24