Skip to content

Commit

Permalink
8283849: AsyncGetCallTrace may crash JVM on guarantee
Browse files Browse the repository at this point in the history
Reviewed-by: phh
Backport-of: 19639855311a47ed532547743ea3873eb8b016d3
  • Loading branch information
Jaroslav Bachorik committed Jul 8, 2022
1 parent ce73401 commit 6e8292f
Show file tree
Hide file tree
Showing 4 changed files with 20 additions and 2 deletions.
12 changes: 10 additions & 2 deletions hotspot/src/share/vm/code/codeCache.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -270,6 +270,10 @@ bool CodeCache::contains(void *p) {
return _heap->contains(p);
}

static bool is_in_asgct() {
Thread* current_thread = Thread::current_or_null();
return current_thread != NULL && current_thread->is_Java_thread() && ((JavaThread*)current_thread)->in_asgct();
}

// This method is safe to call without holding the CodeCache_lock, as long as a dead codeblob is not
// looked up (i.e., one that has been marked for deletion). It only dependes on the _segmap to contain
Expand All @@ -278,8 +282,12 @@ CodeBlob* CodeCache::find_blob(void* start) {
CodeBlob* result = find_blob_unsafe(start);
if (result == NULL) return NULL;
// We could potientially look up non_entrant methods
guarantee(!result->is_zombie() || result->is_locked_by_vm() || is_error_reported(), "unsafe access to zombie method");
return result;
bool is_zombie = result != NULL && result->is_zombie();
bool is_result_safe = !is_zombie || result->is_locked_by_vm() || is_error_reported();
guarantee(is_result_safe || is_in_asgct(), "unsafe access to zombie method");
// When in ASGCT the previous gurantee will pass for a zombie method but we still don't want that code blob returned in order
// to minimize the chance of accessing dead memory
return is_result_safe ? result : NULL;
}

nmethod* CodeCache::find_nmethod(void* start) {
Expand Down
4 changes: 4 additions & 0 deletions hotspot/src/share/vm/prims/forte.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -553,6 +553,9 @@ void AsyncGetCallTrace(ASGCT_CallTrace *trace, jint depth, void* ucontext) {
return;
}

// !important! make sure all to call thread->set_in_asgct(false) before every return
thread->set_in_asgct(true);

switch (thread->thread_state()) {
case _thread_new:
case _thread_uninitialized:
Expand Down Expand Up @@ -610,6 +613,7 @@ void AsyncGetCallTrace(ASGCT_CallTrace *trace, jint depth, void* ucontext) {
trace->num_frames = ticks_unknown_state; // -7
break;
}
thread->set_in_asgct(false);
}


Expand Down
1 change: 1 addition & 0 deletions hotspot/src/share/vm/runtime/thread.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1448,6 +1448,7 @@ void JavaThread::initialize() {
set_monitor_chunks(NULL);
set_next(NULL);
set_thread_state(_thread_new);
_in_asgct = false;
_terminated = _not_terminated;
_privileged_stack_top = NULL;
_array_for_gc = NULL;
Expand Down
5 changes: 5 additions & 0 deletions hotspot/src/share/vm/runtime/thread.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -781,6 +781,7 @@ typedef void (*ThreadFunction)(JavaThread*, TRAPS);
class JavaThread: public Thread {
friend class VMStructs;
private:
bool _in_asgct; // Is set when this JavaThread is handling ASGCT call
JavaThread* _next; // The next thread in the Threads list
oop _threadObj; // The Java level thread object

Expand Down Expand Up @@ -1782,6 +1783,10 @@ class JavaThread: public Thread {
public:
uint get_claimed_par_id() { return _claimed_par_id; }
void set_claimed_par_id(uint id) { _claimed_par_id = id;}

// AsyncGetCallTrace support
inline bool in_asgct(void) {return _in_asgct;}
inline void set_in_asgct(bool value) {_in_asgct = value;}
};

// Inline implementation of JavaThread::current
Expand Down

1 comment on commit 6e8292f

@openjdk-notifier
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please sign in to comment.