Skip to content

Commit

Permalink
Throw exception for null in RuntimeJNIAccess/RuntimeReflection reg.
Browse files Browse the repository at this point in the history
Don't allow null values to be passed to the `register` method of
`RuntimeJNIAccess` and `RuntimeReflection`. Since these are public APIs
GraalVM should either handle null values (by ignoring them in this case)
or throw a `NullPointerException` before creating an asynchronous task
to perform the registration in the analysis, which then results in
`NullPointerException`s being thrown later when it's no longer possible
to understand where the null value originate from.
  • Loading branch information
zakkak committed Nov 1, 2023
1 parent 9fb0c04 commit e6c12dd
Show file tree
Hide file tree
Showing 5 changed files with 145 additions and 3 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,13 @@

public interface ReflectionRegistry {
default void register(ConfigurationCondition condition, Class<?>... classes) {
Arrays.stream(classes).forEach(clazz -> register(condition, false, clazz));
Arrays.stream(classes).forEach(clazz -> {
if (clazz == null) {
throw new NullPointerException("Cannot register null value as class for reflection. " +
"Please ensure that all values you register are not null.");
}
register(condition, false, clazz);
});
}

void register(ConfigurationCondition condition, boolean unsafeAllocated, Class<?> clazz);
Expand Down
4 changes: 2 additions & 2 deletions substratevm/mx.substratevm/mx_substratevm.py
Original file line number Diff line number Diff line change
@@ -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
Expand Down Expand Up @@ -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'])

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,14 @@ public abstract class ConditionalConfigurationRegistry {
private final Map<String, Collection<Runnable>> pendingReachabilityHandlers = new ConcurrentHashMap<>();

protected void registerConditionalConfiguration(ConfigurationCondition condition, Runnable runnable) {
if (condition == null) {
throw new NullPointerException("Cannot use null value as condition for conditional configuration. " +
"Please ensure that you register a non-null condition.");
}
if (runnable == null) {
throw new NullPointerException("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();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -272,6 +272,10 @@ public void register(ConfigurationCondition condition, boolean queriedOnly, Exec
checkNotSealed();
register(analysisUniverse -> registerConditionalConfiguration(condition, () -> {
for (Executable executable : executables) {
if (executable == null) {
throw new NullPointerException("Cannot register null value as executable for reflection. " +
"Please ensure that all values you register are not null.");
}
analysisUniverse.getBigbang().postTask(debug -> registerMethod(queriedOnly, executable));
}
}));
Expand Down Expand Up @@ -404,6 +408,10 @@ public void register(ConfigurationCondition condition, boolean finalIsWritable,
private void registerInternal(ConfigurationCondition condition, Field... fields) {
register(analysisUniverse -> registerConditionalConfiguration(condition, () -> {
for (Field field : fields) {
if (field == null) {
throw new NullPointerException("Cannot register null value as field for reflection. " +
"Please ensure that all values you register are not null.");
}
analysisUniverse.getBigbang().postTask(debug -> registerField(field));
}
}));
Expand Down
Original file line number Diff line number Diff line change
@@ -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
}
}

0 comments on commit e6c12dd

Please sign in to comment.