Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Backport: [GR-49816] Throw exception for null pointers passed to RuntimeJNIAccess / RuntimeReflection register methods #15

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions substratevm/mx.substratevm/mx_substratevm.py
Original file line number Diff line number Diff line change
@@ -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
Expand Down Expand Up @@ -296,7 +296,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 @@ -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;

Expand All @@ -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();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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) {
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));
}
Expand Down Expand Up @@ -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.";
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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))));
Expand Down Expand Up @@ -281,6 +283,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) {
Expand Down Expand Up @@ -409,6 +412,7 @@ public void registerConstructorLookup(ConfigurationCondition condition, Class<?>

@Override
public void register(ConfigurationCondition condition, boolean finalIsWritable, Field... fields) {
requireNonNull(fields, "field");
checkNotSealed();
registerInternal(condition, fields);
}
Expand Down Expand Up @@ -1072,4 +1076,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.";
}
}
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
}
}
Loading