changeset 51254:a5573f4f6392 lworld

Summary: Add value types consistency checks Reviewed-by: acorn
author fparain
date Wed, 11 Jul 2018 12:41:33 -0400
parents 5cbed62223a4
children 44e73ab2123d
files src/hotspot/share/interpreter/linkResolver.cpp src/hotspot/share/interpreter/linkResolver.hpp src/hotspot/share/oops/constantPool.cpp src/hotspot/share/oops/instanceKlass.cpp src/hotspot/share/oops/instanceKlass.hpp src/hotspot/share/oops/klassVtable.cpp test/hotspot/jtreg/compiler/valhalla/valuetypes/TestLWorld.java test/hotspot/jtreg/runtime/valhalla/valuetypes/UninitializedValueFieldsTest.java test/hotspot/jtreg/runtime/valhalla/valuetypes/ValueOops.java test/hotspot/jtreg/runtime/valhalla/valuetypes/ValueTypesTest.java
diffstat 10 files changed, 159 insertions(+), 37 deletions(-) [+]
line wrap: on
line diff
--- a/src/hotspot/share/interpreter/linkResolver.cpp	Wed Jul 11 15:02:10 2018 +0200
+++ b/src/hotspot/share/interpreter/linkResolver.cpp	Wed Jul 11 12:41:33 2018 -0400
@@ -674,6 +674,15 @@
   }
 }
 
+void LinkResolver::check_method_value_types_consistency(const LinkInfo& link_info,
+                                                        const methodHandle& resolved_method,
+                                                        TRAPS) {
+  InstanceKlass::check_signature_for_value_types_consistency(resolved_method->signature(),
+                                                             InstanceKlass::cast(link_info.current_klass()),
+                                                             resolved_method->method_holder() ,
+                                                             CHECK);
+}
+
 void LinkResolver::check_field_loader_constraints(Symbol* field, Symbol* sig,
                                                   Klass* current_klass,
                                                   Klass* sel_klass, TRAPS) {
@@ -705,6 +714,15 @@
   }
 }
 
+void LinkResolver::check_field_value_types_consistency(Symbol* sig,
+                                                  Klass* current_klass,
+                                                  Klass* sel_klass, TRAPS) {
+  InstanceKlass::check_symbol_for_value_types_consistency(sig,
+                                                             InstanceKlass::cast(current_klass),
+                                                             InstanceKlass::cast(sel_klass),
+                                                             CHECK);
+}
+
 methodHandle LinkResolver::resolve_method(const LinkInfo& link_info,
                                           Bytecodes::Code code, TRAPS) {
 
@@ -769,6 +787,9 @@
 
     // check loader constraints
     check_method_loader_constraints(link_info, resolved_method, "method", CHECK_NULL);
+
+    // check ValueTypes attribute consistency
+    check_method_value_types_consistency(link_info, resolved_method, CHECK_NULL);
   }
 
   return resolved_method;
@@ -861,6 +882,10 @@
                                CHECK_NULL);
 
     check_method_loader_constraints(link_info, resolved_method, "interface method", CHECK_NULL);
+
+
+    // check ValueTypes attribute consistency
+    check_method_value_types_consistency(link_info, resolved_method, CHECK_NULL);
   }
 
   if (code != Bytecodes::_invokestatic && resolved_method->is_static()) {
@@ -1021,6 +1046,7 @@
 
   if (sel_klass != current_klass) {
     check_field_loader_constraints(field, sig, current_klass, sel_klass, CHECK);
+    check_field_value_types_consistency(sig, current_klass, sel_klass, CHECK);
   }
 
   // return information. note that the klass is set to the actual klass containing the
@@ -1216,6 +1242,8 @@
       // check loader constraints if found a different method
       } else if (sel_method() != resolved_method()) {
         check_method_loader_constraints(link_info, sel_method, "method", CHECK);
+        // check ValueTypes attribute consistency
+        check_method_value_types_consistency(link_info, resolved_method, CHECK);
       }
     }
 
--- a/src/hotspot/share/interpreter/linkResolver.hpp	Wed Jul 11 15:02:10 2018 +0200
+++ b/src/hotspot/share/interpreter/linkResolver.hpp	Wed Jul 11 12:41:33 2018 -0400
@@ -220,9 +220,16 @@
   static void check_method_loader_constraints(const LinkInfo& link_info,
                                               const methodHandle& resolved_method,
                                               const char* method_type, TRAPS);
+ static void check_method_value_types_consistency(const LinkInfo& link_info,
+                                                    const methodHandle& resolved_method,
+                                                    TRAPS);
   static void check_field_loader_constraints(Symbol* field, Symbol* sig,
                                              Klass* current_klass,
                                              Klass* sel_klass, TRAPS);
+  static void check_field_value_types_consistency(Symbol* sig,
+                                               Klass* current_klass,
+                                               Klass* sel_klass, TRAPS);
+
 
   static methodHandle resolve_interface_method(const LinkInfo& link_info, Bytecodes::Code code, TRAPS);
   static methodHandle resolve_method          (const LinkInfo& link_info, Bytecodes::Code code, TRAPS);
--- a/src/hotspot/share/oops/constantPool.cpp	Wed Jul 11 15:02:10 2018 +0200
+++ b/src/hotspot/share/oops/constantPool.cpp	Wed Jul 11 12:41:33 2018 -0400
@@ -439,6 +439,14 @@
   }
 }
 
+void check_value_types_consistency(const constantPoolHandle& this_cp, Klass* resolved_klass, TRAPS) {
+  bool opinion0 = resolved_klass->is_value();
+  bool opinion1 = this_cp->pool_holder()->is_declared_value_type(resolved_klass->name());
+  if (opinion0 != opinion1) {
+    THROW(vmSymbols::java_lang_IncompatibleClassChangeError());
+  }
+}
+
 Klass* ConstantPool::klass_at_impl(const constantPoolHandle& this_cp, int which,
                                    bool save_resolution_error, TRAPS) {
   assert(THREAD->is_Java_thread(), "must be a Java thread");
@@ -482,6 +490,10 @@
     verify_constant_pool_resolve(this_cp, k, THREAD);
   }
 
+  if (!HAS_PENDING_EXCEPTION) {
+    check_value_types_consistency(this_cp, k, THREAD);
+  }
+
   // Failed to resolve class. We must record the errors so that subsequent attempts
   // to resolve this constant pool entry fail with the same error (JVMS 5.4.3).
   if (HAS_PENDING_EXCEPTION) {
--- a/src/hotspot/share/oops/instanceKlass.cpp	Wed Jul 11 15:02:10 2018 +0200
+++ b/src/hotspot/share/oops/instanceKlass.cpp	Wed Jul 11 12:41:33 2018 -0400
@@ -629,25 +629,36 @@
   // are not pre-loaded. Their loading is triggered by either anewarray
   // or multianewarray bytecodes.
 
-  {
+  if (has_value_types_attribute()) {
     ResourceMark rm(THREAD);
     for (int i = 0; i < methods()->length(); i++) {
       Method* m = methods()->at(i);
       for (SignatureStream ss(m->signature()); !ss.is_done(); ss.next()) {
         Symbol* sig = ss.as_symbol(THREAD);
-        if (is_declared_value_type(sig)) {
-          // Get current loader and protection domain first.
-          oop loader = class_loader();
-          oop protection_domain = this->protection_domain();
-
-          Klass* klass = SystemDictionary::resolve_or_fail(sig,
-                                                           Handle(THREAD, loader), Handle(THREAD, protection_domain), true,
-                                                           CHECK_false);
-          if (klass == NULL) {
-            THROW_(vmSymbols::java_lang_LinkageError(), false);
+        if (ss.is_object()) {
+          Symbol* symb = sig;
+          if (ss.is_array()) {
+            int i=0;
+            while (sig->byte_at(i) == '[') i++;
+            if (i == sig->utf8_length() - 1 ) continue; // primitive array
+            symb = SymbolTable::lookup(sig->as_C_string() + i + 1,
+                                       sig->utf8_length() - 3, CHECK_false);
           }
-          if (!klass->is_value()) {
-            THROW_(vmSymbols::java_lang_IncompatibleClassChangeError(), false);
+          if (is_declared_value_type(symb)) {
+            oop loader = class_loader();
+            oop protection_domain = this->protection_domain();
+            Klass* klass = SystemDictionary::resolve_or_fail(symb,
+                                                             Handle(THREAD, loader), Handle(THREAD, protection_domain), true,
+                                                             CHECK_false);
+            if (symb != sig) {
+              symb->decrement_refcount();
+            }
+            if (klass == NULL) {
+              THROW_(vmSymbols::java_lang_LinkageError(), false);
+            }
+            if (!klass->is_value()) {
+              THROW_(vmSymbols::java_lang_IncompatibleClassChangeError(), false);
+            }
           }
         }
       }
@@ -3261,6 +3272,61 @@
   return false;
 }
 
+void InstanceKlass::check_signature_for_value_types_consistency(Symbol* signature,
+                                                             InstanceKlass* k1,
+                                                             InstanceKlass* k2, TRAPS) {
+  if (signature->utf8_length() == 1) return;  // Primitive signature
+  if (!(k1->has_value_types_attribute() || k2->has_value_types_attribute())) return;
+  ResourceMark rm(THREAD);
+  for (SignatureStream sstream(signature); !sstream.is_done(); sstream.next()) {
+    if (sstream.is_object()) {
+      Symbol* sym = sstream.as_symbol(THREAD);
+      Symbol* name = sym;
+      if (sstream.is_array()) {
+        int i=0;
+        while (sym->byte_at(i) == '[') i++;
+        if (i == sym->utf8_length() - 1 ) continue; // primitive array
+        assert(sym->byte_at(i) == 'L', "Must be a L-type");
+        name = SymbolTable::lookup(sym->as_C_string() + i + 1,
+                                           sym->utf8_length() - 2 - i, CHECK);
+      }
+      bool opinion1 = k1->is_declared_value_type(name);
+      bool opinion2 = k2->is_declared_value_type(name);
+      if (sym != name) name->decrement_refcount();
+      if (opinion1 != opinion2) {
+        THROW(vmSymbols::java_lang_IncompatibleClassChangeError());
+      }
+    }
+  }
+}
+
+void InstanceKlass::check_symbol_for_value_types_consistency(Symbol* sym,
+                                                             InstanceKlass* k1,
+                                                             InstanceKlass* k2, TRAPS) {
+  if (sym->utf8_length() == 1) return;  // Primitive signature
+  if (!(k1->has_value_types_attribute() || k2->has_value_types_attribute())) return;
+  assert(sym->byte_at(0) == 'L' || sym->byte_at(0) == '[', "Sanity check");
+  ResourceMark rm(THREAD);
+  Symbol* name;
+  if (sym->byte_at(0) == 'L') {
+    name = SymbolTable::lookup(sym->as_C_string() + 1,
+                               sym->utf8_length() - 2, CHECK);
+  } else {
+    int i=0;
+    while (sym->byte_at(i) == '[') i++;
+    if (i == sym->utf8_length() - 1 ) return; // primitive array
+    assert(sym->byte_at(i) == 'L', "Must be a L-type");
+    name = SymbolTable::lookup(sym->as_C_string() + i + 1,
+                               sym->utf8_length() - 2 - i, CHECK);
+  }
+  bool opinion1 = k1->is_declared_value_type(name);
+  bool opinion2 = k2->is_declared_value_type(name);
+  name->decrement_refcount();
+  if (opinion1 != opinion2) {
+    THROW(vmSymbols::java_lang_IncompatibleClassChangeError());
+  }
+}
+
 void InstanceKlass::print_class_load_logging(ClassLoaderData* loader_data,
                                              const char* module_name,
                                              const ClassFileStream* cfs) const {
--- a/src/hotspot/share/oops/instanceKlass.hpp	Wed Jul 11 15:02:10 2018 +0200
+++ b/src/hotspot/share/oops/instanceKlass.hpp	Wed Jul 11 12:41:33 2018 -0400
@@ -497,12 +497,13 @@
 
   Array<ValueTypes>* value_types() const       { return _value_types; }
   void set_value_types(Array<ValueTypes>* f)   { _value_types = f; }
-
+  bool has_value_types_attribute() const { return _value_types != NULL; }
   bool is_declared_value_type(int index);
   bool is_declared_value_type(Symbol* symbol);
-
   static bool is_declared_value_type(Array<ValueTypes>* value_types, int index);
   static bool is_declared_value_type(ConstantPool* constants, Array<ValueTypes>* value_types, Symbol* symbol);
+  static void check_signature_for_value_types_consistency(Symbol* sig, InstanceKlass* k1, InstanceKlass* k2, TRAPS);
+  static void check_symbol_for_value_types_consistency(Symbol* sym, InstanceKlass* k1, InstanceKlass* k2, TRAPS);
 
   enum InnerClassAttributeOffset {
     // From http://mirror.eng/products/jdk/1.1/docs/guide/innerclasses/spec/innerclasses.doc10.html#18814
--- a/src/hotspot/share/oops/klassVtable.cpp	Wed Jul 11 15:02:10 2018 +0200
+++ b/src/hotspot/share/oops/klassVtable.cpp	Wed Jul 11 12:41:33 2018 -0400
@@ -526,6 +526,10 @@
               THROW_MSG_(vmSymbols::java_lang_LinkageError(), buf, false);
             }
           }
+          InstanceKlass::check_signature_for_value_types_consistency(signature,
+                                                                     InstanceKlass::cast(target_klass),
+                                                                     InstanceKlass::cast(super_klass),
+                                                                     CHECK_(false));
         }
 
         put_method_at(target_method(), i);
@@ -1257,6 +1261,10 @@
             THROW_MSG(vmSymbols::java_lang_LinkageError(), buf);
           }
         }
+        InstanceKlass::check_signature_for_value_types_consistency(m->signature(),
+                                                                   InstanceKlass::cast(interf),
+                                                                   target()->method_holder(),
+                                                                   CHECK);
       }
 
       // ime may have moved during GC so recalculate address
--- a/test/hotspot/jtreg/compiler/valhalla/valuetypes/TestLWorld.java	Wed Jul 11 15:02:10 2018 +0200
+++ b/test/hotspot/jtreg/compiler/valhalla/valuetypes/TestLWorld.java	Wed Jul 11 12:41:33 2018 -0400
@@ -315,8 +315,8 @@
             aconst_null().
             invokevirtual(TestLWorld.class, "test7", "(Lcompiler/valhalla/valuetypes/MyValue1;)J", false).
             return_();
-        }
-        );
+        },
+        MyValue1.class);
 
     @DontCompile
     public void test7_verifier(boolean warmup) throws Throwable {
@@ -435,8 +435,8 @@
             CODE.
             aconst_null().
             areturn();
-        }
-        );
+        },
+        MyValue1.class);
 
     @Test
     public void test13() throws Throwable {
@@ -464,8 +464,8 @@
             aconst_null().
             putfield(TestLWorld.class, "valueField1", "Lcompiler/valhalla/valuetypes/MyValue1;").
             return_();
-        }
-        );
+        },
+        MyValue1.class);
 
     @Test
     public void test14() throws Throwable {
@@ -524,8 +524,8 @@
             swap().
             putfield(TestLWorld.class, "valueField1", "Lcompiler/valhalla/valuetypes/MyValue1;").
             return_();
-        }
-        );
+        },
+        MyValue1.class);
 
     @Test
     public void test16(boolean flag) throws Throwable {
@@ -597,8 +597,8 @@
             swap().
             putfield(TestLWorld.class, "valueField1", "Lcompiler/valhalla/valuetypes/MyValue1;").
             return_();
-        }
-        );
+        },
+        MyValue1.class);
 
     @Test
     public void test19(boolean flag) throws Throwable {
@@ -1367,8 +1367,8 @@
             aconst_null().
             aastore().
             return_();
-        }
-        );
+        },
+        MyValue1.class);
 
     @Test()
     public void test49(MyValue1[] va, int index) throws Throwable {
@@ -1691,8 +1691,8 @@
             aload_3().
             aastore().
             return_();
-        }
-        );
+        },
+	MyValue1.class, MyValue2.class);
 
     @Test()
     public void test58(MyValue1[] va, int index, MyValue2 v) throws Throwable {
--- a/test/hotspot/jtreg/runtime/valhalla/valuetypes/UninitializedValueFieldsTest.java	Wed Jul 11 15:02:10 2018 +0200
+++ b/test/hotspot/jtreg/runtime/valhalla/valuetypes/UninitializedValueFieldsTest.java	Wed Jul 11 12:41:33 2018 -0400
@@ -34,12 +34,12 @@
  */
 public class UninitializedValueFieldsTest {
     static __NotFlattened Point nonFlattenableStaticPoint;
-    static Point staticPoint;
+    static __Flattenable Point staticPoint;
 
     Point instancePoint;
 
     static __NotFlattened JumboValue sj1;
-    static JumboValue sj2;
+    static __Flattenable JumboValue sj2;
 
     __NotFlattened JumboValue j1;
     JumboValue j2;
--- a/test/hotspot/jtreg/runtime/valhalla/valuetypes/ValueOops.java	Wed Jul 11 15:02:10 2018 +0200
+++ b/test/hotspot/jtreg/runtime/valhalla/valuetypes/ValueOops.java	Wed Jul 11 12:41:33 2018 -0400
@@ -325,7 +325,7 @@
                         .invokestatic(ValueOops.class, "doGc", "()V", false) // Stack,LVT
                         .pop()
                         .areturn();
-                    });
+                    }, vtClass);
             Person person = (Person) moveValueThroughStackAndLvt.invokeExact(createDefaultPerson());
             validateDefaultPerson(person);
             doGc();
@@ -604,7 +604,7 @@
                             .aastore()
                             .pop()
                             .return_();
-                        }).invoke(oopMaps);
+                        }, FooValue.class).invoke(oopMaps);
             } catch (Throwable t) { fail("exerciseVBytecodeExprStackWithDefault", t); }
         }
 
@@ -627,7 +627,7 @@
                             .aastore()
                             .pop()
                             .return_();
-                        }).invoke(fa, oopMaps);
+					     }, FooValue.class).invoke(fa, oopMaps);
             } catch (Throwable t) { fail("exerciseVBytecodeExprStackWithRefs", t); }
         }
     }
--- a/test/hotspot/jtreg/runtime/valhalla/valuetypes/ValueTypesTest.java	Wed Jul 11 15:02:10 2018 +0200
+++ b/test/hotspot/jtreg/runtime/valhalla/valuetypes/ValueTypesTest.java	Wed Jul 11 12:41:33 2018 -0400
@@ -129,7 +129,7 @@
                     .label("end")
                     .iconst_0()
                     .return_(TypeTag.Z);
-                });
+                }, valueClass);
         boolean result = (boolean) fromExecStackToLocalVar.invokeExact();
         System.out.println(result);
         assertTrue(result, "Invariant");
@@ -198,7 +198,7 @@
                     .label("failed")
                     .iconst_0()
                     .return_(TypeTag.Z);
-                });
+                }, valueClass);
         boolean result = (boolean) fromExecStackToFields.invokeExact();
         System.out.println(result);
         assertTrue(result, "Invariant");
@@ -272,7 +272,7 @@
                     .label("failed")
                     .iconst_0()
                     .return_(TypeTag.Z);
-                });
+                }, valueClass);
         boolean result = (boolean) fromExecStackToValueArray.invokeExact();
         System.out.println(result);
         assertTrue(result, "Invariant");