Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

8271490: [ppc] [s390]: Crash in JavaThread::pd_get_top_frame_for_profiling #68

Closed
wants to merge 1 commit into from
Closed
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
@@ -52,7 +52,6 @@ void RegisterMap::check_location_valid() {
#endif // ASSERT

bool frame::safe_for_sender(JavaThread *thread) {
bool safe = false;
address sp = (address)_sp;
address fp = (address)_fp;
address unextended_sp = (address)_unextended_sp;
@@ -70,28 +69,23 @@ bool frame::safe_for_sender(JavaThread *thread) {

// An fp must be within the stack and above (but not equal) sp.
bool fp_safe = thread->is_in_stack_range_excl(fp, sp);
// An interpreter fp must be within the stack and above (but not equal) sp.
// Moreover, it must be at least the size of the ijava_state structure.
// An interpreter fp must be fp_safe.
// Moreover, it must be at a distance at least the size of the ijava_state structure.
bool fp_interp_safe = fp_safe && ((fp - sp) >= ijava_state_size);

// We know sp/unextended_sp are safe, only fp is questionable here

// If the current frame is known to the code cache then we can attempt to
// to construct the sender and do some validation of it. This goes a long way
// construct the sender and do some validation of it. This goes a long way
// toward eliminating issues when we get in frame construction code

if (_cb != NULL ){
// Entry frame checks
if (is_entry_frame()) {
// An entry frame must have a valid fp.
return fp_safe && is_entry_frame_valid(thread);
}

// Now check if the frame is complete and the test is
// reliable. Unfortunately we can only check frame completeness for
// runtime stubs and nmethods. Other generic buffer blobs are more
// problematic so we just assume they are OK. Adapter blobs never have a
// complete frame and are never OK
// First check if the frame is complete and the test is reliable.
// Unfortunately we can only check frame completeness for runtime stubs
// and nmethods. Other generic buffer blobs are more problematic
// so we just assume they are OK.
// Adapter blobs never have a complete frame and are never OK
if (!_cb->is_frame_complete_at(_pc)) {
if (_cb->is_compiled() || _cb->is_adapter_blob() || _cb->is_runtime_stub()) {
return false;
@@ -103,10 +97,23 @@ bool frame::safe_for_sender(JavaThread *thread) {
return false;
}

// Entry frame checks
if (is_entry_frame()) {
// An entry frame must have a valid fp.
return fp_safe && is_entry_frame_valid(thread);
}

if (is_interpreted_frame() && !fp_interp_safe) {
return false;
}

// At this point, there still is a chance that fp_safe is false.
// In particular, (fp == NULL) might be true. So let's check and
// bail out before we actually dereference from fp.
if (!fp_safe) {
return false;
}

abi_minframe* sender_abi = (abi_minframe*) fp;
intptr_t* sender_sp = (intptr_t*) fp;
address sender_pc = (address) sender_abi->lr;;
@@ -55,7 +55,6 @@ void RegisterMap::check_location_valid() {
// Profiling/safepoint support

bool frame::safe_for_sender(JavaThread *thread) {
bool safe = false;
address sp = (address)_sp;
address fp = (address)_fp;
address unextended_sp = (address)_unextended_sp;
@@ -73,28 +72,23 @@ bool frame::safe_for_sender(JavaThread *thread) {

// An fp must be within the stack and above (but not equal) sp.
bool fp_safe = thread->is_in_stack_range_excl(fp, sp);
// An interpreter fp must be within the stack and above (but not equal) sp.
// Moreover, it must be at least the size of the z_ijava_state structure.
// An interpreter fp must be fp_safe.
// Moreover, it must be at a distance at least the size of the z_ijava_state structure.
bool fp_interp_safe = fp_safe && ((fp - sp) >= z_ijava_state_size);

// We know sp/unextended_sp are safe, only fp is questionable here

// If the current frame is known to the code cache then we can attempt to
// to construct the sender and do some validation of it. This goes a long way
// construct the sender and do some validation of it. This goes a long way
// toward eliminating issues when we get in frame construction code

if (_cb != NULL ) {
// Entry frame checks
if (is_entry_frame()) {
// An entry frame must have a valid fp.
return fp_safe && is_entry_frame_valid(thread);
}

// Now check if the frame is complete and the test is
// reliable. Unfortunately we can only check frame completeness for
// runtime stubs. Other generic buffer blobs are more
// problematic so we just assume they are OK. Adapter blobs never have a
// complete frame and are never OK. nmethods should be OK on s390.
// First check if the frame is complete and the test is reliable.
// Unfortunately we can only check frame completeness for runtime stubs.
// Other generic buffer blobs are more problematic so we just assume they are OK.
// Adapter blobs never have a complete frame and are never OK.
// nmethods should be OK on s390.
if (!_cb->is_frame_complete_at(_pc)) {
if (_cb->is_adapter_blob() || _cb->is_runtime_stub()) {
return false;
@@ -106,13 +100,26 @@ bool frame::safe_for_sender(JavaThread *thread) {
return false;
}

// Entry frame checks
if (is_entry_frame()) {
// An entry frame must have a valid fp.
return fp_safe && is_entry_frame_valid(thread);
}

if (is_interpreted_frame() && !fp_interp_safe) {
return false;
}

// At this point, there still is a chance that fp_safe is false.
// In particular, (fp == NULL) might be true. So let's check and
// bail out before we actually dereference from fp.
if (!fp_safe) {
return false;
}

z_abi_160* sender_abi = (z_abi_160*) fp;
intptr_t* sender_sp = (intptr_t*) sender_abi->callers_sp;
address sender_pc = (address) sender_abi->return_pc;
address sender_pc = (address) sender_abi->return_pc;

// We must always be able to find a recognizable pc.
CodeBlob* sender_blob = CodeCache::find_blob_unsafe(sender_pc);
@@ -35,6 +35,8 @@ frame JavaThread::pd_last_frame() {
address pc = _anchor.last_Java_pc();

// Last_Java_pc ist not set, if we come here from compiled code.
// Assume spill slot for link register contains a suitable pc.
// Should have been filled by method entry code.
if (pc == NULL) {
pc = (address) *(sp + 2);
}
@@ -64,6 +66,17 @@ bool JavaThread::pd_get_top_frame_for_profiling(frame* fr_addr, void* ucontext,
return false;
}

if (ret_frame.fp() == NULL) {
// The found frame does not have a valid frame pointer.
// Bail out because this will create big trouble later on, either
// - when using istate, calculated as (NULL - ijava_state_size) or
// - when using fp() directly in safe_for_sender()
//
// There is no conclusive description (yet) how this could happen, but it does.
// For more details on what was observed, see thread_linux_s390.cpp
return false;
}

if (ret_frame.is_interpreted_frame()) {
frame::ijava_state *istate = ret_frame.get_ijava_state();
const Method *m = (const Method*)(istate->method);
@@ -35,6 +35,8 @@ frame JavaThread::pd_last_frame() {
address pc = _anchor.last_Java_pc();

// Last_Java_pc ist not set if we come here from compiled code.
// Assume spill slot for Z_R14 (return register) contains a suitable pc.
// Should have been filled by method entry code.
if (pc == NULL) {
pc = (address) *(sp + 14);
}
@@ -51,6 +53,9 @@ bool JavaThread::pd_get_top_frame_for_profiling(frame* fr_addr, void* ucontext,
return true;
}

// At this point, we don't have a last_Java_frame, so
// we try to glean some information out of the ucontext
// if we were running Java code when SIGPROF came in.
if (isInJava) {
ucontext_t* uc = (ucontext_t*) ucontext;
frame ret_frame((intptr_t*)uc->uc_mcontext.gregs[15/*Z_SP*/],
@@ -61,6 +66,38 @@ bool JavaThread::pd_get_top_frame_for_profiling(frame* fr_addr, void* ucontext,
return false;
}

if (ret_frame.fp() == NULL) {
// The found frame does not have a valid frame pointer.
// Bail out because this will create big trouble later on, either
// - when using istate, calculated as (NULL - z_ijava_state_size (= 0x70 (dbg) or 0x68 (rel)) or
// - when using fp() directly in safe_for_sender()
//
// There is no conclusive description (yet) how this could happen, but it does:
//
// We observed a SIGSEGV with the following stack trace (openjdk.jdk11u-dev, 2021-07-07, linuxs390x fastdebug)
// V [libjvm.so+0x12c8f12] JavaThread::pd_get_top_frame_for_profiling(frame*, void*, bool)+0x142
// V [libjvm.so+0xb1020c] JfrGetCallTrace::get_topframe(void*, frame&)+0x3c
// V [libjvm.so+0xba0b08] OSThreadSampler::protected_task(os::SuspendedThreadTaskContext const&)+0x98
// V [libjvm.so+0xff33c4] os::SuspendedThreadTask::internal_do_task()+0x14c
// V [libjvm.so+0xfe3c9c] os::SuspendedThreadTask::run()+0x24
// V [libjvm.so+0xba0c66] JfrThreadSampleClosure::sample_thread_in_java(JavaThread*, JfrStackFrame*, unsigned int)+0x66
// V [libjvm.so+0xba1718] JfrThreadSampleClosure::do_sample_thread(JavaThread*, JfrStackFrame*, unsigned int, JfrSampleType)+0x278
// V [libjvm.so+0xba4f54] JfrThreadSampler::task_stacktrace(JfrSampleType, JavaThread**) [clone .constprop.62]+0x284
// V [libjvm.so+0xba5e54] JfrThreadSampler::run()+0x2ec
// V [libjvm.so+0x12adc9c] Thread::call_run()+0x9c
// V [libjvm.so+0xff5ab0] thread_native_entry(Thread*)+0x128
// siginfo: si_signo: 11 (SIGSEGV), si_code: 1 (SEGV_MAPERR), si_addr: 0xfffffffffffff000
// failing instruction: e320 6008 0004 LG r2,8(r0,r6)
// contents of r6: 0xffffffffffffff90
//
// Here is the sequence of what happens:
// - ret_frame is constructed with _fp == NULL (for whatever reason)
// - ijava_state_unchecked() calculates it's result as
// istate = fp() - z_ijava_state_size() = NULL - 0x68 DEBUG_ONLY(-8)
// - istate->method dereferences memory at offset 8 from istate
return false;
}

if (ret_frame.is_interpreted_frame()) {
frame::z_ijava_state* istate = ret_frame.ijava_state_unchecked();
if (is_in_full_stack((address)istate)) {