Skip to content

Commit 0e847cf

Browse files
committed
8271490: [ppc] [s390]: Crash in JavaThread::pd_get_top_frame_for_profiling
Backport-of: 276b07b36af01d339e48baada7a512451fe34afe
1 parent d9a7fc6 commit 0e847cf

File tree

4 files changed

+93
-29
lines changed

4 files changed

+93
-29
lines changed

src/hotspot/cpu/ppc/frame_ppc.cpp

Lines changed: 21 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,6 @@ void RegisterMap::check_location_valid() {
5252
#endif // ASSERT
5353

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

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

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

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

8382
if (_cb != NULL ){
84-
// Entry frame checks
85-
if (is_entry_frame()) {
86-
// An entry frame must have a valid fp.
87-
return fp_safe && is_entry_frame_valid(thread);
88-
}
8983

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

100+
// Entry frame checks
101+
if (is_entry_frame()) {
102+
// An entry frame must have a valid fp.
103+
return fp_safe && is_entry_frame_valid(thread);
104+
}
105+
106106
if (is_interpreted_frame() && !fp_interp_safe) {
107107
return false;
108108
}
109109

110+
// At this point, there still is a chance that fp_safe is false.
111+
// In particular, (fp == NULL) might be true. So let's check and
112+
// bail out before we actually dereference from fp.
113+
if (!fp_safe) {
114+
return false;
115+
}
116+
110117
abi_minframe* sender_abi = (abi_minframe*) fp;
111118
intptr_t* sender_sp = (intptr_t*) fp;
112119
address sender_pc = (address) sender_abi->lr;;

src/hotspot/cpu/s390/frame_s390.cpp

Lines changed: 22 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,6 @@ void RegisterMap::check_location_valid() {
5555
// Profiling/safepoint support
5656

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

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

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

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

8685
if (_cb != NULL ) {
87-
// Entry frame checks
88-
if (is_entry_frame()) {
89-
// An entry frame must have a valid fp.
90-
return fp_safe && is_entry_frame_valid(thread);
91-
}
9286

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

103+
// Entry frame checks
104+
if (is_entry_frame()) {
105+
// An entry frame must have a valid fp.
106+
return fp_safe && is_entry_frame_valid(thread);
107+
}
108+
109109
if (is_interpreted_frame() && !fp_interp_safe) {
110110
return false;
111111
}
112112

113+
// At this point, there still is a chance that fp_safe is false.
114+
// In particular, (fp == NULL) might be true. So let's check and
115+
// bail out before we actually dereference from fp.
116+
if (!fp_safe) {
117+
return false;
118+
}
119+
113120
z_abi_160* sender_abi = (z_abi_160*) fp;
114121
intptr_t* sender_sp = (intptr_t*) sender_abi->callers_sp;
115-
address sender_pc = (address) sender_abi->return_pc;
122+
address sender_pc = (address) sender_abi->return_pc;
116123

117124
// We must always be able to find a recognizable pc.
118125
CodeBlob* sender_blob = CodeCache::find_blob_unsafe(sender_pc);

src/hotspot/os_cpu/linux_ppc/thread_linux_ppc.cpp

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,8 @@ frame JavaThread::pd_last_frame() {
3535
address pc = _anchor.last_Java_pc();
3636

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

69+
if (ret_frame.fp() == NULL) {
70+
// The found frame does not have a valid frame pointer.
71+
// Bail out because this will create big trouble later on, either
72+
// - when using istate, calculated as (NULL - ijava_state_size) or
73+
// - when using fp() directly in safe_for_sender()
74+
//
75+
// There is no conclusive description (yet) how this could happen, but it does.
76+
// For more details on what was observed, see thread_linux_s390.cpp
77+
return false;
78+
}
79+
6780
if (ret_frame.is_interpreted_frame()) {
6881
frame::ijava_state *istate = ret_frame.get_ijava_state();
6982
const Method *m = (const Method*)(istate->method);

src/hotspot/os_cpu/linux_s390/thread_linux_s390.cpp

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,8 @@ frame JavaThread::pd_last_frame() {
3535
address pc = _anchor.last_Java_pc();
3636

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

56+
// At this point, we don't have a last_Java_frame, so
57+
// we try to glean some information out of the ucontext
58+
// if we were running Java code when SIGPROF came in.
5459
if (isInJava) {
5560
ucontext_t* uc = (ucontext_t*) ucontext;
5661
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,
6166
return false;
6267
}
6368

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

0 commit comments

Comments
 (0)