8163808: Fix asserts and logging for old classfile vtable construction
Reviewed-by: coleenp, lfoltan, rprotacio, ctornqvi
--- a/hotspot/src/share/vm/classfile/classFileParser.cpp Fri Aug 19 16:02:21 2016 +0200
+++ b/hotspot/src/share/vm/classfile/classFileParser.cpp Fri Aug 19 11:32:43 2016 -0400
@@ -5844,6 +5844,7 @@
_super_klass,
_methods,
_access_flags,
+ _major_version,
_loader_data->class_loader(),
_class_name,
_local_interfaces,
--- a/hotspot/src/share/vm/oops/klassVtable.cpp Fri Aug 19 16:02:21 2016 +0200
+++ b/hotspot/src/share/vm/oops/klassVtable.cpp Fri Aug 19 11:32:43 2016 -0400
@@ -60,7 +60,7 @@
void klassVtable::compute_vtable_size_and_num_mirandas(
int* vtable_length_ret, int* num_new_mirandas,
GrowableArray<Method*>* all_mirandas, const Klass* super,
- Array<Method*>* methods, AccessFlags class_flags,
+ Array<Method*>* methods, AccessFlags class_flags, u2 major_version,
Handle classloader, Symbol* classname, Array<Klass*>* local_interfaces,
TRAPS) {
NoSafepointVerifier nsv;
@@ -77,7 +77,7 @@
assert(methods->at(i)->is_method(), "must be a Method*");
methodHandle mh(THREAD, methods->at(i));
- if (needs_new_vtable_entry(mh, super, classloader, classname, class_flags, THREAD)) {
+ if (needs_new_vtable_entry(mh, super, classloader, classname, class_flags, major_version, THREAD)) {
vtable_length += vtableEntry::size(); // we need a new entry
}
}
@@ -256,10 +256,15 @@
// 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
- // calculation.
- assert(initialized <= _length, "vtable initialization failed");
- for(;initialized < _length; initialized++) {
- put_method_at(NULL, initialized);
+ // calculation, for classfile versions for which we do not do transitive override
+ // calculations.
+ if (ik()->major_version() >= VTABLE_TRANSITIVE_OVERRIDE_VERSION) {
+ assert(initialized == _length, "vtable initialization failed");
+ } else {
+ assert(initialized <= _length, "vtable initialization failed");
+ for(;initialized < _length; initialized++) {
+ table()[initialized].clear();
+ }
}
NOT_PRODUCT(verify(tty, true));
}
@@ -298,9 +303,9 @@
ResourceMark rm(THREAD);
outputStream* logst = Log(vtables)::trace_stream();
char* sig = target_method()->name_and_sig_as_C_string();
- logst->print("transitive overriding superclass %s with %s::%s index %d, original flags: ",
+ logst->print("transitive overriding superclass %s with %s index %d, original flags: ",
supersuperklass->internal_name(),
- _klass->internal_name(), sig, vtable_index);
+ sig, vtable_index);
super_method->print_linkage_flags(logst);
logst->print("overriders flags: ");
target_method->print_linkage_flags(logst);
@@ -330,11 +335,11 @@
outputStream* logst = Log(vtables)::trace_stream();
char* sig = target_method()->name_and_sig_as_C_string();
if (overrides) {
- logst->print("overriding with %s::%s index %d, original flags: ",
- target_klass->internal_name(), sig, i);
+ logst->print("overriding with %s index %d, original flags: ",
+ sig, i);
} else {
- logst->print("NOT overriding with %s::%s index %d, original flags: ",
- target_klass->internal_name(), sig, i);
+ logst->print("NOT overriding with %s index %d, original flags: ",
+ sig, i);
}
super_method->print_linkage_flags(logst);
logst->print("overriders flags: ");
@@ -566,6 +571,7 @@
Handle classloader,
Symbol* classname,
AccessFlags class_flags,
+ u2 major_version,
TRAPS) {
if (class_flags.is_interface()) {
// Interfaces do not use vtables, except for java.lang.Object methods,
@@ -646,8 +652,12 @@
}
}
- // Start with lookup result and continue to search up
- k = superk->super(); // haven't found an override match yet; continue to look
+ // Start with lookup result and continue to search up, for versions supporting transitive override
+ if (major_version >= VTABLE_TRANSITIVE_OVERRIDE_VERSION) {
+ k = superk->super(); // haven't found an override match yet; continue to look
+ } else {
+ break;
+ }
}
// if the target method is public or protected it may have a matching
@@ -1500,15 +1510,22 @@
void vtableEntry::verify(klassVtable* vt, outputStream* st) {
NOT_PRODUCT(FlagSetting fs(IgnoreLockingAssertions, true));
- assert(method() != NULL, "must have set method");
- method()->verify();
- // we sub_type, because it could be a miranda method
- if (!vt->klass()->is_subtype_of(method()->method_holder())) {
+ KlassHandle vtklass_h = vt->klass();
+ Klass* vtklass = vtklass_h();
+ if (vtklass->is_instance_klass() &&
+ (InstanceKlass::cast(vtklass)->major_version() >= klassVtable::VTABLE_TRANSITIVE_OVERRIDE_VERSION)) {
+ assert(method() != NULL, "must have set method");
+ }
+ if (method() != NULL) {
+ method()->verify();
+ // we sub_type, because it could be a miranda method
+ if (!vtklass_h->is_subtype_of(method()->method_holder())) {
#ifndef PRODUCT
- print();
+ print();
#endif
- fatal("vtableEntry " PTR_FORMAT ": method is from subclass", p2i(this));
- }
+ fatal("vtableEntry " PTR_FORMAT ": method is from subclass", p2i(this));
+ }
+ }
}
#ifndef PRODUCT
--- a/hotspot/src/share/vm/oops/klassVtable.hpp Fri Aug 19 16:02:21 2016 +0200
+++ b/hotspot/src/share/vm/oops/klassVtable.hpp Fri Aug 19 11:32:43 2016 -0400
@@ -90,6 +90,7 @@
const Klass* super,
Array<Method*>* methods,
AccessFlags class_flags,
+ u2 major_version,
Handle classloader,
Symbol* classname,
Array<Klass*>* local_interfaces,
@@ -115,8 +116,14 @@
protected:
friend class vtableEntry;
+
+ public:
+ // Transitive overridng rules for class files < JDK1_7 use the older JVMS rules.
+ // Overriding is determined as we create the vtable, so we use the class file version
+ // of the class whose vtable we are calculating.
+ enum { VTABLE_TRANSITIVE_OVERRIDE_VERSION = 51 } ;
+
private:
- enum { VTABLE_TRANSITIVE_OVERRIDE_VERSION = 51 } ;
void copy_vtable_to(vtableEntry* start);
int initialize_from_super(KlassHandle super);
int index_of(Method* m, int len) const; // same as index_of, but search only up to len
@@ -126,6 +133,7 @@
Handle classloader,
Symbol* classname,
AccessFlags access_flags,
+ u2 major_version,
TRAPS);
bool update_inherited_vtable(InstanceKlass* klass, methodHandle target_method, int super_vtable_len, int default_index, bool checkconstraints, TRAPS);
--- /dev/null Thu Jan 01 00:00:00 1970 +0000
+++ b/hotspot/test/runtime/TransitiveOverrideCFV50/TransitiveOverrideCFV50.java Fri Aug 19 11:32:43 2016 -0400
@@ -0,0 +1,200 @@
+/*
+ * Copyright (c) 2016, Oracle and/or its affiliates. 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 8163808
+ * @modules java.base/jdk.internal.org.objectweb.asm
+ * @run main TransitiveOverrideCFV50
+ */
+
+import java.util.*;
+import java.io.File;
+import java.io.FileOutputStream;
+import jdk.internal.org.objectweb.asm.ClassWriter;
+import jdk.internal.org.objectweb.asm.MethodVisitor;
+import jdk.internal.org.objectweb.asm.AnnotationVisitor;
+import jdk.internal.org.objectweb.asm.Opcodes;
+
+/*
+ * Test mixed classfile version overriding handling.
+ * Key is to generate P2/C with an older classfile version <=50
+ * Correct response is B.m:2 for older classfiles
+ * This test was added to ensure no assertions in debug
+ * note: for P2/C classfile version >=51, correct answer becomes C.m:3
+ * public class P1.A { public int m() { return 1; }
+ *
+ * public class P1.B extends A { int m() { return 2; }
+ *
+ * public class P2.c extends P1.B { public int m() { return 3; }
+ */
+
+public class TransitiveOverrideCFV50 implements Opcodes{
+ static final String classP1A = "P1.A";
+ static final String classP1B = "P1.B";
+ static final String classP2C = "P2.C";
+
+ static final String callerName = classP2C;
+
+ public static void main(String[] args) throws Exception {
+ ClassLoader cl = new ClassLoader() {
+ public Class<?> loadClass(String name) throws ClassNotFoundException {
+ if (findLoadedClass(name) != null) {
+ return findLoadedClass(name);
+ }
+
+ if (classP1A.equals(name)) {
+ byte[] classFile = dumpP1A();
+ return defineClass(classP1A, classFile, 0, classFile.length);
+ }
+ if (classP1B.equals(name)) {
+ byte[] classFile = dumpP1B();
+ return defineClass(classP1B, classFile, 0, classFile.length);
+ }
+ if (classP2C.equals(name)) {
+ byte[] classFile = dumpP2C();
+ return defineClass(classP2C, classFile, 0, classFile.length);
+ }
+
+ return super.loadClass(name);
+ }
+ };
+
+ cl.loadClass(classP1A);
+ cl.loadClass(classP1B);
+ cl.loadClass(classP2C);
+
+ //cl.loadClass(callerName).getDeclaredMethod("test");
+ cl.loadClass(callerName).newInstance();
+
+ int result = (Integer)cl.loadClass(callerName).getDeclaredMethod("test").invoke(null);
+ if (result != 2) {
+ throw new RuntimeException("expected B.m:2, got " + result);
+ }
+ }
+
+ public static byte[] dumpP1A() {
+
+ ClassWriter cw = new ClassWriter(0);
+ MethodVisitor mv;
+
+ cw.visit(V1_7, ACC_PUBLIC + ACC_SUPER, "P1/A", null, "java/lang/Object", null);
+
+ {
+ mv = cw.visitMethod(ACC_PUBLIC, "<init>", "()V", null, null);
+ mv.visitCode();
+ mv.visitVarInsn(ALOAD, 0);
+ mv.visitMethodInsn(INVOKESPECIAL, "java/lang/Object", "<init>", "()V", false);
+ mv.visitInsn(RETURN);
+ mv.visitMaxs(1, 1);
+ mv.visitEnd();
+ }
+ {
+ mv = cw.visitMethod(ACC_PUBLIC, "m", "()I", null, null);
+ mv.visitCode();
+ mv.visitFieldInsn(GETSTATIC, "java/lang/System", "out", "Ljava/io/PrintStream;");
+ mv.visitLdcInsn("A.m");
+ mv.visitMethodInsn(INVOKEVIRTUAL, "java/io/PrintStream", "println", "(Ljava/lang/String;)V", false);
+ mv.visitIntInsn(BIPUSH, 1);
+ mv.visitInsn(IRETURN);
+ mv.visitMaxs(2, 1);
+ mv.visitEnd();
+ }
+ cw.visitEnd();
+
+ return cw.toByteArray();
+ }
+ public static byte[] dumpP1B() {
+
+ ClassWriter cw = new ClassWriter(0);
+ MethodVisitor mv;
+
+ cw.visit(V1_8, ACC_PUBLIC + ACC_SUPER, "P1/B", null, "P1/A", null);
+
+ {
+ mv = cw.visitMethod(ACC_PUBLIC, "<init>", "()V", null, null);
+ mv.visitCode();
+ mv.visitVarInsn(ALOAD, 0);
+ mv.visitMethodInsn(INVOKESPECIAL, "P1/A", "<init>", "()V", false);
+ mv.visitInsn(RETURN);
+ mv.visitMaxs(1, 1);
+ mv.visitEnd();
+ }
+ {
+ mv = cw.visitMethod(0, "m", "()I", null, null);
+ mv.visitCode();
+ mv.visitFieldInsn(GETSTATIC, "java/lang/System", "out", "Ljava/io/PrintStream;");
+ mv.visitLdcInsn("B.m");
+ mv.visitMethodInsn(INVOKEVIRTUAL, "java/io/PrintStream", "println", "(Ljava/lang/String;)V", false);
+ mv.visitIntInsn(BIPUSH, 2);
+ mv.visitInsn(IRETURN);
+ mv.visitMaxs(2, 1);
+ mv.visitEnd();
+ }
+ cw.visitEnd();
+
+ return cw.toByteArray();
+ }
+ public static byte[] dumpP2C() {
+
+ ClassWriter cw = new ClassWriter(0);
+ MethodVisitor mv;
+
+ cw.visit(V1_6, ACC_PUBLIC + ACC_SUPER, "P2/C", null, "P1/B", null);
+
+ {
+ mv = cw.visitMethod(ACC_PUBLIC, "<init>", "()V", null, null);
+ mv.visitCode();
+ mv.visitVarInsn(ALOAD, 0);
+ mv.visitMethodInsn(INVOKESPECIAL, "P1/B", "<init>", "()V", false);
+ mv.visitInsn(RETURN);
+ mv.visitMaxs(1, 1);
+ mv.visitEnd();
+ }
+ {
+ mv = cw.visitMethod(ACC_PUBLIC, "m", "()I", null, null);
+ mv.visitCode();
+ mv.visitFieldInsn(GETSTATIC, "java/lang/System", "out", "Ljava/io/PrintStream;");
+ mv.visitLdcInsn("P2/C.m");
+ mv.visitMethodInsn(INVOKEVIRTUAL, "java/io/PrintStream", "println", "(Ljava/lang/String;)V", false);
+ mv.visitIntInsn(BIPUSH, 3);
+ mv.visitInsn(IRETURN);
+ mv.visitMaxs(2, 1);
+ mv.visitEnd();
+ }
+ {
+ mv = cw.visitMethod(ACC_PUBLIC + ACC_STATIC, "test", "()I", null, null);
+ mv.visitCode();
+ mv.visitTypeInsn(NEW, "P2/C");
+ mv.visitInsn(DUP);
+ mv.visitMethodInsn(INVOKESPECIAL, "P2/C", "<init>", "()V", false);
+ mv.visitMethodInsn(INVOKEVIRTUAL, "P1/A", "m", "()I", false);
+ mv.visitInsn(IRETURN);
+ mv.visitMaxs(3, 2);
+ mv.visitEnd();
+ }
+
+ cw.visitEnd();
+
+ return cw.toByteArray();
+ }
+}
--- a/hotspot/test/runtime/logging/VtablesTest.java Fri Aug 19 16:02:21 2016 +0200
+++ b/hotspot/test/runtime/logging/VtablesTest.java Fri Aug 19 11:32:43 2016 -0400
@@ -46,10 +46,10 @@
output.shouldContain("copy vtable from ClassA to ClassB");
output.shouldContain("Initializing: ClassB");
output.shouldContain("adding ClassB.Method1()V");
- output.shouldContain("] overriding with ClassB::ClassB.Method2()V");
+ output.shouldContain("] overriding with ClassB.Method2()V");
output.shouldContain("invokevirtual resolved method: caller-class:ClassB");
output.shouldContain("invokevirtual selected method: receiver-class:ClassB");
- output.shouldContain("NOT overriding with p2.D::p2.D.nooverride()V");
+ output.shouldContain("NOT overriding with p2.D.nooverride()V");
output.shouldHaveExitValue(0);
pb = ProcessTools.createJavaProcessBuilder("-Xlog:vtables=trace", "p1/C");