From 041924ab32602bf0c77a50dfe33667866c5b6b37 Mon Sep 17 00:00:00 2001 From: Johannes Bechberger Date: Tue, 7 Jun 2022 08:08:06 +0000 Subject: [PATCH] 8282477: [x86, aarch64] vmassert(_last_Java_pc == NULL, "already walkable"); fails with async profiler Reviewed-by: mdoerr Backport-of: 4b2c82200fdc01de868cf414e40a4d891e753f89 --- src/hotspot/cpu/aarch64/frame_aarch64.cpp | 16 +++---------- .../cpu/aarch64/javaFrameAnchor_aarch64.hpp | 4 ++-- src/hotspot/cpu/arm/javaFrameAnchor_arm.hpp | 2 +- src/hotspot/cpu/ppc/javaFrameAnchor_ppc.hpp | 2 +- src/hotspot/cpu/s390/javaFrameAnchor_s390.hpp | 2 +- src/hotspot/cpu/x86/frame_x86.cpp | 24 ++++--------------- src/hotspot/cpu/x86/javaFrameAnchor_x86.hpp | 3 +-- src/hotspot/cpu/zero/javaFrameAnchor_zero.hpp | 2 +- src/hotspot/share/prims/jvmtiExport.cpp | 2 +- .../share/runtime/interfaceSupport.inline.hpp | 8 +++---- src/hotspot/share/runtime/java.cpp | 2 +- src/hotspot/share/runtime/safepoint.cpp | 2 +- src/hotspot/share/runtime/thread.cpp | 2 +- src/hotspot/share/runtime/thread.hpp | 2 +- 14 files changed, 23 insertions(+), 50 deletions(-) diff --git a/src/hotspot/cpu/aarch64/frame_aarch64.cpp b/src/hotspot/cpu/aarch64/frame_aarch64.cpp index 760eb2467c8..f9299d22006 100644 --- a/src/hotspot/cpu/aarch64/frame_aarch64.cpp +++ b/src/hotspot/cpu/aarch64/frame_aarch64.cpp @@ -350,13 +350,9 @@ frame frame::sender_for_entry_frame(RegisterMap* map) const { assert(jfa->last_Java_sp() > sp(), "must be above this frame on stack"); // Since we are walking the stack now this nested anchor is obviously walkable // even if it wasn't when it was stacked. - if (!jfa->walkable()) { - // Capture _last_Java_pc (if needed) and mark anchor walkable. - jfa->capture_last_Java_pc(); - } + jfa->make_walkable(); map->clear(); assert(map->include_argument_oops(), "should be set by clear"); - vmassert(jfa->last_Java_pc() != NULL, "not walkable"); frame fr(jfa->last_Java_sp(), jfa->last_Java_fp(), jfa->last_Java_pc()); return fr; @@ -817,20 +813,14 @@ frame::frame(void* sp, void* fp, void* pc) { void frame::pd_ps() {} #endif -void JavaFrameAnchor::make_walkable(JavaThread* thread) { +void JavaFrameAnchor::make_walkable() { // last frame set? if (last_Java_sp() == NULL) return; // already walkable? if (walkable()) return; - vmassert(Thread::current() == (Thread*)thread, "not current thread"); vmassert(last_Java_sp() != NULL, "not called from Java code?"); vmassert(last_Java_pc() == NULL, "already walkable"); - capture_last_Java_pc(); + _last_Java_pc = (address)_last_Java_sp[-1]; vmassert(walkable(), "something went wrong"); } -void JavaFrameAnchor::capture_last_Java_pc() { - vmassert(_last_Java_sp != NULL, "no last frame set"); - vmassert(_last_Java_pc == NULL, "already walkable"); - _last_Java_pc = (address)_last_Java_sp[-1]; -} diff --git a/src/hotspot/cpu/aarch64/javaFrameAnchor_aarch64.hpp b/src/hotspot/cpu/aarch64/javaFrameAnchor_aarch64.hpp index 6ff3c037407..a11bf0dca81 100644 --- a/src/hotspot/cpu/aarch64/javaFrameAnchor_aarch64.hpp +++ b/src/hotspot/cpu/aarch64/javaFrameAnchor_aarch64.hpp @@ -65,8 +65,8 @@ public: } bool walkable(void) { return _last_Java_sp != NULL && _last_Java_pc != NULL; } - void make_walkable(JavaThread* thread); - void capture_last_Java_pc(void); + + void make_walkable(); intptr_t* last_Java_sp(void) const { return _last_Java_sp; } diff --git a/src/hotspot/cpu/arm/javaFrameAnchor_arm.hpp b/src/hotspot/cpu/arm/javaFrameAnchor_arm.hpp index 85585543f49..6aefc1b0a8c 100644 --- a/src/hotspot/cpu/arm/javaFrameAnchor_arm.hpp +++ b/src/hotspot/cpu/arm/javaFrameAnchor_arm.hpp @@ -65,7 +65,7 @@ public: // Always walkable bool walkable(void) { return true; } // Never any thing to do since we are always walkable and can find address of return addresses - void make_walkable(JavaThread* thread) { } + void make_walkable() { } intptr_t* last_Java_sp(void) const { return _last_Java_sp; } diff --git a/src/hotspot/cpu/ppc/javaFrameAnchor_ppc.hpp b/src/hotspot/cpu/ppc/javaFrameAnchor_ppc.hpp index a30ea1c175b..d78ba49cd64 100644 --- a/src/hotspot/cpu/ppc/javaFrameAnchor_ppc.hpp +++ b/src/hotspot/cpu/ppc/javaFrameAnchor_ppc.hpp @@ -67,7 +67,7 @@ public: // Always walkable. bool walkable(void) { return true; } // Never any thing to do since we are always walkable and can find address of return addresses. - void make_walkable(JavaThread* thread) { } + void make_walkable() { } intptr_t* last_Java_sp(void) const { return _last_Java_sp; } diff --git a/src/hotspot/cpu/s390/javaFrameAnchor_s390.hpp b/src/hotspot/cpu/s390/javaFrameAnchor_s390.hpp index 6c17541883d..9f401d89c28 100644 --- a/src/hotspot/cpu/s390/javaFrameAnchor_s390.hpp +++ b/src/hotspot/cpu/s390/javaFrameAnchor_s390.hpp @@ -72,7 +72,7 @@ // We don't have to flush registers, so the stack is always walkable. inline bool walkable(void) { return true; } - inline void make_walkable(JavaThread* thread) { } + inline void make_walkable() { } public: diff --git a/src/hotspot/cpu/x86/frame_x86.cpp b/src/hotspot/cpu/x86/frame_x86.cpp index cc4e33f47e8..3faf6f99db8 100644 --- a/src/hotspot/cpu/x86/frame_x86.cpp +++ b/src/hotspot/cpu/x86/frame_x86.cpp @@ -344,13 +344,9 @@ frame frame::sender_for_entry_frame(RegisterMap* map) const { assert(jfa->last_Java_sp() > sp(), "must be above this frame on stack"); // Since we are walking the stack now this nested anchor is obviously walkable // even if it wasn't when it was stacked. - if (!jfa->walkable()) { - // Capture _last_Java_pc (if needed) and mark anchor walkable. - jfa->capture_last_Java_pc(); - } + jfa->make_walkable(); map->clear(); assert(map->include_argument_oops(), "should be set by clear"); - vmassert(jfa->last_Java_pc() != NULL, "not walkable"); frame fr(jfa->last_Java_sp(), jfa->last_Java_fp(), jfa->last_Java_pc()); return fr; @@ -380,13 +376,9 @@ frame frame::sender_for_optimized_entry_frame(RegisterMap* map) const { assert(jfa->last_Java_sp() > sp(), "must be above this frame on stack"); // Since we are walking the stack now this nested anchor is obviously walkable // even if it wasn't when it was stacked. - if (!jfa->walkable()) { - // Capture _last_Java_pc (if needed) and mark anchor walkable. - jfa->capture_last_Java_pc(); - } + jfa->make_walkable(); map->clear(); assert(map->include_argument_oops(), "should be set by clear"); - vmassert(jfa->last_Java_pc() != NULL, "not walkable"); frame fr(jfa->last_Java_sp(), jfa->last_Java_fp(), jfa->last_Java_pc()); return fr; @@ -724,20 +716,12 @@ frame::frame(void* sp, void* fp, void* pc) { void frame::pd_ps() {} #endif -void JavaFrameAnchor::make_walkable(JavaThread* thread) { +void JavaFrameAnchor::make_walkable() { // last frame set? if (last_Java_sp() == NULL) return; // already walkable? if (walkable()) return; - vmassert(Thread::current() == (Thread*)thread, "not current thread"); - vmassert(last_Java_sp() != NULL, "not called from Java code?"); vmassert(last_Java_pc() == NULL, "already walkable"); - capture_last_Java_pc(); - vmassert(walkable(), "something went wrong"); -} - -void JavaFrameAnchor::capture_last_Java_pc() { - vmassert(_last_Java_sp != NULL, "no last frame set"); - vmassert(_last_Java_pc == NULL, "already walkable"); _last_Java_pc = (address)_last_Java_sp[-1]; + vmassert(walkable(), "something went wrong"); } diff --git a/src/hotspot/cpu/x86/javaFrameAnchor_x86.hpp b/src/hotspot/cpu/x86/javaFrameAnchor_x86.hpp index 6fa2b55033b..050e36e9dc4 100644 --- a/src/hotspot/cpu/x86/javaFrameAnchor_x86.hpp +++ b/src/hotspot/cpu/x86/javaFrameAnchor_x86.hpp @@ -63,8 +63,7 @@ public: } bool walkable(void) { return _last_Java_sp != NULL && _last_Java_pc != NULL; } - void make_walkable(JavaThread* thread); - void capture_last_Java_pc(void); + void make_walkable(); intptr_t* last_Java_sp(void) const { return _last_Java_sp; } diff --git a/src/hotspot/cpu/zero/javaFrameAnchor_zero.hpp b/src/hotspot/cpu/zero/javaFrameAnchor_zero.hpp index a51e60a5070..cb92f1b97a4 100644 --- a/src/hotspot/cpu/zero/javaFrameAnchor_zero.hpp +++ b/src/hotspot/cpu/zero/javaFrameAnchor_zero.hpp @@ -73,7 +73,7 @@ return true; } - void make_walkable(JavaThread* thread) { + void make_walkable() { // nothing to do } diff --git a/src/hotspot/share/prims/jvmtiExport.cpp b/src/hotspot/share/prims/jvmtiExport.cpp index dd0f1885c8b..ceab8da5c49 100644 --- a/src/hotspot/share/prims/jvmtiExport.cpp +++ b/src/hotspot/share/prims/jvmtiExport.cpp @@ -169,7 +169,7 @@ public: thread->set_active_handles(new_handles); #endif assert(thread == JavaThread::current(), "thread must be current!"); - thread->frame_anchor()->make_walkable(thread); + thread->frame_anchor()->make_walkable(); }; ~JvmtiEventMark() { diff --git a/src/hotspot/share/runtime/interfaceSupport.inline.hpp b/src/hotspot/share/runtime/interfaceSupport.inline.hpp index 1e345b01caa..7cb6eeff078 100644 --- a/src/hotspot/share/runtime/interfaceSupport.inline.hpp +++ b/src/hotspot/share/runtime/interfaceSupport.inline.hpp @@ -138,7 +138,7 @@ class ThreadInVMForHandshake : public ThreadStateTransition { _original_state(thread->thread_state()) { if (thread->has_last_Java_frame()) { - thread->frame_anchor()->make_walkable(thread); + thread->frame_anchor()->make_walkable(); } thread->set_thread_state(_thread_in_vm); @@ -209,7 +209,7 @@ class ThreadInVMfromNative : public ThreadStateTransition { // we call known native code using this wrapper holding locks. _thread->check_possible_safepoint(); // Once we are in native vm expects stack to be walkable - _thread->frame_anchor()->make_walkable(_thread); + _thread->frame_anchor()->make_walkable(); OrderAccess::storestore(); // Keep thread_state change and make_walkable() separate. _thread->set_thread_state(_thread_in_native); } @@ -222,7 +222,7 @@ class ThreadToNativeFromVM : public ThreadStateTransition { // We are leaving the VM at this point and going directly to native code. // Block, if we are in the middle of a safepoint synchronization. assert(!thread->owns_locks(), "must release all locks when leaving VM"); - thread->frame_anchor()->make_walkable(thread); + thread->frame_anchor()->make_walkable(); trans(_thread_in_vm, _thread_in_native); // Check for pending. async. exceptions or suspends. if (_thread->has_special_runtime_exit_condition()) _thread->handle_special_runtime_exit_condition(false); @@ -250,7 +250,7 @@ class ThreadBlockInVMPreprocess : public ThreadStateTransition { assert(thread->thread_state() == _thread_in_vm, "coming from wrong thread state"); thread->check_possible_safepoint(); // Once we are blocked vm expects stack to be walkable - thread->frame_anchor()->make_walkable(thread); + thread->frame_anchor()->make_walkable(); OrderAccess::storestore(); // Keep thread_state change and make_walkable() separate. thread->set_thread_state(_thread_blocked); } diff --git a/src/hotspot/share/runtime/java.cpp b/src/hotspot/share/runtime/java.cpp index 820bb5c3505..996bb7dbca1 100644 --- a/src/hotspot/share/runtime/java.cpp +++ b/src/hotspot/share/runtime/java.cpp @@ -617,7 +617,7 @@ void vm_perform_shutdown_actions() { JavaThread* jt = thread->as_Java_thread(); // Must always be walkable or have no last_Java_frame when in // thread_in_native - jt->frame_anchor()->make_walkable(jt); + jt->frame_anchor()->make_walkable(); jt->set_thread_state(_thread_in_native); } } diff --git a/src/hotspot/share/runtime/safepoint.cpp b/src/hotspot/share/runtime/safepoint.cpp index 3db6cc8b3ee..3191275884c 100644 --- a/src/hotspot/share/runtime/safepoint.cpp +++ b/src/hotspot/share/runtime/safepoint.cpp @@ -722,7 +722,7 @@ void SafepointSynchronize::block(JavaThread *thread) { } JavaThreadState state = thread->thread_state(); - thread->frame_anchor()->make_walkable(thread); + thread->frame_anchor()->make_walkable(); uint64_t safepoint_id = SafepointSynchronize::safepoint_counter(); // Check that we have a valid thread_state at this point diff --git a/src/hotspot/share/runtime/thread.cpp b/src/hotspot/share/runtime/thread.cpp index 8aed9a450b6..50ecc4d4688 100644 --- a/src/hotspot/share/runtime/thread.cpp +++ b/src/hotspot/share/runtime/thread.cpp @@ -1676,7 +1676,7 @@ void JavaThread::check_and_handle_async_exceptions() { void JavaThread::handle_special_runtime_exit_condition(bool check_asyncs) { if (is_obj_deopt_suspend()) { - frame_anchor()->make_walkable(this); + frame_anchor()->make_walkable(); wait_for_object_deoptimization(); } diff --git a/src/hotspot/share/runtime/thread.hpp b/src/hotspot/share/runtime/thread.hpp index 9700d6be2a5..9a3fe0efe6e 100644 --- a/src/hotspot/share/runtime/thread.hpp +++ b/src/hotspot/share/runtime/thread.hpp @@ -1428,7 +1428,7 @@ class JavaThread: public Thread { public: // Accessing frames frame last_frame() { - _anchor.make_walkable(this); + _anchor.make_walkable(); return pd_last_frame(); } javaVFrame* last_java_vframe(RegisterMap* reg_map); -- 2.43.2