8027229: ICCE expected for >=2 maximally specific default methods

Need to process defaults for interfaces for invokespecial

Reviewed-by: lfoltan, hseigel, coleenp, jrose

hotspot/src/share/vm/classfile/classFileParser.cpp

@@ -4080,7 +4080,7 @@ instanceKlassHandle ClassFileParser::parseClassFile(Symbol* name,
 
     // Generate any default methods - default methods are interface methods
     // that have a default implementation.  This is new with Lambda project.
-    if (has_default_methods && !access_flags.is_interface() ) {
+    if (has_default_methods ) {
       DefaultMethods::generate_default_methods(
           this_klass(), &all_mirandas, CHECK_(nullHandle));
     }

hotspot/src/share/vm/classfile/defaultMethods.cpp

@@ -171,8 +171,12 @@ class HierarchyVisitor : StackObj {
   }
   bool is_cancelled() const { return _cancelled; }
 
+  // This code used to skip interface classes because their only
+  // superclass was j.l.Object which would be also covered by class
+  // superclass hierarchy walks. Now that the starting point can be
+  // an interface, we must ensure we catch j.l.Object as the super.
   static bool has_super(InstanceKlass* cls) {
-    return cls->super() != NULL && !cls->is_interface();
+    return cls->super() != NULL;
   }
 
   Node* node_at_depth(int i) const {
@@ -391,16 +395,21 @@ class MethodFamily : public ResourceObj {
       return;
     }
 
+    // Qualified methods are maximally-specific methods
+    // These include public, instance concrete (=default) and abstract methods
     GrowableArray<Method*> qualified_methods;
     int num_defaults = 0;
     int default_index = -1;
+    int qualified_index = -1;
     for (int i = 0; i < _members.length(); ++i) {
       Pair<Method*,QualifiedState> entry = _members.at(i);
       if (entry.second == QUALIFIED) {
         qualified_methods.append(entry.first);
-        default_index++;
+        qualified_index++;
         if (entry.first->is_default_method()) {
           num_defaults++;
+          default_index = qualified_index;
+
         }
       }
     }
@@ -408,16 +417,10 @@ class MethodFamily : public ResourceObj {
     if (qualified_methods.length() == 0) {
       _exception_message = generate_no_defaults_message(CHECK);
       _exception_name = vmSymbols::java_lang_AbstractMethodError();
-    } else if (qualified_methods.length() == 1) {
-      // leave abstract methods alone, they will be found via normal search path
-      Method* method = qualified_methods.at(0);
-      if (!method->is_abstract()) {
-        _selected_target = qualified_methods.at(0);
-      }
     // If only one qualified method is default, select that
     } else if (num_defaults == 1) {
         _selected_target = qualified_methods.at(default_index);
-    } else {
+    } else if (num_defaults > 1) {
       _exception_message = generate_conflicts_message(&qualified_methods,CHECK);
       _exception_name = vmSymbols::java_lang_IncompatibleClassChangeError();
       if (TraceDefaultMethods) {
@@ -425,6 +428,7 @@ class MethodFamily : public ResourceObj {
         tty->print_cr("");
       }
     }
+    // leave abstract methods alone, they will be found via normal search path
   }
 
   bool contains_signature(Symbol* query) {
@@ -704,8 +708,10 @@ class FindMethodsByErasedSig : public HierarchyVisitor<FindMethodsByErasedSig> {
     Method* m = iklass->find_method(_method_name, _method_signature);
     // private interface methods are not candidates for default methods
     // invokespecial to private interface methods doesn't use default method logic
+    // The overpasses are your supertypes' errors, we do not include them
     // future: take access controls into account for superclass methods
-    if (m != NULL && !m->is_static() && (!iklass->is_interface() || m->is_public())) {
+    if (m != NULL && !m->is_static() && !m->is_overpass() &&
+         (!iklass->is_interface() || m->is_public())) {
       if (_family == NULL) {
         _family = new StatefulMethodFamily();
       }
@@ -781,7 +787,8 @@ void DefaultMethods::generate_default_methods(
 #ifndef PRODUCT
   if (TraceDefaultMethods) {
     ResourceMark rm;  // be careful with these!
-    tty->print_cr("Class %s requires default method processing",
+    tty->print_cr("%s %s requires default method processing",
+        klass->is_interface() ? "Interface" : "Class",
         klass->name()->as_klass_external_name());
     PrintHierarchy printer;
     printer.run(klass);
@@ -806,7 +813,7 @@ void DefaultMethods::generate_default_methods(
  }
 #ifndef PRODUCT
   if (TraceDefaultMethods) {
-    tty->print_cr("Creating overpasses...");
+    tty->print_cr("Creating defaults and overpasses...");
   }
 #endif // ndef PRODUCT
 
@@ -1076,7 +1083,9 @@ static void merge_in_new_methods(InstanceKlass* klass,
   klass->set_initial_method_idnum(new_size);
 
   ClassLoaderData* cld = klass->class_loader_data();
+  if (original_methods ->length() > 0) {
     MetadataFactory::free_array(cld, original_methods);
+  }
   if (original_ordering->length() > 0) {
     klass->set_method_ordering(merged_ordering);
     MetadataFactory::free_array(cld, original_ordering);

hotspot/src/share/vm/interpreter/linkResolver.cpp

@@ -152,11 +152,13 @@ CallInfo::CallInfo(Method* resolved_method, Klass* resolved_klass) {
     // Could be an Object method inherited into an interface, but still a vtable call.
     kind = CallInfo::vtable_call;
   } else if (!resolved_klass->is_interface()) {
-    // A miranda method.  Compute the vtable index.
+    // A default or miranda method.  Compute the vtable index.
     ResourceMark rm;
     klassVtable* vt = InstanceKlass::cast(resolved_klass)->vtable();
-    index = vt->index_of_miranda(resolved_method->name(),
-                                 resolved_method->signature());
+    index = LinkResolver::vtable_index_of_interface_method(resolved_klass,
+                           resolved_method);
+    assert(index >= 0 , "we should have valid vtable index at this point");
+
     kind = CallInfo::vtable_call;
   } else if (resolved_method->has_vtable_index()) {
     // Can occur if an interface redeclares a method of Object.
@@ -279,7 +281,7 @@ void LinkResolver::lookup_instance_method_in_klasses(methodHandle& result, Klass
 }
 
 int LinkResolver::vtable_index_of_interface_method(KlassHandle klass,
-                                          methodHandle resolved_method, TRAPS) {
+                                          methodHandle resolved_method) {
 
   int vtable_index = Method::invalid_vtable_index;
   Symbol* name = resolved_method->name();
@@ -295,7 +297,7 @@ int LinkResolver::vtable_index_of_interface_method(KlassHandle klass,
   }
   if (vtable_index == Method::invalid_vtable_index) {
     // get vtable_index for miranda methods
-    ResourceMark rm(THREAD);
+    ResourceMark rm;
     klassVtable *vt = InstanceKlass::cast(klass())->vtable();
     vtable_index = vt->index_of_miranda(name, signature);
   }
@@ -1118,7 +1120,7 @@ void LinkResolver::runtime_resolve_virtual_method(CallInfo& result,
   // do lookup based on receiver klass using the vtable index
   if (resolved_method->method_holder()->is_interface()) { // miranda method
     vtable_index = vtable_index_of_interface_method(resolved_klass,
-                           resolved_method, CHECK);
+                           resolved_method);
     assert(vtable_index >= 0 , "we should have valid vtable index at this point");
 
     InstanceKlass* inst = InstanceKlass::cast(recv_klass());
@@ -1268,14 +1270,6 @@ void LinkResolver::runtime_resolve_interface_method(CallInfo& result, methodHand
                                                       sel_method->name(),
                                                       sel_method->signature()));
   }
-  // setup result
-  if (!resolved_method->has_itable_index()) {
-    int vtable_index = resolved_method->vtable_index();
-    assert(vtable_index == sel_method->vtable_index(), "sanity check");
-    result.set_virtual(resolved_klass, recv_klass, resolved_method, sel_method, vtable_index, CHECK);
-    return;
-  }
-  int itable_index = resolved_method()->itable_index();
 
   if (TraceItables && Verbose) {
     ResourceMark rm(THREAD);
@@ -1296,8 +1290,16 @@ void LinkResolver::runtime_resolve_interface_method(CallInfo& result, methodHand
     }
     tty->cr();
   }
+  // setup result
+  if (!resolved_method->has_itable_index()) {
+    int vtable_index = resolved_method->vtable_index();
+    assert(vtable_index == sel_method->vtable_index(), "sanity check");
+    result.set_virtual(resolved_klass, recv_klass, resolved_method, sel_method, vtable_index, CHECK);
+  } else {
+    int itable_index = resolved_method()->itable_index();
     result.set_interface(resolved_klass, recv_klass, resolved_method, sel_method, itable_index, CHECK);
   }
+}
 
 
 methodHandle LinkResolver::linktime_resolve_interface_method_or_null(

hotspot/src/share/vm/interpreter/linkResolver.hpp

@@ -130,7 +130,6 @@ class LinkResolver: AllStatic {
   static void lookup_polymorphic_method         (methodHandle& result, KlassHandle klass, Symbol* name, Symbol* signature,
                                                  KlassHandle current_klass, Handle *appendix_result_or_null, Handle *method_type_result, TRAPS);
 
-  static int vtable_index_of_interface_method(KlassHandle klass, methodHandle resolved_method, TRAPS);
   static void resolve_klass           (KlassHandle& result, constantPoolHandle  pool, int index, TRAPS);
 
   static void resolve_pool  (KlassHandle& resolved_klass, Symbol*& method_name, Symbol*& method_signature, KlassHandle& current_klass, constantPoolHandle pool, int index, TRAPS);
@@ -186,6 +185,7 @@ class LinkResolver: AllStatic {
   static methodHandle resolve_interface_call_or_null(KlassHandle receiver_klass, KlassHandle resolved_klass, Symbol* method_name, Symbol* method_signature, KlassHandle current_klass);
   static methodHandle resolve_static_call_or_null   (KlassHandle resolved_klass, Symbol* method_name, Symbol* method_signature, KlassHandle current_klass);
   static methodHandle resolve_special_call_or_null  (KlassHandle resolved_klass, Symbol* method_name, Symbol* method_signature, KlassHandle current_klass);
+  static int vtable_index_of_interface_method(KlassHandle klass, methodHandle resolved_method);
 
   // same as above for compile-time resolution; returns vtable_index if current_klass if linked
   static int resolve_virtual_vtable_index  (KlassHandle receiver_klass, KlassHandle resolved_klass, Symbol* method_name, Symbol* method_signature, KlassHandle current_klass);

hotspot/src/share/vm/oops/klassVtable.cpp

@@ -86,7 +86,11 @@ void klassVtable::compute_vtable_size_and_num_mirandas(
   get_mirandas(&new_mirandas, all_mirandas, super, methods, NULL, local_interfaces);
   *num_new_mirandas = new_mirandas.length();
 
+  // Interfaces do not need interface methods in their vtables
+  // This includes miranda methods and during later processing, default methods
+  if (!class_flags.is_interface()) {
     vtable_length += *num_new_mirandas * vtableEntry::size();
+  }
 
   if (Universe::is_bootstrapping() && vtable_length == 0) {
     // array classes don't have their superclass set correctly during
@@ -224,7 +228,11 @@ void klassVtable::initialize_vtable(bool checkconstraints, TRAPS) {
     }
 
     // add miranda methods; it will also return the updated initialized
+    // Interfaces do not need interface methods in their vtables
+    // This includes miranda methods and during later processing, default methods
+    if (!ik()->is_interface()) {
       initialized = fill_in_mirandas(initialized);
+    }
 
     // In class hierarchies where the accessibility is not increasing (i.e., going from private ->
     // package_private -> public/protected), the vtable might actually be smaller than our initial
@@ -332,10 +340,16 @@ bool klassVtable::update_inherited_vtable(InstanceKlass* klass, methodHandle tar
     // An interface never allocates new vtable slots, only inherits old ones.
     // This method will either be assigned its own itable index later,
     // or be assigned an inherited vtable index in the loop below.
-    // default methods store their vtable indices in the inheritors default_vtable_indices
-    assert (default_index == -1, "interfaces don't store resolved default methods");
+    // default methods inherited by classes store their vtable indices
+    // in the inheritor's default_vtable_indices
+    // default methods inherited by interfaces may already have a
+    // valid itable index, if so, don't change it
+    // overpass methods in an interface will be assigned an itable index later
+    // by an inheriting class
+    if (!is_default || !target_method()->has_itable_index()) {
       target_method()->set_vtable_index(Method::pending_itable_index);
     }
+  }
 
   // we need a new entry if there is no superclass
   if (klass->super() == NULL) {
@@ -494,8 +508,18 @@ void klassVtable::put_method_at(Method* m, int index) {
 #ifndef PRODUCT
   if (PrintVtables && Verbose) {
     ResourceMark rm;
-    tty->print_cr("adding %s::%s at index %d", _klass->internal_name(),
-      (m != NULL) ? m->name()->as_C_string() : "<NULL>", index);
+    const char* sig = (m != NULL) ? m->name_and_sig_as_C_string() : "<NULL>";
+    tty->print("adding %s at index %d, flags: ", sig, index);
+    if (m != NULL) {
+      m->access_flags().print_on(tty);
+      if (m->is_default_method()) {
+        tty->print("default ");
+      }
+      if (m->is_overpass()) {
+        tty->print("overpass");
+      }
+    }
+    tty->cr();
   }
 #endif
   table()[index].set(m);
@@ -631,9 +655,11 @@ bool klassVtable::is_miranda_entry_at(int i) {
   if (mhk->is_interface()) {
     assert(m->is_public(), "should be public");
     assert(ik()->implements_interface(method_holder) , "this class should implement the interface");
-    assert(is_miranda(m, ik()->methods(), ik()->default_methods(), ik()->super()), "should be a miranda_method");
+    // the search could find a miranda or a default method
+    if (is_miranda(m, ik()->methods(), ik()->default_methods(), ik()->super())) {
       return true;
     }
+  }
   return false;
 }
 
@@ -644,9 +670,10 @@ bool klassVtable::is_miranda_entry_at(int i) {
 // the caller must make sure that the method belongs to an interface implemented by the class
 // Miranda methods only include public interface instance methods
 // Not private methods, not static methods, not default == concrete abstract
+// Miranda methods also do not include overpass methods in interfaces
 bool klassVtable::is_miranda(Method* m, Array<Method*>* class_methods,
                              Array<Method*>* default_methods, Klass* super) {
-  if (m->is_static() || m->is_private()) {
+  if (m->is_static() || m->is_private() || m->is_overpass()) {
     return false;
   }
   Symbol* name = m->name();
@@ -744,6 +771,8 @@ void klassVtable::get_mirandas(GrowableArray<Method*>* new_mirandas,
 // Discover miranda methods ("miranda" = "interface abstract, no binding"),
 // and append them into the vtable starting at index initialized,
 // return the new value of initialized.
+// Miranda methods use vtable entries, but do not get assigned a vtable_index
+// The vtable_index is discovered by searching from the end of the vtable
 int klassVtable::fill_in_mirandas(int initialized) {
   GrowableArray<Method*> mirandas(20);
   get_mirandas(&mirandas, NULL, ik()->super(), ik()->methods(),
@@ -977,6 +1006,25 @@ int klassItable::assign_itable_indices_for_interface(Klass* klass) {
     if (interface_method_needs_itable_index(m)) {
       assert(!m->is_final_method(), "no final interface methods");
       // If m is already assigned a vtable index, do not disturb it.
+      if (TraceItables && Verbose) {
+        ResourceMark rm;
+        const char* sig = (m != NULL) ? m->name_and_sig_as_C_string() : "<NULL>";
+        if (m->has_vtable_index()) {
+          tty->print("itable index %d for method: %s, flags: ", m->vtable_index(), sig);
+        } else {
+          tty->print("itable index %d for method: %s, flags: ", ime_num, sig);
+        }
+        if (m != NULL) {
+          m->access_flags().print_on(tty);
+          if (m->is_default_method()) {
+            tty->print("default ");
+          }
+          if (m->is_overpass()) {
+            tty->print("overpass");
+          }
+        }
+        tty->cr();
+      }
       if (!m->has_vtable_index()) {
         assert(m->vtable_index() == Method::pending_itable_index, "set by initialize_vtable");
         m->set_itable_index(ime_num);