From 0145ec8ffca6b1d9671085b2cedb18cf05b750a0 Mon Sep 17 00:00:00 2001 From: Fabio Niephaus Date: Thu, 19 Jan 2023 12:55:01 +0100 Subject: [PATCH 1/2] Improve `OutOfMemoryError` handling. - Use `-XX:+ExitOnOutOfMemoryError` for the builder, so that it fails on the first OOME - Show an actionable error message --- .../com/oracle/svm/core/util/ExitStatus.java | 6 ++++- .../com/oracle/svm/driver/NativeImage.java | 27 ++++++++++++++++--- 2 files changed, 29 insertions(+), 4 deletions(-) diff --git a/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/util/ExitStatus.java b/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/util/ExitStatus.java index d395895112dc..0b56b0d8e6ef 100644 --- a/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/util/ExitStatus.java +++ b/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/util/ExitStatus.java @@ -29,7 +29,11 @@ public enum ExitStatus { OK(0), BUILDER_ERROR(1), FALLBACK_IMAGE(2), - BUILDER_INTERRUPT_WITHOUT_REASON(3), + + // 3 used by `-XX:+ExitOnOutOfMemoryError` (see src/hotspot/share/utilities/debug.cpp) + OUT_OF_MEMORY(3), + + BUILDER_INTERRUPT_WITHOUT_REASON(4), DRIVER_ERROR(20), DRIVER_TO_BUILDER_ERROR(21), WATCHDOG_EXIT(30), diff --git a/substratevm/src/com.oracle.svm.driver/src/com/oracle/svm/driver/NativeImage.java b/substratevm/src/com.oracle.svm.driver/src/com/oracle/svm/driver/NativeImage.java index 1a00fa86eaed..c18ce54fafd7 100644 --- a/substratevm/src/com.oracle.svm.driver/src/com/oracle/svm/driver/NativeImage.java +++ b/substratevm/src/com.oracle.svm.driver/src/com/oracle/svm/driver/NativeImage.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2016, 2020, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2016, 2023, Oracle and/or its affiliates. All rights reserved. * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. * * This code is free software; you can redistribute it and/or modify it @@ -804,7 +804,11 @@ private void prepareImageBuildArgs() { if (!"0".equals(xmxVal)) { addImageBuilderJavaArgs(oXmx + xmxVal); } - /* Prevent JVM that runs the image builder to steal focus */ + + /* Let builder exit on first OutOfMemoryError. */ + addImageBuilderJavaArgs("-XX:+ExitOnOutOfMemoryError"); + + /* Prevent JVM that runs the image builder to steal focus. */ addImageBuilderJavaArgs("-Djava.awt.headless=true"); addImageBuilderJavaArgs("-Dorg.graalvm.version=" + graalvmVersion); addImageBuilderJavaArgs("-Dorg.graalvm.vendor=" + graalvmVendor); @@ -1515,7 +1519,7 @@ protected static void build(BuildConfiguration config, Function Date: Sun, 29 Jan 2023 20:05:45 -0800 Subject: [PATCH 2/2] Do not attempt to initialize classes with large array allocations at build time --- .../EarlyClassInitializerAnalysis.java | 31 ++++++++++++++++++ .../ProvenSafeClassInitializationSupport.java | 4 +++ ...estClassInitializationMustBeSafeEarly.java | 32 ++++++++++++++++++- 3 files changed, 66 insertions(+), 1 deletion(-) diff --git a/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/classinitialization/EarlyClassInitializerAnalysis.java b/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/classinitialization/EarlyClassInitializerAnalysis.java index 51bc43061517..557d78148236 100644 --- a/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/classinitialization/EarlyClassInitializerAnalysis.java +++ b/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/classinitialization/EarlyClassInitializerAnalysis.java @@ -51,12 +51,17 @@ import org.graalvm.compiler.nodes.graphbuilderconf.InlineInvokePlugin; import org.graalvm.compiler.nodes.graphbuilderconf.InvocationPlugins; import org.graalvm.compiler.nodes.java.AccessFieldNode; +import org.graalvm.compiler.nodes.java.NewArrayNode; +import org.graalvm.compiler.nodes.java.NewMultiArrayNode; import org.graalvm.compiler.options.OptionValues; import org.graalvm.compiler.phases.OptimisticOptimizations; import org.graalvm.compiler.phases.tiers.HighTierContext; import org.graalvm.compiler.phases.util.Providers; import org.graalvm.compiler.serviceprovider.JavaVersionUtil; +import org.graalvm.nativeimage.ImageSingletons; +import org.graalvm.nativeimage.impl.RuntimeClassInitializationSupport; +import com.oracle.graal.pointsto.infrastructure.OriginalClassProvider; import com.oracle.graal.pointsto.phases.NoClassInitializationPlugin; import com.oracle.graal.pointsto.util.GraalAccess; import com.oracle.svm.core.ParsingReason; @@ -68,6 +73,7 @@ import com.oracle.svm.hosted.snippets.ReflectionPlugins; import com.oracle.svm.hosted.snippets.SubstrateGraphBuilderPlugins; +import jdk.vm.ci.meta.JavaConstant; import jdk.vm.ci.meta.ResolvedJavaField; import jdk.vm.ci.meta.ResolvedJavaMethod; import jdk.vm.ci.meta.ResolvedJavaType; @@ -276,6 +282,31 @@ public void nodeAdded(Node node) { throw new ClassInitializerHasSideEffectsException("Access of thread-local value"); } else if (node instanceof UnsafeAccessNode) { throw VMError.shouldNotReachHere("Intrinsification of Unsafe methods is not enabled during bytecode parsing"); + + } else if (node instanceof NewArrayNode) { + checkArrayAllocationLength(((NewArrayNode) node).length()); + } else if (node instanceof NewMultiArrayNode) { + var dimensions = ((NewMultiArrayNode) node).dimensions(); + for (var dimension : dimensions) { + checkArrayAllocationLength(dimension); + } + } + } + + private static void checkArrayAllocationLength(ValueNode lengthNode) { + JavaConstant lengthConstant = lengthNode.asJavaConstant(); + if (lengthConstant != null) { + int length = lengthConstant.asInt(); + if (length < 0 || length > 100_000) { + /* + * Ensure that also the late class initialization after static analysis does not + * attempt to initialize. + */ + Class clazz = OriginalClassProvider.getJavaClass(lengthNode.graph().method().getDeclaringClass()); + ((ProvenSafeClassInitializationSupport) ImageSingletons.lookup(RuntimeClassInitializationSupport.class)).mustNotBeProvenSafe.add(clazz); + + throw new ClassInitializerHasSideEffectsException("Allocation of too large array in class initializer"); + } } } } diff --git a/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/classinitialization/ProvenSafeClassInitializationSupport.java b/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/classinitialization/ProvenSafeClassInitializationSupport.java index 0d6c841381f6..92f3e3c5041f 100644 --- a/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/classinitialization/ProvenSafeClassInitializationSupport.java +++ b/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/classinitialization/ProvenSafeClassInitializationSupport.java @@ -64,6 +64,7 @@ class ProvenSafeClassInitializationSupport extends ClassInitializationSupport { private final EarlyClassInitializerAnalysis earlyClassInitializerAnalysis; private final Set> provenSafeEarly = ConcurrentHashMap.newKeySet(); private Set> provenSafeLate = ConcurrentHashMap.newKeySet(); + final Set> mustNotBeProvenSafe = ConcurrentHashMap.newKeySet(); ProvenSafeClassInitializationSupport(MetaAccessProvider metaAccess, ImageClassLoader loader) { super(metaAccess, loader); @@ -76,6 +77,9 @@ InitKind computeInitKindAndMaybeInitializeClass(Class clazz) { } boolean canBeProvenSafe(Class clazz) { + if (mustNotBeProvenSafe.contains(clazz)) { + return false; + } InitKind initKind = specifiedInitKindFor(clazz); return initKind == null || (initKind.isRunTime() && !isStrictlyDefined(clazz)); } diff --git a/substratevm/src/com.oracle.svm.test/src/com/oracle/svm/test/clinit/TestClassInitializationMustBeSafeEarly.java b/substratevm/src/com.oracle.svm.test/src/com/oracle/svm/test/clinit/TestClassInitializationMustBeSafeEarly.java index 60042e76a9ea..f25620659f45 100644 --- a/substratevm/src/com.oracle.svm.test/src/com/oracle/svm/test/clinit/TestClassInitializationMustBeSafeEarly.java +++ b/substratevm/src/com.oracle.svm.test/src/com/oracle/svm/test/clinit/TestClassInitializationMustBeSafeEarly.java @@ -447,6 +447,26 @@ private static DevirtualizedCallSuperMustBeSafeEarly createProvider() { } } +class LargeAllocation1MustBeDelayed { + static final Object value = computeValue(); + + private static Object computeValue() { + Object[] result = new Object[1_000_000]; + for (int i = 0; i < result.length; i++) { + result[i] = new int[1_000_000]; + } + return result; + } +} + +class LargeAllocation2MustBeDelayed { + static final Object value = computeValue(); + + private static Object computeValue() { + return new int[Integer.MAX_VALUE][Integer.MAX_VALUE]; + } +} + class TestClassInitializationMustBeSafeEarlyFeature implements Feature { static final Class[] checkedClasses = new Class[]{ @@ -479,7 +499,8 @@ class TestClassInitializationMustBeSafeEarlyFeature implements Feature { ReferencesOtherPureClassMustBeSafeEarly.class, HelperClassMustBeSafeEarly.class, CycleMustBeSafeLate.class, HelperClassMustBeSafeLate.class, ReflectionMustBeSafeEarly.class, ForNameMustBeSafeEarly.class, ForNameMustBeDelayed.class, - DevirtualizedCallMustBeDelayed.class, DevirtualizedCallSuperMustBeSafeEarly.class, DevirtualizedCallSubMustBeSafeEarly.class, DevirtualizedCallUsageMustBeDelayed.class + DevirtualizedCallMustBeDelayed.class, DevirtualizedCallSuperMustBeSafeEarly.class, DevirtualizedCallSubMustBeSafeEarly.class, DevirtualizedCallUsageMustBeDelayed.class, + LargeAllocation1MustBeDelayed.class, LargeAllocation2MustBeDelayed.class, }; private static void checkClasses(boolean checkSafeEarly, boolean checkSafeLate) { @@ -660,6 +681,15 @@ public static void main(String[] args) { assertSame("field", ReflectionMustBeSafeEarly.f2.getName()); System.out.println(DevirtualizedCallUsageMustBeDelayed.value); + + if (System.currentTimeMillis() == 0) { + /* + * Make the class initializers reachable at run time, but do not actually execute them + * because they will allocate a lot of memory before throwing an OutOfMemoryError. + */ + System.out.println(LargeAllocation1MustBeDelayed.value); + System.out.println(LargeAllocation2MustBeDelayed.value); + } } private static void assertSame(Object expected, Object actual) {