diff --git a/substratevm/mx.substratevm/mx_substratevm.py b/substratevm/mx.substratevm/mx_substratevm.py index 2fc9dad14ff8..ff659d0e128b 100644 --- a/substratevm/mx.substratevm/mx_substratevm.py +++ b/substratevm/mx.substratevm/mx_substratevm.py @@ -1,7 +1,7 @@ # # ---------------------------------------------------------------------------------------------------- # -# Copyright (c) 2018, 2021, Oracle and/or its affiliates. All rights reserved. +# Copyright (c) 2018, 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 @@ -307,7 +307,7 @@ def native_image_func(args, **kwargs): yield native_image_func native_image_context.hosted_assertions = ['-J-ea', '-J-esa'] -_native_unittest_features = '--features=com.oracle.svm.test.ImageInfoTest$TestFeature,com.oracle.svm.test.ServiceLoaderTest$TestFeature,com.oracle.svm.test.SecurityServiceTest$TestFeature' +_native_unittest_features = '--features=com.oracle.svm.test.ImageInfoTest$TestFeature,com.oracle.svm.test.ServiceLoaderTest$TestFeature,com.oracle.svm.test.SecurityServiceTest$TestFeature,com.oracle.svm.test.ReflectionRegistrationTest$TestFeature' IMAGE_ASSERTION_FLAGS = ['-H:+VerifyGraalGraphs', '-H:+VerifyPhases'] DEVMODE_FLAGS = ['-Ob'] diff --git a/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/ConditionalConfigurationRegistry.java b/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/ConditionalConfigurationRegistry.java index 1ee6e315c3d2..f386e97be495 100644 --- a/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/ConditionalConfigurationRegistry.java +++ b/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/ConditionalConfigurationRegistry.java @@ -27,6 +27,7 @@ import java.util.ArrayList; import java.util.List; import java.util.Map; +import java.util.Objects; import java.util.concurrent.ConcurrentHashMap; import org.graalvm.nativeimage.hosted.Feature; @@ -38,6 +39,8 @@ public abstract class ConditionalConfigurationRegistry { private final Map> pendingReachabilityHandlers = new ConcurrentHashMap<>(); protected void registerConditionalConfiguration(ConfigurationCondition condition, Runnable runnable) { + Objects.requireNonNull(condition, "Cannot use null value as condition for conditional configuration. Please ensure that you register a non-null condition."); + Objects.requireNonNull(runnable, "Cannot use null value as runnable for conditional configuration. Please ensure that you register a non-null runnable."); if (ConfigurationCondition.alwaysTrue().equals(condition)) { /* analysis optimization to include new types as early as possible */ runnable.run(); diff --git a/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/ReachabilityHandlerFeature.java b/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/ReachabilityHandlerFeature.java index a1e8c65b810f..e8a04028df35 100644 --- a/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/ReachabilityHandlerFeature.java +++ b/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/ReachabilityHandlerFeature.java @@ -32,6 +32,7 @@ import java.util.List; import java.util.Map; import java.util.Set; +import java.util.concurrent.ConcurrentHashMap; import java.util.function.BiConsumer; import java.util.function.Consumer; @@ -44,6 +45,7 @@ import com.oracle.svm.core.SubstrateOptions; import com.oracle.svm.core.feature.InternalFeature; import com.oracle.svm.core.feature.AutomaticallyRegisteredFeature; +import com.oracle.svm.core.util.ConcurrentIdentityHashMap; import com.oracle.svm.core.util.UserError; import com.oracle.svm.core.util.VMError; import com.oracle.svm.hosted.FeatureImpl.BeforeAnalysisAccessImpl; @@ -52,8 +54,8 @@ @AutomaticallyRegisteredFeature public class ReachabilityHandlerFeature implements InternalFeature, ReachabilityHandler { - private final IdentityHashMap> activeHandlers = new IdentityHashMap<>(); - private final IdentityHashMap>> triggeredHandlers = new IdentityHashMap<>(); + private final Map> activeHandlers = new ConcurrentIdentityHashMap<>(); + private final Map>> triggeredHandlers = new ConcurrentIdentityHashMap<>(); public static ReachabilityHandlerFeature singleton() { return ImageSingletons.lookup(ReachabilityHandlerFeature.class); @@ -93,7 +95,7 @@ private void registerReachabilityHandler(BeforeAnalysisAccess a, Object callback BeforeAnalysisAccessImpl access = (BeforeAnalysisAccessImpl) a; AnalysisMetaAccess metaAccess = access.getMetaAccess(); - Set triggerSet = activeHandlers.computeIfAbsent(callback, c -> new HashSet<>()); + var triggerSet = activeHandlers.computeIfAbsent(callback, c -> ConcurrentHashMap.newKeySet()); for (Object trigger : triggers) { if (trigger instanceof Class) { @@ -125,7 +127,7 @@ public void duringAnalysis(DuringAnalysisAccess a) { Set triggers = activeHandlers.get(callback); if (callback instanceof Consumer) { if (isTriggered(access, triggers)) { - triggeredHandlers.put(callback, null); + triggeredHandlers.put(callback, Map.of()); toExactCallback(callback).accept(access); completedCallbacks.add(callback); } diff --git a/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/jni/JNIAccessFeature.java b/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/jni/JNIAccessFeature.java index 2e1a8a50ba95..8787613218fb 100644 --- a/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/jni/JNIAccessFeature.java +++ b/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/jni/JNIAccessFeature.java @@ -33,6 +33,7 @@ import java.util.Collections; import java.util.List; import java.util.Map; +import java.util.Objects; import java.util.Set; import java.util.concurrent.ConcurrentHashMap; import java.util.function.Predicate; @@ -201,18 +202,21 @@ private class JNIRuntimeAccessibilitySupportImpl extends ConditionalConfiguratio @Override public void register(ConfigurationCondition condition, boolean unsafeAllocated, Class clazz) { assert !unsafeAllocated : "unsafeAllocated can be only set via Unsafe.allocateInstance, not via JNI."; + Objects.requireNonNull(clazz, () -> nullErrorMessage("class")); abortIfSealed(); registerConditionalConfiguration(condition, () -> newClasses.add(clazz)); } @Override public void register(ConfigurationCondition condition, boolean queriedOnly, Executable... methods) { + requireNonNull(methods, "methods"); abortIfSealed(); registerConditionalConfiguration(condition, () -> newMethods.addAll(Arrays.asList(methods))); } @Override public void register(ConfigurationCondition condition, boolean finalIsWritable, Field... fields) { + requireNonNull(fields, "field"); abortIfSealed(); registerConditionalConfiguration(condition, () -> registerFields(finalIsWritable, fields)); } @@ -612,4 +616,14 @@ private static boolean anyFieldMatches(ResolvedJavaType sub, String name) { return false; } } + + private static void requireNonNull(Object[] values, String kind) { + for (Object value : values) { + Objects.requireNonNull(value, () -> nullErrorMessage(kind)); + } + } + + private static String nullErrorMessage(String kind) { + return "Cannot register null value as " + kind + " for JNI access. Please ensure that all values you register are not null."; + } } diff --git a/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/reflect/ReflectionDataBuilder.java b/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/reflect/ReflectionDataBuilder.java index 1faacb5671e9..d9861e5ffca0 100644 --- a/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/reflect/ReflectionDataBuilder.java +++ b/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/reflect/ReflectionDataBuilder.java @@ -44,6 +44,7 @@ import java.util.HashSet; import java.util.List; import java.util.Map; +import java.util.Objects; import java.util.Optional; import java.util.Set; import java.util.concurrent.Callable; @@ -127,6 +128,7 @@ public class ReflectionDataBuilder extends ConditionalConfigurationRegistry impl @Override public void register(ConfigurationCondition condition, boolean unsafeInstantiated, Class clazz) { + Objects.requireNonNull(clazz, () -> nullErrorMessage("class")); checkNotSealed(); registerConditionalConfiguration(condition, () -> { if (unsafeInstantiated) { @@ -148,6 +150,7 @@ public void registerClassLookupException(ConfigurationCondition condition, Strin @Override public void register(ConfigurationCondition condition, boolean queriedOnly, Executable... methods) { + requireNonNull(methods, "methods"); checkNotSealed(); registerConditionalConfiguration(condition, () -> registerMethods(queriedOnly, methods)); } @@ -171,6 +174,7 @@ private void registerMethods(boolean queriedOnly, Executable[] methods) { @Override public void register(ConfigurationCondition condition, boolean finalIsWritable, Field... fields) { + requireNonNull(fields, "field"); checkNotSealed(); registerConditionalConfiguration(condition, () -> registerFields(fields)); } @@ -925,4 +929,14 @@ static ExecutableAccessibility max(ExecutableAccessibility a, ExecutableAccessib return a == Accessed || b == Accessed ? Accessed : QueriedOnly; } } + + private static void requireNonNull(Object[] values, String kind) { + for (Object value : values) { + Objects.requireNonNull(value, () -> nullErrorMessage(kind)); + } + } + + private static String nullErrorMessage(String kind) { + return "Cannot register null value as " + kind + " for reflection. Please ensure that all values you register are not null."; + } } diff --git a/substratevm/src/com.oracle.svm.test/src/com/oracle/svm/test/ReflectionRegistrationTest.java b/substratevm/src/com.oracle.svm.test/src/com/oracle/svm/test/ReflectionRegistrationTest.java new file mode 100644 index 000000000000..555cc683657d --- /dev/null +++ b/substratevm/src/com.oracle.svm.test/src/com/oracle/svm/test/ReflectionRegistrationTest.java @@ -0,0 +1,120 @@ +/* + * Copyright (c) 2023, 2023, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2023, 2023, Red Hat Inc. 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 + * under the terms of the GNU General Public License version 2 only, as + * published by the Free Software Foundation. Oracle designates this + * particular file as subject to the "Classpath" exception as provided + * by Oracle in the LICENSE file that accompanied this code. + * + * This code is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License + * version 2 for more details (a copy is included in the LICENSE file that + * accompanied this code). + * + * You should have received a copy of the GNU General Public License version + * 2 along with this work; if not, write to the Free Software Foundation, + * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA. + * + * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA + * or visit www.oracle.com if you need additional information or have any + * questions. + */ +package com.oracle.svm.test; + +import com.oracle.svm.hosted.FeatureImpl; +import com.oracle.svm.hosted.substitute.SubstitutionReflectivityFilter; +import org.graalvm.nativeimage.ImageSingletons; +import org.graalvm.nativeimage.hosted.Feature; +import org.graalvm.nativeimage.hosted.RuntimeReflection; +import org.graalvm.nativeimage.impl.RuntimeReflectionSupport; +import org.junit.Test; + +import java.lang.reflect.Executable; +import java.lang.reflect.Field; + +/** + * Tests the {@link RuntimeReflection}. + */ +public class ReflectionRegistrationTest { + + public static class TestFeature implements Feature { + + @SuppressWarnings("unused")// + int unusedVariableOne = 1; + @SuppressWarnings("unused")// + int unusedVariableTwo = 2; + + @Override + public void beforeAnalysis(final BeforeAnalysisAccess access) { + try { + RuntimeReflection.register((Class) null); + assert false; + } catch (NullPointerException e) { + assert e.getMessage().startsWith("Cannot register null value"); + } + try { + RuntimeReflection.register((Executable) null); + assert false; + } catch (NullPointerException e) { + assert e.getMessage().startsWith("Cannot register null value"); + } + try { + RuntimeReflection.register((Field) null); + assert false; + } catch (NullPointerException e) { + assert e.getMessage().startsWith("Cannot register null value"); + } + + try { + ImageSingletons.lookup(RuntimeReflectionSupport.class).register(null, this.getClass()); + assert false; + } catch (NullPointerException e) { + assert e.getMessage().startsWith("Cannot use null value"); + } + + try { + ImageSingletons.lookup(RuntimeReflectionSupport.class).register(null, true, this.getClass().getMethods()); + assert false; + } catch (NullPointerException e) { + assert e.getMessage().startsWith("Cannot use null value"); + } + + try { + ImageSingletons.lookup(RuntimeReflectionSupport.class).register(null, true, this.getClass().getFields()); + assert false; + } catch (NullPointerException e) { + assert e.getMessage().startsWith("Cannot use null value"); + } + + FeatureImpl.BeforeAnalysisAccessImpl impl = (FeatureImpl.BeforeAnalysisAccessImpl) access; + try { + SubstitutionReflectivityFilter.shouldExclude((Class) null, impl.getMetaAccess(), impl.getUniverse()); + assert false; + } catch (NullPointerException e) { + // expected + } + try { + SubstitutionReflectivityFilter.shouldExclude((Executable) null, impl.getMetaAccess(), impl.getUniverse()); + assert false; + } catch (NullPointerException e) { + // expected + } + try { + SubstitutionReflectivityFilter.shouldExclude((Field) null, impl.getMetaAccess(), impl.getUniverse()); + assert false; + } catch (NullPointerException e) { + // expected + } + } + + } + + @Test + public void test() { + // nothing to do + } +}