From da02ef8607ab22ce6efe81ca2426c63a25daf532 Mon Sep 17 00:00:00 2001 From: Christian Haeubl Date: Fri, 5 May 2023 15:04:28 +0200 Subject: [PATCH 1/4] Use PosixUtils.installSignalHandler() instead of Signal.sigaction(). Remove option EnableSignalAPI. --- sdk/mx.sdk/mx_sdk_vm_impl.py | 1 - .../posix/PosixSubstrateSegfaultHandler.java | 23 +------- .../posix/PosixSubstrateSigprofHandler.java | 20 +------ .../com/oracle/svm/core/posix/PosixUtils.java | 59 +++++++++++++++---- .../svm/core/posix/SunMiscSubstitutions.java | 11 ++-- .../oracle/svm/core/posix/headers/Signal.java | 8 +-- .../Target_jdk_internal_misc_Signal.java | 4 +- .../svm/core/DumpHeapOnSignalFeature.java | 4 +- .../com/oracle/svm/core/SubstrateOptions.java | 19 +----- .../oracle/svm/core/VMInspectionOptions.java | 12 +--- 10 files changed, 68 insertions(+), 93 deletions(-) diff --git a/sdk/mx.sdk/mx_sdk_vm_impl.py b/sdk/mx.sdk/mx_sdk_vm_impl.py index e37f603d5d03..9659e1e24660 100644 --- a/sdk/mx.sdk/mx_sdk_vm_impl.py +++ b/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] diff --git a/substratevm/src/com.oracle.svm.core.posix/src/com/oracle/svm/core/posix/PosixSubstrateSegfaultHandler.java b/substratevm/src/com.oracle.svm.core.posix/src/com/oracle/svm/core/posix/PosixSubstrateSegfaultHandler.java index 73ba9bf4e9a6..7f34cb65847d 100644 --- a/substratevm/src/com.oracle.svm.core.posix/src/com/oracle/svm/core/posix/PosixSubstrateSegfaultHandler.java +++ b/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()); } } 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 4b1c28a25e77..bc8cea77af81 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 @@ -36,7 +36,6 @@ 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; @@ -44,13 +43,11 @@ 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 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 diff --git a/substratevm/src/com.oracle.svm.core.posix/src/com/oracle/svm/core/posix/PosixUtils.java b/substratevm/src/com.oracle.svm.core.posix/src/com/oracle/svm/core/posix/PosixUtils.java index 66c87249d3f9..918bbc8b2031 100644 --- a/substratevm/src/com.oracle.svm.core.posix/src/com/oracle/svm/core/posix/PosixUtils.java +++ b/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."); + + 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 diff --git a/substratevm/src/com.oracle.svm.core.posix/src/com/oracle/svm/core/posix/SunMiscSubstitutions.java b/substratevm/src/com.oracle.svm.core.posix/src/com/oracle/svm/core/posix/SunMiscSubstitutions.java index 140b3f34fdbd..4c90f4c357cb 100644 --- a/substratevm/src/com.oracle.svm.core.posix/src/com/oracle/svm/core/posix/SunMiscSubstitutions.java +++ b/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)); } diff --git a/substratevm/src/com.oracle.svm.core.posix/src/com/oracle/svm/core/posix/headers/Signal.java b/substratevm/src/com.oracle.svm.core.posix/src/com/oracle/svm/core/posix/headers/Signal.java index 404a228892f8..59accc941d08 100644 --- a/substratevm/src/com.oracle.svm.core.posix/src/com/oracle/svm/core/posix/headers/Signal.java +++ b/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); diff --git a/substratevm/src/com.oracle.svm.core.windows/src/com/oracle/svm/core/windows/Target_jdk_internal_misc_Signal.java b/substratevm/src/com.oracle.svm.core.windows/src/com/oracle/svm/core/windows/Target_jdk_internal_misc_Signal.java index 7ccfa2927849..225da4d99506 100644 --- a/substratevm/src/com.oracle.svm.core.windows/src/com/oracle/svm/core/windows/Target_jdk_internal_misc_Signal.java +++ b/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."); diff --git a/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/DumpHeapOnSignalFeature.java b/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/DumpHeapOnSignalFeature.java index 0eb2753f1445..30d293c3704c 100644 --- a/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/DumpHeapOnSignalFeature.java +++ b/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(); } } diff --git a/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/SubstrateOptions.java b/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/SubstrateOptions.java index 337d103453ce..6ae59829f964 100644 --- a/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/SubstrateOptions.java +++ b/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/SubstrateOptions.java @@ -652,6 +652,7 @@ private static void validateGenerateDebugInfo(HostedOptionKey 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 Debug = new HostedOptionKey<>(0) { + @Override public void update(EconomicMap, Object> values, Object newValue) { GenerateDebugInfo.update(values, newValue); } @@ -788,22 +789,6 @@ protected void onValueUpdate(EconomicMap, 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 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 EnableSignalAPI = new HostedOptionKey<>(null) { - @Override - public Boolean getValueOrDefault(UnmodifiableEconomicMap, 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 EnableSignalHandling = new RuntimeOptionKey<>(null) { @Override @@ -824,7 +809,7 @@ public Boolean getValue(OptionValues values) { public static final RuntimeOptionKey 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; diff --git a/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/VMInspectionOptions.java b/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/VMInspectionOptions.java index a249ed5fcbf6..7973a3c39126 100644 --- a/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/VMInspectionOptions.java +++ b/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/VMInspectionOptions.java @@ -129,22 +129,16 @@ public static boolean hasJmxClientSupport() { } @Option(help = "Dumps all runtime compiled methods on SIGUSR2.", type = OptionType.User) // - public static final HostedOptionKey DumpRuntimeCompilationOnSignal = new HostedOptionKey<>(false, VMInspectionOptions::validateOnSignalOption); + public static final HostedOptionKey DumpRuntimeCompilationOnSignal = new HostedOptionKey<>(false); @Option(help = "Dumps all thread stacktraces on SIGQUIT/SIGBREAK.", type = OptionType.User) // - public static final HostedOptionKey DumpThreadStacksOnSignal = new HostedOptionKey<>(false, VMInspectionOptions::validateOnSignalOption); - - private static void validateOnSignalOption(HostedOptionKey optionKey) { - if (optionKey.getValue() && !SubstrateOptions.EnableSignalAPI.getValue()) { - throw UserError.abort("The option %s requires the Signal API, but the Signal API is disabled. Please enable with `-H:+%s`.", - optionKey.getName(), SubstrateOptions.EnableSignalAPI.getName()); - } - } + public static final HostedOptionKey DumpThreadStacksOnSignal = new HostedOptionKey<>(false); static class DeprecatedOptions { @Option(help = "Enables features that allow the VM to be inspected during run time.", type = OptionType.User, // deprecated = true, deprecationMessage = "Please use --" + ENABLE_MONITORING_OPTION) // static final HostedOptionKey AllowVMInspection = new HostedOptionKey<>(false) { + @Override protected void onValueUpdate(EconomicMap, Object> values, Boolean oldValue, Boolean newValue) { EnableMonitoringFeatures.update(values, newValue ? "all" : ""); DumpRuntimeCompilationOnSignal.update(values, true); From 5bb0493c9b8e6d19592f8b63ddb34bdcbaa33a0c Mon Sep 17 00:00:00 2001 From: Christian Haeubl Date: Tue, 2 May 2023 17:48:13 +0200 Subject: [PATCH 2/4] Fix an overflow. --- .../com/oracle/svm/core/IsolateArgumentParser.java | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/IsolateArgumentParser.java b/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/IsolateArgumentParser.java index 73d52698a480..65b3ae40c224 100644 --- a/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/IsolateArgumentParser.java +++ b/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/IsolateArgumentParser.java @@ -63,10 +63,10 @@ public class IsolateArgumentParser { private static final CGlobalData 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; } From 798dd0822a2d0cad8c292b93fc6a104ac65da285 Mon Sep 17 00:00:00 2001 From: Christian Haeubl Date: Fri, 5 May 2023 16:59:07 +0200 Subject: [PATCH 3/4] Disallow @Uninterruptible(calleeMustBe = true) if there isn't a proper reason used for documentation. --- .../svm/hosted/code/UninterruptibleAnnotationChecker.java | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/code/UninterruptibleAnnotationChecker.java b/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/code/UninterruptibleAnnotationChecker.java index 15e58be1f46f..4d26a75781ec 100644 --- a/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/code/UninterruptibleAnnotationChecker.java +++ b/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/code/UninterruptibleAnnotationChecker.java @@ -127,6 +127,11 @@ private void checkSpecifiedOptions(HostedMethod method, Uninterruptible annotati violations.add("Method " + method.format("%H.%n(%p)") + " uses an unspecific reason but is annotated with 'callerMustBe = true'. Please document in the reason why the callers need to be uninterruptible."); } + + if (!annotation.calleeMustBe()) { + violations.add("Method " + method.format("%H.%n(%p)") + + " uses an unspecific reason but is annotated with 'calleeMustBe = false'. Please document in the reason why it is safe to execute interruptible code."); + } } else if (isSimilarToUnspecificReason(annotation.reason())) { violations.add("Method " + method.format("%H.%n(%p)") + " uses a reason that is similar to the unspecific reason '" + Uninterruptible.CALLED_FROM_UNINTERRUPTIBLE_CODE + "'. " + "If the method has an inherent reason for being uninterruptible, besides being called from uninterruptible code, then please improve the reason. " + From aeb56809f71d4ab89577358530e7f1cebe65de1a Mon Sep 17 00:00:00 2001 From: Christian Haeubl Date: Fri, 12 May 2023 14:33:38 +0200 Subject: [PATCH 4/4] Mark the option EnableSignalHandling as immutable. Add documentation to RuntimeOptionKeyFlag. --- .../com/oracle/svm/core/posix/PosixUtils.java | 2 +- .../com/oracle/svm/core/SubstrateOptions.java | 2 +- .../com/oracle/svm/core/jdk/RuntimeSupport.java | 2 +- .../oracle/svm/core/option/RuntimeOptionKey.java | 16 ++++++++++++---- 4 files changed, 15 insertions(+), 7 deletions(-) diff --git a/substratevm/src/com.oracle.svm.core.posix/src/com/oracle/svm/core/posix/PosixUtils.java b/substratevm/src/com.oracle.svm.core.posix/src/com/oracle/svm/core/posix/PosixUtils.java index 918bbc8b2031..07d21a57f7cc 100644 --- a/substratevm/src/com.oracle.svm.core.posix/src/com/oracle/svm/core/posix/PosixUtils.java +++ b/substratevm/src/com.oracle.svm.core.posix/src/com/oracle/svm/core/posix/PosixUtils.java @@ -302,7 +302,7 @@ public static void installSignalHandler(int signum, Signal.AdvancedSignalDispatc * 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."); + VMError.guarantee(SubstrateOptions.EnableSignalHandling.getValue(), "Trying to install a signal handler while signal handling is disabled."); if (VMOperation.isInProgress()) { /* diff --git a/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/SubstrateOptions.java b/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/SubstrateOptions.java index 6ae59829f964..4b50cb8ebb0f 100644 --- a/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/SubstrateOptions.java +++ b/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/SubstrateOptions.java @@ -790,7 +790,7 @@ protected void onValueUpdate(EconomicMap, Object> values, Integer o public static final HostedOptionKey DisableTypeIdResultVerification = new HostedOptionKey<>(true); @Option(help = "Enables signal handling", stability = OptionStability.EXPERIMENTAL, type = Expert)// - public static final RuntimeOptionKey EnableSignalHandling = new RuntimeOptionKey<>(null) { + public static final RuntimeOptionKey EnableSignalHandling = new RuntimeOptionKey<>(null, Immutable) { @Override public Boolean getValueOrDefault(UnmodifiableEconomicMap, Object> values) { if (values.containsKey(this)) { diff --git a/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/jdk/RuntimeSupport.java b/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/jdk/RuntimeSupport.java index 10a81b3ffc48..a128ea048f90 100644 --- a/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/jdk/RuntimeSupport.java +++ b/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/jdk/RuntimeSupport.java @@ -104,7 +104,7 @@ public void initialize() { /** * Adds a hook which will execute during the shutdown process. Note it is possible for the - * {@link #shutdownHooks} to called without the {@link #startupHooks} executing first. + * {@link #shutdownHooks} to be called without the {@link #startupHooks} executing first. */ @Platforms(Platform.HOSTED_ONLY.class) public void addShutdownHook(Hook hook) { diff --git a/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/option/RuntimeOptionKey.java b/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/option/RuntimeOptionKey.java index dee7c4f5153b..e92e493ef6cc 100644 --- a/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/option/RuntimeOptionKey.java +++ b/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/option/RuntimeOptionKey.java @@ -24,8 +24,9 @@ */ package com.oracle.svm.core.option; -import com.oracle.svm.core.SubstrateUtil; -import com.oracle.svm.core.jdk.RuntimeSupport; +import java.util.Objects; +import java.util.function.Consumer; + import org.graalvm.collections.EconomicMap; import org.graalvm.compiler.api.replacements.Fold; import org.graalvm.compiler.options.Option; @@ -34,8 +35,8 @@ import org.graalvm.nativeimage.Platform; import org.graalvm.nativeimage.Platforms; -import java.util.Objects; -import java.util.function.Consumer; +import com.oracle.svm.core.SubstrateUtil; +import com.oracle.svm.core.jdk.RuntimeSupport; /** * Defines a runtime {@link Option}, in contrast to a {@link HostedOptionKey hosted option}. @@ -133,7 +134,14 @@ private static int computeFlags(RuntimeOptionKeyFlag[] flags) { } public enum RuntimeOptionKeyFlag { + /** If this flag is set, then option value is propagated to all compilation isolates. */ RelevantForCompilationIsolates, + + /** + * If this flag is set, then the option value can only be changed during startup, i.e., + * before the startup hooks are executed (see {@link RuntimeSupport#initialize()}). This + * flag should be used for runtime options that are accessed in startup hooks. + */ Immutable, } }