Skip to content

[GR-39967] Skip constant folding of reflection methods with side effects. #5156

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.

Sign up for GitHub

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

Merged
merged 1 commit into from
Oct 6, 2022
Merged

[GR-39967] Skip constant folding of reflection methods with side effects. #5156

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
82 changes: 78 additions & 4 deletions ...atevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/snippets/ReflectionPlugins.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@
import java.util.List;
import java.util.Objects;
import java.util.Set;
import java.util.function.Predicate;
import java.util.function.Supplier;
import java.util.stream.Collectors;
import java.util.stream.Stream;
Expand All @@ -58,6 +59,8 @@
import org.graalvm.compiler.nodes.graphbuilderconf.InvocationPlugins.Registration;
import org.graalvm.compiler.options.Option;
import org.graalvm.nativeimage.ImageSingletons;
import org.graalvm.nativeimage.hosted.RuntimeReflection;
import org.graalvm.nativeimage.impl.RuntimeClassInitializationSupport;

import com.oracle.graal.pointsto.infrastructure.OriginalClassProvider;
import com.oracle.graal.pointsto.meta.AnalysisUniverse;
Expand All @@ -72,6 +75,7 @@
import com.oracle.svm.core.util.VMError;
import com.oracle.svm.hosted.ExceptionSynthesizer;
import com.oracle.svm.hosted.ImageClassLoader;
import com.oracle.svm.hosted.classinitialization.ClassInitializationSupport;
import com.oracle.svm.hosted.substitute.AnnotationSubstitutionProcessor;
import com.oracle.svm.hosted.substitute.DeletedElementException;
import com.oracle.svm.util.ModuleSupport;
Expand Down Expand Up @@ -165,6 +169,7 @@ public static void registerInvocationPlugins(ImageClassLoader imageClassLoader,
ByteOrder.class));

private void registerMethodHandlesPlugins(InvocationPlugins plugins) {

registerFoldInvocationPlugins(plugins, MethodHandles.class,
"publicLookup", "privateLookupIn",
"arrayConstructor", "arrayLength", "arrayElementGetter", "arrayElementSetter", "arrayElementVarHandle",
Expand All @@ -174,16 +179,18 @@ private void registerMethodHandlesPlugins(InvocationPlugins plugins) {
"in",
"findStatic", "findVirtual", "findConstructor", "findClass", "accessClass", "findSpecial",
"findGetter", "findSetter", "findVarHandle",
"findStaticGetter", "findStaticSetter", "findStaticVarHandle",
"findStaticGetter", "findStaticSetter",
"unreflect", "unreflectSpecial", "unreflectConstructor",
"unreflectGetter", "unreflectSetter", "unreflectVarHandle");
"unreflectGetter", "unreflectSetter");

registerFoldInvocationPlugins(plugins, MethodType.class,
"methodType", "genericMethodType",
"changeParameterType", "insertParameterTypes", "appendParameterTypes", "replaceParameterTypes", "dropParameterTypes",
"changeReturnType", "erase", "generic", "wrap", "unwrap",
"parameterType", "parameterCount", "returnType", "lastParameterType");

registerConditionalFoldInvocationPlugins(plugins);

Registration r = new Registration(plugins, MethodHandles.class);
r.register(new RequiredInlineOnlyInvocationPlugin("lookup") {
@Override
Expand All @@ -193,6 +200,53 @@ public boolean apply(GraphBuilderContext b, ResolvedJavaMethod targetMethod, Rec
});
}

/**
* For some methods check if folding an invocation using reflection, i.e., by executing the
* target method and capturing the result, has undesired side effects, such as triggering
* initialization of classes that should be initialized at run time. This is based on knowledge
* about the reflection API methods implementation.
*/
Comment on lines +203 to +208
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@cstancu Is this still an issue now that the new class initialization strategy ( #4684) is being used by default?

This change seems to significantly increase the number of metadata in the image heap.

private void registerConditionalFoldInvocationPlugins(InvocationPlugins plugins) {
Method methodHandlesLookupFindStaticVarHandle = ReflectionUtil.lookupMethod(MethodHandles.Lookup.class, "findStaticVarHandle", Class.class, String.class, Class.class);
registerFoldInvocationPlugin(plugins, methodHandlesLookupFindStaticVarHandle, (args) -> {
/* VarHandles.makeFieldHandle() triggers init of receiver class (JDK-8291065). */
Object classArg = args[0];
if (classArg instanceof Class<?>) {
if (shouldInitializeAtRuntime((Class<?>) classArg)) {
/* Skip the folding and register the field for run time reflection. */
if (reason == ParsingReason.PointsToAnalysis) {
Field field = ReflectionUtil.lookupField(true, (Class<?>) args[0], (String) args[1]);
if (field != null) {
RuntimeReflection.register(field);
}
}
return false;
}
}
return true;
});

Method methodHandlesLookupUnreflectVarHandle = ReflectionUtil.lookupMethod(MethodHandles.Lookup.class, "unreflectVarHandle", Field.class);
registerFoldInvocationPlugin(plugins, methodHandlesLookupUnreflectVarHandle, (args) -> {
/*
* VarHandles.makeFieldHandle() triggers init of static field's declaring class
* (JDK-8291065).
*/
Object fieldArg = args[0];
if (fieldArg instanceof Field) {
Field field = (Field) fieldArg;
if (isStatic(field) && shouldInitializeAtRuntime(field.getDeclaringClass())) {
/* Skip the folding and register the field for run time reflection. */
if (reason == ParsingReason.PointsToAnalysis) {
RuntimeReflection.register(field);
}
return false;
}
}
return true;
});
}

private void registerClassPlugins(InvocationPlugins plugins) {
registerFoldInvocationPlugins(plugins, Class.class,
"getField", "getMethod", "getConstructor",
Expand Down Expand Up @@ -326,7 +380,13 @@ private void registerFoldInvocationPlugins(InvocationPlugins plugins, Class<?> d
}
}

private static final Predicate<Object[]> alwaysAllowConstantFolding = args -> true;

private void registerFoldInvocationPlugin(InvocationPlugins plugins, Method reflectionMethod) {
registerFoldInvocationPlugin(plugins, reflectionMethod, alwaysAllowConstantFolding);
}

private void registerFoldInvocationPlugin(InvocationPlugins plugins, Method reflectionMethod, Predicate<Object[]> allowConstantFolding) {
if (!ALLOWED_CONSTANT_CLASSES.contains(reflectionMethod.getReturnType()) && !reflectionMethod.getReturnType().isPrimitive()) {
throw VMError.shouldNotReachHere("Return type of method " + reflectionMethod + " is not on the allow-list for types that are immutable");
}
Expand All @@ -341,12 +401,13 @@ private void registerFoldInvocationPlugin(InvocationPlugins plugins, Method refl
plugins.register(reflectionMethod.getDeclaringClass(), new RequiredInvocationPlugin(reflectionMethod.getName(), parameterTypes.toArray(new Class<?>[0])) {
@Override
public boolean defaultHandler(GraphBuilderContext b, ResolvedJavaMethod targetMethod, Receiver receiver, ValueNode... args) {
return foldInvocationUsingReflection(b, targetMethod, reflectionMethod, receiver, args);
return foldInvocationUsingReflection(b, targetMethod, reflectionMethod, receiver, args, allowConstantFolding);
}
});
}

private boolean foldInvocationUsingReflection(GraphBuilderContext b, ResolvedJavaMethod targetMethod, Method reflectionMethod, Receiver receiver, ValueNode[] args) {
private boolean foldInvocationUsingReflection(GraphBuilderContext b, ResolvedJavaMethod targetMethod, Method reflectionMethod, Receiver receiver, ValueNode[] args,
Predicate<Object[]> allowConstantFolding) {
assert b.getMetaAccess().lookupJavaMethod(reflectionMethod).equals(targetMethod) : "Fold method mismatch: " + reflectionMethod + " != " + targetMethod;

Object receiverValue;
Expand Down Expand Up @@ -376,6 +437,10 @@ private boolean foldInvocationUsingReflection(GraphBuilderContext b, ResolvedJav
}
}

if (!allowConstantFolding.test(argValues)) {
return false;
}

/* String representation of the parameters for debug printing. */
Supplier<String> targetParameters = () -> (receiverValue == null ? "" : receiverValue.toString() + "; ") +
Stream.of(argValues).map(arg -> arg instanceof Object[] ? Arrays.toString((Object[]) arg) : Objects.toString(arg)).collect(Collectors.joining(", "));
Expand All @@ -401,6 +466,15 @@ private boolean foldInvocationUsingReflection(GraphBuilderContext b, ResolvedJav
return pushConstant(b, targetMethod, targetParameters, returnKind, returnValue, false) != null;
}

private static boolean shouldInitializeAtRuntime(Class<?> classArg) {
ClassInitializationSupport classInitializationSupport = (ClassInitializationSupport) ImageSingletons.lookup(RuntimeClassInitializationSupport.class);
return classInitializationSupport.shouldInitializeAtRuntime(classArg);
}

private static boolean isStatic(Field field) {
return Modifier.isStatic(field.getModifiers());
}

private Object unbox(GraphBuilderContext b, ValueNode arg, JavaKind argKind) {
if (!arg.isJavaConstant()) {
/*
Expand Down