8026478: -XX:+VerifyAdapterSharing is broken

Summary: Fix by considering all checks in StubRoutines
Reviewed-by: kvn, twisti

src/share/vm/runtime/sharedRuntime.cpp

@@ -2400,7 +2400,7 @@ AdapterHandlerEntry* AdapterHandlerLibrary::get_adapter(methodHandle method) {
   ResourceMark rm;
 
   NOT_PRODUCT(int insts_size);
-  AdapterBlob* B = NULL;
+  AdapterBlob* new_adapter = NULL;
   AdapterHandlerEntry* entry = NULL;
   AdapterFingerPrint* fingerprint = NULL;
   {
@@ -2432,7 +2432,8 @@ AdapterHandlerEntry* AdapterHandlerLibrary::get_adapter(methodHandle method) {
 
 #ifdef ASSERT
     AdapterHandlerEntry* shared_entry = NULL;
-    if (VerifyAdapterSharing && entry != NULL) {
+    // Start adapter sharing verification only after the VM is booted.
+    if (VerifyAdapterSharing && (entry != NULL)) {
       shared_entry = entry;
       entry = NULL;
     }
@@ -2448,41 +2449,44 @@ AdapterHandlerEntry* AdapterHandlerLibrary::get_adapter(methodHandle method) {
     // Make a C heap allocated version of the fingerprint to store in the adapter
     fingerprint = new AdapterFingerPrint(total_args_passed, sig_bt);
 
-    // Create I2C & C2I handlers
+    // StubRoutines::code2() is initialized after this function can be called. As a result,
+    // VerifyAdapterCalls and VerifyAdapterSharing can fail if we re-use code that generated
+    // prior to StubRoutines::code2() being set. Checks refer to checks generated in an I2C
+    // stub that ensure that an I2C stub is called from an interpreter frame.
+    bool contains_all_checks = StubRoutines::code2() != NULL;
 
+    // Create I2C & C2I handlers
     BufferBlob* buf = buffer_blob(); // the temporary code buffer in CodeCache
     if (buf != NULL) {
       CodeBuffer buffer(buf);
       short buffer_locs[20];
       buffer.insts()->initialize_shared_locs((relocInfo*)buffer_locs,
                                              sizeof(buffer_locs)/sizeof(relocInfo));
-      MacroAssembler _masm(&buffer);
 
+      MacroAssembler _masm(&buffer);
       entry = SharedRuntime::generate_i2c2i_adapters(&_masm,
                                                      total_args_passed,
                                                      comp_args_on_stack,
                                                      sig_bt,
                                                      regs,
                                                      fingerprint);
-
 #ifdef ASSERT
       if (VerifyAdapterSharing) {
         if (shared_entry != NULL) {
-          assert(shared_entry->compare_code(buf->code_begin(), buffer.insts_size(), total_args_passed, sig_bt),
-                 "code must match");
+          assert(shared_entry->compare_code(buf->code_begin(), buffer.insts_size()), "code must match");
           // Release the one just created and return the original
           _adapters->free_entry(entry);
           return shared_entry;
         } else  {
-          entry->save_code(buf->code_begin(), buffer.insts_size(), total_args_passed, sig_bt);
+          entry->save_code(buf->code_begin(), buffer.insts_size());
         }
       }
 #endif
 
-      B = AdapterBlob::create(&buffer);
+      new_adapter = AdapterBlob::create(&buffer);
       NOT_PRODUCT(insts_size = buffer.insts_size());
     }
-    if (B == NULL) {
+    if (new_adapter == NULL) {
       // CodeCache is full, disable compilation
       // Ought to log this but compile log is only per compile thread
       // and we're some non descript Java thread.
@@ -2490,7 +2494,7 @@ AdapterHandlerEntry* AdapterHandlerLibrary::get_adapter(methodHandle method) {
       CompileBroker::handle_full_code_cache();
       return NULL; // Out of CodeCache space
     }
-    entry->relocate(B->content_begin());
+    entry->relocate(new_adapter->content_begin());
 #ifndef PRODUCT
     // debugging suppport
     if (PrintAdapterHandlers || PrintStubCode) {
@@ -2509,22 +2513,25 @@ AdapterHandlerEntry* AdapterHandlerLibrary::get_adapter(methodHandle method) {
       }
     }
 #endif
-
-    _adapters->add(entry);
+    // Add the entry only if the entry contains all required checks (see sharedRuntime_xxx.cpp)
+    // The checks are inserted only if -XX:+VerifyAdapterCalls is specified.
+    if (contains_all_checks || !VerifyAdapterCalls) {
+      _adapters->add(entry);
+    }
   }
   // Outside of the lock
-  if (B != NULL) {
+  if (new_adapter != NULL) {
     char blob_id[256];
     jio_snprintf(blob_id,
                  sizeof(blob_id),
                  "%s(%s)@" PTR_FORMAT,
-                 B->name(),
+                 new_adapter->name(),
                  fingerprint->as_string(),
-                 B->content_begin());
-    Forte::register_stub(blob_id, B->content_begin(), B->content_end());
+                 new_adapter->content_begin());
+    Forte::register_stub(blob_id, new_adapter->content_begin(),new_adapter->content_end());
 
     if (JvmtiExport::should_post_dynamic_code_generated()) {
-      JvmtiExport::post_dynamic_code_generated(blob_id, B->content_begin(), B->content_end());
+      JvmtiExport::post_dynamic_code_generated(blob_id, new_adapter->content_begin(), new_adapter->content_end());
     }
   }
   return entry;
@@ -2556,7 +2563,6 @@ void AdapterHandlerEntry::deallocate() {
   delete _fingerprint;
 #ifdef ASSERT
   if (_saved_code) FREE_C_HEAP_ARRAY(unsigned char, _saved_code, mtCode);
-  if (_saved_sig)  FREE_C_HEAP_ARRAY(Basictype, _saved_sig, mtCode);
 #endif
 }
 
@@ -2565,26 +2571,19 @@ void AdapterHandlerEntry::deallocate() {
 // Capture the code before relocation so that it can be compared
 // against other versions.  If the code is captured after relocation
 // then relative instructions won't be equivalent.
-void AdapterHandlerEntry::save_code(unsigned char* buffer, int length, int total_args_passed, BasicType* sig_bt) {
+void AdapterHandlerEntry::save_code(unsigned char* buffer, int length) {
   _saved_code = NEW_C_HEAP_ARRAY(unsigned char, length, mtCode);
-  _code_length = length;
+  _saved_code_length = length;
   memcpy(_saved_code, buffer, length);
-  _total_args_passed = total_args_passed;
-  _saved_sig = NEW_C_HEAP_ARRAY(BasicType, _total_args_passed, mtCode);
-  memcpy(_saved_sig, sig_bt, _total_args_passed * sizeof(BasicType));
 }
 
 
-bool AdapterHandlerEntry::compare_code(unsigned char* buffer, int length, int total_args_passed, BasicType* sig_bt) {
-  if (length != _code_length) {
+bool AdapterHandlerEntry::compare_code(unsigned char* buffer, int length) {
+  if (length != _saved_code_length) {
     return false;
   }
-  for (int i = 0; i < length; i++) {
-    if (buffer[i] != _saved_code[i]) {
-      return false;
-    }
-  }
-  return true;
+
+  return (memcmp(buffer, _saved_code, length) == 0) ? true : false;
 }
 #endif
 

src/share/vm/runtime/sharedRuntime.hpp

@@ -612,9 +612,7 @@ class AdapterHandlerEntry : public BasicHashtableEntry<mtCode> {
   // Captures code and signature used to generate this adapter when
   // verifing adapter equivalence.
   unsigned char* _saved_code;
-  int            _code_length;
-  BasicType*     _saved_sig;
-  int            _total_args_passed;
+  int            _saved_code_length;
 #endif
 
   void init(AdapterFingerPrint* fingerprint, address i2c_entry, address c2i_entry, address c2i_unverified_entry) {
@@ -624,9 +622,7 @@ class AdapterHandlerEntry : public BasicHashtableEntry<mtCode> {
     _c2i_unverified_entry = c2i_unverified_entry;
 #ifdef ASSERT
     _saved_code = NULL;
-    _code_length = 0;
-    _saved_sig = NULL;
-    _total_args_passed = 0;
+    _saved_code_length = 0;
 #endif
   }
 
@@ -639,7 +635,6 @@ class AdapterHandlerEntry : public BasicHashtableEntry<mtCode> {
   address get_i2c_entry()            const { return _i2c_entry; }
   address get_c2i_entry()            const { return _c2i_entry; }
   address get_c2i_unverified_entry() const { return _c2i_unverified_entry; }
-
   address base_address();
   void relocate(address new_base);
 
@@ -651,8 +646,8 @@ class AdapterHandlerEntry : public BasicHashtableEntry<mtCode> {
 
 #ifdef ASSERT
   // Used to verify that code generated for shared adapters is equivalent
-  void save_code(unsigned char* code, int length, int total_args_passed, BasicType* sig_bt);
-  bool compare_code(unsigned char* code, int length, int total_args_passed, BasicType* sig_bt);
+  void save_code   (unsigned char* code, int length);
+  bool compare_code(unsigned char* code, int length);
 #endif
 
   //virtual void print_on(outputStream* st) const;  DO NOT USE