Skip to content

Commit

Permalink
[Misc] Fix bugs in previous commit.
Browse files Browse the repository at this point in the history
Summary:
1. Fix StackOverflow casued by coroutine switch. Coroutine switch should update thread stack information correctly.
2. Make SharedRuntime::monitor_exit_helper can work in JRT_LEAF(no safepoint) for coroutine.
3. _class_to_be_initialized should be bound to WispThread in UseWispMonitor.
4. Thread constructor's thread id is protected by synchronized. It may introduce another unintended context switch which cause assertion failure. Use AtomicInteger instead to avoid it.
5. Fix macros THREAD do not change to WispThread when enable UseWispMonitor. Therefore, methods that use THREAD will do uncorrectly.
6. Update thread object for coroutine WispThread. Refine print_stack_header_on to not return oop.
7. Fix Exceptions::_throw_msg use wrong thread in Wisp.
8. Fix setThreadWrapper in WispTask. Add method to print info in native to avoid synchroize.
9. Fix ReservedStackTest crash in slowdebug. The crash root cause is stack_guard_state is inconsistent. The following contributes to it:
    - stack_guard_state is not updated correclty in coroutine switch. I think it is not a critical reason. In switch, stack_guard_state should always be stack_guard_enabled.
    - During reserved stack activation, its stack is too large to exceed to yellow zone to trigger another nested enable_stack_yellow_reserved_zone(). We detect the nested yellow zone and fix the state in this patch.

    After the patch, the crash is fixed but the test is still fail because of https://bugs.openjdk.org/browse/JDK-8231031.

    Several improvements as follows:
        - Make coroutine stack and thread share the same StackOverflow.

10. Fix JRT_LEAF method can not do oops correctly when use Wisp. In java17, InterpreterRuntime::monitorexit and some other monitorexit methods are defined as JRT_LEAF and call them with call_VM_leaf. JRT_LEAF don't allow to call Java methods or break safepoint, so we need to define a new method monitorexit_wisp with JRT_ENTRY_NO_ASYNC and call it with call_VM to make GC do correctly.
11. Fix setThreadWrapper repeatedly when Task is reused by Thread and it's last wrapper is WispThreadWrapper.

Test Plan: all wisp cases

Reviewed-by: yulei

Issue:
#103
  • Loading branch information
nebula-xm authored and ZhaiMo15 committed Jul 31, 2023
1 parent 1bcf919 commit adffc51
Show file tree
Hide file tree
Showing 43 changed files with 498 additions and 278 deletions.
1 change: 1 addition & 0 deletions make/data/hotspot-symbols/symbols-unix
Original file line number Diff line number Diff line change
Expand Up @@ -194,6 +194,7 @@ JVM_SetNativeThreadName
JVM_SetPrimitiveArrayElement
JVM_SetThreadPriority
JVM_SetWispTask
JVM_UpdateThreadObjectForWispThread
JVM_Sleep
JVM_StartThread
JVM_StopThread
Expand Down
12 changes: 10 additions & 2 deletions src/hotspot/cpu/aarch64/interp_masm_aarch64.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -852,7 +852,11 @@ void InterpreterMacroAssembler::unlock_object(Register lock_reg)
assert(lock_reg == c_rarg1, "The argument is only for looks. It must be rarg1");

if (UseHeavyMonitors) {
call_VM_leaf(CAST_FROM_FN_PTR(address, InterpreterRuntime::monitorexit), lock_reg);
if (UseWispMonitor) {
call_VM(noreg, CAST_FROM_FN_PTR(address, InterpreterRuntime::monitorexit_wisp), lock_reg);
} else {
call_VM_leaf(CAST_FROM_FN_PTR(address, InterpreterRuntime::monitorexit), lock_reg);
}
} else {
Label done;

Expand Down Expand Up @@ -888,7 +892,11 @@ void InterpreterMacroAssembler::unlock_object(Register lock_reg)

// Call the runtime routine for slow case.
str(obj_reg, Address(lock_reg, BasicObjectLock::obj_offset_in_bytes())); // restore obj
call_VM_leaf(CAST_FROM_FN_PTR(address, InterpreterRuntime::monitorexit), lock_reg);
if (UseWispMonitor) {
call_VM(noreg, CAST_FROM_FN_PTR(address, InterpreterRuntime::monitorexit_wisp), lock_reg);
} else {
call_VM_leaf(CAST_FROM_FN_PTR(address, InterpreterRuntime::monitorexit), lock_reg);
}

bind(done);

Expand Down
5 changes: 3 additions & 2 deletions src/hotspot/cpu/aarch64/sharedRuntime_aarch64.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1909,7 +1909,7 @@ nmethod* SharedRuntime::generate_native_wrapper(MacroAssembler* masm,
Label reguard;
Label reguard_done;
__ ldrb(rscratch1, Address(rthread, JavaThread::stack_guard_state_offset()));
__ cmpw(rscratch1, StackOverflow::stack_guard_yellow_reserved_disabled);
__ cmpw(rscratch1, static_cast<int>(StackOverflow::stack_guard_yellow_reserved_disabled));
__ br(Assembler::EQ, reguard);
__ bind(reguard_done);

Expand Down Expand Up @@ -2058,6 +2058,7 @@ nmethod* SharedRuntime::generate_native_wrapper(MacroAssembler* masm,
__ ldr(r19, Address(rthread, in_bytes(Thread::pending_exception_offset())));
__ str(zr, Address(rthread, in_bytes(Thread::pending_exception_offset())));

// TODO WispMonitor yield
rt_call(masm, CAST_FROM_FN_PTR(address, SharedRuntime::complete_monitor_unlocking_C));

#ifdef ASSERT
Expand Down Expand Up @@ -3298,7 +3299,7 @@ void NativeInvokerGenerator::generate() {
Label L_reguard;
Label L_after_reguard;
__ ldrb(rscratch1, Address(rthread, JavaThread::stack_guard_state_offset()));
__ cmpw(rscratch1, StackOverflow::stack_guard_yellow_reserved_disabled);
__ cmpw(rscratch1, static_cast<int>(StackOverflow::stack_guard_yellow_reserved_disabled));
__ br(Assembler::EQ, L_reguard);
__ bind(L_after_reguard);

Expand Down
12 changes: 10 additions & 2 deletions src/hotspot/cpu/arm/interp_masm_arm.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -990,7 +990,11 @@ void InterpreterMacroAssembler::unlock_object(Register Rlock) {
assert(Rlock == R0, "the first argument");

if (UseHeavyMonitors) {
call_VM_leaf(CAST_FROM_FN_PTR(address, InterpreterRuntime::monitorexit), Rlock);
if (UseWispMonitor) {
call_VM(noreg, CAST_FROM_FN_PTR(address, InterpreterRuntime::monitorexit_wisp), Rlock);
} else {
call_VM_leaf(CAST_FROM_FN_PTR(address, InterpreterRuntime::monitorexit), Rlock);
}
} else {
Label done, slow_case;

Expand Down Expand Up @@ -1030,7 +1034,11 @@ void InterpreterMacroAssembler::unlock_object(Register Rlock) {

// Call the runtime routine for slow case.
str(Robj, Address(Rlock, obj_offset)); // restore obj
call_VM_leaf(CAST_FROM_FN_PTR(address, InterpreterRuntime::monitorexit), Rlock);
if (UseWispMonitor) {
call_VM(noreg, CAST_FROM_FN_PTR(address, InterpreterRuntime::monitorexit_wisp), Rlock);
} else {
call_VM_leaf(CAST_FROM_FN_PTR(address, InterpreterRuntime::monitorexit), Rlock);
}

bind(done);
}
Expand Down
2 changes: 1 addition & 1 deletion src/hotspot/cpu/arm/sharedRuntime_arm.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1235,7 +1235,7 @@ nmethod* SharedRuntime::generate_native_wrapper(MacroAssembler* masm,
__ ldr_s32(R2, Address(Rthread, JavaThread::stack_guard_state_offset()));
__ str_32(Rtemp, Address(Rthread, JavaThread::thread_state_offset()));

__ cmp(R2, StackOverflow::stack_guard_yellow_reserved_disabled);
__ cmp(R2, static_cast<unsigned>(StackOverflow::stack_guard_yellow_reserved_disabled));
__ b(reguard, eq);
__ bind(reguard_done);

Expand Down
4 changes: 2 additions & 2 deletions src/hotspot/cpu/arm/templateInterpreterGenerator_arm.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -962,9 +962,9 @@ address TemplateInterpreterGenerator::generate_native_entry(bool synchronized) {
{
__ ldr_u32(Rtemp, Address(Rthread, JavaThread::stack_guard_state_offset()));
__ cmp_32(Rtemp, StackOverflow::stack_guard_yellow_reserved_disabled);
__ call(CAST_FROM_FN_PTR(address, SharedRuntime::reguard_yellow_pages), relocInfo::none, eq);
__ call(CAST_FROM_FN_PTR(address, SharedRuntime::reguard_yellow_pages), relocInfo::none, eq);
#if R9_IS_SCRATCHED
__ restore_method();
__ restore_method();
#endif
}

Expand Down
12 changes: 10 additions & 2 deletions src/hotspot/cpu/ppc/interp_masm_ppc_64.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1022,7 +1022,11 @@ void InterpreterMacroAssembler::lock_object(Register monitor, Register object) {
// Throw IllegalMonitorException if object is not locked by current thread.
void InterpreterMacroAssembler::unlock_object(Register monitor) {
if (UseHeavyMonitors) {
call_VM_leaf(CAST_FROM_FN_PTR(address, InterpreterRuntime::monitorexit), monitor);
if (UseWispMonitor) {
call_VM(noreg, CAST_FROM_FN_PTR(address, InterpreterRuntime::monitorexit_wisp), monitor);
} else {
call_VM_leaf(CAST_FROM_FN_PTR(address, InterpreterRuntime::monitorexit), monitor);
}
} else {

// template code:
Expand Down Expand Up @@ -1094,7 +1098,11 @@ void InterpreterMacroAssembler::unlock_object(Register monitor) {
// The lock has been converted into a heavy lock and hence
// we need to get into the slow case.
bind(slow_case);
call_VM_leaf(CAST_FROM_FN_PTR(address, InterpreterRuntime::monitorexit), monitor);
if (UseWispMonitor) {
call_VM(noreg, CAST_FROM_FN_PTR(address, InterpreterRuntime::monitorexit_wisp), monitor);
} else {
call_VM_leaf(CAST_FROM_FN_PTR(address, InterpreterRuntime::monitorexit), monitor);
}
// }

Label done;
Expand Down
2 changes: 1 addition & 1 deletion src/hotspot/cpu/ppc/sharedRuntime_ppc.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2362,7 +2362,7 @@ nmethod *SharedRuntime::generate_native_wrapper(MacroAssembler *masm,

Label no_reguard;
__ lwz(r_temp_1, thread_(stack_guard_state));
__ cmpwi(CCR0, r_temp_1, StackOverflow::stack_guard_yellow_reserved_disabled);
__ cmpwi(CCR0, r_temp_1, static_cast<int>(StackOverflow::stack_guard_yellow_reserved_disabled));
__ bne(CCR0, no_reguard);

save_native_result(masm, ret_type, workspace_slot_offset);
Expand Down
12 changes: 10 additions & 2 deletions src/hotspot/cpu/s390/interp_masm_s390.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1077,7 +1077,11 @@ void InterpreterMacroAssembler::lock_object(Register monitor, Register object) {
void InterpreterMacroAssembler::unlock_object(Register monitor, Register object) {

if (UseHeavyMonitors) {
call_VM_leaf(CAST_FROM_FN_PTR(address, InterpreterRuntime::monitorexit), monitor);
if (UseWispMonitor) {
call_VM(noreg, CAST_FROM_FN_PTR(address, InterpreterRuntime::monitorexit_wisp), monitor);
} else {
call_VM_leaf(CAST_FROM_FN_PTR(address, InterpreterRuntime::monitorexit), monitor);
}
return;
}

Expand Down Expand Up @@ -1151,7 +1155,11 @@ void InterpreterMacroAssembler::unlock_object(Register monitor, Register object)
// The lock has been converted into a heavy lock and hence
// we need to get into the slow case.
z_stg(object, obj_entry); // Restore object entry, has been cleared above.
call_VM_leaf(CAST_FROM_FN_PTR(address, InterpreterRuntime::monitorexit), monitor);
if (UseWispMonitor) {
call_VM(noreg, CAST_FROM_FN_PTR(address, InterpreterRuntime::monitorexit_wisp), monitor);
} else {
call_VM_leaf(CAST_FROM_FN_PTR(address, InterpreterRuntime::monitorexit), monitor);
}

// }

Expand Down
2 changes: 1 addition & 1 deletion src/hotspot/cpu/s390/sharedRuntime_s390.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2056,7 +2056,7 @@ nmethod *SharedRuntime::generate_native_wrapper(MacroAssembler *masm,
Label no_reguard;

__ z_cli(Address(Z_thread, JavaThread::stack_guard_state_offset() + in_ByteSize(sizeof(StackOverflow::StackGuardState) - 1)),
StackOverflow::stack_guard_yellow_reserved_disabled);
static_cast<int64_t>(StackOverflow::stack_guard_yellow_reserved_disabled));

__ z_bre(no_reguard);

Expand Down
14 changes: 11 additions & 3 deletions src/hotspot/cpu/x86/interp_masm_x86.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1164,7 +1164,7 @@ void InterpreterMacroAssembler::remove_activation(

NOT_LP64(get_thread(rthread);)

cmpl(Address(rthread, JavaThread::stack_guard_state_offset()), StackOverflow::stack_guard_enabled);
cmpl(Address(rthread, JavaThread::stack_guard_state_offset()), static_cast<int32_t>(StackOverflow::stack_guard_enabled));
jcc(Assembler::equal, no_reserved_zone_enabling);

cmpptr(rbx, Address(rthread, JavaThread::reserved_stack_activation_offset()));
Expand Down Expand Up @@ -1333,7 +1333,11 @@ void InterpreterMacroAssembler::unlock_object(Register lock_reg) {
"The argument is only for looks. It must be c_rarg1");

if (UseHeavyMonitors) {
call_VM_leaf(CAST_FROM_FN_PTR(address, InterpreterRuntime::monitorexit), lock_reg);
if (UseWispMonitor) {
call_VM(noreg, CAST_FROM_FN_PTR(address, InterpreterRuntime::monitorexit_wisp), lock_reg);
} else {
call_VM_leaf(CAST_FROM_FN_PTR(address, InterpreterRuntime::monitorexit), lock_reg);
}
} else {
Label done;

Expand Down Expand Up @@ -1377,7 +1381,11 @@ void InterpreterMacroAssembler::unlock_object(Register lock_reg) {

// Call the runtime routine for slow case.
movptr(Address(lock_reg, BasicObjectLock::obj_offset_in_bytes()), obj_reg); // restore obj
call_VM_leaf(CAST_FROM_FN_PTR(address, InterpreterRuntime::monitorexit), lock_reg);
if (UseWispMonitor) {
call_VM(noreg, CAST_FROM_FN_PTR(address, InterpreterRuntime::monitorexit_wisp), lock_reg);
} else {
call_VM_leaf(CAST_FROM_FN_PTR(address, InterpreterRuntime::monitorexit), lock_reg);
}

bind(done);

Expand Down
14 changes: 9 additions & 5 deletions src/hotspot/cpu/x86/sharedRuntime_x86_32.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2138,13 +2138,17 @@ nmethod* SharedRuntime::generate_native_wrapper(MacroAssembler* masm,
// should be a peal
// +wordSize because of the push above
// args are (oop obj, BasicLock* lock, JavaThread* thread)
__ push(thread);
__ lea(rax, Address(rbp, lock_slot_rbp_offset));
__ push(rax);
if (UseWispMonitor) {
__ call_VM(noreg, CAST_FROM_FN_PTR(address, SharedRuntime::complete_wisp_monitor_unlocking_C), obj_reg, rax);
} else {
__ push(thread);
__ push(rax);
__ push(obj_reg);
__ call(RuntimeAddress(CAST_FROM_FN_PTR(address, SharedRuntime::complete_monitor_unlocking_C)));
__ addptr(rsp, 3*wordSize);
}

__ push(obj_reg);
__ call(RuntimeAddress(CAST_FROM_FN_PTR(address, SharedRuntime::complete_monitor_unlocking_C)));
__ addptr(rsp, 3*wordSize);
#ifdef ASSERT
{
Label L;
Expand Down
Loading

0 comments on commit adffc51

Please sign in to comment.