Uploaded image for project: 'JDK'
  1. JDK
  2. JDK-8141551

C2 can not handle returns with incompatible interface arrays

    Details

    • Type: Bug
    • Status: Resolved
    • Priority: P4
    • Resolution: Fixed
    • Affects Version/s: 9
    • Fix Version/s: 9
    • Component/s: hotspot
    • Labels:
    • Subcomponent:
    • Resolved In Build:
      b96
    • CPU:
      generic
    • OS:
      generic

      Backports

        Description

         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 pruduct 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 comppilation (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.

          Issue Links

            Activity

            Hide
            hgupdate HG Updates added a comment -
            URL: http://hg.openjdk.java.net/jdk9/hs-comp/hotspot/rev/b425a78e8512
            User: kvn
            Date: 2015-11-20 00:02:54 +0000
            Show
            hgupdate HG Updates added a comment - URL: http://hg.openjdk.java.net/jdk9/hs-comp/hotspot/rev/b425a78e8512 User: kvn Date: 2015-11-20 00:02:54 +0000
            Hide
            hgupdate HG Updates added a comment -
            URL: http://hg.openjdk.java.net/jdk9/jdk9/hotspot/rev/b425a78e8512
            User: lana
            Date: 2015-12-10 00:26:59 +0000
            Show
            hgupdate HG Updates added a comment - URL: http://hg.openjdk.java.net/jdk9/jdk9/hotspot/rev/b425a78e8512 User: lana Date: 2015-12-10 00:26:59 +0000

              People

              • Assignee:
                simonis Volker Simonis
                Reporter:
                simonis Volker Simonis
              • Votes:
                0 Vote for this issue
                Watchers:
                2 Start watching this issue

                Dates

                • Created:
                  Updated:
                  Resolved: