Skip to content

Commit f9b357b

Browse files
peter-hoferpeter-hofer
committed
[GR-12843] [GR-12881] Reflection allows access to deleted fields, build fails for deleted static final fields (#784).
PullRequest: graal/2591
2 parents efaeba3 + f42aba0 commit f9b357b

File tree

5 files changed

+75
-21
lines changed

5 files changed

+75
-21
lines changed

‎substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/snippets/ReflectionPlugins.java

+17-9
Original file line numberDiff line numberDiff line change
@@ -43,14 +43,17 @@
4343
import org.graalvm.compiler.options.Option;
4444
import org.graalvm.nativeimage.ImageSingletons;
4545

46+
import com.oracle.svm.core.annotate.Delete;
4647
import com.oracle.svm.core.meta.SubstrateObjectConstant;
4748
import com.oracle.svm.core.option.HostedOptionKey;
4849
import com.oracle.svm.core.reflect.ReflectionPluginExceptions;
4950
import com.oracle.svm.core.util.VMError;
5051
import com.oracle.svm.hosted.ImageClassLoader;
52+
import com.oracle.svm.hosted.NativeImageOptions;
5153

5254
import jdk.vm.ci.meta.JavaConstant;
5355
import jdk.vm.ci.meta.JavaKind;
56+
import jdk.vm.ci.meta.MetaAccessProvider;
5457
import jdk.vm.ci.meta.ResolvedJavaMethod;
5558

5659
public class ReflectionPlugins {
@@ -171,12 +174,12 @@ private static boolean processForName(GraphBuilderContext b, ResolvedJavaMethod
171174
String className = snippetReflection.asObject(String.class, name.asJavaConstant());
172175
Class<?> clazz = imageClassLoader.findClassByName(className, false);
173176
if (clazz == null) {
174-
if (shouldNotIntrinsify(analysis, hosted, throwClassNotFoundExceptionMethod)) {
177+
if (shouldNotIntrinsify(analysis, hosted, b.getMetaAccess(), throwClassNotFoundExceptionMethod)) {
175178
return false;
176179
}
177180
throwClassNotFoundException(b, targetMethod, className);
178181
} else {
179-
if (shouldNotIntrinsify(analysis, hosted, clazz)) {
182+
if (shouldNotIntrinsify(analysis, hosted, b.getMetaAccess(), clazz)) {
180183
return false;
181184
}
182185
JavaConstant hub = b.getConstantReflection().asJavaClass(b.getMetaAccess().lookupJavaType(clazz));
@@ -196,12 +199,12 @@ private static boolean processGetField(GraphBuilderContext b, ResolvedJavaMethod
196199
String target = clazz.getTypeName() + "." + fieldName;
197200
try {
198201
Field field = declared ? clazz.getDeclaredField(fieldName) : clazz.getField(fieldName);
199-
if (shouldNotIntrinsify(analysis, hosted, field)) {
202+
if (shouldNotIntrinsify(analysis, hosted, b.getMetaAccess(), field)) {
200203
return false;
201204
}
202205
pushConstant(b, targetMethod, snippetReflection.forObject(field), target);
203206
} catch (NoSuchFieldException e) {
204-
if (shouldNotIntrinsify(analysis, hosted, throwNoSuchFieldExceptionMethod)) {
207+
if (shouldNotIntrinsify(analysis, hosted, b.getMetaAccess(), throwNoSuchFieldExceptionMethod)) {
205208
return false;
206209
}
207210
throwNoSuchFieldException(b, targetMethod, target);
@@ -223,12 +226,12 @@ private static boolean processGetMethod(GraphBuilderContext b, ResolvedJavaMetho
223226
String target = clazz.getTypeName() + "." + methodName + "(" + Stream.of(paramTypes).map(Class::getTypeName).collect(Collectors.joining(", ")) + ")";
224227
try {
225228
Method method = declared ? clazz.getDeclaredMethod(methodName, paramTypes) : clazz.getMethod(methodName, paramTypes);
226-
if (shouldNotIntrinsify(analysis, hosted, method)) {
229+
if (shouldNotIntrinsify(analysis, hosted, b.getMetaAccess(), method)) {
227230
return false;
228231
}
229232
pushConstant(b, targetMethod, snippetReflection.forObject(method), target);
230233
} catch (NoSuchMethodException e) {
231-
if (shouldNotIntrinsify(analysis, hosted, throwNoSuchMethodExceptionMethod)) {
234+
if (shouldNotIntrinsify(analysis, hosted, b.getMetaAccess(), throwNoSuchMethodExceptionMethod)) {
232235
return false;
233236
}
234237
throwNoSuchMethodException(b, targetMethod, target);
@@ -253,12 +256,12 @@ private static boolean processGetConstructor(GraphBuilderContext b, ResolvedJava
253256
String target = clazz.getTypeName() + ".<init>(" + Stream.of(paramTypes).map(Class::getTypeName).collect(Collectors.joining(", ")) + ")";
254257
try {
255258
Constructor<?> constructor = declared ? clazz.getDeclaredConstructor(paramTypes) : clazz.getConstructor(paramTypes);
256-
if (shouldNotIntrinsify(analysis, hosted, constructor)) {
259+
if (shouldNotIntrinsify(analysis, hosted, b.getMetaAccess(), constructor)) {
257260
return false;
258261
}
259262
pushConstant(b, targetMethod, snippetReflection.forObject(constructor), target);
260263
} catch (NoSuchMethodException e) {
261-
if (shouldNotIntrinsify(analysis, hosted, throwNoSuchMethodExceptionMethod)) {
264+
if (shouldNotIntrinsify(analysis, hosted, b.getMetaAccess(), throwNoSuchMethodExceptionMethod)) {
262265
return false;
263266
}
264267
throwNoSuchMethodException(b, targetMethod, target);
@@ -271,12 +274,17 @@ private static boolean processGetConstructor(GraphBuilderContext b, ResolvedJava
271274
}
272275

273276
/** Check if the element should be intrinsified. */
274-
private static boolean shouldNotIntrinsify(boolean analysis, boolean hosted, Object element) {
277+
private static boolean shouldNotIntrinsify(boolean analysis, boolean hosted, MetaAccessProvider metaAccess, Object element) {
275278
if (!hosted) {
276279
/* We are analyzing the static initializers and should always intrinsify. */
277280
return false;
278281
}
279282
if (analysis) {
283+
if (NativeImageOptions.ReportUnsupportedElementsAtRuntime.getValue()) {
284+
if (element instanceof Field && metaAccess.lookupJavaField((Field) element).isAnnotationPresent(Delete.class)) {
285+
return true; /* Fail during the reflective field lookup at runtime. */
286+
}
287+
}
280288
/* We are during analysis, we should intrinsify and mark the objects as intrinsified. */
281289
ImageSingletons.lookup(ReflectionPluginRegistry.class).add(element);
282290
return false;

‎substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/substitute/AnnotatedField.java

+9-2
Original file line numberDiff line numberDiff line change
@@ -27,8 +27,9 @@
2727
import java.lang.annotation.Annotation;
2828
import java.util.Arrays;
2929

30+
import com.oracle.svm.core.annotate.Delete;
31+
import com.oracle.svm.core.annotate.InjectAccessors;
3032
import com.oracle.svm.core.meta.ReadableJavaField;
31-
import com.oracle.svm.core.util.VMError;
3233
import com.oracle.svm.hosted.c.GraalAccess;
3334

3435
import jdk.vm.ci.meta.JavaConstant;
@@ -75,7 +76,13 @@ public JavaConstant readValue(JavaConstant receiver) {
7576

7677
@Override
7778
public boolean allowConstantFolding() {
78-
throw VMError.shouldNotReachHere();
79+
/*
80+
* We assume that fields for which this class is used always have altered behavior for which
81+
* constant folding is not valid.
82+
*/
83+
assert (injectedAnnotation instanceof Delete) || (injectedAnnotation instanceof InjectAccessors) : "Unknown annotation @" +
84+
injectedAnnotation.annotationType().getSimpleName() + ", should constant folding be permitted?";
85+
return false;
7986
}
8087

8188
@Override

‎substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/substitute/DeletedMethod.java

+1-1
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,7 @@ public DeletedMethod(ResolvedJavaMethod original, Delete deleteAnnotation) {
5858
this.deleteAnnotation = deleteAnnotation;
5959
}
6060

61-
static final Method reportErrorMethod;
61+
public static final Method reportErrorMethod;
6262

6363
static {
6464
try {

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

+16-5
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,9 @@
4444
import org.graalvm.nativeimage.Feature.DuringAnalysisAccess;
4545
import org.graalvm.nativeimage.impl.RuntimeReflectionSupport;
4646

47+
import com.oracle.graal.pointsto.meta.AnalysisMetaAccess;
4748
import com.oracle.graal.pointsto.meta.AnalysisType;
49+
import com.oracle.svm.core.annotate.Delete;
4850
import com.oracle.svm.core.hub.ClassForNameSupport;
4951
import com.oracle.svm.core.hub.DynamicHub;
5052
import com.oracle.svm.core.util.UserError;
@@ -117,7 +119,6 @@ protected void duringAnalysis(DuringAnalysisAccess a) {
117119
Field declaredPublicFieldsField = findField(originalReflectionDataClass, "declaredPublicFields");
118120
Field declaredPublicMethodsField = findField(originalReflectionDataClass, "declaredPublicMethods");
119121

120-
Field[] emptyFields = new Field[0];
121122
Method[] emptyMethods = new Method[0];
122123
Constructor<?>[] emptyConstructors = new Constructor<?>[0];
123124

@@ -176,14 +177,14 @@ protected void duringAnalysis(DuringAnalysisAccess a) {
176177
try {
177178
Object originalReflectionData = reflectionDataMethod.invoke(clazz);
178179
hub.setReflectionData(new DynamicHub.ReflectionData(
179-
filter(declaredFieldsField.get(originalReflectionData), reflectionFields.keySet(), emptyFields),
180-
filter(publicFieldsField.get(originalReflectionData), reflectionFields.keySet(), emptyFields),
180+
filterFields(declaredFieldsField.get(originalReflectionData), reflectionFields.keySet(), access.getMetaAccess()),
181+
filterFields(publicFieldsField.get(originalReflectionData), reflectionFields.keySet(), access.getMetaAccess()),
181182
filter(declaredMethodsField.get(originalReflectionData), reflectionMethods, emptyMethods),
182183
filter(publicMethodsField.get(originalReflectionData), reflectionMethods, emptyMethods),
183184
filter(declaredConstructorsField.get(originalReflectionData), reflectionMethods, emptyConstructors),
184185
filter(publicConstructorsField.get(originalReflectionData), reflectionMethods, emptyConstructors),
185186
nullaryConstructor(declaredConstructorsField.get(originalReflectionData), reflectionMethods),
186-
filter(declaredPublicFieldsField.get(originalReflectionData), reflectionFields.keySet(), emptyFields),
187+
filterFields(declaredPublicFieldsField.get(originalReflectionData), reflectionFields.keySet(), access.getMetaAccess()),
187188
filter(declaredPublicMethodsField.get(originalReflectionData), reflectionMethods, emptyMethods),
188189
enclosingMethodOrConstructor(clazz)));
189190
} catch (IllegalAccessException | IllegalArgumentException | InvocationTargetException ex) {
@@ -225,7 +226,7 @@ private Executable enclosingMethodOrConstructor(Class<?> clazz) {
225226
if (enclosingMethod == null && enclosingConstructor == null) {
226227
return null;
227228
} else if (enclosingMethod != null && enclosingConstructor != null) {
228-
throw VMError.shouldNotReachHere("Classs has both an enclosingMethod and an enclosingConstructor: " + clazz + ", " + enclosingMethod + ", " + enclosingConstructor);
229+
throw VMError.shouldNotReachHere("Class has both an enclosingMethod and an enclosingConstructor: " + clazz + ", " + enclosingMethod + ", " + enclosingConstructor);
229230
}
230231

231232
Executable enclosingMethodOrConstructor = enclosingMethod != null ? enclosingMethod : enclosingConstructor;
@@ -247,6 +248,16 @@ private static <T> T[] filter(Object elements, Set<?> filter, T[] prototypeArray
247248
return result.toArray(prototypeArray);
248249
}
249250

251+
private static Field[] filterFields(Object fields, Set<Field> filter, AnalysisMetaAccess metaAccess) {
252+
List<Field> result = new ArrayList<>();
253+
for (Field field : (Field[]) fields) {
254+
if (filter.contains(field) && !metaAccess.lookupJavaField(field).isAnnotationPresent(Delete.class)) {
255+
result.add(field);
256+
}
257+
}
258+
return result.toArray(new Field[0]);
259+
}
260+
250261
private static Method findMethod(Class<?> declaringClass, String methodName, Class<?>... parameterTypes) {
251262
try {
252263
Method result = declaringClass.getDeclaredMethod(methodName, parameterTypes);

‎substratevm/src/com.oracle.svm.reflect/src/com/oracle/svm/reflect/hosted/ReflectionSubstitutionType.java

+32-4
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,7 @@
4040
import org.graalvm.compiler.core.common.type.TypeReference;
4141
import org.graalvm.compiler.debug.DebugContext;
4242
import org.graalvm.compiler.nodes.CallTargetNode.InvokeKind;
43+
import org.graalvm.compiler.nodes.ConstantNode;
4344
import org.graalvm.compiler.nodes.LogicNode;
4445
import org.graalvm.compiler.nodes.NodeView;
4546
import org.graalvm.compiler.nodes.PiNode;
@@ -62,11 +63,15 @@
6263
import org.graalvm.nativeimage.impl.RuntimeReflectionSupport;
6364

6465
import com.oracle.graal.pointsto.meta.HostedProviders;
66+
import com.oracle.svm.core.annotate.Delete;
67+
import com.oracle.svm.core.meta.SubstrateObjectConstant;
6568
import com.oracle.svm.core.util.VMError;
6669
import com.oracle.svm.hosted.annotation.CustomSubstitutionField;
6770
import com.oracle.svm.hosted.annotation.CustomSubstitutionMethod;
6871
import com.oracle.svm.hosted.annotation.CustomSubstitutionType;
6972
import com.oracle.svm.hosted.phases.HostedGraphKit;
73+
import com.oracle.svm.hosted.substitute.AnnotationSubstitutionProcessor;
74+
import com.oracle.svm.hosted.substitute.DeletedMethod;
7075
import com.oracle.svm.reflect.helpers.ExceptionHelpers;
7176
import com.oracle.svm.reflect.hosted.ReflectionSubstitutionType.ReflectionSubstitutionMethod;
7277

@@ -163,8 +168,8 @@ public ReflectionSubstitutionType(ResolvedJavaType original, Member member) {
163168
private static ReflectionSubstitutionMethod createWriteMethod(ResolvedJavaMethod method, Field field, JavaKind kind) {
164169
ReflectionDataBuilder reflectionDataBuilder = (ReflectionDataBuilder) ImageSingletons.lookup(RuntimeReflectionSupport.class);
165170
if (Modifier.isFinal(field.getModifiers()) && !reflectionDataBuilder.inspectFinalFieldWritableForAnalysis(field)) {
166-
return new ThrowingMethod(method, IllegalAccessException.class, "Cannot set final field: " + field.getName() + ". " +
167-
"Enable by specifying \"allowWrite\" for this field in the reflection configuration.");
171+
return new ThrowingMethod(method, IllegalAccessException.class, "Cannot set final field: " + field.getDeclaringClass().getName() +
172+
"." + field.getName() + ". " + "Enable by specifying \"allowWrite\" for this field in the reflection configuration.");
168173
}
169174
return new ReflectiveWriteMethod(method, field, kind);
170175
}
@@ -349,6 +354,23 @@ private static ValueNode doImplicitCast(HostedGraphKit graphKit, JavaKind from,
349354
return doImplicitCast(graphKit, JavaKind.Int, to, intermediate);
350355
}
351356

357+
private static boolean isDeletedField(ResolvedJavaField field) {
358+
return field.isAnnotationPresent(Delete.class);
359+
}
360+
361+
private static void handleDeletedField(HostedGraphKit graphKit, HostedProviders providers, ResolvedJavaField field, JavaKind returnKind) {
362+
Delete deleteAnnotation = field.getAnnotation(Delete.class);
363+
String msg = AnnotationSubstitutionProcessor.deleteErrorMessage(field, deleteAnnotation.value(), false);
364+
ValueNode msgNode = ConstantNode.forConstant(SubstrateObjectConstant.forObject(msg), providers.getMetaAccess(), graphKit.getGraph());
365+
ResolvedJavaMethod reportErrorMethod = providers.getMetaAccess().lookupJavaMethod(DeletedMethod.reportErrorMethod);
366+
graphKit.createInvokeWithExceptionAndUnwind(reportErrorMethod, InvokeKind.Static, graphKit.getFrameState(), graphKit.bci(), graphKit.bci(), msgNode);
367+
ConstantNode returnValue = null;
368+
if (returnKind != JavaKind.Void) {
369+
returnValue = graphKit.unique(ConstantNode.defaultForKind(returnKind));
370+
}
371+
graphKit.createReturn(returnValue, returnKind);
372+
}
373+
352374
private static class ReflectiveReadMethod extends ReflectionSubstitutionMethod {
353375

354376
private final Field field;
@@ -365,7 +387,10 @@ public StructuredGraph buildGraph(DebugContext ctx, ResolvedJavaMethod method, H
365387
HostedGraphKit graphKit = new HostedGraphKit(ctx, providers, method);
366388
ResolvedJavaField targetField = providers.getMetaAccess().lookupJavaField(field);
367389

368-
if (canImplicitCast(targetField.getJavaKind(), kind)) {
390+
if (isDeletedField(targetField)) {
391+
handleDeletedField(graphKit, providers, targetField, kind);
392+
393+
} else if (canImplicitCast(targetField.getJavaKind(), kind)) {
369394

370395
ValueNode receiver;
371396
if (targetField.isStatic()) {
@@ -406,7 +431,10 @@ public StructuredGraph buildGraph(DebugContext ctx, ResolvedJavaMethod method, H
406431
ResolvedJavaField targetField = providers.getMetaAccess().lookupJavaField(field);
407432

408433
JavaKind fieldKind = targetField.getJavaKind();
409-
if (kind == JavaKind.Object || canImplicitCast(kind, fieldKind)) {
434+
if (isDeletedField(targetField)) {
435+
handleDeletedField(graphKit, providers, targetField, JavaKind.Void);
436+
437+
} else if (kind == JavaKind.Object || canImplicitCast(kind, fieldKind)) {
410438

411439
ValueNode receiver;
412440
if (targetField.isStatic()) {

0 commit comments

Comments
 (0)