From ded2ab37057621bdf65d8c5f7a8de4cf8a1d5bc0 Mon Sep 17 00:00:00 2001 From: hseigel Date: Mon, 16 Dec 2013 08:24:33 -0500 Subject: [PATCH] 8027804: JCK resolveMethod test fails expecting AbstractMethodError Summary: Create AME overpass methods and fix method search logic Reviewed-by: kamg, acorn, lfoltan, coleenp --- src/share/vm/classfile/defaultMethods.cpp | 28 +++++++++++++++++++---- src/share/vm/interpreter/linkResolver.cpp | 13 ++++++++--- src/share/vm/oops/instanceKlass.cpp | 17 ++++++++++---- src/share/vm/oops/instanceKlass.hpp | 3 ++- src/share/vm/oops/klassVtable.cpp | 6 +++-- 5 files changed, 51 insertions(+), 16 deletions(-) diff --git a/src/share/vm/classfile/defaultMethods.cpp b/src/share/vm/classfile/defaultMethods.cpp index 129eecbad..8a60a6b36 100644 --- a/src/share/vm/classfile/defaultMethods.cpp +++ b/src/share/vm/classfile/defaultMethods.cpp @@ -349,6 +349,7 @@ class MethodFamily : public ResourceObj { } Symbol* generate_no_defaults_message(TRAPS) const; + Symbol* generate_method_message(Symbol *klass_name, Method* method, TRAPS) const; Symbol* generate_conflicts_message(GrowableArray* methods, TRAPS) const; public: @@ -414,21 +415,25 @@ class MethodFamily : public ResourceObj { } } - if (qualified_methods.length() == 0) { - _exception_message = generate_no_defaults_message(CHECK); + if (num_defaults == 0) { + if (qualified_methods.length() == 0) { + _exception_message = generate_no_defaults_message(CHECK); + } else { + assert(root != NULL, "Null root class"); + _exception_message = generate_method_message(root->name(), qualified_methods.at(0), CHECK); + } _exception_name = vmSymbols::java_lang_AbstractMethodError(); // If only one qualified method is default, select that } else if (num_defaults == 1) { _selected_target = qualified_methods.at(default_index); } else if (num_defaults > 1) { - _exception_message = generate_conflicts_message(&qualified_methods,CHECK); - _exception_name = vmSymbols::java_lang_IncompatibleClassChangeError(); + _exception_message = generate_conflicts_message(&qualified_methods,CHECK); + _exception_name = vmSymbols::java_lang_IncompatibleClassChangeError(); if (TraceDefaultMethods) { _exception_message->print_value_on(tty); tty->print_cr(""); } } - // leave abstract methods alone, they will be found via normal search path } bool contains_signature(Symbol* query) { @@ -486,6 +491,19 @@ Symbol* MethodFamily::generate_no_defaults_message(TRAPS) const { return SymbolTable::new_symbol("No qualifying defaults found", CHECK_NULL); } +Symbol* MethodFamily::generate_method_message(Symbol *klass_name, Method* method, TRAPS) const { + stringStream ss; + ss.print("Method "); + Symbol* name = method->name(); + Symbol* signature = method->signature(); + ss.write((const char*)klass_name->bytes(), klass_name->utf8_length()); + ss.print("."); + ss.write((const char*)name->bytes(), name->utf8_length()); + ss.write((const char*)signature->bytes(), signature->utf8_length()); + ss.print(" is abstract"); + return SymbolTable::new_symbol(ss.base(), (int)ss.size(), CHECK_NULL); +} + Symbol* MethodFamily::generate_conflicts_message(GrowableArray* methods, TRAPS) const { stringStream ss; ss.print("Conflicting default methods:"); diff --git a/src/share/vm/interpreter/linkResolver.cpp b/src/share/vm/interpreter/linkResolver.cpp index 0187d352f..b63e03bdc 100644 --- a/src/share/vm/interpreter/linkResolver.cpp +++ b/src/share/vm/interpreter/linkResolver.cpp @@ -300,7 +300,7 @@ int LinkResolver::vtable_index_of_interface_method(KlassHandle klass, Symbol* signature = resolved_method->signature(); // First check in default method array - if (!resolved_method->is_abstract() && + if (!resolved_method->is_abstract() && (InstanceKlass::cast(klass())->default_methods() != NULL)) { int index = InstanceKlass::find_method_index(InstanceKlass::cast(klass())->default_methods(), name, signature); if (index >= 0 ) { @@ -318,7 +318,11 @@ int LinkResolver::vtable_index_of_interface_method(KlassHandle klass, void LinkResolver::lookup_method_in_interfaces(methodHandle& result, KlassHandle klass, Symbol* name, Symbol* signature, TRAPS) { InstanceKlass *ik = InstanceKlass::cast(klass()); - result = methodHandle(THREAD, ik->lookup_method_in_all_interfaces(name, signature)); + + // Specify 'true' in order to skip default methods when searching the + // interfaces. Function lookup_method_in_klasses() already looked for + // the method in the default methods table. + result = methodHandle(THREAD, ik->lookup_method_in_all_interfaces(name, signature, true)); } void LinkResolver::lookup_polymorphic_method(methodHandle& result, @@ -620,7 +624,7 @@ void LinkResolver::resolve_interface_method(methodHandle& resolved_method, bool check_access, bool nostatics, TRAPS) { - // check if klass is interface + // check if klass is interface if (!resolved_klass->is_interface()) { ResourceMark rm(THREAD); char buf[200]; @@ -1287,8 +1291,11 @@ void LinkResolver::runtime_resolve_interface_method(CallInfo& result, methodHand resolved_klass()->external_name()); THROW_MSG(vmSymbols::java_lang_IncompatibleClassChangeError(), buf); } + // do lookup based on receiver klass methodHandle sel_method; + // This search must match the linktime preparation search for itable initialization + // to correctly enforce loader constraints for interface method inheritance lookup_instance_method_in_klasses(sel_method, recv_klass, resolved_method->name(), resolved_method->signature(), CHECK); diff --git a/src/share/vm/oops/instanceKlass.cpp b/src/share/vm/oops/instanceKlass.cpp index 7c63c4810..a6ca58c42 100644 --- a/src/share/vm/oops/instanceKlass.cpp +++ b/src/share/vm/oops/instanceKlass.cpp @@ -1498,13 +1498,18 @@ int InstanceKlass::find_method_by_name( return -1; } -// lookup_method searches both the local methods array and all superclasses methods arrays +// uncached_lookup_method searches both the local class methods array and all +// superclasses methods arrays, skipping any overpass methods in superclasses. Method* InstanceKlass::uncached_lookup_method(Symbol* name, Symbol* signature) const { Klass* klass = const_cast(this); + bool dont_ignore_overpasses = true; // For the class being searched, find its overpasses. while (klass != NULL) { Method* method = InstanceKlass::cast(klass)->find_method(name, signature); - if (method != NULL) return method; + if ((method != NULL) && (dont_ignore_overpasses || !method->is_overpass())) { + return method; + } klass = InstanceKlass::cast(klass)->super(); + dont_ignore_overpasses = false; // Ignore overpass methods in all superclasses. } return NULL; } @@ -1519,7 +1524,7 @@ Method* InstanceKlass::lookup_method_in_ordered_interfaces(Symbol* name, } // Look up interfaces if (m == NULL) { - m = lookup_method_in_all_interfaces(name, signature); + m = lookup_method_in_all_interfaces(name, signature, false); } return m; } @@ -1528,14 +1533,16 @@ Method* InstanceKlass::lookup_method_in_ordered_interfaces(Symbol* name, // Do NOT return private or static methods, new in JDK8 which are not externally visible // They should only be found in the initial InterfaceMethodRef Method* InstanceKlass::lookup_method_in_all_interfaces(Symbol* name, - Symbol* signature) const { + Symbol* signature, + bool skip_default_methods) const { Array* all_ifs = transitive_interfaces(); int num_ifs = all_ifs->length(); InstanceKlass *ik = NULL; for (int i = 0; i < num_ifs; i++) { ik = InstanceKlass::cast(all_ifs->at(i)); Method* m = ik->lookup_method(name, signature); - if (m != NULL && m->is_public() && !m->is_static()) { + if (m != NULL && m->is_public() && !m->is_static() && + (!skip_default_methods || !m->is_default_method())) { return m; } } diff --git a/src/share/vm/oops/instanceKlass.hpp b/src/share/vm/oops/instanceKlass.hpp index 84cc0046a..2b7a73a31 100644 --- a/src/share/vm/oops/instanceKlass.hpp +++ b/src/share/vm/oops/instanceKlass.hpp @@ -525,7 +525,8 @@ class InstanceKlass: public Klass { // lookup a method in all the interfaces that this class implements // (returns NULL if not found) - Method* lookup_method_in_all_interfaces(Symbol* name, Symbol* signature) const; + Method* lookup_method_in_all_interfaces(Symbol* name, Symbol* signature, bool skip_default_methods) const; + // lookup a method in local defaults then in all interfaces // (returns NULL if not found) Method* lookup_method_in_ordered_interfaces(Symbol* name, Symbol* signature) const; diff --git a/src/share/vm/oops/klassVtable.cpp b/src/share/vm/oops/klassVtable.cpp index 509c78853..a7fc062b7 100644 --- a/src/share/vm/oops/klassVtable.cpp +++ b/src/share/vm/oops/klassVtable.cpp @@ -622,7 +622,7 @@ bool klassVtable::needs_new_vtable_entry(methodHandle target_method, // this check for all access permissions. InstanceKlass *sk = InstanceKlass::cast(super); if (sk->has_miranda_methods()) { - if (sk->lookup_method_in_all_interfaces(name, signature) != NULL) { + if (sk->lookup_method_in_all_interfaces(name, signature, false) != NULL) { return false; // found a matching miranda; we do not need a new entry } } @@ -743,7 +743,7 @@ void klassVtable::add_new_mirandas_to_lists( if (is_miranda(im, class_methods, default_methods, super)) { // is it a miranda at all? InstanceKlass *sk = InstanceKlass::cast(super); // check if it is a duplicate of a super's miranda - if (sk->lookup_method_in_all_interfaces(im->name(), im->signature()) == NULL) { + if (sk->lookup_method_in_all_interfaces(im->name(), im->signature(), false) == NULL) { new_mirandas->append(im); } if (all_mirandas != NULL) { @@ -1085,6 +1085,8 @@ void klassItable::initialize_itable_for_interface(int method_table_offset, Klass Method* m = methods->at(i); methodHandle target; if (m->has_itable_index()) { + // This search must match the runtime resolution, i.e. selection search for invokeinterface + // to correctly enforce loader constraints for interface method inheritance LinkResolver::lookup_instance_method_in_klasses(target, _klass, m->name(), m->signature(), CHECK); } if (target == NULL || !target->is_public() || target->is_abstract()) { -- GitLab