8027229: ICCE expected for >=2 maximally specific default methods.
authoracorn
Wed, 13 Nov 2013 07:31:26 -0800
changeset 21556 e75cd34a59e0
parent 21555 2f66a8ee6265
child 21557 55115e0708f1
child 21729 6fe1dafeb82e
8027229: ICCE expected for >=2 maximally specific default methods. Summary: Need to process defaults for interfaces for invokespecial Reviewed-by: lfoltan, hseigel, coleenp, jrose
hotspot/src/share/vm/classfile/classFileParser.cpp
hotspot/src/share/vm/classfile/defaultMethods.cpp
hotspot/src/share/vm/interpreter/linkResolver.cpp
hotspot/src/share/vm/interpreter/linkResolver.hpp
hotspot/src/share/vm/oops/klassVtable.cpp
--- a/hotspot/src/share/vm/classfile/classFileParser.cpp	Fri Nov 08 23:49:20 2013 +0000
+++ b/hotspot/src/share/vm/classfile/classFileParser.cpp	Wed Nov 13 07:31:26 2013 -0800
@@ -4080,7 +4080,7 @@
 
     // 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));
     }
--- a/hotspot/src/share/vm/classfile/defaultMethods.cpp	Fri Nov 08 23:49:20 2013 +0000
+++ b/hotspot/src/share/vm/classfile/defaultMethods.cpp	Wed Nov 13 07:31:26 2013 -0800
@@ -171,8 +171,12 @@
   }
   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 @@
       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 @@
     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
+    // 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 @@
         tty->print_cr("");
       }
     }
+    // leave abstract methods alone, they will be found via normal search path
   }
 
   bool contains_signature(Symbol* query) {
@@ -704,8 +708,10 @@
     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 @@
 #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 @@
  }
 #ifndef PRODUCT
   if (TraceDefaultMethods) {
-    tty->print_cr("Creating overpasses...");
+    tty->print_cr("Creating defaults and overpasses...");
   }
 #endif // ndef PRODUCT
 
@@ -1076,7 +1083,9 @@
   klass->set_initial_method_idnum(new_size);
 
   ClassLoaderData* cld = klass->class_loader_data();
-  MetadataFactory::free_array(cld, original_methods);
+  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);
--- a/hotspot/src/share/vm/interpreter/linkResolver.cpp	Fri Nov 08 23:49:20 2013 +0000
+++ b/hotspot/src/share/vm/interpreter/linkResolver.cpp	Wed Nov 13 07:31:26 2013 -0800
@@ -152,11 +152,13 @@
     // 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 @@
 }
 
 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 @@
   }
   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);
   }
@@ -691,7 +693,7 @@
                   );
     resolved_method->access_flags().print_on(tty);
     if (resolved_method->is_default_method()) {
-      tty->print("default");
+      tty->print("default ");
     }
     if (resolved_method->is_overpass()) {
       tty->print("overpass");
@@ -937,7 +939,7 @@
                );
     resolved_method->access_flags().print_on(tty);
     if (resolved_method->is_default_method()) {
-      tty->print("default");
+      tty->print("default ");
     }
     if (resolved_method->is_overpass()) {
       tty->print("overpass");
@@ -1017,7 +1019,7 @@
                 );
     sel_method->access_flags().print_on(tty);
     if (sel_method->is_default_method()) {
-      tty->print("default");
+      tty->print("default ");
     }
     if (sel_method->is_overpass()) {
       tty->print("overpass");
@@ -1081,7 +1083,7 @@
                   );
     resolved_method->access_flags().print_on(tty);
     if (resolved_method->is_default_method()) {
-      tty->print("default");
+      tty->print("default ");
     }
     if (resolved_method->is_overpass()) {
       tty->print("overpass");
@@ -1118,7 +1120,7 @@
   // 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());
@@ -1175,7 +1177,7 @@
                   );
     selected_method->access_flags().print_on(tty);
     if (selected_method->is_default_method()) {
-      tty->print("default");
+      tty->print("default ");
     }
     if (selected_method->is_overpass()) {
       tty->print("overpass");
@@ -1268,14 +1270,6 @@
                                                       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);
@@ -1289,14 +1283,22 @@
                   );
     sel_method->access_flags().print_on(tty);
     if (sel_method->is_default_method()) {
-      tty->print("default");
+      tty->print("default ");
     }
     if (sel_method->is_overpass()) {
       tty->print("overpass");
     }
     tty->cr();
   }
-  result.set_interface(resolved_klass, recv_klass, resolved_method, sel_method, itable_index, CHECK);
+  // 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);
+  }
 }
 
 
--- a/hotspot/src/share/vm/interpreter/linkResolver.hpp	Fri Nov 08 23:49:20 2013 +0000
+++ b/hotspot/src/share/vm/interpreter/linkResolver.hpp	Wed Nov 13 07:31:26 2013 -0800
@@ -130,7 +130,6 @@
   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 @@
   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);
--- a/hotspot/src/share/vm/oops/klassVtable.cpp	Fri Nov 08 23:49:20 2013 +0000
+++ b/hotspot/src/share/vm/oops/klassVtable.cpp	Wed Nov 13 07:31:26 2013 -0800
@@ -86,7 +86,11 @@
   get_mirandas(&new_mirandas, all_mirandas, super, methods, NULL, local_interfaces);
   *num_new_mirandas = new_mirandas.length();
 
-  vtable_length += *num_new_mirandas * vtableEntry::size();
+  // 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 @@
     }
 
     // add miranda methods; it will also return the updated initialized
-    initialized = fill_in_mirandas(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
@@ -264,12 +272,12 @@
            _klass->internal_name(), sig, vtable_index);
            super_method->access_flags().print_on(tty);
            if (super_method->is_default_method()) {
-             tty->print("default");
+             tty->print("default ");
            }
            tty->print("overriders flags: ");
            target_method->access_flags().print_on(tty);
            if (target_method->is_default_method()) {
-             tty->print("default");
+             tty->print("default ");
            }
         }
 #endif /*PRODUCT*/
@@ -332,9 +340,15 @@
     // 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");
-    target_method()->set_vtable_index(Method::pending_itable_index);
+    // 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
@@ -441,7 +455,7 @@
            target_klass->internal_name(), sig, i);
            super_method->access_flags().print_on(tty);
            if (super_method->is_default_method()) {
-             tty->print("default");
+             tty->print("default ");
            }
            if (super_method->is_overpass()) {
              tty->print("overpass");
@@ -449,7 +463,7 @@
            tty->print("overriders flags: ");
            target_method->access_flags().print_on(tty);
            if (target_method->is_default_method()) {
-             tty->print("default");
+             tty->print("default ");
            }
            if (target_method->is_overpass()) {
              tty->print("overpass");
@@ -468,7 +482,7 @@
            target_klass->internal_name(), sig,i);
            super_method->access_flags().print_on(tty);
            if (super_method->is_default_method()) {
-             tty->print("default");
+             tty->print("default ");
            }
            if (super_method->is_overpass()) {
              tty->print("overpass");
@@ -476,7 +490,7 @@
            tty->print("overriders flags: ");
            target_method->access_flags().print_on(tty);
            if (target_method->is_default_method()) {
-             tty->print("default");
+             tty->print("default ");
            }
            if (target_method->is_overpass()) {
              tty->print("overpass");
@@ -494,8 +508,18 @@
 #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,8 +655,10 @@
   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");
-    return true;
+    // 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 @@
 // 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 @@
 // 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(),
@@ -758,7 +787,7 @@
           sig, initialized);
         meth->access_flags().print_on(tty);
         if (meth->is_default_method()) {
-          tty->print("default");
+          tty->print("default ");
         }
         tty->cr();
       }
@@ -858,7 +887,7 @@
       tty->print("      (%5d)  ", i);
       m->access_flags().print_on(tty);
       if (m->is_default_method()) {
-        tty->print("default");
+        tty->print("default ");
       }
       if (m->is_overpass()) {
         tty->print("overpass");
@@ -977,6 +1006,25 @@
     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);
@@ -1079,7 +1127,7 @@
           tty->print("target_method flags: ");
           target()->access_flags().print_on(tty);
           if (target()->is_default_method()) {
-            tty->print("default");
+            tty->print("default ");
           }
           tty->cr();
         }
@@ -1158,7 +1206,7 @@
       tty->print("      (%5d)  ", i);
       m->access_flags().print_on(tty);
       if (m->is_default_method()) {
-        tty->print("default");
+        tty->print("default ");
       }
       tty->print(" --  ");
       m->print_name(tty);