[GR-63037] Cleanups and refactorings for STATUS_IN_VM.

PullRequest: graal/20362

Author: christianhaeubl

substratevm/src/com.oracle.svm.core.genscavenge/src/com/oracle/svm/core/genscavenge/HeapImpl.java

@@ -262,7 +262,7 @@ public boolean isAllocationDisallowed() {
     /** A guard to place before an allocation, giving the call site and the allocation type. */
     static void exitIfAllocationDisallowed(String callSite, String typeName) {
         if (HeapImpl.getHeapImpl().isAllocationDisallowed()) {
-            NoAllocationVerifier.exit(callSite, typeName);
+            throw NoAllocationVerifier.exit(callSite, typeName);
         }
     }
 

substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/c/function/CFunctionOptions.java

@@ -31,29 +31,40 @@
 
 import org.graalvm.nativeimage.c.function.CFunction;
 
+import com.oracle.svm.core.Uninterruptible;
+import com.oracle.svm.core.thread.VMThreads.StatusSupport;
+
 /**
- * This annotation is used to override or extend the behavior of {@link CFunction}. Must only be
- * used on methods that are annotated with {@link CFunction}.
+ * This annotation is used to override or extend the behavior of {@link CFunction}. May only be used
+ * on methods that are annotated with {@link CFunction}.
  */
 @Retention(RetentionPolicy.RUNTIME)
 @Target(ElementType.METHOD)
 public @interface CFunctionOptions {
-    /**
-     * Describes the thread state transition performed when the C function is invoked.
-     */
     enum Transition {
         /**
-         * The thread state is transitioned from Java to VM, and the Java parts of the stack are
-         * made walkable. If the C code blocks or calls back to Java, it must do an explicit thread
-         * state transition to native to prevent that safepoints (and therefore garbage collections)
-         * of other threads are delayed.
+         * Does a transition to {@link StatusSupport#STATUS_IN_VM}. This prevents safepoints
+         * (similar to {@code NO_TRANSITION}) but also pushes a frame anchor (similar to {@code
+         * TO_NATIVE}) to make the Java part of the stack walkable.
+         *
+         * The executed C code can safely assume that there are no safepoints happening in the VM.
+         * If it is necessary to block in the native code, the C code can do an explicit thread
+         * state transition to {@link StatusSupport#STATUS_IN_NATIVE} to allow safepoints in a
+         * controlled manner.
+         *
+         * Note that this transition does not do a safepoint check when the C code returns back to
+         * Java. This allows C functions to return raw pointers to Java objects, if the Java caller
+         * is an {@link Uninterruptible} method.
+         *
+         * This transition may only be used by trusted native code as it can result in deadlocks
+         * easily. Therefore, it is not part of the Native Image API.
          */
         TO_VM
     }
 
     /**
-     * The Java-to-C thread transition code used when calling the C function. Overrides the
-     * transition that is set via the {@link CFunction} annotation.
+     * The thread state transition performed when calling the C function. Overrides the transition
+     * that is set via the {@link CFunction} annotation.
      */
     Transition transition();
 }

substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/heap/NoAllocationVerifier.java

@@ -43,7 +43,7 @@ public class NoAllocationVerifier implements AutoCloseable {
     public static final String ERROR_MSG = "Attempt to allocate while allocation was explicitly disabled using a NoAllocationVerifier";
 
     /** A guard to place before an allocation, giving the call site and the allocation type. */
-    public static void exit(final String callSite, final String typeName) {
+    public static RuntimeException exit(final String callSite, final String typeName) {
         Log.log().string("[NoAllocationVerifier detected disallowed allocation: ").string(callSite).string(": ").string(typeName).newline();
         if (openVerifiers.get() != null) {
             Log.log().string("[NoAllocationVerifier stack: ");

substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/thread/Safepoint.java

@@ -29,7 +29,6 @@
 
 import java.util.concurrent.atomic.AtomicInteger;
 
-import jdk.graal.compiler.word.Word;
 import org.graalvm.nativeimage.CurrentIsolate;
 import org.graalvm.nativeimage.ImageSingletons;
 import org.graalvm.nativeimage.IsolateThread;
@@ -53,7 +52,6 @@
 import com.oracle.svm.core.log.Log;
 import com.oracle.svm.core.nodes.CFunctionEpilogueNode;
 import com.oracle.svm.core.nodes.CFunctionPrologueNode;
-import com.oracle.svm.core.nodes.CodeSynchronizationNode;
 import com.oracle.svm.core.nodes.SafepointCheckNode;
 import com.oracle.svm.core.option.HostedOptionKey;
 import com.oracle.svm.core.option.RuntimeOptionKey;
@@ -81,6 +79,7 @@
 import jdk.graal.compiler.nodes.extended.ForeignCallNode;
 import jdk.graal.compiler.nodes.extended.MembarNode;
 import jdk.graal.compiler.options.Option;
+import jdk.graal.compiler.word.Word;
 
 /**
  * Support for initiating safepoints, which are a global state in which all threads are paused so
@@ -151,14 +150,13 @@ public final class Safepoint {
     public static final SubstrateForeignCallDescriptor ENTER_SLOW_PATH_SAFEPOINT_CHECK = SnippetRuntime.findForeignCall(Safepoint.class, "enterSlowPathSafepointCheck", NO_SIDE_EFFECT);
     public static final SubstrateForeignCallDescriptor ENTER_SLOW_PATH_TRANSITION_FROM_NATIVE_TO_NEW_STATUS = SnippetRuntime.findForeignCall(Safepoint.class,
                     "enterSlowPathTransitionFromNativeToNewStatus", NO_SIDE_EFFECT);
-    private static final SubstrateForeignCallDescriptor ENTER_SLOW_PATH_TRANSITION_FROM_VM_TO_JAVA = SnippetRuntime.findForeignCall(Safepoint.class, "enterSlowPathTransitionFromVMToJava",
-                    NO_SIDE_EFFECT);
+    private static final SubstrateForeignCallDescriptor ENTER_SLOW_PATH_RUN_PENDING_ACTIONS = SnippetRuntime.findForeignCall(Safepoint.class, "enterSlowPathRunPendingActions", NO_SIDE_EFFECT);
 
     /** All foreign calls defined in this class. */
     public static final SubstrateForeignCallDescriptor[] FOREIGN_CALLS = new SubstrateForeignCallDescriptor[]{
                     ENTER_SLOW_PATH_SAFEPOINT_CHECK,
                     ENTER_SLOW_PATH_TRANSITION_FROM_NATIVE_TO_NEW_STATUS,
-                    ENTER_SLOW_PATH_TRANSITION_FROM_VM_TO_JAVA,
+                    ENTER_SLOW_PATH_RUN_PENDING_ACTIONS,
     };
 
     /** Private constructor: No instances: only statics. */
@@ -250,32 +248,12 @@ private static void slowPathSafepointCheck0(int newStatus, boolean callerHasJava
         }
 
         if (newStatus == StatusSupport.STATUS_IN_JAVA) {
-            // Resetting the safepoint counter or executing the recurring callback must only be done
-            // if the thread is in Java state.
-            slowPathRunJavaStateActions();
+            ActionOnTransitionToJavaSupport.runPendingActions();
+            /* Do this last so that a thrown exception cannot skip any of the above. */
+            ThreadingSupportImpl.onSafepointCheckSlowpath();
         }
     }
 
-    /**
-     * Slow path code run after a safepoint check or after transitioning from VM to Java state. It
-     * resets the safepoint counter, runs recurring callbacks if necessary, and executes pending
-     * {@link ActionOnTransitionToJavaSupport transition actions}.
-     */
-    @Uninterruptible(reason = "Must not contain safepoint checks.")
-    private static void slowPathRunJavaStateActions() {
-        if (ActionOnTransitionToJavaSupport.isActionPending()) {
-            if (ActionOnTransitionToJavaSupport.isSynchronizeCode()) {
-                CodeSynchronizationNode.synchronizeCode();
-            } else {
-                assert false : "Unexpected action pending.";
-            }
-            ActionOnTransitionToJavaSupport.clearActions();
-        }
-
-        // Do this last so an exception cannot skip the above
-        ThreadingSupportImpl.onSafepointCheckSlowpath();
-    }
-
     @NeverInline("Must not be inlined in a caller that has an exception handler: We only support InvokeNode and not InvokeWithExceptionNode between a CFunctionPrologueNode and CFunctionEpilogueNode")
     @Uninterruptible(reason = "Must not contain safepoint checks.")
     private static void freezeAtSafepoint(int newStatus, boolean callerHasJavaFrameAnchor) {
@@ -457,7 +435,7 @@ public static void transitionNativeToJava(boolean popFrameAnchor) {
                 JavaFrameAnchors.popFrameAnchor();
             }
         } else {
-            callSlowPathNativeToNewStatus(Safepoint.ENTER_SLOW_PATH_TRANSITION_FROM_NATIVE_TO_NEW_STATUS, newStatus, popFrameAnchor);
+            callSlowPathNativeToNewStatus(ENTER_SLOW_PATH_TRANSITION_FROM_NATIVE_TO_NEW_STATUS, newStatus, popFrameAnchor);
         }
 
         /*
@@ -479,22 +457,23 @@ public static void slowTransitionNativeToVM() {
         int newStatus = StatusSupport.STATUS_IN_VM;
         boolean needSlowPath = !StatusSupport.compareAndSetNativeToNewStatus(newStatus);
         if (BranchProbabilityNode.probability(BranchProbabilityNode.VERY_SLOW_PATH_PROBABILITY, needSlowPath)) {
-            callSlowPathNativeToNewStatus(Safepoint.ENTER_SLOW_PATH_TRANSITION_FROM_NATIVE_TO_NEW_STATUS, newStatus, false);
+            callSlowPathNativeToNewStatus(ENTER_SLOW_PATH_TRANSITION_FROM_NATIVE_TO_NEW_STATUS, newStatus, false);
         }
     }
 
     @Uninterruptible(reason = "Must not contain safepoint checks")
     public static void transitionVMToJava(boolean popFrameAnchor) {
-        // We can directly change the thread status as no other thread will touch the status field
-        // as long as we are in VM status.
+        /* Change the thread status directly (other threads won't touch the status field). */
         StatusSupport.assertStatusVM();
         StatusSupport.setStatusJavaUnguarded();
         if (popFrameAnchor) {
             JavaFrameAnchors.popFrameAnchor();
         }
-        boolean needSlowPath = ThreadingSupportImpl.needsNativeToJavaSlowpath();
+
+        /* Only execute pending actions but don't do a safepoint slowpath call. */
+        boolean needSlowPath = ActionOnTransitionToJavaSupport.isActionPending();
         if (BranchProbabilityNode.probability(BranchProbabilityNode.VERY_SLOW_PATH_PROBABILITY, needSlowPath)) {
-            callSlowPathSafepointCheck(Safepoint.ENTER_SLOW_PATH_TRANSITION_FROM_VM_TO_JAVA);
+            callRunPendingActions(ENTER_SLOW_PATH_RUN_PENDING_ACTIONS);
         }
     }
 
@@ -514,16 +493,11 @@ public static void transitionVMToNative() {
         StatusSupport.setStatusNative();
     }
 
-    @NodeIntrinsic(value = ForeignCallNode.class)
-    private static native void callSlowPathSafepointCheck(@ConstantNodeParameter ForeignCallDescriptor descriptor);
-
     /**
      * Block until I can transition from native to a new thread status. This is not inlined and need
      * not be fast. In fact, it often blocks. But it can not do much except block, since it starts
      * out running with "native" thread status.
      *
-     * Foreign call: {@link #ENTER_SLOW_PATH_TRANSITION_FROM_NATIVE_TO_NEW_STATUS}.
-     *
      * This method cannot use the {@link StubCallingConvention} with callee saved registers: the
      * reference map of the C call and this slow-path call must be the same. This is only guaranteed
      * when both the C call and the call to this slow path do not use callee saved registers.
@@ -543,27 +517,27 @@ private static void enterSlowPathTransitionFromNativeToNewStatus(int newStatus,
         }
     }
 
-    @NodeIntrinsic(value = ForeignCallNode.class)
-    private static native void callSlowPathNativeToNewStatus(@ConstantNodeParameter ForeignCallDescriptor descriptor, int newThreadStatus, boolean popFrameAnchor);
-
     /**
-     * Transitions from VM to Java do not need a safepoint check. We only need to make sure that any
-     * {@link ActionOnTransitionToJavaSupport pending transition action} is executed.
-     *
-     * Foreign call: {@link #ENTER_SLOW_PATH_TRANSITION_FROM_VM_TO_JAVA}.
+     * Runs any {@link ActionOnTransitionToJavaSupport pending transition actions}.
      *
      * This method cannot use the {@link StubCallingConvention} with callee saved registers: the
      * reference map of the C call and this slow-path call must be the same. This is only guaranteed
      * when both the C call and the call to this slow path do not use callee saved registers.
      */
-    @SubstrateForeignCallTarget(stubCallingConvention = false)
+    @SubstrateForeignCallTarget(stubCallingConvention = false, fullyUninterruptible = true)
     @Uninterruptible(reason = "Must not contain safepoint checks.")
-    private static void enterSlowPathTransitionFromVMToJava() {
+    private static void enterSlowPathRunPendingActions() {
         VMError.guarantee(StatusSupport.isStatusJava(), "Must be already back in Java mode");
-
-        slowPathRunJavaStateActions();
+        assert ActionOnTransitionToJavaSupport.isActionPending() : "must not be called otherwise";
+        ActionOnTransitionToJavaSupport.runPendingActions();
     }
 
+    @NodeIntrinsic(value = ForeignCallNode.class)
+    private static native void callRunPendingActions(@ConstantNodeParameter ForeignCallDescriptor descriptor);
+
+    @NodeIntrinsic(value = ForeignCallNode.class)
+    private static native void callSlowPathNativeToNewStatus(@ConstantNodeParameter ForeignCallDescriptor descriptor, int newThreadStatus, boolean popFrameAnchor);
+
     /** Methods for the thread that brings the system to a safepoint. */
     @AutomaticallyRegisteredImageSingleton
     public static final class Master {

substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/thread/VMThreads.java

@@ -41,6 +41,7 @@
 import com.oracle.svm.core.Uninterruptible;
 import com.oracle.svm.core.c.function.CEntryPointErrors;
 import com.oracle.svm.core.c.function.CFunctionOptions;
+import com.oracle.svm.core.config.ConfigurationValues;
 import com.oracle.svm.core.feature.AutomaticallyRegisteredFeature;
 import com.oracle.svm.core.feature.InternalFeature;
 import com.oracle.svm.core.heap.Heap;
@@ -55,6 +56,7 @@
 import com.oracle.svm.core.memory.UntrackedNullableNativeMemory;
 import com.oracle.svm.core.nodes.CFunctionEpilogueNode;
 import com.oracle.svm.core.nodes.CFunctionPrologueNode;
+import com.oracle.svm.core.nodes.CodeSynchronizationNode;
 import com.oracle.svm.core.threadlocal.FastThreadLocal;
 import com.oracle.svm.core.threadlocal.FastThreadLocalBytes;
 import com.oracle.svm.core.threadlocal.FastThreadLocalFactory;
@@ -65,10 +67,12 @@
 import com.oracle.svm.core.util.VMError;
 
 import jdk.graal.compiler.api.directives.GraalDirectives;
+import jdk.graal.compiler.api.replacements.Fold;
 import jdk.graal.compiler.core.common.SuppressFBWarnings;
 import jdk.graal.compiler.replacements.ReplacementsUtil;
 import jdk.graal.compiler.replacements.nodes.AssertionNode;
 import jdk.graal.compiler.word.Word;
+import jdk.vm.ci.aarch64.AArch64;
 
 /**
  * Utility methods for the manipulation and iteration of {@link IsolateThread}s.
@@ -969,6 +973,7 @@ public static String toString(int safepointBehavior) {
 
     /**
      * A thread-local enum conveying any actions needed before thread begins executing Java code.
+     * Only used on aarch64 at the moment.
      */
     public static class ActionOnTransitionToJavaSupport {
 
@@ -982,26 +987,36 @@ public static class ActionOnTransitionToJavaSupport {
 
         @Uninterruptible(reason = "Called from uninterruptible code.", mayBeInlined = true)
         public static boolean isActionPending() {
-            return actionTL.getVolatile() != NO_ACTION;
+            if (!isAarch64()) {
+                return false;
+            }
+            return actionTL.get() != NO_ACTION;
         }
 
         @Uninterruptible(reason = "Called from uninterruptible code.", mayBeInlined = true)
-        public static boolean isSynchronizeCode() {
-            return actionTL.getVolatile() == SYNCHRONIZE_CODE;
-        }
+        public static void runPendingActions() {
+            if (!isAarch64() || !ActionOnTransitionToJavaSupport.isActionPending()) {
+                return;
+            }
 
-        @Uninterruptible(reason = "Called from uninterruptible code.", mayBeInlined = true)
-        public static void clearActions() {
-            actionTL.setVolatile(NO_ACTION);
+            assert actionTL.get() == SYNCHRONIZE_CODE;
+            CodeSynchronizationNode.synchronizeCode();
+            actionTL.set(NO_ACTION);
         }
 
         @Uninterruptible(reason = "Called from uninterruptible code.", mayBeInlined = true)
         public static void setSynchronizeCode(IsolateThread vmThread) {
+            if (!isAarch64()) {
+                return;
+            }
+
             assert StatusSupport.isStatusCreated(vmThread) || VMOperation.isInProgressAtSafepoint() : "Invariant to avoid races between setting and clearing.";
-            actionTL.setVolatile(vmThread, SYNCHRONIZE_CODE);
+            actionTL.set(vmThread, SYNCHRONIZE_CODE);
         }
 
         public static void requestAllThreadsSynchronizeCode() {
+            assert isAarch64();
+
             final IsolateThread myself = CurrentIsolate.getCurrentThread();
             for (IsolateThread vmThread = VMThreads.firstThread(); vmThread.isNonNull(); vmThread = VMThreads.nextThread(vmThread)) {
                 if (myself == vmThread) {
@@ -1010,6 +1025,11 @@ public static void requestAllThreadsSynchronizeCode() {
                 setSynchronizeCode(vmThread);
             }
         }
+
+        @Fold
+        static boolean isAarch64() {
+            return ConfigurationValues.getTarget().arch instanceof AArch64;
+        }
     }
 
     public interface OSThreadHandle extends PointerBase {