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
--- 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);