RFR: 8309209: C2 failed "assert(_stack_guard_state == stack_guard_reserved_disabled) failed: inconsistent state"

[Fei Yang]

On Tue, 27 Jun 2023 23:48:40 GMT, David Holmes <dholmes at openjdk.org> wrote:

>> PPC64 implementation:
>> 
>> --- a/src/hotspot/cpu/ppc/interp_masm_ppc_64.cpp
>> +++ b/src/hotspot/cpu/ppc/interp_masm_ppc_64.cpp
>> @@ -888,6 +888,11 @@ void InterpreterMacroAssembler::remove_activation(TosState state,
>>      // Test if reserved zone needs to be enabled.
>>      Label no_reserved_zone_enabling;
>>  
>> +    // check if already enabled - if so no re-enabling needed
>> +    lwz(R0, in_bytes(JavaThread::stack_guard_state_offset()), R16_thread);
>> +    cmpwi(CCR0, R0, StackOverflow::stack_guard_enabled);
>> +    beq_predict_taken(CCR0, no_reserved_zone_enabling);
>> +
>>      // Compare frame pointers. There is no good stack pointer, as with stack
>>      // frame compression we can get different SPs when we do calls. A subsequent
>>      // call could have a smaller SP, so that this compare succeeds for an
>> 
>> 
>> The other implementation don't look correct to me. `StackGuardState` is an `enum` and should typically have 4 Bytes.
>
>> The other implementation don't look correct to me. StackGuardState is an enum and should typically have 4 Bytes.
> 
> @TheRealMDoerr could you elaborate please. We have the following:
> - x86: we use `void cmpl(Address dst, int32_t imm32);`
> - Aarch64: we use `cmp(Register Rd, unsigned char imm8)` and cast to `u1`
> - RISC-V: we use `sub (Register Rd, Register Rn, int64_t decrement, Register temp = t0);` (perhaps should be `subw` for 32-bit? @RealFYang ?)

@dholmes-ora : Yes, that makes sense to me. And I will also need to change to use 32-bit load (lw) to get the guard state.


diff --git a/src/hotspot/cpu/riscv/interp_masm_riscv.cpp b/src/hotspot/cpu/riscv/interp_masm_riscv.cpp
index edec2e08c83..1e981498fcc 100644
--- a/src/hotspot/cpu/riscv/interp_masm_riscv.cpp
+++ b/src/hotspot/cpu/riscv/interp_masm_riscv.cpp
@@ -764,6 +764,11 @@ void InterpreterMacroAssembler::remove_activation(
     // testing if reserved zone needs to be re-enabled
     Label no_reserved_zone_enabling;

+    // check if already enabled - if so no re-enabling needed
+    lw(t0, Address(xthread, JavaThread::stack_guard_state_offset()));
+    subw(t0, t0, StackOverflow::stack_guard_enabled);
+    beqz(t0, no_reserved_zone_enabling);
+
     ld(t0, Address(xthread, JavaThread::reserved_stack_activation_offset()));
     ble(t1, t0, no_reserved_zone_enabling);

-------------

PR Comment: https://git.openjdk.org/jdk/pull/14669#issuecomment-1610439335