[GR-45994] Signal-handler related simplifications.

sdk/mx.sdk/mx_sdk_vm_impl.py

@@ -1330,7 +1330,6 @@ def contents(self):
                 build_args += ['--language:' + image_config.language, '--tool:all']
 
             if isinstance(image_config, mx_sdk.LanguageLibraryConfig):
-                build_args += ['-H:+EnableSignalAPI']
                 if image_config.main_class:
                     build_args += ['-Dorg.graalvm.launcher.class=' + image_config.main_class]
 

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

@@ -27,26 +27,20 @@
 import org.graalvm.nativeimage.c.function.CEntryPoint;
 import org.graalvm.nativeimage.c.function.CEntryPoint.Publish;
 import org.graalvm.nativeimage.c.function.CEntryPointLiteral;
-import org.graalvm.nativeimage.c.struct.SizeOf;
 import org.graalvm.nativeimage.c.type.VoidPointer;
 import org.graalvm.word.PointerBase;
-import org.graalvm.word.WordFactory;
 
-import com.oracle.svm.core.SubstrateOptions;
 import com.oracle.svm.core.SubstrateSegfaultHandler;
 import com.oracle.svm.core.Uninterruptible;
 import com.oracle.svm.core.c.function.CEntryPointOptions;
 import com.oracle.svm.core.c.function.CEntryPointOptions.NoEpilogue;
 import com.oracle.svm.core.c.function.CEntryPointOptions.NoPrologue;
 import com.oracle.svm.core.feature.AutomaticallyRegisteredImageSingleton;
-import com.oracle.svm.core.graal.stackvalue.UnsafeStackValue;
-import com.oracle.svm.core.headers.LibC;
 import com.oracle.svm.core.heap.RestrictHeapAccess;
 import com.oracle.svm.core.log.Log;
 import com.oracle.svm.core.os.MemoryProtectionProvider;
 import com.oracle.svm.core.posix.headers.Signal;
 import com.oracle.svm.core.posix.headers.Signal.AdvancedSignalDispatcher;
-import com.oracle.svm.core.posix.headers.Signal.sigaction;
 import com.oracle.svm.core.posix.headers.Signal.siginfo_t;
 import com.oracle.svm.core.posix.headers.Signal.ucontext_t;
 import com.oracle.svm.core.util.VMError;
@@ -92,20 +86,7 @@ protected void printSignalInfo(Log log, PointerBase signalInfo) {
 
     @Override
     protected void installInternal() {
-        VMError.guarantee(SubstrateOptions.EnableSignalHandling.getValue(), "Trying to install a signal handler while signal handling is disabled.");
-        int structSigActionSize = SizeOf.get(sigaction.class);
-        sigaction structSigAction = UnsafeStackValue.get(structSigActionSize);
-        LibC.memset(structSigAction, WordFactory.signed(0), WordFactory.unsigned(structSigActionSize));
-        /* Register sa_sigaction signal handler */
-        structSigAction.sa_flags(Signal.SA_SIGINFO() | Signal.SA_NODEFER());
-        structSigAction.sa_sigaction(advancedSignalDispatcher.getFunctionPointer());
-        synchronized (Target_jdk_internal_misc_Signal.class) {
-            /*
-             * Don't want to race with logic within Util_jdk_internal_misc_Signal#handle0 which
-             * reads these signals.
-             */
-            Signal.sigaction(Signal.SignalEnum.SIGSEGV.getCValue(), structSigAction, WordFactory.nullPointer());
-            Signal.sigaction(Signal.SignalEnum.SIGBUS.getCValue(), structSigAction, WordFactory.nullPointer());
-        }
+        PosixUtils.installSignalHandler(Signal.SignalEnum.SIGSEGV, advancedSignalDispatcher.getFunctionPointer(), Signal.SA_NODEFER());
+        PosixUtils.installSignalHandler(Signal.SignalEnum.SIGBUS, advancedSignalDispatcher.getFunctionPointer(), Signal.SA_NODEFER());
     }
 }

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

@@ -36,21 +36,18 @@
 import org.graalvm.nativeimage.c.function.CEntryPoint;
 import org.graalvm.nativeimage.c.function.CEntryPointLiteral;
 import org.graalvm.nativeimage.c.function.CodePointer;
-import org.graalvm.nativeimage.c.struct.SizeOf;
 import org.graalvm.nativeimage.hosted.Feature;
 import org.graalvm.word.Pointer;
 import org.graalvm.word.WordFactory;
 
 import com.oracle.svm.core.IsolateListenerSupport;
 import com.oracle.svm.core.IsolateListenerSupportFeature;
 import com.oracle.svm.core.RegisterDumper;
-import com.oracle.svm.core.SubstrateOptions;
 import com.oracle.svm.core.Uninterruptible;
 import com.oracle.svm.core.c.function.CEntryPointOptions;
 import com.oracle.svm.core.feature.AutomaticallyRegisteredFeature;
 import com.oracle.svm.core.feature.InternalFeature;
 import com.oracle.svm.core.graal.stackvalue.UnsafeStackValue;
-import com.oracle.svm.core.headers.LibC;
 import com.oracle.svm.core.heap.RestrictHeapAccess;
 import com.oracle.svm.core.jfr.JfrFeature;
 import com.oracle.svm.core.jfr.sampler.JfrExecutionSampler;
@@ -60,7 +57,6 @@
 import com.oracle.svm.core.sampler.SubstrateSigprofHandler;
 import com.oracle.svm.core.thread.ThreadListenerSupport;
 import com.oracle.svm.core.util.TimeUtils;
-import com.oracle.svm.core.util.VMError;
 
 public class PosixSubstrateSigprofHandler extends SubstrateSigprofHandler {
     private static final CEntryPointLiteral<Signal.AdvancedSignalDispatcher> advancedSignalDispatcher = CEntryPointLiteral.create(PosixSubstrateSigprofHandler.class,
@@ -85,21 +81,7 @@ private static void dispatch(@SuppressWarnings("unused") int signalNumber, @Supp
     }
 
     private static void registerSigprofSignal(Signal.AdvancedSignalDispatcher dispatcher) {
-        VMError.guarantee(SubstrateOptions.EnableSignalHandling.getValue(), "Trying to install a signal handler while signal handling is disabled.");
-        int structSigActionSize = SizeOf.get(Signal.sigaction.class);
-        Signal.sigaction structSigAction = UnsafeStackValue.get(structSigActionSize);
-        LibC.memset(structSigAction, WordFactory.signed(0), WordFactory.unsigned(structSigActionSize));
-
-        /* Register sa_sigaction signal handler */
-        structSigAction.sa_flags(Signal.SA_SIGINFO() | Signal.SA_NODEFER() | Signal.SA_RESTART());
-        structSigAction.sa_sigaction(dispatcher);
-        /*
-         * Note this can race with other signals being installed. However, using Java
-         * synchronization is disallowed within a VMOperation. If race-free execution becomes
-         * necessary, then a VMMutex will be needed and additional code will need to be
-         * made @Uniterruptible so that a thread owning the VMMutex cannot block at a safepoint.
-         */
-        Signal.sigaction(Signal.SignalEnum.SIGPROF.getCValue(), structSigAction, WordFactory.nullPointer());
+        PosixUtils.installSignalHandler(Signal.SignalEnum.SIGPROF, dispatcher, Signal.SA_NODEFER() | Signal.SA_RESTART());
     }
 
     @Override

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

@@ -56,6 +56,7 @@
 import com.oracle.svm.core.posix.headers.Wait;
 import com.oracle.svm.core.posix.headers.darwin.DarwinTime;
 import com.oracle.svm.core.posix.headers.linux.LinuxTime;
+import com.oracle.svm.core.thread.VMOperation;
 import com.oracle.svm.core.util.VMError;
 
 public class PosixUtils {
@@ -251,6 +252,10 @@ public static int readBytes(int fd, CCharPointer buffer, int bufferLen, int read
         return readBytes;
     }
 
+    public static Signal.SignalDispatcher installSignalHandler(Signal.SignalEnum signum, Signal.SignalDispatcher handler, int flags) {
+        return installSignalHandler(signum.getCValue(), handler, flags);
+    }
+
     /**
      * Emulates the deprecated {@code signal} function via its replacement {@code sigaction},
      * assuming BSD semantics (like glibc does, for example).
@@ -261,26 +266,58 @@ public static int readBytes(int fd, CCharPointer buffer, int bufferLen, int read
      * Note that this method should not be called from an initialization hook:
      * {@code EnableSignalHandling} may not be set correctly at the time initialization hooks run.
      */
-    public static Signal.SignalDispatcher installSignalHandler(int signum, Signal.SignalDispatcher handler) {
-        synchronized (Target_jdk_internal_misc_Signal.class) {
-            return installSignalHandler0(signum, handler);
+    public static Signal.SignalDispatcher installSignalHandler(int signum, Signal.SignalDispatcher handler, int flags) {
+        int structSigActionSize = SizeOf.get(Signal.sigaction.class);
+        Signal.sigaction act = UnsafeStackValue.get(structSigActionSize);
+        LibC.memset(act, WordFactory.signed(0), WordFactory.unsigned(structSigActionSize));
+        act.sa_flags(flags);
+        act.sa_handler(handler);
+
+        Signal.sigaction old = UnsafeStackValue.get(Signal.sigaction.class);
+
+        int result = sigaction(signum, act, old);
+        if (result != 0) {
+            return Signal.SIG_ERR();
         }
+        return old.sa_handler();
     }
 
-    private static Signal.SignalDispatcher installSignalHandler0(int signum, Signal.SignalDispatcher handler) {
-        VMError.guarantee(SubstrateOptions.EnableSignalHandling.getValue(), "Trying to install a signal handler while signal handling is disabled.");
-        Signal.sigaction old = UnsafeStackValue.get(Signal.sigaction.class);
+    public static void installSignalHandler(Signal.SignalEnum signum, Signal.AdvancedSignalDispatcher handler, int flags) {
+        installSignalHandler(signum.getCValue(), handler, flags);
+    }
 
+    public static void installSignalHandler(int signum, Signal.AdvancedSignalDispatcher handler, int flags) {
         int structSigActionSize = SizeOf.get(Signal.sigaction.class);
         Signal.sigaction act = UnsafeStackValue.get(structSigActionSize);
         LibC.memset(act, WordFactory.signed(0), WordFactory.unsigned(structSigActionSize));
+        act.sa_flags(Signal.SA_SIGINFO() | flags);
+        act.sa_sigaction(handler);
 
-        act.sa_flags(Signal.SA_RESTART());
-        act.sa_handler(handler);
-        if (Signal.sigaction(signum, act, old) != 0) {
-            return Signal.SIG_ERR();
+        int result = sigaction(signum, act, WordFactory.nullPointer());
+        PosixUtils.checkStatusIs0(result, "sigaction failed in installSignalHandler().");
+    }
+
+    /*
+     * Avoid races with logic within Util_jdk_internal_misc_Signal#handle0 which reads these
+     * signals.
+     */
+    private static int sigaction(int signum, Signal.sigaction structSigAction, Signal.sigaction old) {
+        VMError.guarantee(SubstrateOptions.EnableSignalHandling.getValue(), "Trying to install a signal handler while signal handling is disabled.");
+
+        if (VMOperation.isInProgress()) {
+            /*
+             * Note this can race with other signals being installed. However, using Java
+             * synchronization is disallowed within a VMOperation. If race-free execution becomes
+             * necessary, then a VMMutex will be needed and additional code will need to be
+             * made @Uninterruptible so that a thread owning the VMMutex cannot block at a
+             * safepoint.
+             */
+            return Signal.sigaction(signum, structSigAction, old);
+        } else {
+            synchronized (Target_jdk_internal_misc_Signal.class) {
+                return Signal.sigaction(signum, structSigAction, old);
+            }
         }
-        return old.sa_handler();
     }
 
     // Checkstyle: stop

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

@@ -67,8 +67,8 @@ private static int findSignal0(String signalName) {
 
     @Substitute
     private static long handle0(int sig, long nativeH) {
-        if (!SubstrateOptions.EnableSignalAPI.getValue()) {
-            throw new IllegalArgumentException("Installing signal handlers is not enabled");
+        if (!SubstrateOptions.EnableSignalHandling.getValue()) {
+            throw new IllegalArgumentException("Signal handlers can't be installed if signal handling is disabled, see option '" + SubstrateOptions.EnableSignalHandling.getName() + "'.");
         }
         return Util_jdk_internal_misc_Signal.handle0(sig, nativeH);
     }
@@ -129,9 +129,6 @@ static boolean isCurrentDispatcher(int sig, SignalDispatcher dispatcher) {
      * is called from within a static synchronized call to ensure race-free execution.
      */
     static long handle0(int sig, long nativeH) {
-        if (!SubstrateOptions.EnableSignalHandling.getValue()) {
-            return sunMiscSignalIgnoreHandler;
-        }
         ensureInitialized();
         final Signal.SignalDispatcher newDispatcher = nativeHToDispatcher(nativeH);
         /* If the dispatcher is the CSunMiscSignal handler, then check if the signal is in range. */
@@ -157,7 +154,7 @@ static long handle0(int sig, long nativeH) {
         }
 
         updateDispatcher(sig, newDispatcher);
-        final Signal.SignalDispatcher oldDispatcher = PosixUtils.installSignalHandler(sig, newDispatcher);
+        final Signal.SignalDispatcher oldDispatcher = PosixUtils.installSignalHandler(sig, newDispatcher, Signal.SA_RESTART());
         CIntPointer sigset = UnsafeStackValue.get(CIntPointer.class);
         sigset.write(1 << (sig - 1));
         Signal.sigprocmask(Signal.SIG_UNBLOCK(), (Signal.sigset_tPointer) sigset, WordFactory.nullPointer());
@@ -456,7 +453,7 @@ private static void installNoopHandler(Signal.SignalEnum signal) {
             /*
              * Replace with no-op signal handler if a custom one has not already been installed.
              */
-            final SignalDispatcher signalResult = PosixUtils.installSignalHandler(signum, NOOP_SIGNAL_HANDLER.getFunctionPointer());
+            final SignalDispatcher signalResult = PosixUtils.installSignalHandler(signum, NOOP_SIGNAL_HANDLER.getFunctionPointer(), Signal.SA_RESTART());
             if (signalResult == Signal.SIG_ERR()) {
                 throw VMError.shouldNotReachHere(String.format("IgnoreSignalsStartupHook: Could not install signal: %s", signal));
             }

substratevm/src/com.oracle.svm.core.posix/src/com/oracle/svm/core/posix/headers/Signal.java

@@ -76,9 +76,9 @@ public interface sigset_tPointer extends PointerBase {
     }
 
     /**
-     * Warning: use {@link #sigaction} or {@link PosixUtils#installSignalHandler}. Do NOT introduce
-     * calls to {@code signal} or {@code sigset}, which are not portable, and when running in
-     * HotSpot, signal chaining (libjsig) will print warnings.
+     * Warning: use {@link PosixUtils#installSignalHandler}. Do NOT introduce calls to
+     * {@code signal} or {@code sigset}, which are not portable, and when running in HotSpot, signal
+     * chaining (libjsig) will print warnings.
      */
     public interface SignalDispatcher extends CFunctionPointer {
         @InvokeCFunctionPointer
@@ -179,7 +179,7 @@ public interface sigaction extends PointerBase {
         sigset_tPointer sa_mask();
     }
 
-    /** @param signum from {@link SignalEnum#getCValue()} */
+    /** Don't call this function directly, see {@link PosixUtils#sigaction}. */
     @CFunction
     public static native int sigaction(int signum, sigaction act, sigaction oldact);
 

substratevm/src/com.oracle.svm.core.windows/src/com/oracle/svm/core/windows/Target_jdk_internal_misc_Signal.java

@@ -59,8 +59,8 @@ final class Target_jdk_internal_misc_Signal {
 
     @Substitute
     private static long handle0(int sig, long nativeH) {
-        if (!SubstrateOptions.EnableSignalAPI.getValue()) {
-            throw new IllegalArgumentException("Installing signal handlers is not enabled");
+        if (!SubstrateOptions.EnableSignalHandling.getValue()) {
+            throw new IllegalArgumentException("Signal handlers can't be installed if signal handling is disabled, see option '" + SubstrateOptions.EnableSignalHandling.getName() + "'.");
         }
         if (!Isolates.isCurrentFirst()) {
             throw new IllegalArgumentException("Only the first isolate can install signal handlers, as signal handling is global for the process.");

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

@@ -46,7 +46,7 @@ public class DumpHeapOnSignalFeature implements InternalFeature {
 
     @Override
     public boolean isInConfiguration(IsInConfigurationAccess access) {
-        return VMInspectionOptions.hasHeapDumpSupport() && SubstrateOptions.EnableSignalAPI.getValue();
+        return VMInspectionOptions.hasHeapDumpSupport();
     }
 
     @Override
@@ -58,7 +58,7 @@ public void beforeAnalysis(BeforeAnalysisAccess access) {
 final class DumpHeapStartupHook implements RuntimeSupport.Hook {
     @Override
     public void execute(boolean isFirstIsolate) {
-        if (isFirstIsolate) {
+        if (isFirstIsolate && SubstrateOptions.EnableSignalHandling.getValue()) {
             DumpHeapReport.install();
         }
     }

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

@@ -63,10 +63,10 @@ public class IsolateArgumentParser {
     private static final CGlobalData<CLongPointer> HOSTED_VALUES = CGlobalDataFactory.createBytes(IsolateArgumentParser::createHostedValues);
     private static final long[] PARSED_OPTION_VALUES = new long[OPTION_COUNT];
 
-    private static final int K = 1024;
-    private static final int M = K * K;
-    private static final int G = K * M;
-    private static final int T = K * G;
+    private static final long K = 1024;
+    private static final long M = K * K;
+    private static final long G = K * M;
+    private static final long T = K * G;
 
     @Platforms(Platform.HOSTED_ONLY.class)
     public IsolateArgumentParser() {
@@ -346,7 +346,7 @@ private static boolean atojulong(CCharPointer s, CLongPointer result) {
             return false;
         }
 
-        int modifier;
+        long modifier;
         switch (tail.read()) {
             case 'T':
             case 't':
@@ -371,7 +371,7 @@ private static boolean atojulong(CCharPointer s, CLongPointer result) {
                 return false;
         }
 
-        UnsignedWord value = n.multiply(modifier);
+        UnsignedWord value = n.multiply(WordFactory.unsigned(modifier));
         if (checkForOverflow(value, n, modifier)) {
             return false;
         }

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

@@ -652,6 +652,7 @@ private static void validateGenerateDebugInfo(HostedOptionKey<Integer> optionKey
     @Option(help = "Control debug information output: 0 - no debuginfo, 1 - AOT code debuginfo, 2 - AOT and runtime code debuginfo (runtime code support only with -H:+UseOldDebugInfo).", //
                     deprecated = true, deprecationMessage = "Please use the -g option.")//
     public static final HostedOptionKey<Integer> Debug = new HostedOptionKey<>(0) {
+        @Override
         public void update(EconomicMap<OptionKey<?>, Object> values, Object newValue) {
             GenerateDebugInfo.update(values, newValue);
         }
@@ -788,24 +789,8 @@ protected void onValueUpdate(EconomicMap<OptionKey<?>, Object> values, Integer o
     @Option(help = "For internal purposes only. Disables type id result verification even when running with assertions enabled.", stability = OptionStability.EXPERIMENTAL, type = OptionType.Debug)//
     public static final HostedOptionKey<Boolean> DisableTypeIdResultVerification = new HostedOptionKey<>(true);
 
-    @Option(help = "Enables the signal API (sun.misc.Signal or jdk.internal.misc.Signal). Defaults to false for shared library and true for executables", stability = OptionStability.EXPERIMENTAL, type = Expert)//
-    public static final HostedOptionKey<Boolean> EnableSignalAPI = new HostedOptionKey<>(null) {
-        @Override
-        public Boolean getValueOrDefault(UnmodifiableEconomicMap<OptionKey<?>, Object> values) {
-            if (values.containsKey(this)) {
-                return (Boolean) values.get(this);
-            }
-            return !SharedLibrary.getValueOrDefault(values);
-        }
-
-        @Override
-        public Boolean getValue(OptionValues values) {
-            return getValueOrDefault(values.getMap());
-        }
-    };
-
     @Option(help = "Enables signal handling", stability = OptionStability.EXPERIMENTAL, type = Expert)//
-    public static final RuntimeOptionKey<Boolean> EnableSignalHandling = new RuntimeOptionKey<>(null) {
+    public static final RuntimeOptionKey<Boolean> EnableSignalHandling = new RuntimeOptionKey<>(null, Immutable) {
         @Override
         public Boolean getValueOrDefault(UnmodifiableEconomicMap<OptionKey<?>, Object> values) {
             if (values.containsKey(this)) {
@@ -824,7 +809,7 @@ public Boolean getValue(OptionValues values) {
     public static final RuntimeOptionKey<String> HeapDumpPath = new RuntimeOptionKey<>("", Immutable);
 
     /* Utility method that follows the `-XX:HeapDumpPath` behavior of the JVM. */
-    public static final String getHeapDumpPath(String defaultFilename) {
+    public static String getHeapDumpPath(String defaultFilename) {
         String heapDumpFilenameOrDirectory = HeapDumpPath.getValue();
         if (heapDumpFilenameOrDirectory.isEmpty()) {
             return defaultFilename;