From 67fd80824cda1ddd56e948d08f3adbee0e608ffe Mon Sep 17 00:00:00 2001 From: twisti Date: Wed, 12 Oct 2011 21:00:13 -0700 Subject: [PATCH] 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 | 24 ++++++++++++---------- src/share/vm/ci/ciEnv.hpp | 12 ++++++----- src/share/vm/ci/ciMethod.cpp | 32 +++++++++++++++++------------ src/share/vm/ci/ciMethod.hpp | 2 +- src/share/vm/ci/ciObjectFactory.cpp | 22 +++++++++++++++----- src/share/vm/ci/ciObjectFactory.hpp | 3 ++- src/share/vm/ci/ciSignature.cpp | 22 ++++++++++++++++++-- src/share/vm/ci/ciSignature.hpp | 4 ++++ 8 files changed, 83 insertions(+), 38 deletions(-) diff --git a/src/share/vm/ci/ciEnv.cpp b/src/share/vm/ci/ciEnv.cpp index 82492a08e..136617723 100644 --- a/src/share/vm/ci/ciEnv.cpp +++ b/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);) } } diff --git a/src/share/vm/ci/ciEnv.hpp b/src/share/vm/ci/ciEnv.hpp index f5242ca02..12f4bb226 100644 --- a/src/share/vm/ci/ciEnv.hpp +++ b/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); } diff --git a/src/share/vm/ci/ciMethod.cpp b/src/share/vm/ci/ciMethod.cpp index a985d5e28..6049d7b93 100644 --- a/src/share/vm/ci/ciMethod.cpp +++ b/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); } diff --git a/src/share/vm/ci/ciMethod.hpp b/src/share/vm/ci/ciMethod.hpp index db7175284..45a491f9d 100644 --- a/src/share/vm/ci/ciMethod.hpp +++ b/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(); diff --git a/src/share/vm/ci/ciObjectFactory.cpp b/src/share/vm/ci/ciObjectFactory.cpp index b0e9064a0..9aa6b2611 100644 --- a/src/share/vm/ci/ciObjectFactory.cpp +++ b/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); diff --git a/src/share/vm/ci/ciObjectFactory.hpp b/src/share/vm/ci/ciObjectFactory.hpp index 6222b9f85..26cc2c30c 100644 --- a/src/share/vm/ci/ciObjectFactory.hpp +++ b/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, diff --git a/src/share/vm/ci/ciSignature.cpp b/src/share/vm/ci/ciSignature.cpp index 8754fb4e8..229a904e9 100644 --- a/src/share/vm/ci/ciSignature.cpp +++ b/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() { diff --git a/src/share/vm/ci/ciSignature.hpp b/src/share/vm/ci/ciSignature.hpp index aaeac416c..25ba097cc 100644 --- a/src/share/vm/ci/ciSignature.hpp +++ b/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(); }; -- GitLab