# HG changeset patch # User simonis # Date 1447086448 -3600 # Mon Nov 09 17:27:28 2015 +0100 # Node ID 752abfc415759c3bf9e1a3f4f0cd804633164e04 # Parent e32667cd477c73fb71f9bdfcddba6087e3422618 8141551: C2 can not handle returns with inccompatible interface arrays diff --git a/src/share/vm/opto/cfgnode.cpp b/src/share/vm/opto/cfgnode.cpp --- a/src/share/vm/opto/cfgnode.cpp +++ b/src/share/vm/opto/cfgnode.cpp @@ -984,7 +984,7 @@ #ifdef ASSERT // The following logic has been moved into TypeOopPtr::filter. const Type* jt = t->join_speculative(_type); - if( jt->empty() ) { // Emptied out??? + if (jt->empty()) { // Emptied out??? // Check for evil case of 't' being a class and '_type' expecting an // interface. This can happen because the bytecodes do not contain @@ -995,14 +995,40 @@ // be 'I' or 'j/l/O'. Thus we'll pick 'j/l/O'. If this then flows // into a Phi which "knows" it's an Interface type we'll have to // uplift the type. - if( !t->empty() && ttip && ttip->is_loaded() && ttip->klass()->is_interface() ) - { assert(ft == _type, ""); } // Uplift to interface - else if( !t->empty() && ttkp && ttkp->is_loaded() && ttkp->klass()->is_interface() ) - { assert(ft == _type, ""); } // Uplift to interface - // Otherwise it's something stupid like non-overlapping int ranges - // found on dying counted loops. - else - { assert(ft == Type::TOP, ""); } // Canonical empty value + if (!t->empty() && ttip && ttip->is_loaded() && ttip->klass()->is_interface()) { + assert(ft == _type, ""); // Uplift to interface + } + else if (!t->empty() && ttkp && ttkp->is_loaded() && ttkp->klass()->is_interface()) { + assert(ft == _type, ""); // Uplift to interface + } + else { + // We also have to handle 'evil cases' of interface- vs. class-arrays + const TypeAryPtr* ftap = jt->isa_aryptr(); + const TypeAryPtr* ktap = _type->isa_aryptr(); + if (!t->empty() && ftap != NULL && ktap != NULL) { + // Handle multidimensional arrays + const TypePtr* ftp = ftap->elem()->make_ptr(); + const TypePtr* ktp = ktap->elem()->make_ptr(); + while (ftp && ftp->isa_aryptr() && ktp && ktp->isa_aryptr()) { + ftap = ftp->is_aryptr(); + ktap = ktp->is_aryptr(); + ftp = ftap->elem()->make_ptr(); + ktp = ktap->elem()->make_ptr(); + } + const TypeInstPtr* ktip = ktp ? ktp->isa_instptr() : NULL; + if (ktip != NULL && ktip->is_loaded() && ktip->klass()->is_interface()) { + assert(ft == _type, ""); // Uplift to array of interface + } + else { + // Otherwise it's something stupid like non-overlapping int ranges + // found on dying counted loops. + assert(ft == Type::TOP, ""); // Canonical empty value + } + } + else { + assert(ft == Type::TOP, ""); // Canonical empty value + } + } } else { diff --git a/src/share/vm/opto/parse1.cpp b/src/share/vm/opto/parse1.cpp --- a/src/share/vm/opto/parse1.cpp +++ b/src/share/vm/opto/parse1.cpp @@ -991,13 +991,18 @@ // 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 + // 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); - assert(ret_type->isa_ptr() && C->env()->system_dictionary_modification_counter_changed(), "return value must be well defined"); + 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."); + } } -#endif - C->record_failure(C2Compiler::retry_class_loading_during_parsing()); + return; } _exits.push_node(ret_type->basic_type(), ret_phi); } diff --git a/src/share/vm/opto/type.cpp b/src/share/vm/opto/type.cpp --- a/src/share/vm/opto/type.cpp +++ b/src/share/vm/opto/type.cpp @@ -708,7 +708,14 @@ if (isa_narrowoop() || t->isa_narrowoop()) return mt; if (isa_narrowklass() || t->isa_narrowklass()) return mt; #ifdef ASSERT - assert(mt == t->xmeet(this_t), "meet not commutative"); + if (mt != t->xmeet(this_t)) { + // Array of Interface meet Array of Oop is Not Commutative. + if (!(this_t->make_ptr() && this_t->make_ptr()->isa_aryptr() && + t->make_ptr() && t->make_ptr()->isa_aryptr() && + interface_vs_oop(t))) { + fatal("meet not commutative"); + } + } const Type* dual_join = mt->_dual; const Type *t2t = dual_join->xmeet(t->_dual); const Type *t2this = dual_join->xmeet(this_t->_dual); @@ -716,6 +723,7 @@ // Interface meet Oop is Not Symmetric: // Interface:AnyNull meet Oop:AnyNull == Interface:AnyNull // Interface:NotNull meet Oop:NotNull == java/lang/Object:NotNull + // Array of Interface meet Array of Oop is also Not Symmetric. if( !interface_vs_oop(t) && (t2t != t->_dual || t2this != this_t->_dual) ) { tty->print_cr("=== Meet Not Symmetric ==="); @@ -2029,7 +2037,11 @@ bool TypeAry::interface_vs_oop(const Type *t) const { const TypeAry* t_ary = t->is_ary(); if (t_ary) { - return _elem->interface_vs_oop(t_ary->_elem); + const TypePtr* this_ptr = _elem->make_ptr(); // In case it is narrow_oop + const TypePtr* t_ptr = t_ary->_elem->make_ptr(); + if(this_ptr != NULL && t_ptr != NULL) { + return this_ptr->interface_vs_oop(t_ptr); + } } return false; } @@ -3134,8 +3146,28 @@ // be 'I' or 'j/l/O'. Thus we'll pick 'j/l/O'. If this then flows // into a Phi which "knows" it's an Interface type we'll have to // uplift the type. - if (!empty() && ktip != NULL && ktip->is_loaded() && ktip->klass()->is_interface()) - return kills; // Uplift to interface + if (!empty()) { + if (ktip != NULL && ktip->is_loaded() && ktip->klass()->is_interface()) { + return kills; // Uplift to interface + } + const TypeAryPtr* ftap = ft->isa_aryptr(); + const TypeAryPtr* ktap = kills->isa_aryptr(); + if (ftap != NULL && ktap != NULL) { + // Handle multidimensional arrays + const TypePtr* ftp = ftap->elem()->make_ptr(); + const TypePtr* ktp = ktap->elem()->make_ptr(); + while (ftp && ftp->isa_aryptr() && ktp && ktp->isa_aryptr()) { + ftap = ftp->is_aryptr(); + ktap = ktp->is_aryptr(); + ftp = ftap->elem()->make_ptr(); + ktp = ktap->elem()->make_ptr(); + } + ktip = ktp ? ktp->isa_instptr() : NULL; + if (ktip != NULL && ktip->is_loaded() && ktip->klass()->is_interface()) { + return kills; // Uplift to array of interface + } + } + } return Type::TOP; // Canonical empty value } @@ -4142,10 +4174,25 @@ (tap->_klass_is_exact && !tap->klass()->is_subtype_of(klass())) || // 'this' is exact and super or unrelated: (this->_klass_is_exact && !klass()->is_subtype_of(tap->klass())))) { - if (above_centerline(ptr)) { - tary = TypeAry::make(Type::BOTTOM, tary->_size, tary->_stable); - } - return make(NotNull, NULL, tary, lazy_klass, false, off, InstanceBot, speculative, depth); + + const TypeAry* tta = tap->_ary; + const TypeAry* mta = tary; + // Handle multidimensional arrays + const TypePtr* ftp = mta->_elem->make_ptr(); + const TypePtr* ktp = tta->_elem->make_ptr(); + while (ftp && ftp->isa_aryptr() && ktp && ktp->isa_aryptr()) { + mta = ftp->is_aryptr()->_ary; + tta = ktp->is_aryptr()->_ary; + ftp = mta->_elem->make_ptr(); + ktp = tta->_elem->make_ptr(); + } + const TypeInstPtr* ktip = ktp ? ktp->isa_instptr() : NULL; + if (ktip == NULL || !ktip->is_loaded() || !ktip->klass()->is_interface()) { + if (above_centerline(ptr)) { + tary = TypeAry::make(Type::BOTTOM, tary->_size, tary->_stable); + } + } + return make(NotNull, NULL, tary, lazy_klass, false, off, InstanceBot, speculative, depth); } bool xk = false; diff --git a/test/compiler/types/TestMeetIncompatibleInterfaceArrays.java b/test/compiler/types/TestMeetIncompatibleInterfaceArrays.java new file mode 100644 --- /dev/null +++ b/test/compiler/types/TestMeetIncompatibleInterfaceArrays.java @@ -0,0 +1,353 @@ +/* + * Copyright 2015 SAP AG. All Rights Reserved. + * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. + * + * This code is free software; you can redistribute it and/or modify it + * under the terms of the GNU General Public License version 2 only, as + * published by the Free Software Foundation. + * + * This code is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License + * version 2 for more details (a copy is included in the LICENSE file that + * accompanied this code). + * + * You should have received a copy of the GNU General Public License version + * 2 along with this work; if not, write to the Free Software Foundation, + * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA. + * + * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA + * or visit www.oracle.com if you need additional information or have any + * questions. + */ + +/* + * @test + * @bug 8141551 + * @summary C2 can not handle returns with inccompatible interface arrays + * @modules java.base/jdk.internal.org.objectweb.asm + * java.base/sun.misc + * @library /testlibrary /../../test/lib + * @build sun.hotspot.WhiteBox + * @run main ClassFileInstaller sun.hotspot.WhiteBox + * sun.hotspot.WhiteBox$WhiteBoxPermission + * @run main/othervm + * -Xbootclasspath/a:. + * -XX:+UnlockDiagnosticVMOptions + * -XX:+WhiteBoxAPI + * -Xbatch + * -XX:CompileThreshold=1 + * -XX:-TieredCompilation + * -XX:CICompilerCount=1 + * -XX:+PrintCompilation + * -XX:+PrintInlining + * -XX:CompileCommand=compileonly,MeetIncompatibleInterfaceArrays*.run + * -XX:CompileCommand=dontinline,TestMeetIncompatibleInterfaceArrays$Helper.createI2* + * -XX:CompileCommand=quiet + * TestMeetIncompatibleInterfaceArrays 0 + * @run main/othervm + * -Xbootclasspath/a:. + * -XX:+UnlockDiagnosticVMOptions + * -XX:+WhiteBoxAPI + * -Xbatch + * -XX:CompileThreshold=1 + * -XX:-TieredCompilation + * -XX:CICompilerCount=1 + * -XX:+PrintCompilation + * -XX:+PrintInlining + * -XX:CompileCommand=compileonly,MeetIncompatibleInterfaceArrays*.run + * -XX:CompileCommand=inline,TestMeetIncompatibleInterfaceArrays$Helper.createI2* + * -XX:CompileCommand=quiet + * TestMeetIncompatibleInterfaceArrays 1 + * @run main/othervm + * -Xbootclasspath/a:. + * -XX:+UnlockDiagnosticVMOptions + * -XX:+WhiteBoxAPI + * -Xbatch + * -XX:CompileThreshold=1 + * -XX:Tier0InvokeNotifyFreqLog=0 -XX:Tier2InvokeNotifyFreqLog=0 -XX:Tier3InvokeNotifyFreqLog=0 -XX:Tier23InlineeNotifyFreqLog=0 + * -XX:Tier3InvocationThreshold=2 -XX:Tier3MinInvocationThreshold=2 -XX:Tier3CompileThreshold=2 + * -XX:Tier4InvocationThreshold=1 -XX:Tier4MinInvocationThreshold=1 -XX:Tier4CompileThreshold=1 + * -XX:+TieredCompilation + * -XX:CICompilerCount=2 + * -XX:+PrintCompilation + * -XX:+PrintInlining + * -XX:CompileCommand=compileonly,MeetIncompatibleInterfaceArrays*.run + * -XX:CompileCommand=compileonly,TestMeetIncompatibleInterfaceArrays$Helper.createI2* + * -XX:CompileCommand=inline,TestMeetIncompatibleInterfaceArrays$Helper.createI2* + * -XX:CompileCommand=quiet + * TestMeetIncompatibleInterfaceArrays 2 + * + * @author volker.simonis@gmail.com + */ + +import java.io.FileOutputStream; +import java.lang.reflect.InvocationTargetException; +import java.lang.reflect.Method; +import jdk.internal.org.objectweb.asm.ClassWriter; +import jdk.internal.org.objectweb.asm.MethodVisitor; +import static jdk.internal.org.objectweb.asm.Opcodes.*; +import sun.hotspot.WhiteBox; + +public class TestMeetIncompatibleInterfaceArrays extends ClassLoader { + + private static final WhiteBox WB = WhiteBox.getWhiteBox(); + + 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() }; + } + public static I2[][] createI2Array2() { + return new I2C[][] { new I2C[] { new I2C() } }; + } + public static I2[][][] createI2Array3() { + return new I2C[][][] { new I2C[][] { new I2C[] { new I2C() } } }; + } + public static I2[][][][] createI2Array4() { + return new I2C[][][][] { new I2C[][][] { new I2C[][] { new I2C[] { new I2C() } } } }; + } + public static I2[][][][][] createI2Array5() { + return new I2C[][][][][] { new I2C[][][][] { new I2C[][][] { new I2C[][] { new I2C[] { new I2C() } } } } }; + } + public static I2 createI21Array0() { + return new I21C(); + } + public static I2[] createI21Array1() { + return new I21C[] { new I21C() }; + } + public static I2[][] createI21Array2() { + return new I21C[][] { new I21C[] { new I21C() } }; + } + public static I2[][][] createI21Array3() { + return new I21C[][][] { new I21C[][] { new I21C[] { new I21C() } } }; + } + public static I2[][][][] createI21Array4() { + return new I21C[][][][] { new I21C[][][] { new I21C[][] { new I21C[] { new I21C() } } } }; + } + public static I2[][][][][] createI21Array5() { + return new I21C[][][][][] { new I21C[][][][] { new I21C[][][] { new I21C[][] { new I21C[] { new I21C() } } } } }; + } + } + + // Location for the generated class files + public static final String PATH = System.getProperty("test.classes", ".") + java.io.File.separator; + + /* + * With 'good == false' this helper method creates the following classes + * (using the nested 'Helper' class and the nested interfaces 'I1' and 'I2'). + * For brevity I omit the enclosing class 'TestMeetIncompatibleInterfaceArrays' in the + * following examples: + * + * 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()); + * } + * } + * ... + * // MeetIncompatibleInterfaceArrays4ASM is special because it creates + * // an illegal class which will be rejected by the verifier. + * public class MeetIncompatibleInterfaceArrays4ASM { + * public static I1[][][][] run() { + * return Helper.createI2Array3(); // returns I1[][][] which gives a verifier error because return expects I1[][][][] + * } + * public static void test() { + * I1[][][][][] i1 = run(); + * System.out.println(i1[0][0][0][0][0].getName()); + * } + * ... + * public class MeetIncompatibleInterfaceArrays5ASM { + * public static I1[][][][][] run() { + * return Helper.createI2Array5(); // returns I2[][][][][] + * } + * public static void test() { + * I1[][][][][] i1 = run(); + * System.out.println(i1[0][0][0][0][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 + * + * The "test()" method calls the "getName()" function from I1 on the objects returned by "run()". + * This will epectedly fail with an "IncompatibleClassChangeError" because the objects returned + * by "run()" (and by createI2Array()) are actually of type "I2C" and only implement "I2" but not "I1". + * + * + * With 'good == true' this helper method will create the following classes: + * + * public class MeetIncompatibleInterfaceArraysGood0ASM { + * public static I1 run() { + * return Helper.createI21Array0(); // returns I2 + * } + * public static void test() { + * I1 i1 = run(); + * System.out.println(i1.getName()); + * } + * } + * + * Calling "test()" on these objects will succeed and output "I2 and I1" because now the "run()" + * method calls "createI21Array()" which actually return an object (or an array of objects) of + * type "I21C" which implements both "I2" and "I1". + * + * Notice that at the bytecode level, the code for the "run()" and "test()" methods in + * "MeetIncompatibleInterfaceArraysASM" and "MeetIncompatibleInterfaceArraysGoodASM" look exactly + * the same. I.e. the verifier has no chance to verify if the I2 object returned by "createI1Array()" + * or "createI21Array()" implements "I1" or not. That's actually the reason why both versions of + * generated classes are legal from a verifier point of view. + * + */ + static void generateTestClass(int dim, boolean good) throws Exception { + String baseClassName = "MeetIncompatibleInterfaceArrays"; + if (good) + baseClassName += "Good"; + String createName = "createI2" + (good ? "1" : "") + "Array"; + String a = ""; + for (int i = 0; i < dim; i++) + a += "["; + ClassWriter cw = new ClassWriter(ClassWriter.COMPUTE_FRAMES); + cw.visit(V1_8, ACC_PUBLIC, baseClassName + dim + "ASM", null, "java/lang/Object", null); + MethodVisitor constr = cw.visitMethod(ACC_PUBLIC, "", "()V", null, null); + constr.visitCode(); + constr.visitVarInsn(ALOAD, 0); + constr.visitMethodInsn(INVOKESPECIAL, "java/lang/Object", "", "()V", false); + constr.visitInsn(RETURN); + constr.visitMaxs(0, 0); + constr.visitEnd(); + MethodVisitor run = cw.visitMethod(ACC_PUBLIC | ACC_STATIC, "run", + "()" + a + "LTestMeetIncompatibleInterfaceArrays$I1;", null, null); + run.visitCode(); + if (dim == 4) { + run.visitMethodInsn(INVOKESTATIC, "TestMeetIncompatibleInterfaceArrays$Helper", createName + 3, + "()" + "[[[" + "LTestMeetIncompatibleInterfaceArrays$I2;", false); + } else { + run.visitMethodInsn(INVOKESTATIC, "TestMeetIncompatibleInterfaceArrays$Helper", createName + dim, + "()" + a + "LTestMeetIncompatibleInterfaceArrays$I2;", false); + } + run.visitInsn(ARETURN); + run.visitMaxs(0, 0); + run.visitEnd(); + MethodVisitor test = cw.visitMethod(ACC_PUBLIC | ACC_STATIC, "test", "()V", null, null); + test.visitCode(); + test.visitMethodInsn(INVOKESTATIC, baseClassName + dim + "ASM", "run", + "()" + a + "LTestMeetIncompatibleInterfaceArrays$I1;", false); + test.visitVarInsn(ASTORE, 0); + if (dim > 0) { + test.visitVarInsn(ALOAD, 0); + for (int i = 1; i <= dim; i++) { + test.visitInsn(ICONST_0); + test.visitInsn(AALOAD); + } + test.visitVarInsn(ASTORE, 1); + } + test.visitFieldInsn(GETSTATIC, "java/lang/System", "out", "Ljava/io/PrintStream;"); + test.visitVarInsn(ALOAD, dim > 0 ? 1 : 0); + test.visitMethodInsn(INVOKEINTERFACE, "TestMeetIncompatibleInterfaceArrays$I1", "getName", + "()Ljava/lang/String;", true); + test.visitMethodInsn(INVOKEVIRTUAL, "java/io/PrintStream", "println", "(Ljava/lang/Object;)V", false); + test.visitInsn(RETURN); + test.visitMaxs(0, 0); + test.visitEnd(); + + // Get the bytes of the class.. + byte[] b = cw.toByteArray(); + // ..and write them into a class file (for debugging) + FileOutputStream fos = new FileOutputStream(PATH + baseClassName + dim + "ASM.class"); + fos.write(b); + fos.close(); + + } + + public static String[][] tier = { { "interpreted", "C2 (tier 4) without inlining", "C2 (tier4) without inlining" }, + { "interpreted", "C2 (tier 4) with inlining", "C2 (tier4) with inlining" }, + { "interpreted", "C1 (tier 3) with inlining", "C2 (tier4) with inlining" } }; + + public static void main(String[] args) throws Exception { + final int pass = Integer.parseInt(args.length > 0 ? args[0] : "0"); + + // Load and initialize some classes required for compilation + Class.forName("TestMeetIncompatibleInterfaceArrays$I1"); + Class.forName("TestMeetIncompatibleInterfaceArrays$I2"); + Class.forName("TestMeetIncompatibleInterfaceArrays$Helper"); + + for (int g = 0; g < 2; g++) { + String baseClassName = "MeetIncompatibleInterfaceArrays"; + boolean good = (g == 0) ? false : true; + if (good) + baseClassName += "Good"; + for (int i = 0; i < 6; i++) { + System.out.println(); + System.out.println("Creating " + baseClassName + i + "ASM.class"); + System.out.println("========================================" + "=" + "========="); + // Create the "MeetIncompatibleInterfaceArraysASM" class + generateTestClass(i, good); + Class c = null; + try { + c = Class.forName(baseClassName + i + "ASM"); + } catch (VerifyError ve) { + if (i == 4) { + System.out.println("OK - must be (" + ve.getMessage() + ")."); + } else { + throw ve; + } + continue; + } + // Call MeetIncompatibleInterfaceArraysASM.test() + Method m = c.getMethod("test"); + Method r = c.getMethod("run"); + for (int j = 0; j < 3; j++) { + System.out.println((j + 1) + ". invokation of " + baseClassName + i + "ASM.test() [should be " + + tier[pass][j] + "]"); + try { + m.invoke(null); + } catch (InvocationTargetException ite) { + if (good) { + throw ite; + } else { + if (ite.getCause() instanceof IncompatibleClassChangeError) { + System.out.println(" OK - catched InvocationTargetException(" + + ite.getCause().getMessage() + ")."); + } else { + throw ite; + } + } + } + } + System.out.println("Method " + r + (WB.isMethodCompiled(r) ? " has" : " has not") + " been compiled."); + if (!WB.isMethodCompiled(r)) { + throw new Exception("Method " + r + " must be compiled!"); + } + } + } + } +}