8059357: ClassVerifier redundantly checks constant pool entries multiple times
authorhseigel
Wed, 27 Mar 2019 09:29:34 -0400
changeset 54308 1dcacbe612ae
parent 54307 df62ea1da6ad
child 54309 c4c225b49c5f
8059357: ClassVerifier redundantly checks constant pool entries multiple times Summary: Store translated signature verificationTypes in a hashtable where they can be fetched, instead of having to re-translating the signatures Reviewed-by: iklam, coleenp
src/hotspot/share/classfile/verifier.cpp
src/hotspot/share/classfile/verifier.hpp
--- a/src/hotspot/share/classfile/verifier.cpp	Wed Mar 27 18:33:48 2019 +0530
+++ b/src/hotspot/share/classfile/verifier.cpp	Wed Mar 27 09:29:34 2019 -0400
@@ -574,7 +574,7 @@
 ClassVerifier::ClassVerifier(
     InstanceKlass* klass, TRAPS)
     : _thread(THREAD), _previous_symbol(NULL), _symbols(NULL), _exception_type(NULL),
-      _message(NULL), _klass(klass) {
+      _message(NULL), _method_signatures_table(NULL), _klass(klass) {
   _this_type = VerificationType::reference_type(klass->name());
 }
 
@@ -601,6 +601,13 @@
 void ClassVerifier::verify_class(TRAPS) {
   log_info(verification)("Verifying class %s with new format", _klass->external_name());
 
+  // Either verifying both local and remote classes or just remote classes.
+  assert(BytecodeVerificationRemote, "Should not be here");
+
+  // Create hash table containing method signatures.
+  method_signatures_table_type method_signatures_table;
+  set_method_signatures_table(&method_signatures_table);
+
   Array<Method*>* methods = _klass->methods();
   int num_methods = methods->length();
 
@@ -625,6 +632,55 @@
   }
 }
 
+// Translate the signature entries into verification types and save them in
+// the growable array.  Also, save the count of arguments.
+void ClassVerifier::translate_signature(Symbol* const method_sig,
+                                        sig_as_verification_types* sig_verif_types,
+                                        TRAPS) {
+  SignatureStream sig_stream(method_sig);
+  VerificationType sig_type[2];
+  int sig_i = 0;
+  GrowableArray<VerificationType>* verif_types = sig_verif_types->sig_verif_types();
+
+  // Translate the signature arguments into verification types.
+  while (!sig_stream.at_return_type()) {
+    int n = change_sig_to_verificationType(&sig_stream, sig_type, CHECK_VERIFY(this));
+    assert(n <= 2, "Unexpected signature type");
+
+    // Store verification type(s).  Longs and Doubles each have two verificationTypes.
+    for (int x = 0; x < n; x++) {
+      verif_types->push(sig_type[x]);
+    }
+    sig_i += n;
+    sig_stream.next();
+  }
+
+  // Set final arg count, not including the return type.  The final arg count will
+  // be compared with sig_verify_types' length to see if there is a return type.
+  sig_verif_types->set_num_args(sig_i);
+
+  // Store verification type(s) for the return type, if there is one.
+  if (sig_stream.type() != T_VOID) {
+    int n = change_sig_to_verificationType(&sig_stream, sig_type, CHECK_VERIFY(this));
+    assert(n <= 2, "Unexpected signature return type");
+    for (int y = 0; y < n; y++) {
+      verif_types->push(sig_type[y]);
+    }
+  }
+}
+
+void ClassVerifier::create_method_sig_entry(sig_as_verification_types* sig_verif_types,
+                                            int sig_index, TRAPS) {
+  // Translate the signature into verification types.
+  ConstantPool* cp = _klass->constants();
+  Symbol* const method_sig = cp->symbol_at(sig_index);
+  translate_signature(method_sig, sig_verif_types, CHECK_VERIFY(this));
+
+  // Add the list of this signature's verification types to the table.
+  bool is_unique = method_signatures_table()->put(sig_index, sig_verif_types);
+  assert(is_unique, "Duplicate entries in method_signature_table");
+}
+
 void ClassVerifier::verify_method(const methodHandle& m, TRAPS) {
   HandleMark hm(THREAD);
   _method = m;   // initialize _method
@@ -2734,44 +2790,28 @@
     ref_class_type = cp_ref_index_to_type(index, cp, CHECK_VERIFY(this));
   }
 
-  // For a small signature length, we just allocate 128 bytes instead
-  // of parsing the signature once to find its size.
-  // -3 is for '(', ')' and return descriptor; multiply by 2 is for
-  // longs/doubles to be consertive.
   assert(sizeof(VerificationType) == sizeof(uintptr_t),
         "buffer type must match VerificationType size");
-  uintptr_t on_stack_sig_types_buffer[128];
-  // If we make a VerificationType[128] array directly, the compiler calls
-  // to the c-runtime library to do the allocation instead of just
-  // stack allocating it.  Plus it would run constructors.  This shows up
-  // in performance profiles.
+
+  // Get the UTF8 index for this signature.
+  int sig_index = cp->signature_ref_index_at(cp->name_and_type_ref_index_at(index));
 
-  VerificationType* sig_types;
-  int size = (method_sig->utf8_length() - 3) * 2;
-  if (size > 128) {
-    // Long and double occupies two slots here.
-    ArgumentSizeComputer size_it(method_sig);
-    size = size_it.size();
-    sig_types = NEW_RESOURCE_ARRAY_IN_THREAD(THREAD, VerificationType, size);
-  } else{
-    sig_types = (VerificationType*)on_stack_sig_types_buffer;
+  // Get the signature's verification types.
+  sig_as_verification_types* mth_sig_verif_types;
+  sig_as_verification_types** mth_sig_verif_types_ptr = method_signatures_table()->get(sig_index);
+  if (mth_sig_verif_types_ptr != NULL) {
+    // Found the entry for the signature's verification types in the hash table.
+    mth_sig_verif_types = *mth_sig_verif_types_ptr;
+    assert(mth_sig_verif_types != NULL, "Unexpected NULL sig_as_verification_types value");
+  } else {
+    // Not found, add the entry to the table.
+    GrowableArray<VerificationType>* verif_types = new GrowableArray<VerificationType>(10);
+    mth_sig_verif_types = new sig_as_verification_types(verif_types);
+    create_method_sig_entry(mth_sig_verif_types, sig_index, CHECK_VERIFY(this));
   }
-  SignatureStream sig_stream(method_sig);
-  int sig_i = 0;
-  while (!sig_stream.at_return_type()) {
-    sig_i += change_sig_to_verificationType(
-      &sig_stream, &sig_types[sig_i], CHECK_VERIFY(this));
-    sig_stream.next();
-  }
-  int nargs = sig_i;
 
-#ifdef ASSERT
-  {
-    ArgumentSizeComputer size_it(method_sig);
-    assert(nargs == size_it.size(), "Argument sizes do not match");
-    assert(nargs <= (method_sig->utf8_length() - 3) * 2, "estimate of max size isn't conservative enough");
-  }
-#endif
+  // Get the number of arguments for this signature.
+  int nargs = mth_sig_verif_types->num_args();
 
   // Check instruction operands
   u2 bci = bcs->bci();
@@ -2844,10 +2884,16 @@
     }
 
   }
+
+  // Get the verification types for the method's arguments.
+  GrowableArray<VerificationType>* sig_verif_types = mth_sig_verif_types->sig_verif_types();
+  assert(sig_verif_types != NULL, "Missing signature's array of verification types");
   // Match method descriptor with operand stack
-  for (int i = nargs - 1; i >= 0; i--) {  // Run backwards
-    current_frame->pop_stack(sig_types[i], CHECK_VERIFY(this));
+  // The arguments are on the stack in descending order.
+  for (int i = nargs - 1; i >= 0; i--) { // Run backwards
+    current_frame->pop_stack(sig_verif_types->at(i), CHECK_VERIFY(this));
   }
+
   // Check objectref on operand stack
   if (opcode != Bytecodes::_invokestatic &&
       opcode != Bytecodes::_invokedynamic) {
@@ -2919,7 +2965,8 @@
     }
   }
   // Push the result type.
-  if (sig_stream.type() != T_VOID) {
+  int sig_verif_types_len = sig_verif_types->length();
+  if (sig_verif_types_len > nargs) {  // There's a return type
     if (method_name == vmSymbols::object_initializer_name()) {
       // <init> method must have a void return type
       /* Unreachable?  Class file parser verifies that methods with '<' have
@@ -2928,11 +2975,13 @@
           "Return type must be void in <init> method");
       return;
     }
-    VerificationType return_type[2];
-    int n = change_sig_to_verificationType(
-      &sig_stream, return_type, CHECK_VERIFY(this));
-    for (int i = 0; i < n; i++) {
-      current_frame->push_stack(return_type[i], CHECK_VERIFY(this)); // push types backwards
+
+    assert(sig_verif_types_len <= nargs + 2,
+           "Signature verification types array return type is bogus");
+    for (int i = nargs; i < sig_verif_types_len; i++) {
+      assert(i == nargs || sig_verif_types->at(i).is_long2() ||
+             sig_verif_types->at(i).is_double2(), "Unexpected return verificationType");
+      current_frame->push_stack(sig_verif_types->at(i), CHECK_VERIFY(this));
     }
   }
 }
--- a/src/hotspot/share/classfile/verifier.hpp	Wed Mar 27 18:33:48 2019 +0530
+++ b/src/hotspot/share/classfile/verifier.hpp	Wed Mar 27 09:29:34 2019 -0400
@@ -31,6 +31,7 @@
 #include "runtime/handles.hpp"
 #include "utilities/exceptions.hpp"
 #include "utilities/growableArray.hpp"
+#include "utilities/resourceHash.hpp"
 
 // The verifier class
 class Verifier : AllStatic {
@@ -246,6 +247,33 @@
   void stackmap_details(outputStream* ss, const Method* method) const;
 };
 
+class sig_as_verification_types : public ResourceObj {
+ private:
+  int _num_args;  // Number of arguments, not including return type.
+  GrowableArray<VerificationType>* _sig_verif_types;
+
+ public:
+
+  sig_as_verification_types(GrowableArray<VerificationType>* sig_verif_types) :
+    _num_args(0), _sig_verif_types(sig_verif_types) {
+  }
+
+  int num_args() const { return _num_args; }
+  void set_num_args(int num_args) { _num_args = num_args; }
+
+  GrowableArray<VerificationType>* sig_verif_types() { return _sig_verif_types; }
+  void set_sig_verif_types(GrowableArray<VerificationType>* sig_verif_types) {
+    _sig_verif_types = sig_verif_types;
+  }
+
+};
+
+// This hashtable is indexed by the Utf8 constant pool indexes pointed to
+// by constant pool (Interface)Method_refs' NameAndType signature entries.
+typedef ResourceHashtable<int, sig_as_verification_types*,
+                          primitive_hash<int>, primitive_equals<int>, 1007>
+                          method_signatures_table_type;
+
 // A new instance of this class is created for each class being verified
 class ClassVerifier : public StackObj {
  private:
@@ -257,6 +285,8 @@
   Symbol* _exception_type;
   char* _message;
 
+  method_signatures_table_type* _method_signatures_table;
+
   ErrorContext _error_context;  // contains information about an error
 
   void verify_method(const methodHandle& method, TRAPS);
@@ -383,6 +413,13 @@
   // the message_buffer will be filled in with the exception message.
   void verify_class(TRAPS);
 
+  // Translates method signature entries into verificationTypes and saves them
+  // in the growable array.
+  void translate_signature(Symbol* const method_sig, sig_as_verification_types* sig_verif_types, TRAPS);
+
+  // Initializes a sig_as_verification_types entry and puts it in the hash table.
+  void create_method_sig_entry(sig_as_verification_types* sig_verif_types, int sig_index, TRAPS);
+
   // Return status modes
   Symbol* result() const { return _exception_type; }
   bool has_error() const { return result() != NULL; }
@@ -400,6 +437,14 @@
 
   Klass* load_class(Symbol* name, TRAPS);
 
+  method_signatures_table_type* method_signatures_table() const {
+    return _method_signatures_table;
+  }
+
+  void set_method_signatures_table(method_signatures_table_type* method_signatures_table) {
+    _method_signatures_table = method_signatures_table;
+  }
+
   int change_sig_to_verificationType(
     SignatureStream* sig_type, VerificationType* inference_type, TRAPS);