From cb351b0ff07df67dc2033eeec7211ff01f0bdc53 Mon Sep 17 00:00:00 2001 From: Christian Haeubl Date: Tue, 22 Jul 2025 14:25:09 +0200 Subject: [PATCH] Fix segfaults caused by graph scheduling. --- .../posix/PosixSubstrateSigprofHandler.java | 13 +++- .../svm/core/SubstrateSegfaultHandler.java | 1 + .../graal/snippets/CEntryPointSnippets.java | 62 ++++++++++++++----- 3 files changed, 59 insertions(+), 17 deletions(-) diff --git a/substratevm/src/com.oracle.svm.core.posix/src/com/oracle/svm/core/posix/PosixSubstrateSigprofHandler.java b/substratevm/src/com.oracle.svm.core.posix/src/com/oracle/svm/core/posix/PosixSubstrateSigprofHandler.java index 880e7dd52f34..934567062760 100644 --- a/substratevm/src/com.oracle.svm.core.posix/src/com/oracle/svm/core/posix/PosixSubstrateSigprofHandler.java +++ b/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(), diff --git a/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/SubstrateSegfaultHandler.java b/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/SubstrateSegfaultHandler.java index 57fdc864a506..de2a100cc384 100644 --- a/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/SubstrateSegfaultHandler.java +++ b/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); diff --git a/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/graal/snippets/CEntryPointSnippets.java b/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/graal/snippets/CEntryPointSnippets.java index a8a0c833993d..5d00a0363cf8 100644 --- a/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/graal/snippets/CEntryPointSnippets.java +++ b/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,6 +311,7 @@ private static int createIsolate(CEntryPointCreateIsolateParameters providedPara IsolateArgumentParser.singleton().tearDown(arguments); return error; } + Isolate isolate = isolatePtr.read(); initBaseRegisters(Isolates.getHeapBase(isolate)); @@ -317,7 +319,7 @@ private static int createIsolate(CEntryPointCreateIsolateParameters providedPara } @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(); }