Skip to content

Commit 2bd60f0

Browse files
christianhaeublchristianhaeubl
committed
[GR-45994] Signal-handler related simplifications.
PullRequest: graal/14509
2 parents dbdcce5 + aeb5680 commit 2bd60f0

File tree

14 files changed

+93
-105
lines changed

14 files changed

+93
-105
lines changed

‎sdk/mx.sdk/mx_sdk_vm_impl.py‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1330,7 +1330,6 @@ def contents(self):
13301330
build_args += ['--language:' + image_config.language, '--tool:all']
13311331

13321332
if isinstance(image_config, mx_sdk.LanguageLibraryConfig):
1333-
build_args += ['-H:+EnableSignalAPI']
13341333
if image_config.main_class:
13351334
build_args += ['-Dorg.graalvm.launcher.class=' + image_config.main_class]
13361335

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

Lines changed: 2 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -27,26 +27,20 @@
2727
import org.graalvm.nativeimage.c.function.CEntryPoint;
2828
import org.graalvm.nativeimage.c.function.CEntryPoint.Publish;
2929
import org.graalvm.nativeimage.c.function.CEntryPointLiteral;
30-
import org.graalvm.nativeimage.c.struct.SizeOf;
3130
import org.graalvm.nativeimage.c.type.VoidPointer;
3231
import org.graalvm.word.PointerBase;
33-
import org.graalvm.word.WordFactory;
3432

35-
import com.oracle.svm.core.SubstrateOptions;
3633
import com.oracle.svm.core.SubstrateSegfaultHandler;
3734
import com.oracle.svm.core.Uninterruptible;
3835
import com.oracle.svm.core.c.function.CEntryPointOptions;
3936
import com.oracle.svm.core.c.function.CEntryPointOptions.NoEpilogue;
4037
import com.oracle.svm.core.c.function.CEntryPointOptions.NoPrologue;
4138
import com.oracle.svm.core.feature.AutomaticallyRegisteredImageSingleton;
42-
import com.oracle.svm.core.graal.stackvalue.UnsafeStackValue;
43-
import com.oracle.svm.core.headers.LibC;
4439
import com.oracle.svm.core.heap.RestrictHeapAccess;
4540
import com.oracle.svm.core.log.Log;
4641
import com.oracle.svm.core.os.MemoryProtectionProvider;
4742
import com.oracle.svm.core.posix.headers.Signal;
4843
import com.oracle.svm.core.posix.headers.Signal.AdvancedSignalDispatcher;
49-
import com.oracle.svm.core.posix.headers.Signal.sigaction;
5044
import com.oracle.svm.core.posix.headers.Signal.siginfo_t;
5145
import com.oracle.svm.core.posix.headers.Signal.ucontext_t;
5246
import com.oracle.svm.core.util.VMError;
@@ -92,20 +86,7 @@ protected void printSignalInfo(Log log, PointerBase signalInfo) {
9286

9387
@Override
9488
protected void installInternal() {
95-
VMError.guarantee(SubstrateOptions.EnableSignalHandling.getValue(), "Trying to install a signal handler while signal handling is disabled.");
96-
int structSigActionSize = SizeOf.get(sigaction.class);
97-
sigaction structSigAction = UnsafeStackValue.get(structSigActionSize);
98-
LibC.memset(structSigAction, WordFactory.signed(0), WordFactory.unsigned(structSigActionSize));
99-
/* Register sa_sigaction signal handler */
100-
structSigAction.sa_flags(Signal.SA_SIGINFO() | Signal.SA_NODEFER());
101-
structSigAction.sa_sigaction(advancedSignalDispatcher.getFunctionPointer());
102-
synchronized (Target_jdk_internal_misc_Signal.class) {
103-
/*
104-
* Don't want to race with logic within Util_jdk_internal_misc_Signal#handle0 which
105-
* reads these signals.
106-
*/
107-
Signal.sigaction(Signal.SignalEnum.SIGSEGV.getCValue(), structSigAction, WordFactory.nullPointer());
108-
Signal.sigaction(Signal.SignalEnum.SIGBUS.getCValue(), structSigAction, WordFactory.nullPointer());
109-
}
89+
PosixUtils.installSignalHandler(Signal.SignalEnum.SIGSEGV, advancedSignalDispatcher.getFunctionPointer(), Signal.SA_NODEFER());
90+
PosixUtils.installSignalHandler(Signal.SignalEnum.SIGBUS, advancedSignalDispatcher.getFunctionPointer(), Signal.SA_NODEFER());
11091
}
11192
}

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

Lines changed: 1 addition & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -36,21 +36,18 @@
3636
import org.graalvm.nativeimage.c.function.CEntryPoint;
3737
import org.graalvm.nativeimage.c.function.CEntryPointLiteral;
3838
import org.graalvm.nativeimage.c.function.CodePointer;
39-
import org.graalvm.nativeimage.c.struct.SizeOf;
4039
import org.graalvm.nativeimage.hosted.Feature;
4140
import org.graalvm.word.Pointer;
4241
import org.graalvm.word.WordFactory;
4342

4443
import com.oracle.svm.core.IsolateListenerSupport;
4544
import com.oracle.svm.core.IsolateListenerSupportFeature;
4645
import com.oracle.svm.core.RegisterDumper;
47-
import com.oracle.svm.core.SubstrateOptions;
4846
import com.oracle.svm.core.Uninterruptible;
4947
import com.oracle.svm.core.c.function.CEntryPointOptions;
5048
import com.oracle.svm.core.feature.AutomaticallyRegisteredFeature;
5149
import com.oracle.svm.core.feature.InternalFeature;
5250
import com.oracle.svm.core.graal.stackvalue.UnsafeStackValue;
53-
import com.oracle.svm.core.headers.LibC;
5451
import com.oracle.svm.core.heap.RestrictHeapAccess;
5552
import com.oracle.svm.core.jfr.JfrFeature;
5653
import com.oracle.svm.core.jfr.sampler.JfrExecutionSampler;
@@ -60,7 +57,6 @@
6057
import com.oracle.svm.core.sampler.SubstrateSigprofHandler;
6158
import com.oracle.svm.core.thread.ThreadListenerSupport;
6259
import com.oracle.svm.core.util.TimeUtils;
63-
import com.oracle.svm.core.util.VMError;
6460

6561
public class PosixSubstrateSigprofHandler extends SubstrateSigprofHandler {
6662
private static final CEntryPointLiteral<Signal.AdvancedSignalDispatcher> advancedSignalDispatcher = CEntryPointLiteral.create(PosixSubstrateSigprofHandler.class,
@@ -85,21 +81,7 @@ private static void dispatch(@SuppressWarnings("unused") int signalNumber, @Supp
8581
}
8682

8783
private static void registerSigprofSignal(Signal.AdvancedSignalDispatcher dispatcher) {
88-
VMError.guarantee(SubstrateOptions.EnableSignalHandling.getValue(), "Trying to install a signal handler while signal handling is disabled.");
89-
int structSigActionSize = SizeOf.get(Signal.sigaction.class);
90-
Signal.sigaction structSigAction = UnsafeStackValue.get(structSigActionSize);
91-
LibC.memset(structSigAction, WordFactory.signed(0), WordFactory.unsigned(structSigActionSize));
92-
93-
/* Register sa_sigaction signal handler */
94-
structSigAction.sa_flags(Signal.SA_SIGINFO() | Signal.SA_NODEFER() | Signal.SA_RESTART());
95-
structSigAction.sa_sigaction(dispatcher);
96-
/*
97-
* Note this can race with other signals being installed. However, using Java
98-
* synchronization is disallowed within a VMOperation. If race-free execution becomes
99-
* necessary, then a VMMutex will be needed and additional code will need to be
100-
* made @Uniterruptible so that a thread owning the VMMutex cannot block at a safepoint.
101-
*/
102-
Signal.sigaction(Signal.SignalEnum.SIGPROF.getCValue(), structSigAction, WordFactory.nullPointer());
84+
PosixUtils.installSignalHandler(Signal.SignalEnum.SIGPROF, dispatcher, Signal.SA_NODEFER() | Signal.SA_RESTART());
10385
}
10486

10587
@Override

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

Lines changed: 48 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,7 @@
5656
import com.oracle.svm.core.posix.headers.Wait;
5757
import com.oracle.svm.core.posix.headers.darwin.DarwinTime;
5858
import com.oracle.svm.core.posix.headers.linux.LinuxTime;
59+
import com.oracle.svm.core.thread.VMOperation;
5960
import com.oracle.svm.core.util.VMError;
6061

6162
public class PosixUtils {
@@ -251,6 +252,10 @@ public static int readBytes(int fd, CCharPointer buffer, int bufferLen, int read
251252
return readBytes;
252253
}
253254

255+
public static Signal.SignalDispatcher installSignalHandler(Signal.SignalEnum signum, Signal.SignalDispatcher handler, int flags) {
256+
return installSignalHandler(signum.getCValue(), handler, flags);
257+
}
258+
254259
/**
255260
* Emulates the deprecated {@code signal} function via its replacement {@code sigaction},
256261
* assuming BSD semantics (like glibc does, for example).
@@ -261,26 +266,58 @@ public static int readBytes(int fd, CCharPointer buffer, int bufferLen, int read
261266
* Note that this method should not be called from an initialization hook:
262267
* {@code EnableSignalHandling} may not be set correctly at the time initialization hooks run.
263268
*/
264-
public static Signal.SignalDispatcher installSignalHandler(int signum, Signal.SignalDispatcher handler) {
265-
synchronized (Target_jdk_internal_misc_Signal.class) {
266-
return installSignalHandler0(signum, handler);
269+
public static Signal.SignalDispatcher installSignalHandler(int signum, Signal.SignalDispatcher handler, int flags) {
270+
int structSigActionSize = SizeOf.get(Signal.sigaction.class);
271+
Signal.sigaction act = UnsafeStackValue.get(structSigActionSize);
272+
LibC.memset(act, WordFactory.signed(0), WordFactory.unsigned(structSigActionSize));
273+
act.sa_flags(flags);
274+
act.sa_handler(handler);
275+
276+
Signal.sigaction old = UnsafeStackValue.get(Signal.sigaction.class);
277+
278+
int result = sigaction(signum, act, old);
279+
if (result != 0) {
280+
return Signal.SIG_ERR();
267281
}
282+
return old.sa_handler();
268283
}
269284

270-
private static Signal.SignalDispatcher installSignalHandler0(int signum, Signal.SignalDispatcher handler) {
271-
VMError.guarantee(SubstrateOptions.EnableSignalHandling.getValue(), "Trying to install a signal handler while signal handling is disabled.");
272-
Signal.sigaction old = UnsafeStackValue.get(Signal.sigaction.class);
285+
public static void installSignalHandler(Signal.SignalEnum signum, Signal.AdvancedSignalDispatcher handler, int flags) {
286+
installSignalHandler(signum.getCValue(), handler, flags);
287+
}
273288

289+
public static void installSignalHandler(int signum, Signal.AdvancedSignalDispatcher handler, int flags) {
274290
int structSigActionSize = SizeOf.get(Signal.sigaction.class);
275291
Signal.sigaction act = UnsafeStackValue.get(structSigActionSize);
276292
LibC.memset(act, WordFactory.signed(0), WordFactory.unsigned(structSigActionSize));
293+
act.sa_flags(Signal.SA_SIGINFO() | flags);
294+
act.sa_sigaction(handler);
277295

278-
act.sa_flags(Signal.SA_RESTART());
279-
act.sa_handler(handler);
280-
if (Signal.sigaction(signum, act, old) != 0) {
281-
return Signal.SIG_ERR();
296+
int result = sigaction(signum, act, WordFactory.nullPointer());
297+
PosixUtils.checkStatusIs0(result, "sigaction failed in installSignalHandler().");
298+
}
299+
300+
/*
301+
* Avoid races with logic within Util_jdk_internal_misc_Signal#handle0 which reads these
302+
* signals.
303+
*/
304+
private static int sigaction(int signum, Signal.sigaction structSigAction, Signal.sigaction old) {
305+
VMError.guarantee(SubstrateOptions.EnableSignalHandling.getValue(), "Trying to install a signal handler while signal handling is disabled.");
306+
307+
if (VMOperation.isInProgress()) {
308+
/*
309+
* Note this can race with other signals being installed. However, using Java
310+
* synchronization is disallowed within a VMOperation. If race-free execution becomes
311+
* necessary, then a VMMutex will be needed and additional code will need to be
312+
* made @Uninterruptible so that a thread owning the VMMutex cannot block at a
313+
* safepoint.
314+
*/
315+
return Signal.sigaction(signum, structSigAction, old);
316+
} else {
317+
synchronized (Target_jdk_internal_misc_Signal.class) {
318+
return Signal.sigaction(signum, structSigAction, old);
319+
}
282320
}
283-
return old.sa_handler();
284321
}
285322

286323
// Checkstyle: stop

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

Lines changed: 4 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -67,8 +67,8 @@ private static int findSignal0(String signalName) {
6767

6868
@Substitute
6969
private static long handle0(int sig, long nativeH) {
70-
if (!SubstrateOptions.EnableSignalAPI.getValue()) {
71-
throw new IllegalArgumentException("Installing signal handlers is not enabled");
70+
if (!SubstrateOptions.EnableSignalHandling.getValue()) {
71+
throw new IllegalArgumentException("Signal handlers can't be installed if signal handling is disabled, see option '" + SubstrateOptions.EnableSignalHandling.getName() + "'.");
7272
}
7373
return Util_jdk_internal_misc_Signal.handle0(sig, nativeH);
7474
}
@@ -129,9 +129,6 @@ static boolean isCurrentDispatcher(int sig, SignalDispatcher dispatcher) {
129129
* is called from within a static synchronized call to ensure race-free execution.
130130
*/
131131
static long handle0(int sig, long nativeH) {
132-
if (!SubstrateOptions.EnableSignalHandling.getValue()) {
133-
return sunMiscSignalIgnoreHandler;
134-
}
135132
ensureInitialized();
136133
final Signal.SignalDispatcher newDispatcher = nativeHToDispatcher(nativeH);
137134
/* 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) {
157154
}
158155

159156
updateDispatcher(sig, newDispatcher);
160-
final Signal.SignalDispatcher oldDispatcher = PosixUtils.installSignalHandler(sig, newDispatcher);
157+
final Signal.SignalDispatcher oldDispatcher = PosixUtils.installSignalHandler(sig, newDispatcher, Signal.SA_RESTART());
161158
CIntPointer sigset = UnsafeStackValue.get(CIntPointer.class);
162159
sigset.write(1 << (sig - 1));
163160
Signal.sigprocmask(Signal.SIG_UNBLOCK(), (Signal.sigset_tPointer) sigset, WordFactory.nullPointer());
@@ -455,7 +452,7 @@ private static void installNoopHandler(Signal.SignalEnum signal) {
455452
/*
456453
* Replace with no-op signal handler if a custom one has not already been installed.
457454
*/
458-
final SignalDispatcher signalResult = PosixUtils.installSignalHandler(signum, NOOP_SIGNAL_HANDLER.getFunctionPointer());
455+
final SignalDispatcher signalResult = PosixUtils.installSignalHandler(signum, NOOP_SIGNAL_HANDLER.getFunctionPointer(), Signal.SA_RESTART());
459456
if (signalResult == Signal.SIG_ERR()) {
460457
throw VMError.shouldNotReachHere(String.format("IgnoreSignalsStartupHook: Could not install signal: %s", signal));
461458
}

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

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -76,9 +76,9 @@ public interface sigset_tPointer extends PointerBase {
7676
}
7777

7878
/**
79-
* Warning: use {@link #sigaction} or {@link PosixUtils#installSignalHandler}. Do NOT introduce
80-
* calls to {@code signal} or {@code sigset}, which are not portable, and when running in
81-
* HotSpot, signal chaining (libjsig) will print warnings.
79+
* Warning: use {@link PosixUtils#installSignalHandler}. Do NOT introduce calls to
80+
* {@code signal} or {@code sigset}, which are not portable, and when running in HotSpot, signal
81+
* chaining (libjsig) will print warnings.
8282
*/
8383
public interface SignalDispatcher extends CFunctionPointer {
8484
@InvokeCFunctionPointer
@@ -179,7 +179,7 @@ public interface sigaction extends PointerBase {
179179
sigset_tPointer sa_mask();
180180
}
181181

182-
/** @param signum from {@link SignalEnum#getCValue()} */
182+
/** Don't call this function directly, see {@link PosixUtils#sigaction}. */
183183
@CFunction
184184
public static native int sigaction(int signum, sigaction act, sigaction oldact);
185185

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -59,8 +59,8 @@ final class Target_jdk_internal_misc_Signal {
5959

6060
@Substitute
6161
private static long handle0(int sig, long nativeH) {
62-
if (!SubstrateOptions.EnableSignalAPI.getValue()) {
63-
throw new IllegalArgumentException("Installing signal handlers is not enabled");
62+
if (!SubstrateOptions.EnableSignalHandling.getValue()) {
63+
throw new IllegalArgumentException("Signal handlers can't be installed if signal handling is disabled, see option '" + SubstrateOptions.EnableSignalHandling.getName() + "'.");
6464
}
6565
if (!Isolates.isCurrentFirst()) {
6666
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‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,7 @@ public class DumpHeapOnSignalFeature implements InternalFeature {
4646

4747
@Override
4848
public boolean isInConfiguration(IsInConfigurationAccess access) {
49-
return VMInspectionOptions.hasHeapDumpSupport() && SubstrateOptions.EnableSignalAPI.getValue();
49+
return VMInspectionOptions.hasHeapDumpSupport();
5050
}
5151

5252
@Override
@@ -58,7 +58,7 @@ public void beforeAnalysis(BeforeAnalysisAccess access) {
5858
final class DumpHeapStartupHook implements RuntimeSupport.Hook {
5959
@Override
6060
public void execute(boolean isFirstIsolate) {
61-
if (isFirstIsolate) {
61+
if (isFirstIsolate && SubstrateOptions.EnableSignalHandling.getValue()) {
6262
DumpHeapReport.install();
6363
}
6464
}

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

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -63,10 +63,10 @@ public class IsolateArgumentParser {
6363
private static final CGlobalData<CLongPointer> HOSTED_VALUES = CGlobalDataFactory.createBytes(IsolateArgumentParser::createHostedValues);
6464
private static final long[] PARSED_OPTION_VALUES = new long[OPTION_COUNT];
6565

66-
private static final int K = 1024;
67-
private static final int M = K * K;
68-
private static final int G = K * M;
69-
private static final int T = K * G;
66+
private static final long K = 1024;
67+
private static final long M = K * K;
68+
private static final long G = K * M;
69+
private static final long T = K * G;
7070

7171
@Platforms(Platform.HOSTED_ONLY.class)
7272
public IsolateArgumentParser() {
@@ -346,7 +346,7 @@ private static boolean atojulong(CCharPointer s, CLongPointer result) {
346346
return false;
347347
}
348348

349-
int modifier;
349+
long modifier;
350350
switch (tail.read()) {
351351
case 'T':
352352
case 't':
@@ -371,7 +371,7 @@ private static boolean atojulong(CCharPointer s, CLongPointer result) {
371371
return false;
372372
}
373373

374-
UnsignedWord value = n.multiply(modifier);
374+
UnsignedWord value = n.multiply(WordFactory.unsigned(modifier));
375375
if (checkForOverflow(value, n, modifier)) {
376376
return false;
377377
}

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

Lines changed: 3 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -652,6 +652,7 @@ private static void validateGenerateDebugInfo(HostedOptionKey<Integer> optionKey
652652
@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).", //
653653
deprecated = true, deprecationMessage = "Please use the -g option.")//
654654
public static final HostedOptionKey<Integer> Debug = new HostedOptionKey<>(0) {
655+
@Override
655656
public void update(EconomicMap<OptionKey<?>, Object> values, Object newValue) {
656657
GenerateDebugInfo.update(values, newValue);
657658
}
@@ -788,24 +789,8 @@ protected void onValueUpdate(EconomicMap<OptionKey<?>, Object> values, Integer o
788789
@Option(help = "For internal purposes only. Disables type id result verification even when running with assertions enabled.", stability = OptionStability.EXPERIMENTAL, type = OptionType.Debug)//
789790
public static final HostedOptionKey<Boolean> DisableTypeIdResultVerification = new HostedOptionKey<>(true);
790791

791-
@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)//
792-
public static final HostedOptionKey<Boolean> EnableSignalAPI = new HostedOptionKey<>(null) {
793-
@Override
794-
public Boolean getValueOrDefault(UnmodifiableEconomicMap<OptionKey<?>, Object> values) {
795-
if (values.containsKey(this)) {
796-
return (Boolean) values.get(this);
797-
}
798-
return !SharedLibrary.getValueOrDefault(values);
799-
}
800-
801-
@Override
802-
public Boolean getValue(OptionValues values) {
803-
return getValueOrDefault(values.getMap());
804-
}
805-
};
806-
807792
@Option(help = "Enables signal handling", stability = OptionStability.EXPERIMENTAL, type = Expert)//
808-
public static final RuntimeOptionKey<Boolean> EnableSignalHandling = new RuntimeOptionKey<>(null) {
793+
public static final RuntimeOptionKey<Boolean> EnableSignalHandling = new RuntimeOptionKey<>(null, Immutable) {
809794
@Override
810795
public Boolean getValueOrDefault(UnmodifiableEconomicMap<OptionKey<?>, Object> values) {
811796
if (values.containsKey(this)) {
@@ -824,7 +809,7 @@ public Boolean getValue(OptionValues values) {
824809
public static final RuntimeOptionKey<String> HeapDumpPath = new RuntimeOptionKey<>("", Immutable);
825810

826811
/* Utility method that follows the `-XX:HeapDumpPath` behavior of the JVM. */
827-
public static final String getHeapDumpPath(String defaultFilename) {
812+
public static String getHeapDumpPath(String defaultFilename) {
828813
String heapDumpFilenameOrDirectory = HeapDumpPath.getValue();
829814
if (heapDumpFilenameOrDirectory.isEmpty()) {
830815
return defaultFilename;

0 commit comments

Comments
 (0)