changeset 13579:5e389fdf564c mvt

8184919: Ensure JVMS spec rules for creation and loading of DVT are followed Summary: Attempt to resolve DVT if MVT enabled and name matches mangling Reviewed-by: fparain, mcimadamore
author dsimms
date Wed, 13 Sep 2017 11:37:04 +0200
parents cf519e23a3f1
children 010b6b654b23
files src/share/vm/classfile/classFileParser.cpp src/share/vm/classfile/systemDictionary.cpp src/share/vm/classfile/systemDictionary.hpp src/share/vm/oops/instanceKlass.cpp src/share/vm/oops/instanceKlass.hpp src/share/vm/runtime/fieldType.cpp src/share/vm/runtime/fieldType.hpp test/runtime/valhalla/valuetypes/DeriveValueTypeCreation.java
diffstat 8 files changed, 246 insertions(+), 37 deletions(-) [+]
line wrap: on
line diff
--- a/src/share/vm/classfile/classFileParser.cpp	Thu Sep 07 13:52:56 2017 -0400
+++ b/src/share/vm/classfile/classFileParser.cpp	Wed Sep 13 11:37:04 2017 +0200
@@ -2182,6 +2182,9 @@
 void ClassFileParser::ClassAnnotationCollector::apply_to(InstanceKlass* ik) {
   assert(ik != NULL, "invariant");
   ik->set_is_contended(is_contended());
+  if (is_value_capable_class()) {
+    ik->set_has_vcc_annotation();
+  }
 }
 
 #define MAX_ARGS_SIZE 255
--- a/src/share/vm/classfile/systemDictionary.cpp	Thu Sep 07 13:52:56 2017 -0400
+++ b/src/share/vm/classfile/systemDictionary.cpp	Wed Sep 13 11:37:04 2017 +0200
@@ -61,6 +61,7 @@
 #include "oops/oop.inline.hpp"
 #include "oops/symbol.hpp"
 #include "oops/typeArrayKlass.hpp"
+#include "oops/valueKlass.hpp"
 #include "prims/jvm.h"
 #include "prims/jvmtiEnvBase.hpp"
 #include "prims/resolvedMethodTable.hpp"
@@ -288,6 +289,34 @@
   return k;
 }
 
+// Temporary Minimal Value Type support code. Attempt to load VCC for DVT descriptor
+Klass* SystemDictionary::resolve_dvt_or_null(Symbol* class_name, Handle class_loader, Handle protection_domain, TRAPS) {
+  assert(EnableMVT && FieldType::is_dvt_postfix(class_name), "Invariant");
+
+  ResourceMark rm(THREAD);
+  // Given a "Q-type" descriptor and EnableMVT, original exception is not so interesting
+  CLEAR_PENDING_EXCEPTION;
+
+  TempNewSymbol vcc_name = SymbolTable::new_symbol(FieldType::dvt_unmangle_vcc(class_name), CHECK_NULL);
+  Klass* vcc = do_resolve_instance_class_or_null(vcc_name, class_loader, protection_domain, CHECK_NULL);
+  if (vcc == NULL) {
+    return NULL;
+  }
+  Klass* dvt = do_resolve_instance_class_or_null(class_name, class_loader, protection_domain, CHECK_NULL);
+  if ((dvt !=NULL) && dvt->is_value() && (ValueKlass::cast(dvt)->get_vcc_klass() == vcc)) {
+    return dvt;
+  }
+  if (vcc->is_instance_klass() && (!InstanceKlass::cast(vcc)->has_vcc_annotation())) {
+    static const char not_vcc_msg[] =
+        "Failed to resolve %s, found possible ValueCapableClass name mangle match is not ValueCapableClass annotated: %s";
+    size_t buflen = strlen(not_vcc_msg) + class_name->utf8_length() + vcc_name->utf8_length();
+    char* buf = NEW_RESOURCE_ARRAY_IN_THREAD(THREAD, char, buflen);
+    jio_snprintf(buf, buflen, not_vcc_msg, class_name->as_C_string(), vcc_name->as_C_string());
+    THROW_MSG_NULL(vmSymbols::java_lang_NoClassDefFoundError(), buf);
+  }
+  return NULL;
+}
+
 
 // Must be called for any super-class or super-interface resolution
 // during class definition to allow class circularity checking
@@ -643,11 +672,22 @@
 #endif // INCLUDE_TRACE
 }
 
+Klass* SystemDictionary::resolve_instance_class_or_null(Symbol* name,
+                                                        Handle class_loader,
+                                                        Handle protection_domain,
+                                                        TRAPS) {
+  Klass* k = do_resolve_instance_class_or_null(name, class_loader, protection_domain, THREAD);
+  if (EnableMVT && (k == NULL) && FieldType::is_dvt_postfix(name)) {
+    k = resolve_dvt_or_null(name, class_loader, protection_domain, THREAD);
+  }
+  return k;
+}
+
 // Be careful when modifying this code: once you have run
 // placeholders()->find_and_add(PlaceholderTable::LOAD_INSTANCE),
 // you need to find_and_remove it before returning.
 // So be careful to not exit with a CHECK_ macro betweeen these calls.
-Klass* SystemDictionary::resolve_instance_class_or_null(Symbol* name,
+Klass* SystemDictionary::do_resolve_instance_class_or_null(Symbol* name,
                                                         Handle class_loader,
                                                         Handle protection_domain,
                                                         TRAPS) {
--- a/src/share/vm/classfile/systemDictionary.hpp	Thu Sep 07 13:52:56 2017 -0400
+++ b/src/share/vm/classfile/systemDictionary.hpp	Wed Sep 13 11:37:04 2017 +0200
@@ -624,7 +624,9 @@
 
   // Basic loading operations
   static Klass* resolve_instance_class_or_null(Symbol* class_name, Handle class_loader, Handle protection_domain, TRAPS);
+  static Klass* do_resolve_instance_class_or_null(Symbol* class_name, Handle class_loader, Handle protection_domain, TRAPS);
   static Klass* resolve_array_class_or_null(Symbol* class_name, Handle class_loader, Handle protection_domain, TRAPS);
+  static Klass* resolve_dvt_or_null(Symbol* class_name, Handle class_loader, Handle protection_domain, TRAPS);
   static InstanceKlass* handle_parallel_super_load(Symbol* class_name, Symbol* supername, Handle class_loader, Handle protection_domain, Handle lockObject, TRAPS);
   // Wait on SystemDictionary_lock; unlocks lockObject before
   // waiting; relocks lockObject with correct recursion count
--- a/src/share/vm/oops/instanceKlass.cpp	Thu Sep 07 13:52:56 2017 -0400
+++ b/src/share/vm/oops/instanceKlass.cpp	Wed Sep 13 11:37:04 2017 +0200
@@ -3865,8 +3865,8 @@
 #endif
 
 #define THROW_DVT_ERROR(s) \
-  Exceptions::fthrow(THREAD_AND_LOCATION, vmSymbols::java_lang_InternalError(), \
-      "DeriveValueType class '%s' %s", external_name(),(s)); \
+  Exceptions::fthrow(THREAD_AND_LOCATION, vmSymbols::java_lang_IncompatibleClassChangeError(), \
+      "ValueCapableClass class '%s' %s", external_name(),(s)); \
       return
 
 void InstanceKlass::create_value_capable_class(Handle class_loader, Handle protection_domain, TRAPS) {
--- a/src/share/vm/oops/instanceKlass.hpp	Thu Sep 07 13:52:56 2017 -0400
+++ b/src/share/vm/oops/instanceKlass.hpp	Wed Sep 13 11:37:04 2017 +0200
@@ -213,10 +213,11 @@
  public:
   enum {
     _extra_is_being_redefined   = 1 << 0, // used for locking redefinition
-    _extra_has_value_fields     = 1 << 1, // has value fields and related embedded section is not empty
-    _extra_is_bufferable        = 1 << 2, // value can be buffered out side of the Java heap
-    _extra_has_vcc_klass        = 1 << 3, // has a pointer to its Value Capable Class (MVT)
-    _extra_has_resolved_methods = 1 << 4  // resolved methods table entries added for this class
+    _extra_has_resolved_methods = 1 << 1, // resolved methods table entries added for this class
+    _extra_has_value_fields     = 1 << 2, // has value fields and related embedded section is not empty
+    _extra_is_bufferable        = 1 << 3, // value can be buffered out side of the Java heap
+    _extra_has_vcc_klass        = 1 << 4, // has a pointer to its Value Capable Class (MVT)
+    _extra_has_vcc_annotation   = 1 << 5
   };
 
  protected:
@@ -399,6 +400,13 @@
     _extra_flags |= _extra_has_vcc_klass;
   }
 
+  bool has_vcc_annotation() const {
+    return (_extra_flags &_extra_has_vcc_annotation) != 0;
+  }
+
+  void set_has_vcc_annotation() {
+    _extra_flags |= _extra_has_vcc_annotation;
+  }
 
   // field sizes
   int nonstatic_field_size() const         { return _nonstatic_field_size; }
--- a/src/share/vm/runtime/fieldType.cpp	Thu Sep 07 13:52:56 2017 -0400
+++ b/src/share/vm/runtime/fieldType.cpp	Wed Sep 13 11:37:04 2017 +0200
@@ -74,6 +74,31 @@
   return false;
 }
 
+static const char dvt_postfix[] = "$Value";
+static const int dvt_postfix_len = 6;
+
+bool FieldType::is_dvt_postfix(Symbol* signature) {
+  assert(strlen(dvt_postfix) == dvt_postfix_len, "Invariant");
+  int sig_length = signature->utf8_length();
+  int pos = sig_length - dvt_postfix_len;
+  if (pos <= 0) {
+    return false;
+  }
+  for (int i = 0; i < dvt_postfix_len; i++) {
+    if (signature->byte_at(pos) != dvt_postfix[i]) {
+      return false;
+    }
+    pos++;
+  }
+  return true;
+}
+
+char* FieldType::dvt_unmangle_vcc(Symbol* signature) {
+  assert(is_dvt_postfix(signature), "Unmangle that which is not managled");
+  char* str = signature->as_C_string();
+  str[signature->utf8_length() -dvt_postfix_len] = '\0';
+  return str;
+}
 
 BasicType FieldType::get_array_info(Symbol* signature, FieldArrayInfo& fd, TRAPS) {
   assert(basic_type(signature) == T_ARRAY, "must be array");
--- a/src/share/vm/runtime/fieldType.hpp	Thu Sep 07 13:52:56 2017 -0400
+++ b/src/share/vm/runtime/fieldType.hpp	Wed Sep 13 11:37:04 2017 +0200
@@ -77,6 +77,10 @@
         (signature->byte_at(sig_length - 1) == ';'));
   }
 
+  // MVT name mangling, VM derived value type naming Foo->Foo$Value
+  static bool is_dvt_postfix(Symbol* signature);
+  static char* dvt_unmangle_vcc(Symbol* signature);
+
   // Parse field and extract array information. Works for T_ARRAY only.
   static BasicType get_array_info(Symbol* signature, FieldArrayInfo& ai, TRAPS);
 };
--- a/test/runtime/valhalla/valuetypes/DeriveValueTypeCreation.java	Thu Sep 07 13:52:56 2017 -0400
+++ b/test/runtime/valhalla/valuetypes/DeriveValueTypeCreation.java	Wed Sep 13 11:37:04 2017 +0200
@@ -22,14 +22,15 @@
  */
 package runtime.valhalla.valuetypes;
 
-import java.io.IOException;
-import java.io.RandomAccessFile;
 import java.lang.reflect.Field;
+import java.lang.reflect.InvocationTargetException;
 import java.lang.reflect.Method;
 import static java.lang.reflect.Modifier.*;
 import java.net.URL;
-import java.util.Enumeration;
+import java.util.ArrayList;
+import java.util.HashMap;
 
+import jdk.experimental.bytecode.*;
 import jdk.experimental.value.ValueType;
 
 import jdk.internal.org.objectweb.asm.*;
@@ -41,6 +42,7 @@
  * @test DeriveValueTypeCreation
  * @summary Derive Value Type creation test
  * @library /test/lib
+ * @compile DeriveValueTypeCreation.java
  * @modules java.base/jdk.internal.org.objectweb.asm
  * @build runtime.valhalla.valuetypes.ValueCapableClass
  * @run main/othervm -Xint -XX:+EnableMVT runtime.valhalla.valuetypes.DeriveValueTypeCreation
@@ -49,6 +51,11 @@
 public class DeriveValueTypeCreation {
 
     public static final String VCC_CLASSNAME = "runtime.valhalla.valuetypes.ValueCapableClass";
+    public static final String DVT_SUFFIX = "$Value";
+
+    public static final int TEST_DEF_CLASS_ACCESS = ACC_SUPER | ACC_PUBLIC | ACC_FINAL;
+    public static final int TEST_DEF_FIELD_ACCESS = ACC_PUBLIC | ACC_FINAL;
+    public static final String OBJECT_CLASS_DESC  = "java/lang/Object";
 
     public static void main(String[] args) {
         DeriveValueTypeCreation test = new DeriveValueTypeCreation();
@@ -58,6 +65,7 @@
     public void run() {
         loadAndRunTest();
         notValueCapableClasses();
+        loadDvtFirst();
     }
 
     void loadAndRunTest() {
@@ -94,7 +102,7 @@
 
         // DVT class matches our expectations for the current implementation...
         Class<?> vtClass = vt.valueClass();
-        if (!vtClass.getName().equals(clazz.getName() + "$Value")) {
+        if (!vtClass.getName().equals(clazz.getName() + DVT_SUFFIX)) {
             fail("ValueType.valueClass() failed");
         }
         if (!vtClass.getSuperclass().getName().equals("java.lang.__Value")) {
@@ -114,23 +122,19 @@
     }
 
     void notValueCapableClasses() {
-        int vccKlassAccess = ACC_SUPER | ACC_PUBLIC | ACC_FINAL;
-        int vccFieldAccess = ACC_PUBLIC | ACC_FINAL;
-        String vccSuperClass = "java/lang/Object";
-
-        // First a control test to check createTestClass is working
+        // First a control test to check createTestVccClass is working
         try {
-            Class<?> cls = createTestClass("Control_Case_is_a_VCC", vccKlassAccess, vccSuperClass, "I", vccFieldAccess);
+            Class<?> cls = createTestVccClass("Control_Case_is_a_VCC", TEST_DEF_CLASS_ACCESS, OBJECT_CLASS_DESC, "I", TEST_DEF_FIELD_ACCESS);
             checkValueCapableClass(cls);
         }
         catch (Exception e) {
             fail("Control test failed", e);
         }
 
-        testFailCase("Not_a_final_class", ACC_SUPER | ACC_PUBLIC, vccSuperClass, "I", vccFieldAccess, "not a final class");
-        testFailCase("No_fields", vccKlassAccess, vccSuperClass, null, vccFieldAccess, "has no instance fields");
-        testFailCase("Not_final_field", vccKlassAccess, vccSuperClass, "I", ACC_PUBLIC, "contains non-final instance field");
-        testFailCase("Super_not_Object", vccKlassAccess, "java/lang/Throwable", "I", vccFieldAccess, "does not derive from Object");
+        testFailCase("Not_a_final_class", ACC_SUPER | ACC_PUBLIC, OBJECT_CLASS_DESC, "I", TEST_DEF_FIELD_ACCESS, "not a final class");
+        testFailCase("No_fields", TEST_DEF_CLASS_ACCESS, OBJECT_CLASS_DESC, null, TEST_DEF_FIELD_ACCESS, "has no instance fields");
+        testFailCase("Not_final_field", TEST_DEF_CLASS_ACCESS, OBJECT_CLASS_DESC, "I", ACC_PUBLIC, "contains non-final instance field");
+        testFailCase("Super_not_Object", TEST_DEF_CLASS_ACCESS, "java/lang/Throwable", "I", TEST_DEF_FIELD_ACCESS, "does not derive from Object");
     }
 
     void testFailCase(String clsName,
@@ -140,7 +144,7 @@
                       int fieldAccess,
                       String errMsgRequired) {
         try {
-            createTestClass(clsName, klassAccess, superKlass, fieldType, fieldAccess);
+            createTestVccClass(clsName, klassAccess, superKlass, fieldType, fieldAccess);
             fail(clsName + " : failed to fail with Error");
         }
         catch (ClassNotFoundException cnfe) {
@@ -153,38 +157,161 @@
         }
     }
 
-    Class<?> createTestClass(String name,
+    byte[] createTestVccClassBytes(String name,
+                                boolean vccAnnotation) {
+        return createTestVccClassBytes(name, TEST_DEF_CLASS_ACCESS, vccAnnotation, OBJECT_CLASS_DESC, "I", TEST_DEF_FIELD_ACCESS);
+    }
+
+    byte[] createTestVccClassBytes(String name,
+                                int klassAccess,
+                                String superKlass,
+                                String fieldType,
+                                int fieldAccess)  {
+        return createTestVccClassBytes(name, klassAccess, true, superKlass, fieldType, fieldAccess);
+    }
+
+    byte[] createTestVccClassBytes(String name,
+                                int klassAccess,
+                                boolean vccAnnotation,
+                                String superKlass,
+                                String fieldType,
+                                int fieldAccess)  {
+        ClassWriter cw = new ClassWriter(0);
+        cw.visit(52, klassAccess, name, null, superKlass, null);
+        if (vccAnnotation ) {
+            cw.visitAnnotation("Ljvm/internal/value/ValueCapableClass;", true);
+        }
+        if (fieldType != null) {
+            cw.visitField(fieldAccess, "x", fieldType, null, null);
+        }
+        cw.visitEnd();
+        return cw.toByteArray();
+    }
+
+    Class<?> createTestVccClass(String name,
                              int klassAccess,
                              String superKlass,
                              String fieldType,
                              int fieldAccess) throws ClassNotFoundException {
-        ClassWriter cw = new ClassWriter(0);
-        cw.visit(52, klassAccess, name, null, superKlass, null);
-        cw.visitAnnotation("Ljvm/internal/value/ValueCapableClass;", true);
-        if (fieldType != null) {
-            cw.visitField(fieldAccess, "x", fieldType, null, null);
-        }
-        cw.visitEnd();
-        return new TestClassLoader(name, cw.toByteArray()).loadClass(name);
+        return new TestClassLoader(name,
+                                   createTestVccClassBytes(name, klassAccess, superKlass, fieldType, fieldAccess)).loadClass(name);
     }
 
     class TestClassLoader extends ClassLoader {
 
-        String name;
-        byte[] classBytes;
+        HashMap<String, byte[]> namedBytes = new HashMap<>();
+        ArrayList<String> findNames = new ArrayList<String>();
 
+        TestClassLoader() {}
         TestClassLoader(String name, byte[] classBytes) {
-            this.name = name;
-            this.classBytes = classBytes;
+            addNamedBytes(name, classBytes);
+        }
+
+        void addNamedBytes(String name, byte[] classBytes) {
+            namedBytes.put(name, classBytes);
         }
 
         @Override
         public Class findClass(String name) throws ClassNotFoundException {
-            if (this.name.equals(name)) {
+            byte[] classBytes = null;
+            synchronized (findNames) {
+                findNames.add(name);
+                classBytes = namedBytes.get(name);
+            }
+            if (classBytes != null) {
                 return defineClass(name, classBytes, 0, classBytes.length);
             }
-            throw new ClassNotFoundException();
+            throw new ClassNotFoundException(name);
+        }
+
+        public ArrayList<String> getFindNames() {
+            return findNames;
         }
     }
 
+    void loadDvtFirst() {
+        try {
+            loadDvtFirstNoVcc();
+            loadDvtFirstNotVcc();
+            loadDvtFirstBadVcc();
+            loadDvtFirstVcc();
+        } catch (Throwable t) {
+            fail("loadDvtFirst failed", t);
+        }
+    }
+
+    void loadDvtFirstNoVcc() throws Throwable {
+        String vccName = "TestNoVcc";
+        try {
+            newDvtUserInstance(vccName, null, false);
+        } catch (NoClassDefFoundError ncdfe) {}
+        try {
+            newDvtUserInstance(vccName, null, true);
+        } catch (NoClassDefFoundError ncdfe) {}
+    }
+
+    void loadDvtFirstNotVcc() throws Throwable {
+        String vccName = "TestNotVcc";
+        byte[] vccBytes = createTestVccClassBytes(vccName, false);
+        try {
+            newDvtUserInstance(vccName, vccBytes, false);
+        } catch (NoClassDefFoundError ncdfe) {}
+        try {
+            newDvtUserInstance(vccName, vccBytes, true);
+        } catch (NoClassDefFoundError ncdfe) {}
+    }
+
+    void loadDvtFirstBadVcc() throws Throwable {
+        String vccName = "TestBadVcc";
+        byte[] vccBytes = createTestVccClassBytes(vccName, TEST_DEF_CLASS_ACCESS,
+                                                  true, OBJECT_CLASS_DESC, "I",
+                                                  ACC_PUBLIC);
+        try {
+            newDvtUserInstance(vccName, vccBytes, false);
+        } catch (IncompatibleClassChangeError icce) {}
+        try {
+            newDvtUserInstance(vccName, vccBytes, true);
+        } catch (IncompatibleClassChangeError icce) {}
+    }
+
+    void loadDvtFirstVcc() throws Throwable {
+        String vccName = "TestValidVcc";
+        byte[] vccBytes = createTestVccClassBytes(vccName, TEST_DEF_CLASS_ACCESS,
+                                                  true, OBJECT_CLASS_DESC, "I",
+                                                  TEST_DEF_FIELD_ACCESS);
+        newDvtUserInstance(vccName, vccBytes, false);
+        newDvtUserInstance(vccName, vccBytes, true);
+    }
+
+    void newDvtUserInstance(String vccName, byte[] vccBytes, boolean withField) throws Throwable {
+        TestClassLoader cl = new TestClassLoader();
+        if (vccBytes != null) {
+            cl.addNamedBytes(vccName, vccBytes);
+        }
+        String dvtUserName = "UseValidDvt";
+        String dvtName = vccName + DVT_SUFFIX;
+        String dvtFieldDesc = "Q" + dvtName + ";";
+        String dvtClassDesc = ";" + dvtFieldDesc;
+        byte[] classBytes = createTestDvtUserClassBytes(dvtUserName, dvtClassDesc, (withField) ? dvtFieldDesc : null);
+        cl.addNamedBytes(dvtUserName, classBytes);
+        try {
+            Class.forName(dvtUserName, true, cl).getDeclaredConstructor().newInstance();
+        } catch (InvocationTargetException ite) { throw ite.getTargetException(); }
+    }
+
+    byte[] createTestDvtUserClassBytes(String className, String dvtDesc, String dvtFieldDesc) {
+        BasicClassBuilder builder = new BasicClassBuilder(className, 53, 1)
+            .withFlags(Flag.ACC_PUBLIC)
+            .withSuperclass("java/lang/Object")
+            .withMethod("<init>", "()V", M ->
+                M.withFlags(Flag.ACC_PUBLIC).withCode(TypedCodeBuilder::new, C ->
+                    C
+                    .load(0).invokespecial("java/lang/Object", "<init>", "()V", false)
+                    .iconst_1().anewarray(dvtDesc).pop()
+                    .return_()));
+        if (dvtFieldDesc != null) {
+            builder.withField("dvtField", dvtFieldDesc);
+        }
+        return builder.build();
+    }
 }