[GR-66563] [GR-66569] Better frame anchor verification and fully uninterruptible exception handling.

PullRequest: graal/21264

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/deopt/Deoptimizer.java

@@ -25,6 +25,7 @@
 package com.oracle.svm.core.deopt;
 
 import static com.oracle.svm.core.Uninterruptible.CALLED_FROM_UNINTERRUPTIBLE_CODE;
+import static com.oracle.svm.core.stack.JavaFrameAnchors.verifyTopFrameAnchor;
 
 import java.lang.annotation.ElementType;
 import java.lang.annotation.Retention;
@@ -813,7 +814,7 @@ private static boolean isNonNullValue(UnsignedWord pointer) {
 
     @DeoptStub(stubType = StubType.LazyEntryStub)
     @Uninterruptible(reason = "Rewriting stack; gpReturnValue holds object reference.")
-    public static UnsignedWord lazyDeoptStubObjectReturn(Pointer framePointer, UnsignedWord gpReturnValue, UnsignedWord fpReturnValue) {
+    public static UnsignedWord lazyDeoptStubObjectReturn(Pointer originalStackPointer, UnsignedWord gpReturnValue, UnsignedWord fpReturnValue) {
         try {
             assert PointerUtils.isAMultiple(KnownIntrinsics.readStackPointer(), Word.unsigned(ConfigurationValues.getTarget().stackAlignment));
             assert Options.LazyDeoptimization.getValue();
@@ -828,7 +829,7 @@ public static UnsignedWord lazyDeoptStubObjectReturn(Pointer framePointer, Unsig
                 gpReturnValueObject = ((Pointer) gpReturnValue).toObject();
             }
 
-            lazyDeoptStubCore(framePointer, gpReturnValue, fpReturnValue, hasException, gpReturnValueObject);
+            lazyDeoptStubCore(originalStackPointer, gpReturnValue, fpReturnValue, hasException, gpReturnValueObject);
             throw UnreachableNode.unreachable();
 
         } catch (Throwable t) {
@@ -838,7 +839,7 @@ public static UnsignedWord lazyDeoptStubObjectReturn(Pointer framePointer, Unsig
 
     @DeoptStub(stubType = StubType.LazyEntryStub)
     @Uninterruptible(reason = "Rewriting stack.")
-    public static UnsignedWord lazyDeoptStubPrimitiveReturn(Pointer framePointer, UnsignedWord gpReturnValue, UnsignedWord fpReturnValue) {
+    public static UnsignedWord lazyDeoptStubPrimitiveReturn(Pointer originalStackPointer, UnsignedWord gpReturnValue, UnsignedWord fpReturnValue) {
         /*
          * Note: when we dispatch an exception, we enter lazyDeoptStubObjectReturn instead, since
          * that involves returning an exception object.
@@ -849,7 +850,7 @@ public static UnsignedWord lazyDeoptStubPrimitiveReturn(Pointer framePointer, Un
             assert VMThreads.StatusSupport.isStatusJava() : "Deopt stub execution must not be visible to other threads.";
             assert !ExceptionUnwind.getLazyDeoptStubShouldReturnToExceptionHandler();
 
-            lazyDeoptStubCore(framePointer, gpReturnValue, fpReturnValue, false, null);
+            lazyDeoptStubCore(originalStackPointer, gpReturnValue, fpReturnValue, false, null);
             throw UnreachableNode.unreachable();
 
         } catch (Throwable t) {
@@ -864,32 +865,32 @@ public static UnsignedWord lazyDeoptStubPrimitiveReturn(Pointer framePointer, Un
      * the code info, and construct the {@link DeoptimizedFrame}.
      */
     @Uninterruptible(reason = "Rewriting stack.")
-    private static UnsignedWord lazyDeoptStubCore(Pointer framePointer, UnsignedWord gpReturnValue, UnsignedWord fpReturnValue, boolean hasException, Object gpReturnValueObject) {
-        CodePointer deoptStubAddress = FrameAccess.singleton().readReturnAddress(CurrentIsolate.getCurrentThread(), framePointer);
+    private static UnsignedWord lazyDeoptStubCore(Pointer originalStackPointer, UnsignedWord gpReturnValue, UnsignedWord fpReturnValue, boolean hasException, Object gpReturnValueObject) {
+        CodePointer deoptStubAddress = FrameAccess.singleton().readReturnAddress(CurrentIsolate.getCurrentThread(), originalStackPointer);
         assert isLazyDeoptStub(deoptStubAddress);
 
         /* The original return address is at offset 0 from the stack pointer */
-        CodePointer originalReturnAddress = framePointer.readWord(0);
+        CodePointer originalReturnAddress = originalStackPointer.readWord(0);
         VMError.guarantee(originalReturnAddress.isNonNull());
 
         DeoptimizedFrame deoptFrame;
         try {
-            deoptFrame = constructLazilyDeoptimizedFrameInterruptibly(framePointer, originalReturnAddress, hasException);
+            deoptFrame = constructLazilyDeoptimizedFrameInterruptibly(originalStackPointer, originalReturnAddress, hasException);
         } catch (OutOfMemoryError ex) {
             /*
              * If a OutOfMemoryError occurs during lazy deoptimization, we cannot let the frame
              * being deoptimized handle the exception, because it might have been invalidated due to
              * incorrect assumptions. Note that since unwindExceptionSkippingCaller does not return,
              * this try...catch must not have a finally block, as it will not be executed.
              */
-            ExceptionUnwind.unwindExceptionSkippingCaller(ex, framePointer);
+            ExceptionUnwind.unwindExceptionSkippingCaller(ex, originalStackPointer);
             throw UnreachableNode.unreachable();
         }
 
         DeoptimizationCounters.counters().deoptCount.inc();
         VMError.guarantee(deoptFrame != null, "was not able to lazily construct a deoptimized frame");
 
-        Pointer newSp = computeNewFramePointer(framePointer, deoptFrame);
+        Pointer newSp = computeNewStackPointer(originalStackPointer, deoptFrame);
 
         /* Build the content of the deopt target stack frames. */
         deoptFrame.buildContent(newSp);
@@ -900,9 +901,9 @@ private static UnsignedWord lazyDeoptStubCore(Pointer framePointer, UnsignedWord
          * can only be called from the current thread. Thus, there is no use case for eager
          * deoptimization to happen if the current thread is executing the lazy deopt stub.
          */
-        VMError.guarantee(framePointer.readWord(0) == originalReturnAddress, "Eager deoptimization should not occur when lazy deoptimization is in progress");
+        VMError.guarantee(originalStackPointer.readWord(0) == originalReturnAddress, "Eager deoptimization should not occur when lazy deoptimization is in progress");
 
-        CodePointer returnAddressAfter = FrameAccess.singleton().readReturnAddress(CurrentIsolate.getCurrentThread(), framePointer);
+        CodePointer returnAddressAfter = FrameAccess.singleton().readReturnAddress(CurrentIsolate.getCurrentThread(), originalStackPointer);
         VMError.guarantee(returnAddressAfter == deoptStubAddress, "Return address must remain unchanged during deoptimization");
 
         recentDeoptimizationEvents.append(deoptFrame.getCompletedMessage());
@@ -962,7 +963,7 @@ private static DeoptimizedFrame constructLazilyDeoptimizedFrameInterruptibly0(Po
      * When {@link #eagerDeoptStub} is "called", the stack looks like this:
      *
      * <pre>
-     *    :                                :
+     *    :                                :   highest stack address
      *    |                                |
      *    |                                |   frame of the
      *    +--------------------------------+   deoptimized method
@@ -977,9 +978,8 @@ private static DeoptimizedFrame constructLazilyDeoptimizedFrameInterruptibly0(Po
      * The instructions to compute the parameters must be generated in this method's prologue by a
      * backend-specific FrameContext class.
      *
-     * @param framePointer This is a pointer to the reference which was written in
-     *            {@link #deoptimizeInRange} on the stack (the slot above the original return
-     *            address).
+     * @param originalStackPointer the original stack pointer of the deoptimized method (points to
+     *            the {@link DeoptimizedFrame} object).
      * @param gpReturnValue This is the value which was stored in the general purpose return
      *            register when the deopt stub was reached. It must be restored to the register
      *            before completion of the stub.
@@ -989,18 +989,19 @@ private static DeoptimizedFrame constructLazilyDeoptimizedFrameInterruptibly0(Po
      */
     @DeoptStub(stubType = StubType.EagerEntryStub)
     @Uninterruptible(reason = "Frame holds Objects in unmanaged storage.")
-    public static UnsignedWord eagerDeoptStub(Pointer framePointer, UnsignedWord gpReturnValue, UnsignedWord fpReturnValue) {
+    public static UnsignedWord eagerDeoptStub(Pointer originalStackPointer, UnsignedWord gpReturnValue, UnsignedWord fpReturnValue) {
         try {
             assert PointerUtils.isAMultiple(KnownIntrinsics.readStackPointer(), Word.unsigned(ConfigurationValues.getTarget().stackAlignment));
             VMError.guarantee(VMThreads.StatusSupport.isStatusJava(), "Deopt stub execution must not be visible to other threads.");
-            DeoptimizedFrame frame = (DeoptimizedFrame) ReferenceAccess.singleton().readObjectAt(framePointer, true);
+
+            DeoptimizedFrame frame = (DeoptimizedFrame) ReferenceAccess.singleton().readObjectAt(originalStackPointer, true);
 
             DeoptimizationCounters.counters().deoptCount.inc();
             if (DeoptimizationCounters.Options.ProfileDeoptimization.getValue()) {
                 DeoptimizationCounters.startTime.set(System.nanoTime());
             }
 
-            final Pointer newSp = computeNewFramePointer(framePointer, frame);
+            final Pointer newSp = computeNewStackPointer(originalStackPointer, frame);
 
             /* Build the content of the deopt target stack frames. */
             frame.buildContent(newSp);
@@ -1023,13 +1024,15 @@ public static UnsignedWord eagerDeoptStub(Pointer framePointer, UnsignedWord gpR
     }
 
     @Uninterruptible(reason = CALLED_FROM_UNINTERRUPTIBLE_CODE, mayBeInlined = true)
-    private static Pointer computeNewFramePointer(Pointer originalFramePointer, DeoptimizedFrame deoptimizedFrame) {
-        /* Computation of the new stack pointer: we start with the stack pointer of this frame. */
-        return originalFramePointer
-                        /* Remove the size of the frame that gets deoptimized. */
-                        .add(Word.unsigned(deoptimizedFrame.getSourceTotalFrameSize()))
-                        /* Add the size of the deoptimization target frames. */
-                        .subtract(deoptimizedFrame.getTargetContent().getSize());
+    private static Pointer computeNewStackPointer(Pointer originalStackPointer, DeoptimizedFrame deoptimizedFrame) {
+        /* Remove the size of the frame that gets deoptimized. */
+        Pointer callerStackPointer = originalStackPointer.add(Word.unsigned(deoptimizedFrame.getSourceTotalFrameSize()));
+
+        /* Verify that the top frame anchor is in a part of the stack that is not rewritten. */
+        verifyTopFrameAnchor(callerStackPointer);
+
+        /* Add the size of the deoptimization target frames. */
+        return callerStackPointer.subtract(deoptimizedFrame.getTargetContent().getSize());
     }
 
     /**
@@ -1054,7 +1057,6 @@ private static UnsignedWord rewriteStackStub(Pointer newSp, UnsignedWord gpRetur
         if (DeoptimizationCounters.Options.ProfileDeoptimization.getValue()) {
             DeoptimizationCounters.counters().timeSpentInDeopt.add(System.nanoTime() - DeoptimizationCounters.startTime.get());
         }
-
         return gpReturnValue;
     }
 

substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/graal/snippets/CFunctionSnippets.java

@@ -276,14 +276,20 @@ private static void matchCallStructure(CFunctionPrologueNode prologueNode) {
     }
 }
 
+/**
+ * {@link CFunctionSnippets} may only be used for code that cannot be deoptimized. Otherwise,
+ * deoptimization could destroy stack allocated {@link JavaFrameAnchor} structs when rewriting the
+ * stack.
+ */
 @AutomaticallyRegisteredFeature
 @Platforms(InternalPlatform.NATIVE_ONLY.class)
 class CFunctionSnippetsFeature implements InternalFeature {
-
     @Override
     @SuppressWarnings("unused")
     public void registerLowerings(RuntimeConfiguration runtimeConfig, OptionValues options, Providers providers,
                     Map<Class<? extends Node>, NodeLoweringProvider<?>> lowerings, boolean hosted) {
-        new CFunctionSnippets(options, providers, lowerings);
+        if (hosted) {
+            new CFunctionSnippets(options, providers, lowerings);
+        }
     }
 }