Skip to content

8367142: Avoid InstanceKlass::cast when converting java mirror to InstanceKlass #27158

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 5 commits into from
Closed

8367142: Avoid InstanceKlass::cast when converting java mirror to InstanceKlass #27158

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
adfedb9
tmp: Clean up java mirror handling in JNI methods
iklam Sep 3, 2025
385c3ae
more fixes
iklam Sep 9, 2025
7d79588
@dholmes-ora comments - remove class_to_verify_considering_redefiniti…
iklam Sep 9, 2025
d943d2f
Merge branch 'master' into 8367142-simplify-java-mirror-handling-in-j…
iklam Sep 9, 2025
f8634ef
Removed the (jobject) version of as_Klass/as_InstanceKlass
iklam Sep 10, 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
4 changes: 2 additions & 2 deletions src/hotspot/share/cds/aotMetaspace.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -784,7 +784,7 @@ void AOTMetaspace::link_shared_classes(TRAPS) {
const GrowableArray<OopHandle>* mirrors = collect_classes.mirrors();
for (int i = 0; i < mirrors->length(); i++) {
OopHandle mirror = mirrors->at(i);
InstanceKlass* ik = InstanceKlass::cast(java_lang_Class::as_Klass(mirror.resolve()));
InstanceKlass* ik = java_lang_Class::as_InstanceKlass(mirror.resolve());
if (may_be_eagerly_linked(ik)) {
has_linked |= try_link_class(THREAD, ik);
}
Expand All @@ -804,7 +804,7 @@ void AOTMetaspace::link_shared_classes(TRAPS) {
const GrowableArray<OopHandle>* mirrors = collect_classes.mirrors();
for (int i = 0; i < mirrors->length(); i++) {
OopHandle mirror = mirrors->at(i);
InstanceKlass* ik = InstanceKlass::cast(java_lang_Class::as_Klass(mirror.resolve()));
InstanceKlass* ik = java_lang_Class::as_InstanceKlass(mirror.resolve());
AOTConstantPoolResolver::preresolve_string_cp_entries(ik, CHECK);
}
}
Expand Down
2 changes: 1 addition & 1 deletion src/hotspot/share/cds/unregisteredClasses.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ InstanceKlass* UnregisteredClasses::load_class(Symbol* name, const char* path, T
CHECK_NULL);
assert(result.get_type() == T_OBJECT, "just checking");

return InstanceKlass::cast(java_lang_Class::as_Klass(result.get_oop()));
return java_lang_Class::as_InstanceKlass(result.get_oop());
}

bool UnregisteredClasses::check_for_exclusion(const InstanceKlass* k) {
Expand Down
27 changes: 12 additions & 15 deletions src/hotspot/share/classfile/javaClasses.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -990,7 +990,7 @@ void java_lang_Class::fixup_mirror(Klass* k, TRAPS) {
create_mirror(k, Handle(), Handle(), Handle(), Handle(), CHECK);
}

void java_lang_Class::initialize_mirror_fields(Klass* k,
void java_lang_Class::initialize_mirror_fields(InstanceKlass* ik,
Handle mirror,
Handle protection_domain,
Handle classData,
Expand All @@ -1005,7 +1005,7 @@ void java_lang_Class::initialize_mirror_fields(Klass* k,
set_protection_domain(mirror(), protection_domain());

// Initialize static fields
InstanceKlass::cast(k)->do_local_static_fields(&initialize_static_field, mirror, CHECK);
ik->do_local_static_fields(&initialize_static_field, mirror, CHECK);

// Set classData
set_class_data(mirror(), classData());
Expand Down Expand Up @@ -1111,8 +1111,7 @@ void java_lang_Class::allocate_mirror(Klass* k, bool is_scratch, Handle protecti
// and java_mirror in this klass.
} else {
assert(k->is_instance_klass(), "Must be");

initialize_mirror_fields(k, mirror, protection_domain, classData, THREAD);
initialize_mirror_fields(InstanceKlass::cast(k), mirror, protection_domain, classData, THREAD);
if (HAS_PENDING_EXCEPTION) {
// If any of the fields throws an exception like OOM remove the klass field
// from the mirror so GC doesn't follow it after the klass has been deallocated.
Expand Down Expand Up @@ -2590,7 +2589,7 @@ static void print_stack_element_to_stream(outputStream* st, Handle mirror, int m
ResourceMark rm;
stringStream ss;

InstanceKlass* holder = InstanceKlass::cast(java_lang_Class::as_Klass(mirror()));
InstanceKlass* holder = java_lang_Class::as_InstanceKlass(mirror());
const char* klass_name = holder->external_name();
char* method_name = name->as_C_string();
ss.print("\tat %s.%s(", klass_name, method_name);
Expand Down Expand Up @@ -2969,7 +2968,7 @@ void java_lang_Throwable::get_stack_trace_elements(int depth, Handle backtrace,
THROW(vmSymbols::java_lang_NullPointerException());
}

InstanceKlass* holder = InstanceKlass::cast(java_lang_Class::as_Klass(bte._mirror()));
InstanceKlass* holder = java_lang_Class::as_InstanceKlass(bte._mirror());
methodHandle method (THREAD, holder->method_with_orig_idnum(bte._method_id, bte._version));

java_lang_StackTraceElement::fill_in(stack_trace_element, holder,
Expand Down Expand Up @@ -3055,7 +3054,7 @@ bool java_lang_Throwable::get_top_method_and_bci(oop throwable, Method** method,
// Get first backtrace element.
BacktraceElement bte = iter.next(current);

InstanceKlass* holder = InstanceKlass::cast(java_lang_Class::as_Klass(bte._mirror()));
InstanceKlass* holder = java_lang_Class::as_InstanceKlass(bte._mirror());
assert(holder != nullptr, "first element should be non-null");
Method* m = holder->method_with_orig_idnum(bte._method_id, bte._version);

Expand Down Expand Up @@ -3441,11 +3440,11 @@ void java_lang_reflect_Method::serialize_offsets(SerializeClosure* f) {

Handle java_lang_reflect_Method::create(TRAPS) {
assert(Universe::is_fully_initialized(), "Need to find another solution to the reflection problem");
Klass* klass = vmClasses::reflect_Method_klass();
InstanceKlass* klass = vmClasses::reflect_Method_klass();
// This class is eagerly initialized during VM initialization, since we keep a reference
// to one of the methods
assert(InstanceKlass::cast(klass)->is_initialized(), "must be initialized");
return InstanceKlass::cast(klass)->allocate_instance_handle(THREAD);
assert(klass->is_initialized(), "must be initialized");
return klass->allocate_instance_handle(THREAD);
}

oop java_lang_reflect_Method::clazz(oop reflect) {
Expand Down Expand Up @@ -3914,17 +3913,15 @@ void reflect_ConstantPool::set_cp(oop reflect, ConstantPool* value) {
}

ConstantPool* reflect_ConstantPool::get_cp(oop reflect) {

oop mirror = reflect->obj_field(_oop_offset);
Klass* k = java_lang_Class::as_Klass(mirror);
assert(k->is_instance_klass(), "Must be");
InstanceKlass* ik = java_lang_Class::as_InstanceKlass(mirror);

// Get the constant pool back from the klass. Since class redefinition
// merges the new constant pool into the old, this is essentially the
// same constant pool as the original. If constant pool merging is
// no longer done in the future, this will have to change to save
// the original.
return InstanceKlass::cast(k)->constants();
return ik->constants();
}


Expand Down Expand Up @@ -5531,7 +5528,7 @@ void JavaClasses::check_offsets() {
#endif // PRODUCT

int InjectedField::compute_offset() {
InstanceKlass* ik = InstanceKlass::cast(klass());
InstanceKlass* ik = klass();
for (AllFieldStream fs(ik); !fs.done(); fs.next()) {
if (!may_be_java && !fs.field_flags().is_injected()) {
// Only look at injected fields
Expand Down
12 changes: 7 additions & 5 deletions src/hotspot/share/classfile/javaClasses.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -269,7 +269,7 @@ class java_lang_Class : AllStatic {
static void set_protection_domain(oop java_class, oop protection_domain);
static void set_class_loader(oop java_class, oop class_loader);
static void set_component_mirror(oop java_class, oop comp_mirror);
static void initialize_mirror_fields(Klass* k, Handle mirror, Handle protection_domain,
static void initialize_mirror_fields(InstanceKlass* ik, Handle mirror, Handle protection_domain,
Handle classData, TRAPS);
static void set_mirror_module_field(JavaThread* current, Klass* K, Handle mirror, Handle module);
public:
Expand All @@ -293,8 +293,10 @@ class java_lang_Class : AllStatic {

static void fixup_module_field(Klass* k, Handle module);

// Conversion
// Conversion -- java_class must not be null. The return value is null only if java_class is a primitive type.
static Klass* as_Klass(oop java_class);
static InstanceKlass* as_InstanceKlass(oop java_class);

static void set_klass(oop java_class, Klass* klass);
static BasicType as_BasicType(oop java_class, Klass** reference_klass = nullptr);
static Symbol* as_signature(oop java_class, bool intern_if_not_found);
Expand Down Expand Up @@ -1895,11 +1897,11 @@ class InjectedField {
const vmClassID klass_id;
const vmSymbolID name_index;
const vmSymbolID signature_index;
const bool may_be_java;
const bool may_be_java;


Klass* klass() const { return vmClasses::klass_at(klass_id); }
Symbol* name() const { return lookup_symbol(name_index); }
InstanceKlass* klass() const { return vmClasses::klass_at(klass_id); }
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you fix the indentation?

Copy link
Member Author

Choose a reason for hiding this comment

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

I removed all indentation alignments from this class, as they no longer seem warranted.

Symbol* name() const { return lookup_symbol(name_index); }
Symbol* signature() const { return lookup_symbol(signature_index); }

int compute_offset();
Expand Down
6 changes: 6 additions & 0 deletions src/hotspot/share/classfile/javaClasses.inline.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -291,6 +291,12 @@ inline Klass* java_lang_Class::as_Klass(oop java_class) {
return k;
}

inline InstanceKlass* java_lang_Class::as_InstanceKlass(oop java_class) {
Klass* k = as_Klass(java_class);
assert(k == nullptr || k->is_instance_klass(), "type check");
return static_cast<InstanceKlass*>(k);
}

inline bool java_lang_Class::is_primitive(oop java_class) {
// should assert:
// assert(java_lang_Class::is_instance(java_class), "must be a Class object");
Expand Down
4 changes: 2 additions & 2 deletions src/hotspot/share/classfile/systemDictionary.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1277,10 +1277,10 @@ InstanceKlass* SystemDictionary::load_instance_class_impl(Symbol* class_name, Ha
assert(result.get_type() == T_OBJECT, "just checking");
oop obj = result.get_oop();

// Primitive classes return null since forName() can not be
// Primitive classes return null since forName() cannot be
// used to obtain any of the Class objects representing primitives or void
if ((obj != nullptr) && !(java_lang_Class::is_primitive(obj))) {
InstanceKlass* k = InstanceKlass::cast(java_lang_Class::as_Klass(obj));
InstanceKlass* k = java_lang_Class::as_InstanceKlass(obj);
// For user defined Java class loaders, check that the name returned is
// the same as that requested. This check is done for the bootstrap
// loader when parsing the class file.
Expand Down
2 changes: 1 addition & 1 deletion src/hotspot/share/jfr/leakprofiler/chains/edgeUtils.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ const Symbol* EdgeUtils::field_name(const Edge& edge, jshort* modifiers) {
if (is_static_field(ref_owner, ik, offset)) {
assert(ik->is_mirror_instance_klass(), "invariant");
assert(java_lang_Class::as_Klass(ref_owner)->is_instance_klass(), "invariant");
ik = InstanceKlass::cast(java_lang_Class::as_Klass(ref_owner));
ik = java_lang_Class::as_InstanceKlass(ref_owner);
}
while (ik != nullptr) {
JavaFieldStream jfs(ik);
Expand Down
2 changes: 1 addition & 1 deletion src/hotspot/share/jvmci/jvmciRuntime.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -187,7 +187,7 @@ JRT_ENTRY(void, JVMCIRuntime::dynamic_new_array_or_null(JavaThread* current, oop
JRT_END

JRT_ENTRY(void, JVMCIRuntime::dynamic_new_instance_or_null(JavaThread* current, oopDesc* type_mirror))
InstanceKlass* klass = InstanceKlass::cast(java_lang_Class::as_Klass(type_mirror));
InstanceKlass* klass = java_lang_Class::as_InstanceKlass(type_mirror);

if (klass == nullptr) {
ResourceMark rm(current);
Expand Down
2 changes: 1 addition & 1 deletion src/hotspot/share/prims/jni.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -526,7 +526,7 @@ JNI_ENTRY(jint, jni_ThrowNew(JNIEnv *env, jclass clazz, const char *message))
jint ret = JNI_OK;
DT_RETURN_MARK(ThrowNew, jint, (const jint&)ret);

InstanceKlass* k = InstanceKlass::cast(java_lang_Class::as_Klass(JNIHandles::resolve_non_null(clazz)));
InstanceKlass* k = java_lang_Class::as_InstanceKlass(JNIHandles::resolve_non_null(clazz));
Symbol* name = k->name();
Handle class_loader (THREAD, k->class_loader());
THROW_MSG_LOADER_(name, (char *)message, class_loader, JNI_OK);
Expand Down
Loading