7092712: JSR 292: unloaded invokedynamic call sites can lead to a crash with...

7092712: JSR 292: unloaded invokedynamic call sites can lead to a crash with signature types not on BCP
Reviewed-by: jrose, never

src/share/vm/ci/ciEnv.cpp

@@ -473,6 +473,7 @@ ciKlass* ciEnv::get_klass_by_name_impl(ciKlass* accessing_klass,
   }
 
   if (require_local)  return NULL;
+
   // Not yet loaded into the VM, or not governed by loader constraints.
   // Make a CI representative for it.
   return get_unloaded_klass(accessing_klass, name);
@@ -498,7 +499,7 @@ ciKlass* ciEnv::get_klass_by_index_impl(constantPoolHandle cpool,
                                         bool& is_accessible,
                                         ciInstanceKlass* accessor) {
   EXCEPTION_CONTEXT;
-  KlassHandle klass (THREAD, constantPoolOopDesc::klass_at_if_loaded(cpool, index));
+  KlassHandle klass(THREAD, constantPoolOopDesc::klass_at_if_loaded(cpool, index));
   Symbol* klass_name = NULL;
   if (klass.is_null()) {
     // The klass has not been inserted into the constant pool.
@@ -785,17 +786,17 @@ ciMethod* ciEnv::get_method_by_index_impl(constantPoolHandle cpool,
   // Either the declared holder was not loaded, or the method could
   // not be found.  Create a dummy ciMethod to represent the failed
   // lookup.
-
-  return get_unloaded_method(declared_holder,
-                             get_symbol(name_sym),
-                             get_symbol(sig_sym));
+  ciSymbol* name      = get_symbol(name_sym);
+  ciSymbol* signature = get_symbol(sig_sym);
+  return get_unloaded_method(declared_holder, name, signature, accessor);
 }
 
 
 // ------------------------------------------------------------------
 // ciEnv::get_fake_invokedynamic_method_impl
 ciMethod* ciEnv::get_fake_invokedynamic_method_impl(constantPoolHandle cpool,
-                                                    int index, Bytecodes::Code bc) {
+                                                    int index, Bytecodes::Code bc,
+                                                    ciInstanceKlass* accessor) {
   // Compare the following logic with InterpreterRuntime::resolve_invokedynamic.
   assert(bc == Bytecodes::_invokedynamic, "must be invokedynamic");
 
@@ -807,9 +808,10 @@ ciMethod* ciEnv::get_fake_invokedynamic_method_impl(constantPoolHandle cpool,
   // Call site might not be resolved yet.  We could create a real invoker method from the
   // compiler, but it is simpler to stop the code path here with an unlinked method.
   if (!is_resolved) {
-    ciInstanceKlass* mh_klass = get_object(SystemDictionary::MethodHandle_klass())->as_instance_klass();
-    ciSymbol*        sig_sym  = get_symbol(cpool->signature_ref_at(index));
-    return get_unloaded_method(mh_klass, ciSymbol::invokeExact_name(), sig_sym);
+    ciInstanceKlass* holder    = get_object(SystemDictionary::MethodHandle_klass())->as_instance_klass();
+    ciSymbol*        name      = ciSymbol::invokeExact_name();
+    ciSymbol*        signature = get_symbol(cpool->signature_ref_at(index));
+    return get_unloaded_method(holder, name, signature, accessor);
   }
 
   // Get the invoker methodOop from the constant pool.
@@ -850,9 +852,9 @@ ciMethod* ciEnv::get_method_by_index(constantPoolHandle cpool,
                                      int index, Bytecodes::Code bc,
                                      ciInstanceKlass* accessor) {
   if (bc == Bytecodes::_invokedynamic) {
-    GUARDED_VM_ENTRY(return get_fake_invokedynamic_method_impl(cpool, index, bc);)
+    GUARDED_VM_ENTRY(return get_fake_invokedynamic_method_impl(cpool, index, bc, accessor);)
   } else {
-    GUARDED_VM_ENTRY(return get_method_by_index_impl(cpool, index, bc, accessor);)
+    GUARDED_VM_ENTRY(return get_method_by_index_impl(          cpool, index, bc, accessor);)
   }
 }
 

src/share/vm/ci/ciEnv.hpp

@@ -153,7 +153,8 @@ private:
                                       int method_index, Bytecodes::Code bc,
                                       ciInstanceKlass* loading_klass);
   ciMethod*  get_fake_invokedynamic_method_impl(constantPoolHandle cpool,
-                                                int index, Bytecodes::Code bc);
+                                                int index, Bytecodes::Code bc,
+                                                ciInstanceKlass* accessor);
 
   // Helper methods
   bool       check_klass_accessibility(ciKlass* accessing_klass,
@@ -192,13 +193,14 @@ private:
   // the result.
   ciMethod* get_unloaded_method(ciInstanceKlass* holder,
                                 ciSymbol*        name,
-                                ciSymbol*        signature) {
-    return _factory->get_unloaded_method(holder, name, signature);
+                                ciSymbol*        signature,
+                                ciInstanceKlass* accessor) {
+    return _factory->get_unloaded_method(holder, name, signature, accessor);
   }
 
   // Get a ciKlass representing an unloaded klass.
   // Ensures uniqueness of the result.
-  ciKlass* get_unloaded_klass(ciKlass* accessing_klass,
+  ciKlass* get_unloaded_klass(ciKlass*  accessing_klass,
                               ciSymbol* name) {
     return _factory->get_unloaded_klass(accessing_klass, name, true);
   }
@@ -224,7 +226,7 @@ private:
 
   // See if we already have an unloaded klass for the given name
   // or return NULL if not.
-  ciKlass *check_get_unloaded_klass(ciKlass* accessing_klass, ciSymbol* name) {
+  ciKlass *check_get_unloaded_klass(ciKlass*  accessing_klass, ciSymbol* name) {
     return _factory->get_unloaded_klass(accessing_klass, name, false);
   }
 

src/share/vm/ci/ciMethod.cpp

@@ -148,21 +148,27 @@ ciMethod::ciMethod(methodHandle h_m) : ciObject(h_m) {
 //
 // Unloaded method.
 ciMethod::ciMethod(ciInstanceKlass* holder,
-                   ciSymbol* name,
-                   ciSymbol* signature) : ciObject(ciMethodKlass::make()) {
-  // These fields are always filled in.
-  _name = name;
-  _holder = holder;
-  _signature = new (CURRENT_ENV->arena()) ciSignature(_holder, constantPoolHandle(), signature);
-  _intrinsic_id = vmIntrinsics::_none;
-  _liveness = NULL;
-  _can_be_statically_bound = false;
-  _method_blocks = NULL;
-  _method_data = NULL;
+                   ciSymbol*        name,
+                   ciSymbol*        signature,
+                   ciInstanceKlass* accessor) :
+  ciObject(ciMethodKlass::make()),
+  _name(                   name),
+  _holder(                 holder),
+  _intrinsic_id(           vmIntrinsics::_none),
+  _liveness(               NULL),
+  _can_be_statically_bound(false),
+  _method_blocks(          NULL),
+  _method_data(            NULL)
 #if defined(COMPILER2) || defined(SHARK)
-  _flow = NULL;
-  _bcea = NULL;
+  ,
+  _flow(                   NULL),
+  _bcea(                   NULL)
 #endif // COMPILER2 || SHARK
+{
+  // Usually holder and accessor are the same type but in some cases
+  // the holder has the wrong class loader (e.g. invokedynamic call
+  // sites) so we pass the accessor.
+  _signature = new (CURRENT_ENV->arena()) ciSignature(accessor, constantPoolHandle(), signature);
 }
 
 

src/share/vm/ci/ciMethod.hpp

@@ -88,7 +88,7 @@ class ciMethod : public ciObject {
 #endif
 
   ciMethod(methodHandle h_m);
-  ciMethod(ciInstanceKlass* holder, ciSymbol* name, ciSymbol* signature);
+  ciMethod(ciInstanceKlass* holder, ciSymbol* name, ciSymbol* signature, ciInstanceKlass* accessor);
 
   methodOop get_methodOop() const {
     methodOop m = (methodOop)get_oop();

src/share/vm/ci/ciObjectFactory.cpp

@@ -374,20 +374,32 @@ ciObject* ciObjectFactory::create_new_object(oop o) {
 // unloaded method.  This may need to change.
 ciMethod* ciObjectFactory::get_unloaded_method(ciInstanceKlass* holder,
                                                ciSymbol*        name,
-                                               ciSymbol*        signature) {
-  for (int i=0; i<_unloaded_methods->length(); i++) {
+                                               ciSymbol*        signature,
+                                               ciInstanceKlass* accessor) {
+  ciSignature* that = NULL;
+  for (int i = 0; i < _unloaded_methods->length(); i++) {
     ciMethod* entry = _unloaded_methods->at(i);
     if (entry->holder()->equals(holder) &&
         entry->name()->equals(name) &&
         entry->signature()->as_symbol()->equals(signature)) {
-      // We've found a match.
-      return entry;
+      // Short-circuit slow resolve.
+      if (entry->signature()->accessing_klass() == accessor) {
+        // We've found a match.
+        return entry;
+      } else {
+        // Lazily create ciSignature
+        if (that == NULL)  that = new (arena()) ciSignature(accessor, constantPoolHandle(), signature);
+        if (entry->signature()->equals(that)) {
+          // We've found a match.
+          return entry;
+        }
+      }
     }
   }
 
   // This is a new unloaded method.  Create it and stick it in
   // the cache.
-  ciMethod* new_method = new (arena()) ciMethod(holder, name, signature);
+  ciMethod* new_method = new (arena()) ciMethod(holder, name, signature, accessor);
 
   init_ident_of(new_method);
   _unloaded_methods->append(new_method);

src/share/vm/ci/ciObjectFactory.hpp

@@ -108,7 +108,8 @@ public:
   // Get the ciMethod representing an unloaded/unfound method.
   ciMethod* get_unloaded_method(ciInstanceKlass* holder,
                                 ciSymbol*        name,
-                                ciSymbol*        signature);
+                                ciSymbol*        signature,
+                                ciInstanceKlass* accessor);
 
   // Get a ciKlass representing an unloaded klass.
   ciKlass* get_unloaded_klass(ciKlass* accessing_klass,

src/share/vm/ci/ciSignature.cpp

@@ -80,7 +80,7 @@ ciSignature::ciSignature(ciKlass* accessing_klass, constantPoolHandle cpool, ciS
 }
 
 // ------------------------------------------------------------------
-// ciSignature::return_ciType
+// ciSignature::return_type
 //
 // What is the return type of this signature?
 ciType* ciSignature::return_type() const {
@@ -88,7 +88,7 @@ ciType* ciSignature::return_type() const {
 }
 
 // ------------------------------------------------------------------
-// ciSignature::ciType_at
+// ciSignature::type_at
 //
 // What is the type of the index'th element of this
 // signature?
@@ -98,6 +98,24 @@ ciType* ciSignature::type_at(int index) const {
   return _types->at(index);
 }
 
+// ------------------------------------------------------------------
+// ciSignature::equals
+//
+// Compare this signature to another one.  Signatures with different
+// accessing classes but with signature-types resolved to the same
+// types are defined to be equal.
+bool ciSignature::equals(ciSignature* that) {
+  // Compare signature
+  if (!this->as_symbol()->equals(that->as_symbol()))  return false;
+  // Compare all types of the arguments
+  for (int i = 0; i < _count; i++) {
+    if (this->type_at(i) != that->type_at(i))         return false;
+  }
+  // Compare the return type
+  if (this->return_type() != that->return_type())     return false;
+  return true;
+}
+
 // ------------------------------------------------------------------
 // ciSignature::print_signature
 void ciSignature::print_signature() {

src/share/vm/ci/ciSignature.hpp

@@ -43,6 +43,7 @@ private:
   int _count;
 
   friend class ciMethod;
+  friend class ciObjectFactory;
 
   ciSignature(ciKlass* accessing_klass, constantPoolHandle cpool, ciSymbol* signature);
 
@@ -52,6 +53,7 @@ private:
 
 public:
   ciSymbol* as_symbol() const                    { return _symbol; }
+  ciKlass*  accessing_klass() const              { return _accessing_klass; }
 
   ciType* return_type() const;
   ciType* type_at(int index) const;
@@ -59,6 +61,8 @@ public:
   int       size() const                         { return _size; }
   int       count() const                        { return _count; }
 
+  bool equals(ciSignature* that);
+
   void print_signature();
   void print();
 };