Make the exception handling fully uninterruptible.

Author: christianhaeubl

substratevm/src/com.oracle.svm.core.graal.llvm/src/com/oracle/svm/core/graal/llvm/runtime/LLVMExceptionUnwind.java

@@ -25,10 +25,10 @@
 package com.oracle.svm.core.graal.llvm.runtime;
 
 import static com.oracle.svm.core.util.VMError.shouldNotReachHere;
+import static org.graalvm.nativeimage.c.function.CFunction.Transition.NO_TRANSITION;
 
 import java.util.function.BooleanSupplier;
 
-import jdk.graal.compiler.core.common.NumUtil;
 import org.graalvm.nativeimage.CurrentIsolate;
 import org.graalvm.nativeimage.IsolateThread;
 import org.graalvm.nativeimage.c.CContext;
@@ -50,6 +50,7 @@
 import com.oracle.svm.core.stack.StackOverflowCheck;
 import com.oracle.svm.hosted.code.CEntryPointCallStubSupport;
 
+import jdk.graal.compiler.core.common.NumUtil;
 import jdk.vm.ci.meta.MetaAccessProvider;
 import jdk.vm.ci.meta.ResolvedJavaMethod;
 
@@ -142,6 +143,7 @@ public static ResolvedJavaMethod getRetrieveExceptionMethod(MetaAccessProvider m
     public static ExceptionUnwind createRaiseExceptionHandler() {
         return new ExceptionUnwind() {
             @Override
+            @Uninterruptible(reason = "Code that is fully uninterruptible may throw and catch exceptions. Therefore, the exception handling must be fully uninterruptible as well.")
             protected void customUnwindException(Pointer callerSP) {
                 _Unwind_Exception exceptionStructure = UnsafeStackValue.get(_Unwind_Exception.class);
                 exceptionStructure.set_exception_class(CurrentIsolate.getCurrentThread());
@@ -198,7 +200,7 @@ private interface _Unwind_Exception extends PointerBase {
     private interface _Unwind_Context extends PointerBase {
     }
 
-    @CFunction(value = "_Unwind_RaiseException")
+    @CFunction(value = "_Unwind_RaiseException", transition = NO_TRANSITION)
     public static native int raiseException(_Unwind_Exception exception);
 
     @CFunction(value = "_Unwind_GetIP")

substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/deopt/DeoptimizedFrame.java

@@ -24,14 +24,14 @@
  */
 package com.oracle.svm.core.deopt;
 
+import static com.oracle.svm.core.Uninterruptible.CALLED_FROM_UNINTERRUPTIBLE_CODE;
+
 import java.lang.ref.WeakReference;
 
-import jdk.graal.compiler.word.Word;
 import org.graalvm.nativeimage.PinnedObject;
 import org.graalvm.nativeimage.c.function.CodePointer;
 import org.graalvm.word.Pointer;
 
-import com.oracle.svm.core.NeverInline;
 import com.oracle.svm.core.Uninterruptible;
 import com.oracle.svm.core.code.CodeInfo;
 import com.oracle.svm.core.code.CodeInfoAccess;
@@ -42,17 +42,15 @@
 import com.oracle.svm.core.config.ConfigurationValues;
 import com.oracle.svm.core.deopt.Deoptimizer.TargetContent;
 import com.oracle.svm.core.graal.stackvalue.UnsafeStackValue;
-import com.oracle.svm.core.heap.RestrictHeapAccess;
 import com.oracle.svm.core.log.StringBuilderLog;
 import com.oracle.svm.core.meta.SubstrateObjectConstant;
 import com.oracle.svm.core.monitor.MonitorSupport;
 
 import jdk.graal.compiler.nodes.FrameState;
+import jdk.graal.compiler.word.Word;
 import jdk.vm.ci.code.InstalledCode;
 import jdk.vm.ci.meta.JavaConstant;
 
-import static com.oracle.svm.core.Uninterruptible.CALLED_FROM_UNINTERRUPTIBLE_CODE;
-
 /**
  * The handle to a deoptimized frame. It contains all stack entries which are written to the frame
  * of the deopt target method(s). For details see {@link Deoptimizer}.
@@ -407,6 +405,7 @@ protected void buildContent(Pointer newSp) {
      * deoptimization stub return to the exception handler instead of the regular return address of
      * the deoptimization target.
      */
+    @Uninterruptible(reason = CALLED_FROM_UNINTERRUPTIBLE_CODE, mayBeInlined = true)
     public void takeException() {
         if (rethrowException) {
             /*
@@ -415,6 +414,7 @@ public void takeException() {
              */
             return;
         }
+
         ReturnAddress firstAddressEntry = topFrame.returnAddress;
         CodePointer ip = Word.pointer(firstAddressEntry.returnAddress);
         CodeInfo info = CodeInfoTable.getImageCodeInfo(ip);
@@ -427,13 +427,15 @@ public void takeException() {
         firstAddressEntry.returnAddress += handler;
     }
 
-    @NeverInline("Has more relaxed heap access requirements than caller.")
-    @RestrictHeapAccess(access = RestrictHeapAccess.Access.UNRESTRICTED, reason = "Printing out error and then crashing.")
+    @Uninterruptible(reason = "Does not need to be uninterruptible because it throws a fatal error.", calleeMustBe = false)
     private static void throwMissingExceptionHandler(CodeInfo info, ReturnAddress firstAddressEntry) {
+        throwMissingExceptionHandler0(info, firstAddressEntry);
+    }
+
+    private static void throwMissingExceptionHandler0(CodeInfo info, ReturnAddress firstAddressEntry) {
         CodeInfoQueryResult detailedQueryResult = new CodeInfoQueryResult();
         CodeInfoAccess.lookupCodeInfo(info, Word.pointer(firstAddressEntry.returnAddress), detailedQueryResult);
         FrameInfoQueryResult frameInfo = detailedQueryResult.getFrameInfo();
         throw Deoptimizer.fatalDeoptimizationError("No exception handler registered for deopt target", frameInfo);
     }
-
 }

substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/snippets/ExceptionUnwind.java

@@ -45,14 +45,12 @@
 import com.oracle.svm.core.deopt.DeoptimizationSupport;
 import com.oracle.svm.core.deopt.DeoptimizedFrame;
 import com.oracle.svm.core.deopt.Deoptimizer;
-import com.oracle.svm.core.heap.RestrictHeapAccess;
 import com.oracle.svm.core.log.Log;
 import com.oracle.svm.core.snippets.SnippetRuntime.SubstrateForeignCallDescriptor;
 import com.oracle.svm.core.stack.JavaFrame;
 import com.oracle.svm.core.stack.JavaFrames;
 import com.oracle.svm.core.stack.JavaStackWalk;
 import com.oracle.svm.core.stack.JavaStackWalker;
-import com.oracle.svm.core.stack.StackOverflowCheck;
 import com.oracle.svm.core.thread.VMThreads;
 import com.oracle.svm.core.threadlocal.FastThreadLocalBytes;
 import com.oracle.svm.core.threadlocal.FastThreadLocalFactory;
@@ -100,43 +98,25 @@ static boolean exceptionsAreFatal() {
 
     /** Foreign call: {@link #UNWIND_EXCEPTION_WITHOUT_CALLEE_SAVED_REGISTERS}. */
     @SubstrateForeignCallTarget(stubCallingConvention = true)
-    @Uninterruptible(reason = "Must not execute recurring callbacks or a stack overflow check.", calleeMustBe = false)
-    @RestrictHeapAccess(access = RestrictHeapAccess.Access.NO_ALLOCATION, reason = "Must not allocate when unwinding the stack.")
+    @Uninterruptible(reason = "Code that is fully uninterruptible may throw and catch exceptions. Therefore, the exception handling must be fully uninterruptible as well.")
     private static void unwindExceptionWithoutCalleeSavedRegisters(Throwable exception, Pointer callerSP) {
-        /*
-         * Make the yellow zone available and pause recurring callbacks to avoid that unexpected
-         * exceptions are thrown. This is reverted before execution continues in the exception
-         * handler (see ExceptionStackFrameVisitor.visitFrame).
-         */
-        StackOverflowCheck.singleton().makeYellowZoneAvailable();
-
-        unwindExceptionInterruptible(exception, callerSP, false, false);
+        unwindException(exception, callerSP, false, false);
     }
 
     /** Foreign call: {@link #UNWIND_EXCEPTION_WITH_CALLEE_SAVED_REGISTERS}. */
     @SubstrateForeignCallTarget(stubCallingConvention = true)
-    @Uninterruptible(reason = "Must not execute recurring callbacks or a stack overflow check.", calleeMustBe = false)
-    @RestrictHeapAccess(access = RestrictHeapAccess.Access.NO_ALLOCATION, reason = "Must not allocate when unwinding the stack.")
+    @Uninterruptible(reason = "Code that is fully uninterruptible may throw and catch exceptions. Therefore, the exception handling must be fully uninterruptible as well.")
     private static void unwindExceptionWithCalleeSavedRegisters(Throwable exception, Pointer callerSP) {
-        StackOverflowCheck.singleton().makeYellowZoneAvailable();
-
-        unwindExceptionInterruptible(exception, callerSP, true, false);
+        unwindException(exception, callerSP, true, false);
     }
 
-    @Uninterruptible(reason = "Must not execute recurring callbacks or a stack overflow check.", calleeMustBe = false)
-    @RestrictHeapAccess(access = RestrictHeapAccess.Access.NO_ALLOCATION, reason = "Must not allocate when unwinding the stack.")
+    @Uninterruptible(reason = "Code that is fully uninterruptible may throw and catch exceptions. Therefore, the exception handling must be fully uninterruptible as well.")
     public static void unwindExceptionSkippingCaller(Throwable exception, Pointer callerSP) {
-        StackOverflowCheck.singleton().makeYellowZoneAvailable();
-
-        unwindExceptionInterruptible(exception, callerSP, true, true);
+        unwindException(exception, callerSP, true, true);
     }
 
-    /*
-     * The stack walking objects must be stateless (no instance fields), because multiple threads
-     * can use them simultaneously. All state must be in separate VMThreadLocals.
-     */
-    @RestrictHeapAccess(access = RestrictHeapAccess.Access.NO_ALLOCATION, reason = "Must not allocate when unwinding the stack.")
-    private static void unwindExceptionInterruptible(Throwable exception, Pointer callerSP, boolean fromMethodWithCalleeSavedRegisters, boolean skipCaller) {
+    @Uninterruptible(reason = "Code that is fully uninterruptible may throw and catch exceptions. Therefore, the exception handling must be fully uninterruptible as well.")
+    private static void unwindException(Throwable exception, Pointer callerSP, boolean fromMethodWithCalleeSavedRegisters, boolean skipCaller) {
         if (currentException.get() != null) {
             reportRecursiveUnwind(exception);
             return; /* Unreachable code. */
@@ -168,6 +148,7 @@ private static void unwindExceptionInterruptible(Throwable exception, Pointer ca
      * Exception unwinding cannot be called recursively. The most likely reason to end up here is an
      * exception being thrown while walking the stack to find an exception handler.
      */
+    @Uninterruptible(reason = "Does not need to be uninterruptible because it throws a fatal error.", calleeMustBe = false)
     private static void reportRecursiveUnwind(Throwable exception) {
         Log.log().string("Fatal error: recursion in exception handling: ").string(exception.getClass().getName());
         Log.log().string(" thrown while unwinding ").string(currentException.get().getClass().getName()).newline().newline();
@@ -182,6 +163,7 @@ private static void reportRecursiveUnwind(Throwable exception) {
      * exception checks such as null pointer or array bounds checks. In such cases, exceptions are
      * treated as fatal errors.
      */
+    @Uninterruptible(reason = "Does not need to be uninterruptible because it throws a fatal error.", calleeMustBe = false)
     private static void reportFatalUnwind(Throwable exception) {
         Log.log().string("Fatal error: exception unwind while thread is not in Java state: ");
         Log.log().exception(exception).newline().newline();
@@ -194,13 +176,15 @@ private static void reportFatalUnwind(Throwable exception) {
      * proper exception handling and reporting of "unhandled" user exceptions is at a higher level
      * using a normal Java catch-all exception handler.
      */
+    @Uninterruptible(reason = "Does not need to be uninterruptible because it throws a fatal error.", calleeMustBe = false)
     private static void reportUnhandledException(Throwable exception) {
         Log.log().string("Fatal error: unhandled exception in isolate ").hex(CurrentIsolate.getIsolate()).string(": ");
         Log.log().exception(exception).newline().newline();
         VMError.shouldNotReachHere("Unhandled exception");
     }
 
     /** Hook to allow a {@link Feature} to install custom exception unwind code. */
+    @Uninterruptible(reason = "Code that is fully uninterruptible may throw and catch exceptions. Therefore, the exception handling must be fully uninterruptible as well.")
     protected abstract void customUnwindException(Pointer callerSP);
 
     /**
@@ -212,7 +196,7 @@ private static void reportUnhandledException(Throwable exception) {
      * @param skipCaller Whether the first (caller) frame should be skipped. If this is true, then
      *            the value of fromMethodWithCalleeSavedRegisters will be ignored.
      */
-    @Uninterruptible(reason = "Prevent deoptimization apart from the few places explicitly considered safe for deoptimization")
+    @Uninterruptible(reason = "Code that is fully uninterruptible may throw and catch exceptions. Therefore, the exception handling must be fully uninterruptible as well.")
     private static void defaultUnwindException(Pointer startSP, boolean fromMethodWithCalleeSavedRegisters, boolean skipCaller) {
         IsolateThread thread = CurrentIsolate.getCurrentThread();
         boolean hasCalleeSavedRegisters = fromMethodWithCalleeSavedRegisters;
@@ -236,7 +220,7 @@ private static void defaultUnwindException(Pointer startSP, boolean fromMethodWi
                     DeoptimizedFrame deoptFrame = Deoptimizer.checkEagerDeoptimized(frame);
                     if (deoptFrame != null) {
                         /* Deoptimization entry points always have an exception handler. */
-                        deoptTakeExceptionInterruptible(deoptFrame);
+                        deoptFrame.takeException();
                         jumpToHandler(sp, DeoptimizationSupport.getEagerDeoptStubPointer(), hasCalleeSavedRegisters);
                         UnreachableNode.unreachable();
                         return; /* Unreachable */
@@ -273,15 +257,13 @@ private static void defaultUnwindException(Pointer startSP, boolean fromMethodWi
         }
     }
 
-    @Uninterruptible(reason = "Prevent deoptimization while dispatching to exception handler")
+    @Uninterruptible(reason = "Code that is fully uninterruptible may throw and catch exceptions. Therefore, the exception handling must be fully uninterruptible as well.")
     private static void jumpToHandler(Pointer sp, CodePointer handlerIP, boolean hasCalleeSavedRegisters) {
         verifyTopFrameAnchor(sp);
 
         Throwable exception = currentException.get();
         currentException.set(null);
 
-        StackOverflowCheck.singleton().protectYellowZone();
-
         if (hasCalleeSavedRegisters) {
             /*
              * The fromMethodWithCalleeSavedRegisters parameter of farReturn must be a compile-time
@@ -294,9 +276,4 @@ private static void jumpToHandler(Pointer sp, CodePointer handlerIP, boolean has
         }
         /* Unreachable code: the intrinsic performs a jump to the specified instruction pointer. */
     }
-
-    @Uninterruptible(reason = "Wrap call to interruptible code.", calleeMustBe = false)
-    private static void deoptTakeExceptionInterruptible(DeoptimizedFrame deoptFrame) {
-        deoptFrame.takeException();
-    }
 }