Skip to content

8362306: HotSpotJVMCIRuntime.getMirror can crash #26346

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

Closed
wants to merge 6 commits into from
Closed

8362306: HotSpotJVMCIRuntime.getMirror can crash #26346

Changes from 2 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
cca137a
fixed bug in HotSpotJVMCIRuntime.getMirror for <clinit> and injected …
dougxc Jul 16, 2025
c2c41aa
moved inject field check into check_field for benefit of get_encoded_…
dougxc Jul 16, 2025
c76efe5
fixed bug in use of JVMCI traps
dougxc Jul 16, 2025
7cf4b26
revert from JVMCI_CHECK_NULL back to JVMCIENV in final return stateme…
dougxc Jul 16, 2025
fd44d95
test that getMirror on clinit returns null
dougxc Jul 16, 2025
d62c74d
replaced use of JVMCIENV with JVMCI_CHECK_NULL in getDeclaredFieldsInfo
dougxc Jul 17, 2025
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
22 changes: 13 additions & 9 deletions src/hotspot/share/jvmci/jvmciCompilerToVM.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2984,13 +2984,21 @@ C2V_VMENTRY_NULL(jobject, asReflectionExecutable, (JNIEnv* env, jobject, ARGUMEN
return JNIHandles::make_local(THREAD, executable);
C2V_END

// Checks that `index` denotes a non-injected field in `klass`
static InstanceKlass* check_field(Klass* klass, jint index, JVMCI_TRAPS) {
if (!klass->is_instance_klass()) {
JVMCI_THROW_MSG_NULL(IllegalArgumentException,
err_msg("Expected non-primitive type, got %s", klass->external_name()));
}
InstanceKlass* iklass = InstanceKlass::cast(klass);
if (index < 0 || index >= iklass->total_fields_count()) {
if (index < 0 || index >= iklass->java_fields_count()) {
if (index >= 0 && index < iklass->total_fields_count()) {
fieldDescriptor fd(iklass, index);
if (fd.is_injected()) {
JVMCI_THROW_MSG_NULL(IllegalArgumentException,
err_msg("Cannot get Field for injected %s.%s", klass->external_name(), fd.name()->as_C_string()));
}
}
JVMCI_THROW_MSG_NULL(IllegalArgumentException,
err_msg("Field index %d out of bounds for %s", index, klass->external_name()));
}
Expand All @@ -3002,10 +3010,6 @@ C2V_VMENTRY_NULL(jobject, asReflectionField, (JNIEnv* env, jobject, ARGUMENT_PAI
Klass* klass = UNPACK_PAIR(Klass, klass);
InstanceKlass* iklass = check_field(klass, index, JVMCI_CHECK_NULL);
Copy link
Contributor

Choose a reason for hiding this comment

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

There's another copy of this idiom in getDeclaredFieldsInfo which might be worth fixing.

fieldDescriptor fd(iklass, index);
if (fd.is_injected()) {
JVMCI_THROW_MSG_NULL(IllegalArgumentException,
err_msg("Cannot get Field for injected %s.%s", klass->external_name(), fd.name()->as_C_string()));
}
oop reflected = Reflection::new_field(&fd, CHECK_NULL);
return JNIHandles::make_local(THREAD, reflected);
C2V_END
Expand Down Expand Up @@ -3077,22 +3081,22 @@ C2V_VMENTRY_NULL(jbyteArray, getEncodedClassAnnotationData, (JNIEnv* env, jobjec
jobject filter, jint filter_length, jlong filter_klass_pointers))
CompilerThreadCanCallJava canCallJava(thread, true); // Requires Java support
InstanceKlass* holder = InstanceKlass::cast(UNPACK_PAIR(Klass, klass));
return get_encoded_annotation_data(holder, holder->class_annotations(), true, filter_length, filter_klass_pointers, THREAD, JVMCIENV);
return get_encoded_annotation_data(holder, holder->class_annotations(), true, filter_length, filter_klass_pointers, THREAD, JVMCI_CHECK_NULL);
Copy link
Member

@gilles-duboscq gilles-duboscq Jul 16, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Can JVMCI_CHECK_ be used in return statements? i think the "check and return default value" part would be unreachable.

Copy link
Member Author

Choose a reason for hiding this comment

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

Indeed - I've been bitten by this before! In this case, it's harmless given that the return statement is anyway the last statement in the method. Still, I'll revert my "fix".
I'm also wondering (again?) why the C++ compiler does not warning about unreachable code.

C2V_END

C2V_VMENTRY_NULL(jbyteArray, getEncodedExecutableAnnotationData, (JNIEnv* env, jobject, ARGUMENT_PAIR(method),
jobject filter, jint filter_length, jlong filter_klass_pointers))
CompilerThreadCanCallJava canCallJava(thread, true); // Requires Java support
methodHandle method(THREAD, UNPACK_PAIR(Method, method));
return get_encoded_annotation_data(method->method_holder(), method->annotations(), false, filter_length, filter_klass_pointers, THREAD, JVMCIENV);
return get_encoded_annotation_data(method->method_holder(), method->annotations(), false, filter_length, filter_klass_pointers, THREAD, JVMCI_CHECK_NULL);
C2V_END

C2V_VMENTRY_NULL(jbyteArray, getEncodedFieldAnnotationData, (JNIEnv* env, jobject, ARGUMENT_PAIR(klass), jint index,
jobject filter, jint filter_length, jlong filter_klass_pointers))
CompilerThreadCanCallJava canCallJava(thread, true); // Requires Java support
InstanceKlass* holder = check_field(InstanceKlass::cast(UNPACK_PAIR(Klass, klass)), index, JVMCIENV);
InstanceKlass* holder = check_field(InstanceKlass::cast(UNPACK_PAIR(Klass, klass)), index, JVMCI_CHECK_NULL);
fieldDescriptor fd(holder, index);
return get_encoded_annotation_data(holder, fd.annotations(), false, filter_length, filter_klass_pointers, THREAD, JVMCIENV);
return get_encoded_annotation_data(holder, fd.annotations(), false, filter_length, filter_klass_pointers, THREAD, JVMCI_CHECK_NULL);
C2V_END

C2V_VMENTRY_NULL(jobjectArray, getFailedSpeculations, (JNIEnv* env, jobject, jlong failed_speculations_address, jobjectArray current))
Expand Down