Skip to content

Commit f5699db

Browse files
Doug Simondougxc
Doug Simon
authored and
dougxc
committed
8362306: HotSpotJVMCIRuntime.getMirror can crash
Reviewed-by: gdub, never, cslucas (cherry picked from commit 2b11a28)
1 parent a7b87d0 commit f5699db

File tree

6 files changed

+76
-13
lines changed

6 files changed

+76
-13
lines changed

‎src/hotspot/share/jvmci/jvmciCompilerToVM.cpp‎

Lines changed: 13 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2263,7 +2263,7 @@ C2V_VMENTRY_NULL(jobjectArray, getDeclaredFieldsInfo, (JNIEnv* env, jobject, ARG
22632263
InstanceKlass* iklass = InstanceKlass::cast(klass);
22642264
int java_fields, injected_fields;
22652265
GrowableArray<FieldInfo>* fields = FieldInfoStream::create_FieldInfoArray(iklass->fieldinfo_stream(), &java_fields, &injected_fields);
2266-
JVMCIObjectArray array = JVMCIENV->new_FieldInfo_array(fields->length(), JVMCIENV);
2266+
JVMCIObjectArray array = JVMCIENV->new_FieldInfo_array(fields->length(), JVMCI_CHECK_NULL);
22672267
for (int i = 0; i < fields->length(); i++) {
22682268
JVMCIObject field_info = JVMCIENV->new_FieldInfo(fields->adr_at(i), JVMCI_CHECK_NULL);
22692269
JVMCIENV->put_object_at(array, i, field_info);
@@ -2970,13 +2970,21 @@ C2V_VMENTRY_NULL(jobject, asReflectionExecutable, (JNIEnv* env, jobject, ARGUMEN
29702970
return JNIHandles::make_local(THREAD, executable);
29712971
C2V_END
29722972

2973+
// Checks that `index` denotes a non-injected field in `klass`
29732974
static InstanceKlass* check_field(Klass* klass, jint index, JVMCI_TRAPS) {
29742975
if (!klass->is_instance_klass()) {
29752976
JVMCI_THROW_MSG_NULL(IllegalArgumentException,
29762977
err_msg("Expected non-primitive type, got %s", klass->external_name()));
29772978
}
29782979
InstanceKlass* iklass = InstanceKlass::cast(klass);
2979-
if (index < 0 || index > iklass->total_fields_count()) {
2980+
if (index < 0 || index >= iklass->java_fields_count()) {
2981+
if (index >= 0 && index < iklass->total_fields_count()) {
2982+
fieldDescriptor fd(iklass, index);
2983+
if (fd.is_injected()) {
2984+
JVMCI_THROW_MSG_NULL(IllegalArgumentException,
2985+
err_msg("Cannot get Field for injected %s.%s", klass->external_name(), fd.name()->as_C_string()));
2986+
}
2987+
}
29802988
JVMCI_THROW_MSG_NULL(IllegalArgumentException,
29812989
err_msg("Field index %d out of bounds for %s", index, klass->external_name()));
29822990
}
@@ -2986,15 +2994,15 @@ static InstanceKlass* check_field(Klass* klass, jint index, JVMCI_TRAPS) {
29862994
C2V_VMENTRY_NULL(jobject, asReflectionField, (JNIEnv* env, jobject, ARGUMENT_PAIR(klass), jint index))
29872995
requireInHotSpot("asReflectionField", JVMCI_CHECK_NULL);
29882996
Klass* klass = UNPACK_PAIR(Klass, klass);
2989-
InstanceKlass* iklass = check_field(klass, index, JVMCIENV);
2997+
InstanceKlass* iklass = check_field(klass, index, JVMCI_CHECK_NULL);
29902998
fieldDescriptor fd(iklass, index);
29912999
oop reflected = Reflection::new_field(&fd, CHECK_NULL);
29923000
return JNIHandles::make_local(THREAD, reflected);
29933001
C2V_END
29943002

29953003
static jbyteArray get_encoded_annotation_data(InstanceKlass* holder, AnnotationArray* annotations_array, bool for_class,
29963004
jint filter_length, jlong filter_klass_pointers,
2997-
JavaThread* THREAD, JVMCIEnv* JVMCIENV) {
3005+
JavaThread* THREAD, JVMCI_TRAPS) {
29983006
// Get a ConstantPool object for annotation parsing
29993007
Handle jcp = reflect_ConstantPool::create(CHECK_NULL);
30003008
reflect_ConstantPool::set_cp(jcp(), holder->constants());
@@ -3072,7 +3080,7 @@ C2V_END
30723080
C2V_VMENTRY_NULL(jbyteArray, getEncodedFieldAnnotationData, (JNIEnv* env, jobject, ARGUMENT_PAIR(klass), jint index,
30733081
jobject filter, jint filter_length, jlong filter_klass_pointers))
30743082
CompilerThreadCanCallJava canCallJava(thread, true); // Requires Java support
3075-
InstanceKlass* holder = check_field(InstanceKlass::cast(UNPACK_PAIR(Klass, klass)), index, JVMCIENV);
3083+
InstanceKlass* holder = check_field(InstanceKlass::cast(UNPACK_PAIR(Klass, klass)), index, JVMCI_CHECK_NULL);
30763084
fieldDescriptor fd(holder, index);
30773085
return get_encoded_annotation_data(holder, fd.annotations(), false, filter_length, filter_klass_pointers, THREAD, JVMCIENV);
30783086
C2V_END

‎src/hotspot/share/runtime/fieldDescriptor.hpp‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -84,6 +84,7 @@ class fieldDescriptor {
8484
bool is_static() const { return access_flags().is_static(); }
8585
bool is_final() const { return access_flags().is_final(); }
8686
bool is_stable() const { return field_flags().is_stable(); }
87+
bool is_injected() const { return field_flags().is_injected(); }
8788
bool is_volatile() const { return access_flags().is_volatile(); }
8889
bool is_transient() const { return access_flags().is_transient(); }
8990

‎src/jdk.internal.vm.ci/share/classes/jdk/vm/ci/hotspot/HotSpotJVMCIRuntime.java‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -775,8 +775,8 @@ public boolean test(ResolvedJavaType type) {
775775
* instances
776776
*/
777777
public Class<?> getMirror(ResolvedJavaType type) {
778-
if (type instanceof HotSpotResolvedJavaType && reflection instanceof HotSpotJDKReflection) {
779-
return ((HotSpotJDKReflection) reflection).getMirror((HotSpotResolvedJavaType) type);
778+
if (type instanceof HotSpotResolvedJavaType hType && reflection instanceof HotSpotJDKReflection) {
779+
return ((HotSpotJDKReflection) reflection).getMirror(hType);
780780
}
781781
return null;
782782
}
@@ -790,8 +790,8 @@ public Class<?> getMirror(ResolvedJavaType type) {
790790
* instances to {@link Executable} instances
791791
*/
792792
public Executable getMirror(ResolvedJavaMethod method) {
793-
if (method instanceof HotSpotResolvedJavaMethodImpl && reflection instanceof HotSpotJDKReflection) {
794-
return HotSpotJDKReflection.getMethod((HotSpotResolvedJavaMethodImpl) method);
793+
if (!method.isClassInitializer() && method instanceof HotSpotResolvedJavaMethodImpl hMethod && reflection instanceof HotSpotJDKReflection) {
794+
return HotSpotJDKReflection.getMethod(hMethod);
795795
}
796796
return null;
797797
}
@@ -805,8 +805,8 @@ public Executable getMirror(ResolvedJavaMethod method) {
805805
* instances
806806
*/
807807
public Field getMirror(ResolvedJavaField field) {
808-
if (field instanceof HotSpotResolvedJavaFieldImpl && reflection instanceof HotSpotJDKReflection) {
809-
return HotSpotJDKReflection.getField((HotSpotResolvedJavaFieldImpl) field);
808+
if (!field.isInternal() && field instanceof HotSpotResolvedJavaFieldImpl hField && reflection instanceof HotSpotJDKReflection) {
809+
return HotSpotJDKReflection.getField(hField);
810810
}
811811
return null;
812812
}

‎src/jdk.internal.vm.ci/share/classes/jdk/vm/ci/hotspot/HotSpotResolvedPrimitiveType.java‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -297,6 +297,16 @@ public ResolvedJavaMethod[] getDeclaredConstructors() {
297297
return new ResolvedJavaMethod[0];
298298
}
299299

300+
@Override
301+
public ResolvedJavaMethod[] getDeclaredConstructors(boolean forceLink) {
302+
return new ResolvedJavaMethod[0];
303+
}
304+
305+
@Override
306+
public ResolvedJavaMethod[] getDeclaredMethods(boolean forceLink) {
307+
return new ResolvedJavaMethod[0];
308+
}
309+
300310
@Override
301311
public ResolvedJavaMethod[] getDeclaredMethods() {
302312
return new ResolvedJavaMethod[0];

‎test/hotspot/jtreg/compiler/jvmci/jdk.vm.ci.runtime.test/src/jdk/vm/ci/runtime/test/TestResolvedJavaField.java‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@
3333
* @compile ../../../../../../../../../../../jdk/jdk/internal/vm/AnnotationEncodingDecoding/alt/MemberDeleted.java
3434
* ../../../../../../../../../../../jdk/jdk/internal/vm/AnnotationEncodingDecoding/alt/MemberTypeChanged.java
3535
* @modules jdk.internal.vm.ci/jdk.vm.ci.meta
36+
* jdk.internal.vm.ci/jdk.vm.ci.hotspot
3637
* jdk.internal.vm.ci/jdk.vm.ci.runtime
3738
* jdk.internal.vm.ci/jdk.vm.ci.common
3839
* java.base/jdk.internal.reflect

‎test/hotspot/jtreg/compiler/jvmci/jdk.vm.ci.runtime.test/src/jdk/vm/ci/runtime/test/TestResolvedJavaType.java‎

Lines changed: 45 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@
3434
* ../../../../../../../../../../../jdk/jdk/internal/vm/AnnotationEncodingDecoding/alt/MemberTypeChanged.java
3535
* @modules java.base/jdk.internal.reflect
3636
* jdk.internal.vm.ci/jdk.vm.ci.meta
37+
* jdk.internal.vm.ci/jdk.vm.ci.hotspot
3738
* jdk.internal.vm.ci/jdk.vm.ci.runtime
3839
* jdk.internal.vm.ci/jdk.vm.ci.common
3940
* java.base/jdk.internal.misc
@@ -104,16 +105,25 @@
104105
import jdk.vm.ci.meta.ResolvedJavaType;
105106
import jdk.vm.ci.meta.UnresolvedJavaType;
106107
import sun.reflect.annotation.AnnotationSupport;
108+
import jdk.vm.ci.hotspot.HotSpotJVMCIRuntime;
107109

108110
/**
109111
* Tests for {@link ResolvedJavaType}.
110112
*/
111113
public class TestResolvedJavaType extends TypeUniverse {
112114
private static final Class<? extends Annotation> SIGNATURE_POLYMORPHIC_CLASS = findPolymorphicSignatureClass();
115+
private static final HotSpotJVMCIRuntime runtime = HotSpotJVMCIRuntime.runtime();
113116

114117
public TestResolvedJavaType() {
115118
}
116119

120+
@Test
121+
public void getMirrorTest() {
122+
for (ResolvedJavaType type : javaTypes) {
123+
assertEquals(type.toClassName(), runtime.getMirror(type).getName());
124+
}
125+
}
126+
117127
@Test
118128
public void equalsTest() {
119129
for (ResolvedJavaType t : javaTypes) {
@@ -935,6 +945,9 @@ public Field lookupField(Collection<Field> fields, ResolvedJavaField key) {
935945
return null;
936946
}
937947

948+
/**
949+
* Replicates the semantics of jdk.internal.reflect.Reflection#fieldFilterMap.
950+
*/
938951
private static boolean isHiddenFromReflection(ResolvedJavaField f) {
939952
if (f.getDeclaringClass().equals(metaAccess.lookupJavaType(ConstantPool.class)) && f.getName().equals("constantPoolOop")) {
940953
return true;
@@ -971,7 +984,13 @@ public void getInstanceFieldsTest() {
971984
int reflectFieldIndex = 0;
972985
for (int i = 0; i < fields.length; i++) {
973986
ResolvedJavaField field = fields[i];
974-
if (field.isInternal() || isHiddenFromReflection(field)) {
987+
var mirror = runtime.getMirror(field);
988+
if (field.isInternal()) {
989+
assertNull(field.toString(), mirror);
990+
continue;
991+
}
992+
assertNotNull(field.toString(), mirror);
993+
if (isHiddenFromReflection(field)) {
975994
continue;
976995
}
977996
Field reflectField = reflectFields.get(reflectFieldIndex++);
@@ -1002,6 +1021,8 @@ public void getStaticFieldsTest() {
10021021
assertNotNull(lookupField(actual, f));
10031022
}
10041023
for (ResolvedJavaField rf : actual) {
1024+
var mirror = runtime.getMirror(rf);
1025+
assertNotNull(rf.toString(), mirror);
10051026
if (!isHiddenFromReflection(rf)) {
10061027
assertEquals(lookupField(expected, rf) != null, !rf.isInternal());
10071028
}
@@ -1025,6 +1046,28 @@ public void getDeclaredMethodsTest() {
10251046
expected.add(resolvedMethod);
10261047
}
10271048
Set<ResolvedJavaMethod> actual = new HashSet<>(Arrays.asList(type.getDeclaredMethods()));
1049+
for (ResolvedJavaMethod method : actual) {
1050+
assertNotNull(method.toString(), runtime.getMirror(method));
1051+
}
1052+
assertEquals(expected, actual);
1053+
}
1054+
}
1055+
1056+
@Test
1057+
public void getDeclaredConstructorsTest() {
1058+
for (Class<?> c : classes) {
1059+
ResolvedJavaType type = metaAccess.lookupJavaType(c);
1060+
Constructor<?>[] raw = c.getDeclaredConstructors();
1061+
Set<ResolvedJavaMethod> expected = new HashSet<>();
1062+
for (Constructor<?> m : raw) {
1063+
ResolvedJavaMethod resolvedMethod = metaAccess.lookupJavaMethod(m);
1064+
assertNotNull(resolvedMethod);
1065+
expected.add(resolvedMethod);
1066+
}
1067+
Set<ResolvedJavaMethod> actual = new HashSet<>(Arrays.asList(type.getDeclaredConstructors()));
1068+
for (ResolvedJavaMethod method : actual) {
1069+
assertNotNull(runtime.getMirror(method));
1070+
}
10281071
assertEquals(expected, actual);
10291072
}
10301073
}
@@ -1067,6 +1110,7 @@ private static ResolvedJavaMethod getClassInitializer(Class<?> c) {
10671110
if (clinit != null) {
10681111
assertEquals(0, clinit.getAnnotations().length);
10691112
assertEquals(0, clinit.getDeclaredAnnotations().length);
1113+
assertNull(runtime.getMirror(clinit));
10701114
}
10711115
return clinit;
10721116
}
@@ -1238,7 +1282,6 @@ public void getAnnotationDataTest() throws Exception {
12381282
"initialize",
12391283
"isPrimitive",
12401284
"newArray",
1241-
"getDeclaredConstructors",
12421285
"isInitialized",
12431286
"isLinked",
12441287
"getJavaClass",

0 commit comments

Comments
 (0)