From c35e58a5adf06e25a3b482e2be384af95a84f11a Mon Sep 17 00:00:00 2001 From: Ioi Lam Date: Thu, 27 Jun 2024 20:10:13 +0000 Subject: [PATCH] 8309634: Resolve CONSTANT_MethodRef at CDS dump time Reviewed-by: matsaave, ccheung --- src/hotspot/share/cds/classListParser.cpp | 2 + src/hotspot/share/cds/classListWriter.cpp | 21 +++- src/hotspot/share/cds/classPrelinker.cpp | 18 ++- src/hotspot/share/cds/dumpAllocStats.cpp | 4 + src/hotspot/share/cds/dumpAllocStats.hpp | 12 ++ .../share/interpreter/interpreterRuntime.cpp | 36 +++++- .../share/interpreter/interpreterRuntime.hpp | 7 +- .../share/interpreter/linkResolver.cpp | 106 +++++++++++++----- .../share/interpreter/linkResolver.hpp | 10 +- src/hotspot/share/oops/cpCache.cpp | 95 +++++++++++++++- src/hotspot/share/oops/cpCache.hpp | 4 +- .../share/oops/resolvedMethodEntry.hpp | 25 +++++ .../resolvedConstants/ResolvedConstants.java | 52 ++++++++- 13 files changed, 349 insertions(+), 43 deletions(-) diff --git a/src/hotspot/share/cds/classListParser.cpp b/src/hotspot/share/cds/classListParser.cpp index 9ee5f25aa89..b2695ac2e78 100644 --- a/src/hotspot/share/cds/classListParser.cpp +++ b/src/hotspot/share/cds/classListParser.cpp @@ -830,6 +830,8 @@ void ClassListParser::parse_constant_pool_tag() { // ignore break; case JVM_CONSTANT_Fieldref: + case JVM_CONSTANT_Methodref: + case JVM_CONSTANT_InterfaceMethodref: preresolve_fmi = true; break; break; diff --git a/src/hotspot/share/cds/classListWriter.cpp b/src/hotspot/share/cds/classListWriter.cpp index 97f0bc3476e..78cd092445b 100644 --- a/src/hotspot/share/cds/classListWriter.cpp +++ b/src/hotspot/share/cds/classListWriter.cpp @@ -258,15 +258,26 @@ void ClassListWriter::write_resolved_constants_for(InstanceKlass* ik) { if (field_entries != nullptr) { for (int i = 0; i < field_entries->length(); i++) { ResolvedFieldEntry* rfe = field_entries->adr_at(i); - if (rfe->is_resolved(Bytecodes::_getstatic) || - rfe->is_resolved(Bytecodes::_putstatic) || - rfe->is_resolved(Bytecodes::_getfield) || + if (rfe->is_resolved(Bytecodes::_getfield) || rfe->is_resolved(Bytecodes::_putfield)) { list.at_put(rfe->constant_pool_index(), true); print = true; } } } + + Array* method_entries = cp->cache()->resolved_method_entries(); + if (method_entries != nullptr) { + for (int i = 0; i < method_entries->length(); i++) { + ResolvedMethodEntry* rme = method_entries->adr_at(i); + if (rme->is_resolved(Bytecodes::_invokevirtual) || + rme->is_resolved(Bytecodes::_invokespecial) || + rme->is_resolved(Bytecodes::_invokeinterface)) { + list.at_put(rme->constant_pool_index(), true); + print = true; + } + } + } } if (print) { @@ -276,7 +287,9 @@ void ClassListWriter::write_resolved_constants_for(InstanceKlass* ik) { if (list.at(i)) { constantTag cp_tag = cp->tag_at(i).value(); assert(cp_tag.value() == JVM_CONSTANT_Class || - cp_tag.value() == JVM_CONSTANT_Fieldref, "sanity"); + cp_tag.value() == JVM_CONSTANT_Fieldref || + cp_tag.value() == JVM_CONSTANT_Methodref|| + cp_tag.value() == JVM_CONSTANT_InterfaceMethodref, "sanity"); stream->print(" %d", i); } } diff --git a/src/hotspot/share/cds/classPrelinker.cpp b/src/hotspot/share/cds/classPrelinker.cpp index 223d3937f93..6b866bac995 100644 --- a/src/hotspot/share/cds/classPrelinker.cpp +++ b/src/hotspot/share/cds/classPrelinker.cpp @@ -89,7 +89,9 @@ bool ClassPrelinker::is_resolution_deterministic(ConstantPool* cp, int cp_index) // currently archive only CP entries that are already resolved. Klass* resolved_klass = cp->resolved_klass_at(cp_index); return resolved_klass != nullptr && is_class_resolution_deterministic(cp->pool_holder(), resolved_klass); - } else if (cp->tag_at(cp_index).is_field()) { + } else if (cp->tag_at(cp_index).is_field() || + cp->tag_at(cp_index).is_method() || + cp->tag_at(cp_index).is_interface_method()) { int klass_cp_index = cp->uncached_klass_ref_index_at(cp_index); if (!cp->tag_at(klass_cp_index).is_klass()) { // Not yet resolved @@ -263,6 +265,14 @@ void ClassPrelinker::preresolve_field_and_method_cp_entries(JavaThread* current, CLEAR_PENDING_EXCEPTION; // just ignore } break; + case Bytecodes::_invokespecial: + case Bytecodes::_invokevirtual: + case Bytecodes::_invokeinterface: + maybe_resolve_fmi_ref(ik, m, raw_bc, bcs.get_index_u2(), preresolve_list, THREAD); + if (HAS_PENDING_EXCEPTION) { + CLEAR_PENDING_EXCEPTION; // just ignore + } + break; default: break; } @@ -301,6 +311,12 @@ void ClassPrelinker::maybe_resolve_fmi_ref(InstanceKlass* ik, Method* m, Bytecod InterpreterRuntime::resolve_get_put(bc, raw_index, mh, cp, false /*initialize_holder*/, CHECK); break; + case Bytecodes::_invokevirtual: + case Bytecodes::_invokespecial: + case Bytecodes::_invokeinterface: + InterpreterRuntime::cds_resolve_invoke(bc, raw_index, cp, CHECK); + break; + default: ShouldNotReachHere(); } diff --git a/src/hotspot/share/cds/dumpAllocStats.cpp b/src/hotspot/share/cds/dumpAllocStats.cpp index 30ef1597063..a1dac41e322 100644 --- a/src/hotspot/share/cds/dumpAllocStats.cpp +++ b/src/hotspot/share/cds/dumpAllocStats.cpp @@ -110,4 +110,8 @@ void DumpAllocStats::print_stats(int ro_all, int rw_all) { _num_field_cp_entries, _num_field_cp_entries_archived, percent_of(_num_field_cp_entries_archived, _num_field_cp_entries), _num_field_cp_entries_reverted); + msg.info("Method CP entries = %6d, archived = %6d (%5.1f%%), reverted = %6d", + _num_method_cp_entries, _num_method_cp_entries_archived, + percent_of(_num_method_cp_entries_archived, _num_method_cp_entries), + _num_method_cp_entries_reverted); } diff --git a/src/hotspot/share/cds/dumpAllocStats.hpp b/src/hotspot/share/cds/dumpAllocStats.hpp index 424a98aa738..0332be73120 100644 --- a/src/hotspot/share/cds/dumpAllocStats.hpp +++ b/src/hotspot/share/cds/dumpAllocStats.hpp @@ -71,6 +71,9 @@ public: int _num_klass_cp_entries; int _num_klass_cp_entries_archived; int _num_klass_cp_entries_reverted; + int _num_method_cp_entries; + int _num_method_cp_entries_archived; + int _num_method_cp_entries_reverted; public: enum { RO = 0, RW = 1 }; @@ -84,6 +87,9 @@ public: _num_klass_cp_entries = 0; _num_klass_cp_entries_archived = 0; _num_klass_cp_entries_reverted = 0; + _num_method_cp_entries = 0; + _num_method_cp_entries_archived = 0; + _num_method_cp_entries_reverted = 0; }; CompactHashtableStats* symbol_stats() { return &_symbol_stats; } @@ -122,6 +128,12 @@ public: _num_klass_cp_entries_reverted += reverted ? 1 : 0; } + void record_method_cp_entry(bool archived, bool reverted) { + _num_method_cp_entries ++; + _num_method_cp_entries_archived += archived ? 1 : 0; + _num_method_cp_entries_reverted += reverted ? 1 : 0; + } + void print_stats(int ro_all, int rw_all); }; diff --git a/src/hotspot/share/interpreter/interpreterRuntime.cpp b/src/hotspot/share/interpreter/interpreterRuntime.cpp index fb779b039f4..4f2eae023f6 100644 --- a/src/hotspot/share/interpreter/interpreterRuntime.cpp +++ b/src/hotspot/share/interpreter/interpreterRuntime.cpp @@ -832,7 +832,6 @@ void InterpreterRuntime::resolve_invoke(JavaThread* current, Bytecodes::Code byt // resolve method CallInfo info; constantPoolHandle pool(current, last_frame.method()->constants()); - ConstantPoolCache* cache = pool->cache(); methodHandle resolved_method; @@ -857,10 +856,18 @@ void InterpreterRuntime::resolve_invoke(JavaThread* current, Bytecodes::Code byt resolved_method = methodHandle(current, info.resolved_method()); } // end JvmtiHideSingleStepping + update_invoke_cp_cache_entry(info, bytecode, resolved_method, pool, method_index); +} + +void InterpreterRuntime::update_invoke_cp_cache_entry(CallInfo& info, Bytecodes::Code bytecode, + methodHandle& resolved_method, + constantPoolHandle& pool, + int method_index) { // Don't allow safepoints until the method is cached. NoSafepointVerifier nsv; // check if link resolution caused cpCache to be updated + ConstantPoolCache* cache = pool->cache(); if (cache->resolved_method_entry_at(method_index)->is_resolved(bytecode)) return; #ifdef ASSERT @@ -912,6 +919,33 @@ void InterpreterRuntime::resolve_invoke(JavaThread* current, Bytecodes::Code byt } } +void InterpreterRuntime::cds_resolve_invoke(Bytecodes::Code bytecode, int method_index, + constantPoolHandle& pool, TRAPS) { + LinkInfo link_info(pool, method_index, bytecode, CHECK); + + if (!link_info.resolved_klass()->is_instance_klass() || InstanceKlass::cast(link_info.resolved_klass())->is_linked()) { + CallInfo call_info; + switch (bytecode) { + case Bytecodes::_invokevirtual: LinkResolver::cds_resolve_virtual_call (call_info, link_info, CHECK); break; + case Bytecodes::_invokeinterface: LinkResolver::cds_resolve_interface_call(call_info, link_info, CHECK); break; + case Bytecodes::_invokespecial: LinkResolver::cds_resolve_special_call (call_info, link_info, CHECK); break; + + default: fatal("Unimplemented: %s", Bytecodes::name(bytecode)); + } + methodHandle resolved_method(THREAD, call_info.resolved_method()); + guarantee(resolved_method->method_holder()->is_linked(), ""); + update_invoke_cp_cache_entry(call_info, bytecode, resolved_method, pool, method_index); + } else { + // FIXME: why a shared class is not linked yet? + // Can't link it here since there are no guarantees it'll be prelinked on the next run. + ResourceMark rm; + InstanceKlass* resolved_iklass = InstanceKlass::cast(link_info.resolved_klass()); + log_info(cds, resolve)("Not resolved: class not linked: %s %s %s", + resolved_iklass->is_shared() ? "is_shared" : "", + resolved_iklass->init_state_name(), + resolved_iklass->external_name()); + } +} // First time execution: Resolve symbols, create a permanent MethodType object. void InterpreterRuntime::resolve_invokehandle(JavaThread* current) { diff --git a/src/hotspot/share/interpreter/interpreterRuntime.hpp b/src/hotspot/share/interpreter/interpreterRuntime.hpp index 3a8db1363df..61041694fc6 100644 --- a/src/hotspot/share/interpreter/interpreterRuntime.hpp +++ b/src/hotspot/share/interpreter/interpreterRuntime.hpp @@ -92,9 +92,11 @@ class InterpreterRuntime: AllStatic { static void resolve_from_cache(JavaThread* current, Bytecodes::Code bytecode); - // Used by ClassListParser. + // Used by ClassPrelinker static void resolve_get_put(Bytecodes::Code bytecode, int field_index, methodHandle& m, constantPoolHandle& pool, bool initialize_holder, TRAPS); + static void cds_resolve_invoke(Bytecodes::Code bytecode, int method_index, + constantPoolHandle& pool, TRAPS); private: // Statics & fields @@ -105,6 +107,9 @@ private: static void resolve_invokehandle (JavaThread* current); static void resolve_invokedynamic(JavaThread* current); + static void update_invoke_cp_cache_entry(CallInfo& info, Bytecodes::Code bytecode, + methodHandle& resolved_method, + constantPoolHandle& pool, int method_index); public: // Synchronization static void monitorenter(JavaThread* current, BasicObjectLock* elem); diff --git a/src/hotspot/share/interpreter/linkResolver.cpp b/src/hotspot/share/interpreter/linkResolver.cpp index 2c7decfa714..cadc3e8a2e8 100644 --- a/src/hotspot/share/interpreter/linkResolver.cpp +++ b/src/hotspot/share/interpreter/linkResolver.cpp @@ -142,7 +142,9 @@ void CallInfo::set_common(Klass* resolved_klass, CallKind kind, int index, TRAPS) { - assert(resolved_method->signature() == selected_method->signature(), "signatures must correspond"); + if (selected_method.not_null()) { + assert(resolved_method->signature() == selected_method->signature(), "signatures must correspond"); + } _resolved_klass = resolved_klass; _resolved_method = resolved_method; _selected_method = selected_method; @@ -151,7 +153,9 @@ void CallInfo::set_common(Klass* resolved_klass, _resolved_appendix = Handle(); DEBUG_ONLY(verify()); // verify before making side effects - CompilationPolicy::compile_if_required(selected_method, THREAD); + if (selected_method.not_null()) { + CompilationPolicy::compile_if_required(selected_method, THREAD); + } } // utility query for unreflecting a method @@ -1152,6 +1156,10 @@ void LinkResolver::resolve_special_call(CallInfo& result, runtime_resolve_special_method(result, link_info, methodHandle(THREAD, resolved_method), recv, CHECK); } +void LinkResolver::cds_resolve_special_call(CallInfo& result, const LinkInfo& link_info, TRAPS) { + resolve_special_call(result, Handle(), link_info, CHECK); +} + // throws linktime exceptions Method* LinkResolver::linktime_resolve_special_method(const LinkInfo& link_info, TRAPS) { @@ -1333,7 +1341,17 @@ void LinkResolver::resolve_virtual_call(CallInfo& result, Handle recv, Klass* re runtime_resolve_virtual_method(result, methodHandle(THREAD, resolved_method), link_info.resolved_klass(), recv, receiver_klass, - check_null_and_abstract, CHECK); + check_null_and_abstract, + /*is_abstract_interpretation*/ false, CHECK); +} + +void LinkResolver::cds_resolve_virtual_call(CallInfo& result, const LinkInfo& link_info, TRAPS) { + Method* resolved_method = linktime_resolve_virtual_method(link_info, CHECK); + runtime_resolve_virtual_method(result, methodHandle(THREAD, resolved_method), + link_info.resolved_klass(), + Handle(), nullptr, + /*check_null_and_abstract*/ false, + /*is_abstract_interpretation*/ true, CHECK); } // throws linktime exceptions @@ -1385,7 +1403,11 @@ void LinkResolver::runtime_resolve_virtual_method(CallInfo& result, Handle recv, Klass* recv_klass, bool check_null_and_abstract, + bool is_abstract_interpretation, TRAPS) { + // is_abstract_interpretation is true IFF CDS is resolving method references without + // running any actual bytecode. Therefore, we don't have an actual recv/recv_klass, so + // we cannot check the actual selected_method (which is not needed by CDS anyway). // setup default return values int vtable_index = Method::invalid_vtable_index; @@ -1406,7 +1428,9 @@ void LinkResolver::runtime_resolve_virtual_method(CallInfo& result, vtable_index = vtable_index_of_interface_method(resolved_klass, resolved_method); assert(vtable_index >= 0 , "we should have valid vtable index at this point"); - selected_method = methodHandle(THREAD, recv_klass->method_at_vtable(vtable_index)); + if (!is_abstract_interpretation) { + selected_method = methodHandle(THREAD, recv_klass->method_at_vtable(vtable_index)); + } } else { // at this point we are sure that resolved_method is virtual and not // a default or miranda method; therefore, it must have a valid vtable index. @@ -1420,31 +1444,40 @@ void LinkResolver::runtime_resolve_virtual_method(CallInfo& result, // resolved method, and it can never be changed by an override. if (vtable_index == Method::nonvirtual_vtable_index) { assert(resolved_method->can_be_statically_bound(), "cannot override this method"); - selected_method = resolved_method; + if (!is_abstract_interpretation) { + selected_method = resolved_method; + } } else { - selected_method = methodHandle(THREAD, recv_klass->method_at_vtable(vtable_index)); + if (!is_abstract_interpretation) { + selected_method = methodHandle(THREAD, recv_klass->method_at_vtable(vtable_index)); + } } } - // check if method exists - if (selected_method.is_null()) { - throw_abstract_method_error(resolved_method, recv_klass, CHECK); + if (!is_abstract_interpretation) { + // check if method exists + if (selected_method.is_null()) { + throw_abstract_method_error(resolved_method, recv_klass, CHECK); + } + + // check if abstract + if (check_null_and_abstract && selected_method->is_abstract()) { + // Pass arguments for generating a verbose error message. + throw_abstract_method_error(resolved_method, selected_method, recv_klass, CHECK); + } + + if (log_develop_is_enabled(Trace, vtables)) { + trace_method_resolution("invokevirtual selected method: receiver-class:", + recv_klass, resolved_klass, selected_method(), + false, vtable_index); + } } - // check if abstract - if (check_null_and_abstract && selected_method->is_abstract()) { - // Pass arguments for generating a verbose error message. - throw_abstract_method_error(resolved_method, selected_method, recv_klass, CHECK); - } - - if (log_develop_is_enabled(Trace, vtables)) { - trace_method_resolution("invokevirtual selected method: receiver-class:", - recv_klass, resolved_klass, selected_method(), - false, vtable_index); - } // setup result result.set_virtual(resolved_klass, resolved_method, selected_method, vtable_index, CHECK); - JFR_ONLY(Jfr::on_resolution(result, CHECK);) + if (selected_method.not_null()) { + JFR_ONLY(Jfr::on_resolution(result, CHECK);) + } } void LinkResolver::resolve_interface_call(CallInfo& result, Handle recv, Klass* recv_klass, @@ -1454,7 +1487,16 @@ void LinkResolver::resolve_interface_call(CallInfo& result, Handle recv, Klass* Method* resolved_method = linktime_resolve_interface_method(link_info, CHECK); methodHandle mh(THREAD, resolved_method); runtime_resolve_interface_method(result, mh, link_info.resolved_klass(), - recv, recv_klass, check_null_and_abstract, CHECK); + recv, recv_klass, check_null_and_abstract, + /*is_abstract_interpretation*/ false, CHECK); +} + +void LinkResolver::cds_resolve_interface_call(CallInfo& result, const LinkInfo& link_info, TRAPS) { + Method* resolved_method = linktime_resolve_interface_method(link_info, CHECK); + runtime_resolve_interface_method(result, methodHandle(THREAD, resolved_method), link_info.resolved_klass(), + Handle(), nullptr, + /*check_null_and_abstract*/ false, + /*is_abstract_interpretation*/ true, CHECK); } Method* LinkResolver::linktime_resolve_interface_method(const LinkInfo& link_info, @@ -1473,7 +1515,9 @@ void LinkResolver::runtime_resolve_interface_method(CallInfo& result, Klass* resolved_klass, Handle recv, Klass* recv_klass, - bool check_null_and_abstract, TRAPS) { + bool check_null_and_abstract, + bool is_abstract_interpretation, TRAPS) { + // is_abstract_interpretation -- see comments in runtime_resolve_virtual_method() // check if receiver exists if (check_null_and_abstract && recv.is_null()) { @@ -1481,7 +1525,7 @@ void LinkResolver::runtime_resolve_interface_method(CallInfo& result, } // check if receiver klass implements the resolved interface - if (!recv_klass->is_subtype_of(resolved_klass)) { + if (!is_abstract_interpretation && !recv_klass->is_subtype_of(resolved_klass)) { ResourceMark rm(THREAD); char buf[200]; jio_snprintf(buf, sizeof(buf), "Class %s does not implement the requested interface %s", @@ -1490,10 +1534,14 @@ void LinkResolver::runtime_resolve_interface_method(CallInfo& result, THROW_MSG(vmSymbols::java_lang_IncompatibleClassChangeError(), buf); } - methodHandle selected_method = resolved_method; + methodHandle selected_method; + + if (!is_abstract_interpretation) { + selected_method = resolved_method; + } // resolve the method in the receiver class, unless it is private - if (!resolved_method()->is_private()) { + if (!is_abstract_interpretation && !resolved_method()->is_private()) { // do lookup based on receiver klass // This search must match the linktime preparation search for itable initialization // to correctly enforce loader constraints for interface method inheritance. @@ -1539,7 +1587,7 @@ void LinkResolver::runtime_resolve_interface_method(CallInfo& result, if (resolved_method->has_vtable_index()) { int vtable_index = resolved_method->vtable_index(); log_develop_trace(itables)(" -- vtable index: %d", vtable_index); - assert(vtable_index == selected_method->vtable_index(), "sanity check"); + assert(is_abstract_interpretation || vtable_index == selected_method->vtable_index(), "sanity check"); result.set_virtual(resolved_klass, resolved_method, selected_method, vtable_index, CHECK); } else if (resolved_method->has_itable_index()) { int itable_index = resolved_method()->itable_index(); @@ -1556,7 +1604,9 @@ void LinkResolver::runtime_resolve_interface_method(CallInfo& result, // This sets up the nonvirtual form of "virtual" call (as needed for final and private methods) result.set_virtual(resolved_klass, resolved_method, resolved_method, index, CHECK); } - JFR_ONLY(Jfr::on_resolution(result, CHECK);) + if (!is_abstract_interpretation) { + JFR_ONLY(Jfr::on_resolution(result, CHECK);) + } } diff --git a/src/hotspot/share/interpreter/linkResolver.hpp b/src/hotspot/share/interpreter/linkResolver.hpp index 80f5d230052..340c7d412d5 100644 --- a/src/hotspot/share/interpreter/linkResolver.hpp +++ b/src/hotspot/share/interpreter/linkResolver.hpp @@ -242,13 +242,15 @@ class LinkResolver: AllStatic { Klass* resolved_klass, Handle recv, Klass* recv_klass, - bool check_null_and_abstract, TRAPS); + bool check_null_and_abstract, + bool is_abstract_interpretation, TRAPS); static void runtime_resolve_interface_method (CallInfo& result, const methodHandle& resolved_method, Klass* resolved_klass, Handle recv, Klass* recv_klass, - bool check_null_and_abstract, TRAPS); + bool check_null_and_abstract, + bool is_abstract_interpretation, TRAPS); static bool resolve_previously_linked_invokehandle(CallInfo& result, const LinkInfo& link_info, @@ -325,6 +327,10 @@ class LinkResolver: AllStatic { static void resolve_dynamic_call (CallInfo& result, BootstrapInfo& bootstrap_specifier, TRAPS); + static void cds_resolve_virtual_call (CallInfo& result, const LinkInfo& link_info, TRAPS); + static void cds_resolve_interface_call(CallInfo& result, const LinkInfo& link_info, TRAPS); + static void cds_resolve_special_call (CallInfo& result, const LinkInfo& link_info, TRAPS); + // same as above for compile-time resolution; but returns null handle instead of throwing // an exception on error also, does not initialize klass (i.e., no side effects) static Method* resolve_virtual_call_or_null(Klass* receiver_klass, diff --git a/src/hotspot/share/oops/cpCache.cpp b/src/hotspot/share/oops/cpCache.cpp index 95c3b8edaf5..817a35959f3 100644 --- a/src/hotspot/share/oops/cpCache.cpp +++ b/src/hotspot/share/oops/cpCache.cpp @@ -46,6 +46,7 @@ #include "oops/compressedOops.hpp" #include "oops/constantPool.inline.hpp" #include "oops/cpCache.inline.hpp" +#include "oops/method.inline.hpp" #include "oops/objArrayOop.inline.hpp" #include "oops/oop.inline.hpp" #include "oops/resolvedFieldEntry.hpp" @@ -408,9 +409,7 @@ void ConstantPoolCache::remove_unshareable_info() { remove_resolved_field_entries_if_non_deterministic(); } if (_resolved_method_entries != nullptr) { - for (int i = 0; i < _resolved_method_entries->length(); i++) { - resolved_method_entry_at(i)->remove_unshareable_info(); - } + remove_resolved_method_entries_if_non_deterministic(); } } @@ -448,6 +447,96 @@ void ConstantPoolCache::remove_resolved_field_entries_if_non_deterministic() { ArchiveBuilder::alloc_stats()->record_field_cp_entry(archived, resolved && !archived); } } + +void ConstantPoolCache::remove_resolved_method_entries_if_non_deterministic() { + ConstantPool* cp = constant_pool(); + ConstantPool* src_cp = ArchiveBuilder::current()->get_source_addr(cp); + for (int i = 0; i < _resolved_method_entries->length(); i++) { + ResolvedMethodEntry* rme = _resolved_method_entries->adr_at(i); + int cp_index = rme->constant_pool_index(); + bool archived = false; + bool resolved = rme->is_resolved(Bytecodes::_invokevirtual) || + rme->is_resolved(Bytecodes::_invokespecial) || + rme->is_resolved(Bytecodes::_invokeinterface); + + // Just for safety -- this should not happen, but do not archive if we ever see this. + resolved &= !(rme->is_resolved(Bytecodes::_invokehandle) || + rme->is_resolved(Bytecodes::_invokestatic)); + + if (resolved && can_archive_resolved_method(rme)) { + rme->mark_and_relocate(src_cp); + archived = true; + } else { + rme->remove_unshareable_info(); + } + if (resolved) { + LogStreamHandle(Trace, cds, resolve) log; + if (log.is_enabled()) { + ResourceMark rm; + int klass_cp_index = cp->uncached_klass_ref_index_at(cp_index); + Symbol* klass_name = cp->klass_name_at(klass_cp_index); + Symbol* name = cp->uncached_name_ref_at(cp_index); + Symbol* signature = cp->uncached_signature_ref_at(cp_index); + log.print("%s%s method CP entry [%3d]: %s %s.%s:%s", + (archived ? "archived" : "reverted"), + (rme->is_resolved(Bytecodes::_invokeinterface) ? " interface" : ""), + cp_index, + cp->pool_holder()->name()->as_C_string(), + klass_name->as_C_string(), name->as_C_string(), signature->as_C_string()); + if (archived) { + Klass* resolved_klass = cp->resolved_klass_at(klass_cp_index); + log.print(" => %s%s", + resolved_klass->name()->as_C_string(), + (rme->is_resolved(Bytecodes::_invokestatic) ? " *** static" : "")); + } + } + ArchiveBuilder::alloc_stats()->record_method_cp_entry(archived, resolved && !archived); + } + } +} + +bool ConstantPoolCache::can_archive_resolved_method(ResolvedMethodEntry* method_entry) { + InstanceKlass* pool_holder = constant_pool()->pool_holder(); + if (!(pool_holder->is_shared_boot_class() || pool_holder->is_shared_platform_class() || + pool_holder->is_shared_app_class())) { + // Archiving resolved cp entries for classes from non-builtin loaders + // is not yet supported. + return false; + } + + if (CDSConfig::is_dumping_dynamic_archive()) { + // InstanceKlass::methods() has been resorted. We need to + // update the vtable_index in method_entry (not implemented) + return false; + } + + if (!method_entry->is_resolved(Bytecodes::_invokevirtual)) { + if (method_entry->method() == nullptr) { + return false; + } + if (method_entry->method()->is_continuation_native_intrinsic()) { + return false; // FIXME: corresponding stub is generated on demand during method resolution (see LinkResolver::resolve_static_call). + } + } + + int cp_index = method_entry->constant_pool_index(); + ConstantPool* src_cp = ArchiveBuilder::current()->get_source_addr(constant_pool()); + assert(src_cp->tag_at(cp_index).is_method() || src_cp->tag_at(cp_index).is_interface_method(), "sanity"); + + if (!ClassPrelinker::is_resolution_deterministic(src_cp, cp_index)) { + return false; + } + + if (method_entry->is_resolved(Bytecodes::_invokeinterface) || + method_entry->is_resolved(Bytecodes::_invokevirtual) || + method_entry->is_resolved(Bytecodes::_invokespecial)) { + return true; + } else { + // invokestatic and invokehandle are not supported yet. + return false; + } + +} #endif // INCLUDE_CDS void ConstantPoolCache::deallocate_contents(ClassLoaderData* data) { diff --git a/src/hotspot/share/oops/cpCache.hpp b/src/hotspot/share/oops/cpCache.hpp index 1f91b194623..c741201c833 100644 --- a/src/hotspot/share/oops/cpCache.hpp +++ b/src/hotspot/share/oops/cpCache.hpp @@ -1,5 +1,5 @@ /* - * Copyright (c) 1998, 2023, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 1998, 2024, Oracle and/or its affiliates. All rights reserved. * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. * * This code is free software; you can redistribute it and/or modify it @@ -224,6 +224,8 @@ class ConstantPoolCache: public MetaspaceObj { #if INCLUDE_CDS void remove_resolved_field_entries_if_non_deterministic(); + void remove_resolved_method_entries_if_non_deterministic(); + bool can_archive_resolved_method(ResolvedMethodEntry* method_entry); #endif // RedefineClasses support diff --git a/src/hotspot/share/oops/resolvedMethodEntry.hpp b/src/hotspot/share/oops/resolvedMethodEntry.hpp index e8445223600..8f49608127f 100644 --- a/src/hotspot/share/oops/resolvedMethodEntry.hpp +++ b/src/hotspot/share/oops/resolvedMethodEntry.hpp @@ -82,6 +82,21 @@ class ResolvedMethodEntry { bool _has_table_index; #endif + void copy_from(const ResolvedMethodEntry& other) { + _method = other._method; + _entry_specific = other._entry_specific; + _cpool_index = other._cpool_index; + _number_of_parameters = other._number_of_parameters; + _tos_state = other._tos_state; + _flags = other._flags; + _bytecode1 = other._bytecode1; + _bytecode2 = other._bytecode2; +#ifdef ASSERT + _has_interface_klass = other._has_interface_klass; + _has_table_index = other._has_table_index; +#endif + } + // Constructors public: ResolvedMethodEntry(u2 cpi) : @@ -99,6 +114,16 @@ class ResolvedMethodEntry { ResolvedMethodEntry() : ResolvedMethodEntry(0) {} + ResolvedMethodEntry(const ResolvedMethodEntry& other) { + copy_from(other); + } + + ResolvedMethodEntry& operator=(const ResolvedMethodEntry& other) { + copy_from(other); + return *this; + } + + // Bit shift to get flags enum { is_vfinal_shift = 0, diff --git a/test/hotspot/jtreg/runtime/cds/appcds/resolvedConstants/ResolvedConstants.java b/test/hotspot/jtreg/runtime/cds/appcds/resolvedConstants/ResolvedConstants.java index 96744b546a8..474fa65d6ea 100644 --- a/test/hotspot/jtreg/runtime/cds/appcds/resolvedConstants/ResolvedConstants.java +++ b/test/hotspot/jtreg/runtime/cds/appcds/resolvedConstants/ResolvedConstants.java @@ -54,6 +54,8 @@ public class ResolvedConstants { "-cp", appJar, "-Xlog:cds+resolve=trace"); CDSTestUtils.createArchiveAndCheck(opts) + // Class References --- + // Always resolve reference when a class references itself .shouldMatch("cds,resolve.*archived klass.* ResolvedConstantsApp app => ResolvedConstantsApp app") @@ -70,6 +72,8 @@ public class ResolvedConstants { // class yet (i.e., there's no initiaited class entry for System in the app loader's dictionary) .shouldMatch("cds,resolve.*reverted klass.* ResolvedConstantsApp .*java/lang/System") + // Field References --- + // Always resolve references to fields in the current class or super class(es) .shouldMatch("cds,resolve.*archived field.* ResolvedConstantsBar => ResolvedConstantsBar.b:I") .shouldMatch("cds,resolve.*archived field.* ResolvedConstantsBar => ResolvedConstantsBar.a:I") @@ -80,11 +84,45 @@ public class ResolvedConstants { .shouldMatch("cds,resolve.*reverted field.* ResolvedConstantsFoo ResolvedConstantsBar.a:I") .shouldMatch("cds,resolve.*reverted field.* ResolvedConstantsFoo ResolvedConstantsBar.b:I") - // Do not resolve field references to unrelated classes .shouldMatch("cds,resolve.*reverted field.* ResolvedConstantsApp ResolvedConstantsBar.a:I") .shouldMatch("cds,resolve.*reverted field.* ResolvedConstantsApp ResolvedConstantsBar.b:I") + // Method References --- + + // Should resolve references to own constructor + .shouldMatch("cds,resolve.*archived method .* ResolvedConstantsApp ResolvedConstantsApp.:") + // Should resolve references to super constructor + .shouldMatch("cds,resolve.*archived method .* ResolvedConstantsApp java/lang/Object.:") + + // Should resolve interface methods in VM classes + .shouldMatch("cds,resolve.*archived interface method .* ResolvedConstantsApp java/lang/Runnable.run:") + + // Should resolve references to own non-static method (private or public) + .shouldMatch("archived method.*: ResolvedConstantsBar ResolvedConstantsBar.doBar:") + .shouldMatch("archived method.*: ResolvedConstantsApp ResolvedConstantsApp.privateInstanceCall:") + .shouldMatch("archived method.*: ResolvedConstantsApp ResolvedConstantsApp.publicInstanceCall:") + + // Should not resolve references to static method + .shouldNotMatch(" archived method CP entry.*: ResolvedConstantsApp ResolvedConstantsApp.staticCall:") + + // Should resolve references to method in super type + .shouldMatch(" archived method CP entry.*: ResolvedConstantsBar ResolvedConstantsFoo.doBar:") + + // App class cannot resolve references to methods in boot classes: + // When the app class loader tries to resolve a class X that's normally loaded by + // the boot loader, it's possible for the app class loader to get a different copy of + // X (by using MethodHandles.Lookup.defineClass(), etc). Therefore, let's be on + // the side of safety and revert all such references. + // + // This will be addressed in JDK-8315737. + .shouldMatch("reverted method.*: ResolvedConstantsApp java/io/PrintStream.println:") + .shouldMatch("reverted method.*: ResolvedConstantsBar java/lang/Class.getName:") + + // Should not resolve methods in unrelated classes. + .shouldMatch("reverted method.*: ResolvedConstantsApp ResolvedConstantsBar.doit:") + + // End --- ; } } @@ -92,7 +130,11 @@ public class ResolvedConstants { class ResolvedConstantsApp implements Runnable { public static void main(String args[]) { System.out.println("Hello ResolvedConstantsApp"); - Object a = new ResolvedConstantsApp(); + ResolvedConstantsApp app = new ResolvedConstantsApp(); + ResolvedConstantsApp.staticCall(); + app.privateInstanceCall(); + app.publicInstanceCall(); + Object a = app; ((Runnable)a).run(); ResolvedConstantsFoo foo = new ResolvedConstantsFoo(); @@ -101,6 +143,10 @@ class ResolvedConstantsApp implements Runnable { bar.b ++; bar.doit(); } + private static void staticCall() {} + private void privateInstanceCall() {} + public void publicInstanceCall() {} + public void run() {} } @@ -124,5 +170,7 @@ class ResolvedConstantsBar extends ResolvedConstantsFoo { System.out.println("b = " + b); doBar(this); + + ((ResolvedConstantsFoo)this).doBar(this); } }