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: mdoerr
Backport-of: 93c88690a1c2cbc7ba7fc70ddef9bf5928e4de03
  • Loading branch information
Jaroslav Bachorik committed Jun 13, 2022
1 parent 8563eec commit 1963985
Show file tree
Hide file tree
Showing 4 changed files with 21 additions and 2 deletions.
13 changes: 11 additions & 2 deletions src/hotspot/share/code/codeCache.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -630,14 +630,23 @@ bool CodeCache::contains(nmethod *nm) {
return contains((void *)nm);
}

static bool is_in_asgct() {
Thread* current_thread = Thread::current_or_null_safe();
return current_thread != NULL && current_thread->is_Java_thread() && current_thread->as_Java_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 depends on the _segmap to contain
// valid indices, which it will always do, as long as the CodeBlob is not in the process of being recycled.
CodeBlob* CodeCache::find_blob(void* start) {
CodeBlob* result = find_blob_unsafe(start);
// We could potentially look up non_entrant methods
guarantee(result == NULL || !result->is_zombie() || result->is_locked_by_vm() || VMError::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() || VMError::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;
}

// Lookup that does not fail if you lookup a zombie method (if you call this, be sure to know
Expand Down
4 changes: 4 additions & 0 deletions src/hotspot/share/prims/forte.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -552,6 +552,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 @@ -609,6 +612,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 src/hotspot/share/runtime/thread.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1014,6 +1014,7 @@ void JavaThread::check_for_valid_safepoint_state() {
JavaThread::JavaThread() :
// Initialize fields

_in_asgct(false),
_on_thread_list(false),
DEBUG_ONLY(_java_call_counter(0) COMMA)
_entry_point(nullptr),
Expand Down
5 changes: 5 additions & 0 deletions src/hotspot/share/runtime/thread.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -692,6 +692,7 @@ class JavaThread: public Thread {
friend class ThreadsSMRSupport; // to access _threadObj for exiting_threads_oops_do
friend class HandshakeState;
private:
bool _in_asgct; // Is set when this JavaThread is handling ASGCT call
bool _on_thread_list; // Is set when this JavaThread is added to the Threads list
OopHandle _threadObj; // The Java level thread object

Expand Down Expand Up @@ -1589,6 +1590,10 @@ class JavaThread: public Thread {
static OopStorage* thread_oop_storage();

static void verify_cross_modify_fence_failure(JavaThread *thread) PRODUCT_RETURN;

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

5 comments on commit 1963985

@jbachorik
Copy link

Choose a reason for hiding this comment

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

/backport jdk11u-dev

@openjdk
Copy link

@openjdk openjdk bot commented on 1963985 Jun 13, 2022

Choose a reason for hiding this comment

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

@jbachorik Could not automatically backport 19639855 to openjdk/jdk11u-dev due to conflicts in the following files:

  • src/hotspot/share/runtime/thread.cpp
  • src/hotspot/share/runtime/thread.hpp

To manually resolve these conflicts run the following commands in your personal fork of openjdk/jdk11u-dev:

$ git checkout -b jbachorik-backport-19639855
$ git fetch --no-tags https://git.openjdk.org/jdk17u-dev 19639855311a47ed532547743ea3873eb8b016d3
$ git cherry-pick --no-commit 19639855311a47ed532547743ea3873eb8b016d3
$ # Resolve conflicts
$ git add files/with/resolved/conflicts
$ git commit -m 'Backport 19639855311a47ed532547743ea3873eb8b016d3'

Once you have resolved the conflicts as explained above continue with creating a pull request towards the openjdk/jdk11u-dev with the title Backport 19639855311a47ed532547743ea3873eb8b016d3.

@openjdk-notifier
Copy link

Choose a reason for hiding this comment

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

@jbachorik
Copy link

Choose a reason for hiding this comment

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

/backport jdk8u-dev

@openjdk
Copy link

@openjdk openjdk bot commented on 1963985 Jun 15, 2022

Choose a reason for hiding this comment

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

@jbachorik Could not automatically backport 19639855 to openjdk/jdk8u-dev due to conflicts in the following files:

  • src/hotspot/share/code/codeCache.cpp
  • src/hotspot/share/runtime/thread.cpp
  • src/hotspot/share/runtime/thread.hpp

To manually resolve these conflicts run the following commands in your personal fork of openjdk/jdk8u-dev:

$ git checkout -b jbachorik-backport-19639855
$ git fetch --no-tags https://git.openjdk.org/jdk17u-dev 19639855311a47ed532547743ea3873eb8b016d3
$ git cherry-pick --no-commit 19639855311a47ed532547743ea3873eb8b016d3
$ # Resolve conflicts
$ git add files/with/resolved/conflicts
$ git commit -m 'Backport 19639855311a47ed532547743ea3873eb8b016d3'

Once you have resolved the conflicts as explained above continue with creating a pull request towards the openjdk/jdk8u-dev with the title Backport 19639855311a47ed532547743ea3873eb8b016d3.

Please sign in to comment.