8027804: JCK resolveMethod test fails expecting AbstractMethodError
authorhseigel
Mon, 16 Dec 2013 08:24:33 -0500
changeset 22232 26acfad336c0
parent 22231 1cca24bb3125
child 22233 f0028de67b30
8027804: JCK resolveMethod test fails expecting AbstractMethodError Summary: Create AME overpass methods and fix method search logic Reviewed-by: kamg, acorn, lfoltan, coleenp
hotspot/src/share/vm/classfile/defaultMethods.cpp
hotspot/src/share/vm/interpreter/linkResolver.cpp
hotspot/src/share/vm/oops/instanceKlass.cpp
hotspot/src/share/vm/oops/instanceKlass.hpp
hotspot/src/share/vm/oops/klassVtable.cpp
--- a/hotspot/src/share/vm/classfile/defaultMethods.cpp	Fri Dec 13 09:25:44 2013 +0100
+++ b/hotspot/src/share/vm/classfile/defaultMethods.cpp	Mon Dec 16 08:24:33 2013 -0500
@@ -349,6 +349,7 @@
   }
 
   Symbol* generate_no_defaults_message(TRAPS) const;
+  Symbol* generate_method_message(Symbol *klass_name, Method* method, TRAPS) const;
   Symbol* generate_conflicts_message(GrowableArray<Method*>* methods, TRAPS) const;
 
  public:
@@ -414,21 +415,25 @@
       }
     }
 
-    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 @@
   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<Method*>* methods, TRAPS) const {
   stringStream ss;
   ss.print("Conflicting default methods:");
--- a/hotspot/src/share/vm/interpreter/linkResolver.cpp	Fri Dec 13 09:25:44 2013 +0100
+++ b/hotspot/src/share/vm/interpreter/linkResolver.cpp	Mon Dec 16 08:24:33 2013 -0500
@@ -300,7 +300,7 @@
   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 @@
 
 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 @@
                                             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 @@
                  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);
--- a/hotspot/src/share/vm/oops/instanceKlass.cpp	Fri Dec 13 09:25:44 2013 +0100
+++ b/hotspot/src/share/vm/oops/instanceKlass.cpp	Mon Dec 16 08:24:33 2013 -0500
@@ -1498,13 +1498,18 @@
   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<InstanceKlass*>(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 @@
   }
   // 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 @@
 // 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<Klass*>* 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;
     }
   }
--- a/hotspot/src/share/vm/oops/instanceKlass.hpp	Fri Dec 13 09:25:44 2013 +0100
+++ b/hotspot/src/share/vm/oops/instanceKlass.hpp	Mon Dec 16 08:24:33 2013 -0500
@@ -525,7 +525,8 @@
 
   // 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;
--- a/hotspot/src/share/vm/oops/klassVtable.cpp	Fri Dec 13 09:25:44 2013 +0100
+++ b/hotspot/src/share/vm/oops/klassVtable.cpp	Mon Dec 16 08:24:33 2013 -0500
@@ -622,7 +622,7 @@
   // 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 @@
       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 @@
     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()) {