Skip to content

Commit

Permalink
8336468: Reflection and MethodHandles should use more precise initial…
Browse files Browse the repository at this point in the history
…izer checks

Reviewed-by: liach, coleenp
  • Loading branch information
shipilev committed Sep 26, 2024
1 parent e36ce5f commit 376056c
Show file tree
Hide file tree
Showing 5 changed files with 30 additions and 25 deletions.
3 changes: 2 additions & 1 deletion src/hotspot/share/classfile/javaClasses.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3052,9 +3052,10 @@ void java_lang_ClassFrameInfo::serialize_offsets(SerializeClosure* f) {

static int get_flags(const methodHandle& m) {
int flags = (jushort)( m->access_flags().as_short() & JVM_RECOGNIZED_METHOD_MODIFIERS );
if (m->is_initializer()) {
if (m->is_object_initializer()) {
flags |= java_lang_invoke_MemberName::MN_IS_CONSTRUCTOR;
} else {
// Note: Static initializers can be here. Record them as plain methods.
flags |= java_lang_invoke_MemberName::MN_IS_METHOD;
}
if (m->caller_sensitive()) {
Expand Down
4 changes: 3 additions & 1 deletion src/hotspot/share/prims/jni.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -444,9 +444,11 @@ JNI_ENTRY(jobject, jni_ToReflectedMethod(JNIEnv *env, jclass cls, jmethodID meth
methodHandle m (THREAD, Method::resolve_jmethod_id(method_id));
assert(m->is_static() == (isStatic != 0), "jni_ToReflectedMethod access flags doesn't match");
oop reflection_method;
if (m->is_initializer()) {
if (m->is_object_initializer()) {
reflection_method = Reflection::new_constructor(m, CHECK_NULL);
} else {
// Note: Static initializers can theoretically be here, if JNI users manage
// to get their jmethodID. Record them as plain methods.
reflection_method = Reflection::new_method(m, false, CHECK_NULL);
}
ret = JNIHandles::make_local(THREAD, reflection_method);
Expand Down
35 changes: 18 additions & 17 deletions src/hotspot/share/prims/jvm.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1826,14 +1826,6 @@ JVM_ENTRY(jobjectArray, JVM_GetRecordComponents(JNIEnv* env, jclass ofClass))
}
JVM_END

static bool select_method(const methodHandle& method, bool want_constructor) {
if (want_constructor) {
return (method->is_initializer() && !method->is_static());
} else {
return (!method->is_initializer() && !method->is_overpass());
}
}

static jobjectArray get_class_declared_methods_helper(
JNIEnv *env,
jclass ofClass, jboolean publicOnly,
Expand Down Expand Up @@ -1866,14 +1858,22 @@ static jobjectArray get_class_declared_methods_helper(
GrowableArray<int>* idnums = new GrowableArray<int>(methods_length);
int num_methods = 0;

// Select methods matching the criteria.
for (int i = 0; i < methods_length; i++) {
methodHandle method(THREAD, methods->at(i));
if (select_method(method, want_constructor)) {
if (!publicOnly || method->is_public()) {
idnums->push(method->method_idnum());
++num_methods;
}
Method* method = methods->at(i);
if (want_constructor && !method->is_object_initializer()) {
continue;
}
if (!want_constructor &&
(method->is_object_initializer() || method->is_static_initializer() ||
method->is_overpass())) {
continue;
}
if (publicOnly && !method->is_public()) {
continue;
}
idnums->push(method->method_idnum());
++num_methods;
}

// Allocate result
Expand Down Expand Up @@ -2175,10 +2175,11 @@ static jobject get_method_at_helper(const constantPoolHandle& cp, jint index, bo
THROW_MSG_NULL(vmSymbols::java_lang_RuntimeException(), "Unable to look up method in target class");
}
oop method;
if (!m->is_initializer() || m->is_static()) {
method = Reflection::new_method(m, true, CHECK_NULL);
} else {
if (m->is_object_initializer()) {
method = Reflection::new_constructor(m, CHECK_NULL);
} else {
// new_method accepts <clinit> as Method here
method = Reflection::new_method(m, true, CHECK_NULL);
}
return JNIHandles::make_local(THREAD, method);
}
Expand Down
3 changes: 2 additions & 1 deletion src/hotspot/share/prims/methodHandles.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -313,8 +313,9 @@ oop MethodHandles::init_method_MemberName(Handle mname, CallInfo& info) {
case CallInfo::direct_call:
vmindex = Method::nonvirtual_vtable_index;
if (m->is_static()) {
assert(!m->is_static_initializer(), "Cannot be static initializer");
flags |= IS_METHOD | (JVM_REF_invokeStatic << REFERENCE_KIND_SHIFT);
} else if (m->is_initializer()) {
} else if (m->is_object_initializer()) {
flags |= IS_CONSTRUCTOR | (JVM_REF_invokeSpecial << REFERENCE_KIND_SHIFT);
} else {
// "special" reflects that this is a direct call, not that it
Expand Down
10 changes: 5 additions & 5 deletions src/hotspot/share/runtime/reflection.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -766,10 +766,10 @@ static Handle new_type(Symbol* signature, Klass* k, TRAPS) {
}

oop Reflection::new_method(const methodHandle& method, bool for_constant_pool_access, TRAPS) {
// Allow sun.reflect.ConstantPool to refer to <clinit> methods as java.lang.reflect.Methods.
assert(!method()->is_initializer() ||
(for_constant_pool_access && method()->is_static()),
"should call new_constructor instead");
// Allow jdk.internal.reflect.ConstantPool to refer to <clinit> methods as java.lang.reflect.Methods.
assert(!method()->is_object_initializer() &&
(for_constant_pool_access || !method()->is_static_initializer()),
"Should not be the initializer");
InstanceKlass* holder = method->method_holder();
int slot = method->method_idnum();

Expand Down Expand Up @@ -817,7 +817,7 @@ oop Reflection::new_method(const methodHandle& method, bool for_constant_pool_ac


oop Reflection::new_constructor(const methodHandle& method, TRAPS) {
assert(method()->is_initializer(), "should call new_method instead");
assert(method()->is_object_initializer(), "Should be the initializer");

InstanceKlass* holder = method->method_holder();
int slot = method->method_idnum();
Expand Down

0 comments on commit 376056c

Please sign in to comment.