C2 can not handle returns with incompatible interface arrays

Given the following interfaces and classes:

public static interface I1 { public String getName(); }
public static interface I2 { public String getName(); }
public static class I2C implements I2 { public String getName() { return "I2";} }
public static class I21C implements I2, I1 { public String getName() { return "I2 and I1";} }

public static class Helper {
  public static I2 createI2Array0() {
    return new I2C();
  }
  public static I2[] createI2Array1() {
    return new I2C[] { new I2C() };
  }
}

we can write the following "pseudo" Java code:

public class MeetIncompatibleInterfaceArrays0ASM {
  public static I1 run() {
    return Helper.createI2Array0(); // returns I2
  }
  public static void test() {
    I1 i1 = run();
    System.out.println(i1.getName());
  }
}
public class MeetIncompatibleInterfaceArrays1ASM {
  public static I1[] run() {
    return Helper.createI2Array1(); // returns I2[]
  }
  public static void test() {
    I1[] i1 = run();
    System.out.println(i1[0].getName());
  }
}

Notice that this is not legal Java code. We would have to use a cast in run() to make it legal:

  public static I1[] run() {
    return (I1[])Helper.createI2Array1(); // returns I2[]
  }

But in pure bytecode, the run() methods are perfectly legal:

  public static I1[] run();
    Code:
      0: invokestatic  #16  // Method Helper.createI2Array1:()[LI2;
      3: areturn

C2 can compile MeetIncompatibleInterfaceArrays0ASM.run() but will fail to compile MeetIncompatibleInterfaceArrays1ASM.run() because it can not meet the two array types I1[] and I2[] during type analysis. Currently this leads to an assertion in the debug build but to an infinite compile loop with a final crash because of the lack of native memory in a product build. This is the offending code from Parse::do_exits() in src/share/vm/opto/parse1.cpp:

    if (!_exits.control()->is_top() && _gvn.type(ret_phi)->empty()) {
      // In case of concurrent class loading, the type we set for the
      // ret_phi in build_exits() may have been too optimistic and the
      // ret_phi may be top now.
#ifdef ASSERT
      {
        MutexLockerEx ml(Compile_lock, Mutex::_no_safepoint_check_flag);
        assert(ret_type->isa_ptr() && C->env()->system_dictionary_modification_counter_changed(), "return value must be well defined");
      }
#endif
      C->record_failure(C2Compiler::retry_class_loading_during_parsing());
    }

If the compiler didn't find a valid return type for a method, it simply assumes that this could only happen because of concurrent class loading and retries the compilation (by setting the return value to C2Compiler::retry_class_loading_during_parsing). Only in the debug build he really asserts that concurrent class loading actually happened.

This could should be fixed to something like:

    if (!_exits.control()->is_top() && _gvn.type(ret_phi)->empty()) {
      // In case of concurrent class loading, the type we set for the
      // ret_phi in build_exits() may have been too optimistic and the
      // ret_phi may be top now.
      // Otherwise, we've encountered an error and have to mark the method as
      // not compilable. Just using an assertion instead would be dangerous
      // as this could lead to an infinite compile loop in non-debug builds.
      {
        MutexLockerEx ml(Compile_lock, Mutex::_no_safepoint_check_flag);
        if (C->env()->system_dictionary_modification_counter_changed()) {
          C->record_failure(C2Compiler::retry_class_loading_during_parsing());
        } else {
          C->record_method_not_compilable("Can't determine return type.");
        }
      }
      return;
    }

That is, in the case where no concurrent class loading happened, we simply record the method as not compilable in order to prevent an infinite compile loop on that method. Of course that only fixes the implications of C2's inability to correctly compile the respective method. But it will also help if other unexpected problems occur and it is always better to happily run with an uncompiled method instead of crashing while trying to compile it.

However a real fix should also improve C2'S type analysis in such a way to allow the compilation of such strange methods. This can be done as follows:

C2 already has code which handles anomalies in its type system due to the different handling of interfaces and classes. During the type computation of a Phi-Node in PhiNode::Value() there's special code which uplifts the meet of an interface with a class type to the interface type if the result of the normal meet would otherwise be empty. However this code only handles interface vs. class types but not 'array of interface' vs 'array of class' types as they occur in our example. The proper handling of such cases is added by this changeset together with a complementary fix of TypeOopPtr::filter_helper() which has exactly the same problem when joining two types.

Unfortunately this only partially fixes the problem. There's another code in Parse::return_current() which inserts implicit casts to interface if a method returns a class type to an interface return. This happens with our example if the createI2Array() method is inlined into the run() method and constant propagation finds the 'real' return type of the method which is I2C (instead of the declared I1). Again, this can be fixed by handling returns of array-of-class types to array-of-interface returns in the same way like this is already done for returns of class types to interface returns (i.e. by inserting an implicit cast to the interface type).

Finally, meeting interface with class types in the way described above can result in unsymmetric type lattices (see Cliff Click's blog at [1][2][3] for a nice description of the implications). They would be detected and objected in debug builds by special verification code in Type::meet_helper(). But because this also happens with simple interface and class type meets, such meets are already excluded from the check by the help of the Type::interface_vs_oop() function. This helper is virtual and overloaded for TypeAry and TypeAryPtr but unfortunately, the TypeAry version doesn't handle compressed oops correctly which is fixed by this change as well.

This change also contains a regression test which tries to exercise the offending Java bytecode code in various variations like interpreted, C1, C2, with and without inlining and with arrays of different dimensions. I've also verified the proposed code by running the JCK vm and lang test in mixed mode, with and without tiered compilation and with -Xcomp without seeing any problems.

[1] http://www.cliffc.org/blog/2012/02/12/too-much-theory
[2] http://www.cliffc.org/blog/2012/02/27/too-much-theory-part-2
[3] http://www.cliffc.org/blog/2012/03/24/too-much-theory-part-3