[GR-67902] Fix segfaults caused by graph scheduling.

PullRequest: graal/21566

Author: christianhaeubl

substratevm/src/com.oracle.svm.core.posix/src/com/oracle/svm/core/posix/PosixSubstrateSigprofHandler.java

@@ -33,6 +33,7 @@
 import org.graalvm.nativeimage.c.function.CodePointer;
 import org.graalvm.word.Pointer;
 
+import com.oracle.svm.core.NeverInline;
 import com.oracle.svm.core.RegisterDumper;
 import com.oracle.svm.core.SubstrateOptions;
 import com.oracle.svm.core.Uninterruptible;
@@ -81,15 +82,21 @@ private static void dispatch(@SuppressWarnings("unused") int signalNumber, @Supp
         int savedErrno = LibC.errno();
         try {
             if (tryEnterIsolate()) {
-                CodePointer ip = (CodePointer) RegisterDumper.singleton().getIP(uContext);
-                Pointer sp = (Pointer) RegisterDumper.singleton().getSP(uContext);
-                tryUninterruptibleStackWalk(ip, sp, true);
+                dispatch0(uContext);
             }
         } finally {
             LibC.setErrno(savedErrno);
         }
     }
 
+    @Uninterruptible(reason = "The method executes during signal handling.", callerMustBe = true)
+    @NeverInline("Base registers are set in caller, prevent reads from floating before that.")
+    private static void dispatch0(Signal.ucontext_t uContext) {
+        CodePointer ip = (CodePointer) RegisterDumper.singleton().getIP(uContext);
+        Pointer sp = (Pointer) RegisterDumper.singleton().getSP(uContext);
+        tryUninterruptibleStackWalk(ip, sp, true);
+    }
+
     @Override
     protected void installSignalHandler() {
         PosixSignalHandlerSupport.installNativeSignalHandler(Signal.SignalEnum.SIGPROF, advancedSignalDispatcher.getFunctionPointer(), Signal.SA_RESTART(),

substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/SubstrateSegfaultHandler.java

@@ -259,6 +259,7 @@ public static void dump(PointerBase signalInfo, RegisterDumper.Context context)
     }
 
     @Uninterruptible(reason = "Must be uninterruptible until we get immune to safepoints.")
+    @NeverInline("Base registers are set in caller, prevent reads from floating before that.")
     public static void dump(PointerBase signalInfo, RegisterDumper.Context context, boolean inSVMSegfaultHandler) {
         Pointer sp = (Pointer) RegisterDumper.singleton().getSP(context);
         CodePointer ip = (CodePointer) RegisterDumper.singleton().getIP(context);

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

@@ -256,12 +256,13 @@ public static void initBaseRegisters(PointerBase heapBase, PointerBase codeBase)
     @Snippet(allowMissingProbabilities = true)
     public static int createIsolateSnippet(CEntryPointCreateIsolateParameters parameters) {
         writeCurrentVMThread(Word.nullPointer());
+
         int result = runtimeCall(CREATE_ISOLATE, parameters);
         if (result != CEntryPointErrors.NO_ERROR) {
             return result;
         }
-        ThreadStatusTransition.fromNativeToJava(false);
 
+        ThreadStatusTransition.fromNativeToJava(false);
         return runtimeCallInitializeIsolate(INITIALIZE_ISOLATE, parameters);
     }
 
@@ -310,14 +311,15 @@ private static int createIsolate(CEntryPointCreateIsolateParameters providedPara
             IsolateArgumentParser.singleton().tearDown(arguments);
             return error;
         }
+
         Isolate isolate = isolatePtr.read();
         initBaseRegisters(Isolates.getHeapBase(isolate));
 
         return createIsolate0(isolate, arguments);
     }
 
     @Uninterruptible(reason = "Thread state not yet set up.")
-    @NeverInline(value = "Ensure this code cannot rise above where heap base is set.")
+    @NeverInline("Base registers are set in caller, prevent reads from floating before that.")
     private static int createIsolate0(Isolate isolate, IsolateArguments arguments) {
         assert Heap.getHeap().verifyImageHeapMapping();
         IsolateArgumentParser.singleton().persistOptions(arguments);
@@ -327,10 +329,12 @@ private static int createIsolate0(Isolate isolate, IsolateArguments arguments) {
         if (!VMThreads.ensureInitialized()) {
             return CEntryPointErrors.THREADING_INITIALIZATION_FAILED;
         }
+
         int error = enterAttachThread0(isolate, false, true);
         if (error != CEntryPointErrors.NO_ERROR) {
             return error;
         }
+
         PlatformThreads.singleton().assignMainThread();
         return CEntryPointErrors.NO_ERROR;
     }
@@ -504,13 +508,13 @@ private static int initializeIsolateInterruptibly1(CEntryPointCreateIsolateParam
     @Snippet(allowMissingProbabilities = true)
     public static int attachThreadSnippet(Isolate isolate, boolean startedByIsolate, boolean ensureJavaThread) {
         writeCurrentVMThread(Word.nullPointer());
+
         int error = runtimeCall(ATTACH_THREAD, isolate, startedByIsolate, ensureJavaThread);
         if (error != CEntryPointErrors.NO_ERROR) {
             return error;
         }
 
         ThreadStatusTransition.fromNativeToJava(false);
-
         if (ensureJavaThread) {
             return runtimeCallEnsureJavaThread(ENSURE_JAVA_THREAD);
         }
@@ -534,7 +538,14 @@ private static int enterAttachThread0(Isolate isolate, boolean startedByIsolate,
         if (error != CEntryPointErrors.NO_ERROR) {
             return error;
         }
+
         initBaseRegisters(Isolates.getHeapBase(isolate));
+        return enterAttachThread1(isolate, startedByIsolate, ensureJavaThread, allowAttach, inCrashHandler);
+    }
+
+    @Uninterruptible(reason = "Thread state not set up yet.")
+    @NeverInline("Base registers are set in caller, prevent reads from floating before that.")
+    private static int enterAttachThread1(Isolate isolate, boolean startedByIsolate, boolean ensureJavaThread, boolean allowAttach, boolean inCrashHandler) {
         if (!VMThreads.isInitialized()) {
             return CEntryPointErrors.UNINITIALIZED_ISOLATE;
         }
@@ -567,6 +578,7 @@ private static int attachUnattachedThread(Isolate isolate, boolean startedByIsol
         if (thread.isNull()) {
             return CEntryPointErrors.ALLOCATION_FAILED;
         }
+
         writeCurrentVMThread(thread);
         if (!StackOverflowCheck.singleton().initialize()) {
             return CEntryPointErrors.UNKNOWN_STACK_BOUNDARIES;
@@ -601,8 +613,13 @@ public static int enterFromCrashHandler(Isolate isolate) {
     @Uninterruptible(reason = "Thread state not yet set up.")
     public static void initializeIsolateThreadForCrashHandler(Isolate isolate, IsolateThread thread) {
         initBaseRegisters(Isolates.getHeapBase(isolate));
-
         writeCurrentVMThread(thread);
+        initializeIsolateThreadForCrashHandler0(isolate, thread);
+    }
+
+    @Uninterruptible(reason = CALLED_FROM_UNINTERRUPTIBLE_CODE)
+    @NeverInline("Base registers are set in caller, prevent reads from floating before that.")
+    private static void initializeIsolateThreadForCrashHandler0(Isolate isolate, IsolateThread thread) {
         VMThreads.StatusSupport.setStatusNative();
         VMThreads.IsolateTL.set(thread, isolate);
         SubstrateDiagnostics.setOnlyAttachedForCrashHandler(thread);
@@ -713,13 +730,11 @@ private static boolean initiateTearDownIsolateInterruptibly() {
 
     @Snippet(allowMissingProbabilities = true)
     public static int enterByIsolateSnippet(Isolate isolate) {
-        int result;
         writeCurrentVMThread(Word.nullPointer());
-        result = runtimeCall(ENTER_BY_ISOLATE, isolate);
-        if (result == CEntryPointErrors.NO_ERROR) {
-            if (VMThreads.StatusSupport.isStatusNativeOrSafepoint()) {
-                ThreadStatusTransition.fromNativeToJava(false);
-            }
+
+        int result = runtimeCall(ENTER_BY_ISOLATE, isolate);
+        if (result == CEntryPointErrors.NO_ERROR && VMThreads.StatusSupport.isStatusNativeOrSafepoint()) {
+            ThreadStatusTransition.fromNativeToJava(false);
         }
         return result;
     }
@@ -731,14 +746,23 @@ private static int enterByIsolate(Isolate isolate) {
         if (error != CEntryPointErrors.NO_ERROR) {
             return error;
         }
+
         initBaseRegisters(Isolates.getHeapBase(isolate));
+        return enterByIsolate0();
+    }
+
+    @Uninterruptible(reason = "Thread state not set up yet.")
+    @NeverInline("Base registers are set in caller, prevent reads from floating before that.")
+    private static int enterByIsolate0() {
         if (!VMThreads.isInitialized()) {
             return CEntryPointErrors.UNINITIALIZED_ISOLATE;
         }
+
         IsolateThread thread = VMThreads.singleton().findIsolateThreadForCurrentOSThread(false);
         if (thread.isNull()) {
             return CEntryPointErrors.UNATTACHED_THREAD;
         }
+
         writeCurrentVMThread(thread);
         return CEntryPointErrors.NO_ERROR;
     }
@@ -748,9 +772,11 @@ public static int enterSnippet(IsolateThread thread) {
         if (thread.isNull()) {
             return CEntryPointErrors.NULL_ARGUMENT;
         }
+
         writeCurrentVMThread(thread);
         Isolate isolate = VMThreads.IsolateTL.get(thread);
         initBaseRegisters(Isolates.getHeapBase(isolate));
+
         if (runtimeAssertionsEnabled() || SubstrateOptions.CheckIsolateThreadAtEntry.getValue()) {
             /*
              * Verification must happen before the thread state transition. It locks the raw
@@ -759,9 +785,7 @@ public static int enterSnippet(IsolateThread thread) {
             runtimeCall(VERIFY_ISOLATE_THREAD, thread);
         }
         ThreadStatusTransition.fromNativeToJava(false);
-
         return CEntryPointErrors.NO_ERROR;
-
     }
 
     @Fold
@@ -804,7 +828,7 @@ private static int reportException(Throwable exception) {
 
     private static void reportExceptionInterruptibly(Throwable exception) {
         logException(exception);
-        VMError.shouldNotReachHere("Unhandled exception");
+        throw VMError.shouldNotReachHere("Unhandled exception");
     }
 
     @RestrictHeapAccess(access = NO_ALLOCATION, reason = "Must not allocate in fatal error handling.")
@@ -825,13 +849,23 @@ public static int returnFromJavaToCSnippet() {
 
     @Snippet(allowMissingProbabilities = true)
     public static boolean isAttachedSnippet(Isolate isolate) {
-        return Isolates.checkIsolate(isolate) == CEntryPointErrors.NO_ERROR && runtimeCallIsAttached(IS_ATTACHED, isolate);
+        return runtimeCallIsAttached(IS_ATTACHED, isolate);
     }
 
     @Uninterruptible(reason = "Thread state not yet set up.")
     @SubstrateForeignCallTarget(stubCallingConvention = false)
     private static boolean isAttached(Isolate isolate) {
+        if (Isolates.checkIsolate(isolate) != CEntryPointErrors.NO_ERROR) {
+            return false;
+        }
+
         initBaseRegisters(Isolates.getHeapBase(isolate));
+        return isAttached0();
+    }
+
+    @Uninterruptible(reason = "Thread state not yet set up.")
+    @NeverInline("Base registers are set in caller, prevent reads from floating before that.")
+    private static boolean isAttached0() {
         return VMThreads.isInitialized() && VMThreads.singleton().findIsolateThreadForCurrentOSThread(false).isNonNull();
     }