[GR-49816] Throw exception for null in RuntimeJNIAccess/RuntimeReflection reg.

PullRequest: graal/15955

Author: fniephaus

substratevm/mx.substratevm/mx_substratevm.py

@@ -1,5 +1,5 @@
 #
-# 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
@@ -315,7 +315,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 = svm_experimental_options(['-H:+VerifyGraalGraphs', '-H:+VerifyPhases'])
 

substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/ConditionalConfigurationRegistry.java

@@ -26,6 +26,7 @@
 
 import java.util.Collection;
 import java.util.Map;
+import java.util.Objects;
 import java.util.concurrent.ConcurrentHashMap;
 import java.util.concurrent.ConcurrentLinkedQueue;
 
@@ -38,6 +39,8 @@ public abstract class ConditionalConfigurationRegistry {
     private final Map<String, Collection<Runnable>> 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();

substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/jni/JNIAccessFeature.java

@@ -34,6 +34,7 @@
 import java.util.IdentityHashMap;
 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;
@@ -42,9 +43,6 @@
 import org.graalvm.collections.EconomicSet;
 import org.graalvm.collections.Equivalence;
 import org.graalvm.collections.UnmodifiableMapCursor;
-import jdk.graal.compiler.api.replacements.Fold;
-import jdk.graal.compiler.options.Option;
-import jdk.graal.compiler.word.WordTypes;
 import org.graalvm.nativeimage.ImageSingletons;
 import org.graalvm.nativeimage.c.function.CodePointer;
 import org.graalvm.nativeimage.hosted.Feature;
@@ -98,6 +96,9 @@
 import com.oracle.svm.hosted.substitute.SubstitutionReflectivityFilter;
 import com.oracle.svm.util.ReflectionUtil;
 
+import jdk.graal.compiler.api.replacements.Fold;
+import jdk.graal.compiler.options.Option;
+import jdk.graal.compiler.word.WordTypes;
 import jdk.vm.ci.meta.MetaAccessProvider;
 import jdk.vm.ci.meta.ResolvedJavaField;
 import jdk.vm.ci.meta.ResolvedJavaMethod;
@@ -202,18 +203,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) {
+        public void register(ConfigurationCondition condition, boolean queriedOnly, Executable... executables) {
+            requireNonNull(executables, "executable");
             abortIfSealed();
-            registerConditionalConfiguration(condition, () -> newMethods.addAll(Arrays.asList(methods)));
+            registerConditionalConfiguration(condition, () -> newMethods.addAll(Arrays.asList(executables)));
         }
 
         @Override
         public void register(ConfigurationCondition condition, boolean finalIsWritable, Field... fields) {
+            requireNonNull(fields, "field");
             abortIfSealed();
             registerConditionalConfiguration(condition, () -> registerFields(finalIsWritable, fields));
         }
@@ -622,4 +626,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.";
+    }
 }

substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/reflect/ReflectionDataBuilder.java

@@ -56,6 +56,7 @@
 import java.util.Collections;
 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;
@@ -166,6 +167,7 @@ private void setQueryFlag(Class<?> clazz, int flag) {
 
     @Override
     public void register(ConfigurationCondition condition, boolean unsafeInstantiated, Class<?> clazz) {
+        Objects.requireNonNull(clazz, () -> nullErrorMessage("class"));
         checkNotSealed();
         register(analysisUniverse -> registerConditionalConfiguration(condition,
                         () -> analysisUniverse.getBigbang().postTask(debug -> registerClass(clazz, unsafeInstantiated))));
@@ -269,6 +271,7 @@ public void registerAllSignersQuery(ConfigurationCondition condition, Class<?> c
 
     @Override
     public void register(ConfigurationCondition condition, boolean queriedOnly, Executable... executables) {
+        requireNonNull(executables, "executable");
         checkNotSealed();
         register(analysisUniverse -> registerConditionalConfiguration(condition, () -> {
             for (Executable executable : executables) {
@@ -397,6 +400,7 @@ public void registerConstructorLookup(ConfigurationCondition condition, Class<?>
 
     @Override
     public void register(ConfigurationCondition condition, boolean finalIsWritable, Field... fields) {
+        requireNonNull(fields, "field");
         checkNotSealed();
         registerInternal(condition, fields);
     }
@@ -1060,4 +1064,14 @@ public int getReflectionMethodsCount() {
     public int getReflectionFieldsCount() {
         return registeredFields.size();
     }
+
+    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.";
+    }
 }

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
+    }
+}