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

[Amit Kumar]

On Wed, 28 Jun 2023 06:04:22 GMT, David Holmes <dholmes at openjdk.org> wrote:

>> This appears to be the same kind of issue as reported in [JDK-8146697](https://bugs.openjdk.org/browse/JDK-8146697) way back in Java 9, which was only "fixed" on x86.  The current failure was seen on Aarch64. It seems prudent to apply the same changes to all the other platforms. I've done Aarch64, and took a guess at RISC-V but do not know PPC or S390, so I am looking to others to provide the appropriate equivalent code changes there.
>> 
>> Testing so far is Aarch64 only:
>> - Tiers 1-3
>> - 50x the closed stackoverflow test that failed previously
>> - 25x vmTestbase/nsk/stress/stack/*
>> 
>> As these failures are so rare, passing tests don't really tell us much. This is more an attempt at additional robustness.
>> 
>> Thanks.
>
> David Holmes has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Fix typo

Here is s390x port:

diff --git a/src/hotspot/cpu/s390/interp_masm_s390.cpp b/src/hotspot/cpu/s390/interp_masm_s390.cpp
index 79c758f8b11..7555eb05b1f 100644
--- a/src/hotspot/cpu/s390/interp_masm_s390.cpp
+++ b/src/hotspot/cpu/s390/interp_masm_s390.cpp
@@ -952,6 +952,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
+    guarantee(sizeof(StackOverflow::StackGuardState) == 4, "unexptected size");
+    z_ly(Z_R0, Address(Z_thread, JavaThread::stack_guard_state_offset()));
+    compare32_and_branch(Z_R0, StackOverflow::stack_guard_enabled, bcondEqual, 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



@TheRealMDoerr I have a 2nd patch as well, Would you please confirm which will be better:


diff --git a/src/hotspot/cpu/s390/interp_masm_s390.cpp b/src/hotspot/cpu/s390/interp_masm_s390.cpp
index 79c758f8b11..a6774326286 100644
--- a/src/hotspot/cpu/s390/interp_masm_s390.cpp
+++ b/src/hotspot/cpu/s390/interp_masm_s390.cpp
@@ -952,6 +952,12 @@ 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
+    guarantee(sizeof(StackOverflow::StackGuardState) == 4, "unexptected size");
+    z_cli(Address(Z_thread, JavaThread::stack_guard_state_offset() + in_ByteSize(sizeof(StackOverflow::StackGuardState) - 1)),
+            StackOverflow::stack_guard_enabled);
+    z_bre(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

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

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