ART: Change method lookup to be more consistent to JLS and the RI.
The method lookup for different invoke types was previously
widely different and didn't work well with the dex cache
method array where we have only a single slot for each
MethodId. The new behavior is to perform the same lookup for
all cases, distinguishing only between interface and
non-interface referencing class, and to further align the
behavior with the JLS and the RI. Where the JLS conflicts
with the RI, we follow the JLS semantics.
The new lookup for class methods first searches the methods
declared in the superclass chain (ignoring "copied" methods)
and only then looks in the "copied" methods. If the search
in the superclass chain finds a method that has not been
inherited (i.e. either a private method or a package-access
method where one of the classes in the chain does not belong
to the same package, see JLS 8.4.8), we still search the
"copied" methods as there may actually be a method inherited
from an interface. This follows the JLS semantics where
inherited methods are included in the search (JLS 15.12.2.1)
but conflicts with the RI where the private or
package-access method takes precedence over methods
inherited from interfaces.
Note that this search can find an accessible method that is
not inherited by the qualifying type, either for a package
access method when the referrer is in the same package but
the qualifying type is in another package, or for a private
method where the referrer is in the same class but the
qualifying type is actually a subclass. For the moment we
allow such calls and we shall consider whether to throw
an IncompatibleClassChangeError in this situation in future
to comply with JLS 15.12.4.3.
The new lookup for interface methods searches the interface
class, then all the superinterfaces and then the
java.lang.Object class, see implicitly declared methods in
interfaces, JLS 9.2. The search for the maximally-specific
non-abstract superinterface method is not yet implemented,
but the difference should be difficult to observe as the
usual subsequent call to FindVirtualMethodForInterface()
should yield the same result for any matching method.
The new test 162-method-idx-clash exposes several cases
where we previously completely messed up due to the effects
of the DexCache, or where we were out of line with the RI.
It also tests a case where the JLS and the RI disagree and
we follow the JLS.
Test: art/test/run-test --host --jvm 162-method-resolution
Test: m test-art-host-gtest
Test: testrunner.py --host
Test: testrunner.py --host --interp-ac
Test: Nexus 6P boots.
Test: testrunner.py --target
Bug: 62855082
Bug: 30627598
Change-Id: If450c8cff2751369011d649c25d28a482a2c61a3
diff --git a/runtime/mirror/class.cc b/runtime/mirror/class.cc
index b0e5b6a..6f70b19 100644
--- a/runtime/mirror/class.cc
+++ b/runtime/mirror/class.cc
@@ -396,95 +396,44 @@
}
}
-ArtMethod* Class::FindInterfaceMethod(const StringPiece& name,
- const StringPiece& signature,
- PointerSize pointer_size) {
- // Check the current class before checking the interfaces.
- ArtMethod* method = FindDeclaredVirtualMethod(name, signature, pointer_size);
- if (method != nullptr) {
- return method;
- }
-
- int32_t iftable_count = GetIfTableCount();
- ObjPtr<IfTable> iftable = GetIfTable();
- for (int32_t i = 0; i < iftable_count; ++i) {
- method = iftable->GetInterface(i)->FindDeclaredVirtualMethod(name, signature, pointer_size);
- if (method != nullptr) {
- return method;
+template <typename SignatureType>
+static inline ArtMethod* FindInterfaceMethodWithSignature(ObjPtr<Class> klass,
+ const StringPiece& name,
+ const SignatureType& signature,
+ PointerSize pointer_size)
+ REQUIRES_SHARED(Locks::mutator_lock_) {
+ // If the current class is not an interface, skip the search of its declared methods;
+ // such lookup is used only to distinguish between IncompatibleClassChangeError and
+ // NoSuchMethodError and the caller has already tried to search methods in the class.
+ if (LIKELY(klass->IsInterface())) {
+ // Search declared methods, both direct and virtual.
+ // (This lookup is used also for invoke-static on interface classes.)
+ for (ArtMethod& method : klass->GetDeclaredMethodsSlice(pointer_size)) {
+ if (method.GetName() == name && method.GetSignature() == signature) {
+ return &method;
+ }
}
}
- return nullptr;
-}
-ArtMethod* Class::FindInterfaceMethod(const StringPiece& name,
- const Signature& signature,
- PointerSize pointer_size) {
- // Check the current class before checking the interfaces.
- ArtMethod* method = FindDeclaredVirtualMethod(name, signature, pointer_size);
- if (method != nullptr) {
- return method;
- }
-
- int32_t iftable_count = GetIfTableCount();
- ObjPtr<IfTable> iftable = GetIfTable();
- for (int32_t i = 0; i < iftable_count; ++i) {
- method = iftable->GetInterface(i)->FindDeclaredVirtualMethod(name, signature, pointer_size);
- if (method != nullptr) {
- return method;
+ // TODO: If there is a unique maximally-specific non-abstract superinterface method,
+ // we should return it, otherwise an arbitrary one can be returned.
+ ObjPtr<IfTable> iftable = klass->GetIfTable();
+ for (int32_t i = 0, iftable_count = iftable->Count(); i < iftable_count; ++i) {
+ ObjPtr<Class> iface = iftable->GetInterface(i);
+ for (ArtMethod& method : iface->GetVirtualMethodsSlice(pointer_size)) {
+ if (method.GetName() == name && method.GetSignature() == signature) {
+ return &method;
+ }
}
}
- return nullptr;
-}
-ArtMethod* Class::FindInterfaceMethod(ObjPtr<DexCache> dex_cache,
- uint32_t dex_method_idx,
- PointerSize pointer_size) {
- // Check the current class before checking the interfaces.
- ArtMethod* method = FindDeclaredVirtualMethod(dex_cache, dex_method_idx, pointer_size);
- if (method != nullptr) {
- return method;
- }
-
- int32_t iftable_count = GetIfTableCount();
- ObjPtr<IfTable> iftable = GetIfTable();
- for (int32_t i = 0; i < iftable_count; ++i) {
- method = iftable->GetInterface(i)->FindDeclaredVirtualMethod(
- dex_cache, dex_method_idx, pointer_size);
- if (method != nullptr) {
- return method;
- }
- }
- return nullptr;
-}
-
-ArtMethod* Class::FindDeclaredDirectMethod(const StringPiece& name,
- const StringPiece& signature,
- PointerSize pointer_size) {
- for (auto& method : GetDirectMethods(pointer_size)) {
- if (name == method.GetName() && method.GetSignature() == signature) {
- return &method;
- }
- }
- return nullptr;
-}
-
-ArtMethod* Class::FindDeclaredDirectMethod(const StringPiece& name,
- const Signature& signature,
- PointerSize pointer_size) {
- for (auto& method : GetDirectMethods(pointer_size)) {
- if (name == method.GetName() && signature == method.GetSignature()) {
- return &method;
- }
- }
- return nullptr;
-}
-
-ArtMethod* Class::FindDeclaredDirectMethod(ObjPtr<DexCache> dex_cache,
- uint32_t dex_method_idx,
- PointerSize pointer_size) {
- if (GetDexCache() == dex_cache) {
- for (auto& method : GetDirectMethods(pointer_size)) {
- if (method.GetDexMethodIndex() == dex_method_idx) {
+ // Then search for public non-static methods in the java.lang.Object.
+ if (LIKELY(klass->IsInterface())) {
+ ObjPtr<Class> object_class = klass->GetSuperClass();
+ DCHECK(object_class->IsObjectClass());
+ for (ArtMethod& method : object_class->GetDeclaredMethodsSlice(pointer_size)) {
+ if (method.IsPublic() && !method.IsStatic() &&
+ method.GetName() == name && method.GetSignature() == signature) {
return &method;
}
}
@@ -492,37 +441,220 @@
return nullptr;
}
-ArtMethod* Class::FindDirectMethod(const StringPiece& name,
- const StringPiece& signature,
- PointerSize pointer_size) {
- for (ObjPtr<Class> klass = this; klass != nullptr; klass = klass->GetSuperClass()) {
- ArtMethod* method = klass->FindDeclaredDirectMethod(name, signature, pointer_size);
- if (method != nullptr) {
- return method;
- }
- }
- return nullptr;
+ArtMethod* Class::FindInterfaceMethod(const StringPiece& name,
+ const StringPiece& signature,
+ PointerSize pointer_size) {
+ return FindInterfaceMethodWithSignature(this, name, signature, pointer_size);
}
-ArtMethod* Class::FindDirectMethod(const StringPiece& name,
- const Signature& signature,
- PointerSize pointer_size) {
- for (ObjPtr<Class> klass = this; klass != nullptr; klass = klass->GetSuperClass()) {
- ArtMethod* method = klass->FindDeclaredDirectMethod(name, signature, pointer_size);
- if (method != nullptr) {
- return method;
- }
- }
- return nullptr;
+ArtMethod* Class::FindInterfaceMethod(const StringPiece& name,
+ const Signature& signature,
+ PointerSize pointer_size) {
+ return FindInterfaceMethodWithSignature(this, name, signature, pointer_size);
}
-ArtMethod* Class::FindDirectMethod(ObjPtr<DexCache> dex_cache,
- uint32_t dex_method_idx,
- PointerSize pointer_size) {
- for (ObjPtr<Class> klass = this; klass != nullptr; klass = klass->GetSuperClass()) {
- ArtMethod* method = klass->FindDeclaredDirectMethod(dex_cache, dex_method_idx, pointer_size);
- if (method != nullptr) {
- return method;
+ArtMethod* Class::FindInterfaceMethod(ObjPtr<DexCache> dex_cache,
+ uint32_t dex_method_idx,
+ PointerSize pointer_size) {
+ // We always search by name and signature, ignoring the type index in the MethodId.
+ const DexFile& dex_file = *dex_cache->GetDexFile();
+ const DexFile::MethodId& method_id = dex_file.GetMethodId(dex_method_idx);
+ StringPiece name = dex_file.StringDataByIdx(method_id.name_idx_);
+ const Signature signature = dex_file.GetMethodSignature(method_id);
+ return FindInterfaceMethod(name, signature, pointer_size);
+}
+
+static inline bool IsInheritedMethod(ObjPtr<mirror::Class> klass,
+ ObjPtr<mirror::Class> declaring_class,
+ ArtMethod& method)
+ REQUIRES_SHARED(Locks::mutator_lock_) {
+ DCHECK_EQ(declaring_class, method.GetDeclaringClass());
+ DCHECK_NE(klass, declaring_class);
+ DCHECK(klass->IsArrayClass() ? declaring_class->IsObjectClass()
+ : klass->IsSubClass(declaring_class));
+ uint32_t access_flags = method.GetAccessFlags();
+ if ((access_flags & (kAccPublic | kAccProtected)) != 0) {
+ return true;
+ }
+ if ((access_flags & kAccPrivate) != 0) {
+ return false;
+ }
+ for (; klass != declaring_class; klass = klass->GetSuperClass()) {
+ if (!klass->IsInSamePackage(declaring_class)) {
+ return false;
+ }
+ }
+ return true;
+}
+
+template <typename SignatureType>
+static inline ArtMethod* FindClassMethodWithSignature(ObjPtr<Class> this_klass,
+ const StringPiece& name,
+ const SignatureType& signature,
+ PointerSize pointer_size)
+ REQUIRES_SHARED(Locks::mutator_lock_) {
+ // Search declared methods first.
+ for (ArtMethod& method : this_klass->GetDeclaredMethodsSlice(pointer_size)) {
+ ArtMethod* np_method = method.GetInterfaceMethodIfProxy(pointer_size);
+ if (np_method->GetName() == name && np_method->GetSignature() == signature) {
+ return &method;
+ }
+ }
+
+ // Then search the superclass chain. If we find an inherited method, return it.
+ // If we find a method that's not inherited because of access restrictions,
+ // try to find a method inherited from an interface in copied methods.
+ ObjPtr<Class> klass = this_klass->GetSuperClass();
+ ArtMethod* uninherited_method = nullptr;
+ for (; klass != nullptr; klass = klass->GetSuperClass()) {
+ DCHECK(!klass->IsProxyClass());
+ for (ArtMethod& method : klass->GetDeclaredMethodsSlice(pointer_size)) {
+ if (method.GetName() == name && method.GetSignature() == signature) {
+ if (IsInheritedMethod(this_klass, klass, method)) {
+ return &method;
+ }
+ uninherited_method = &method;
+ break;
+ }
+ }
+ if (uninherited_method != nullptr) {
+ break;
+ }
+ }
+
+ // Then search copied methods.
+ // If we found a method that's not inherited, stop the search in its declaring class.
+ ObjPtr<Class> end_klass = klass;
+ DCHECK_EQ(uninherited_method != nullptr, end_klass != nullptr);
+ klass = this_klass;
+ if (UNLIKELY(klass->IsProxyClass())) {
+ DCHECK(klass->GetCopiedMethodsSlice(pointer_size).empty());
+ klass = klass->GetSuperClass();
+ }
+ for (; klass != end_klass; klass = klass->GetSuperClass()) {
+ DCHECK(!klass->IsProxyClass());
+ for (ArtMethod& method : klass->GetCopiedMethodsSlice(pointer_size)) {
+ if (method.GetName() == name && method.GetSignature() == signature) {
+ return &method; // No further check needed, copied methods are inherited by definition.
+ }
+ }
+ }
+ return uninherited_method; // Return the `uninherited_method` if any.
+}
+
+
+ArtMethod* Class::FindClassMethod(const StringPiece& name,
+ const StringPiece& signature,
+ PointerSize pointer_size) {
+ return FindClassMethodWithSignature(this, name, signature, pointer_size);
+}
+
+ArtMethod* Class::FindClassMethod(const StringPiece& name,
+ const Signature& signature,
+ PointerSize pointer_size) {
+ return FindClassMethodWithSignature(this, name, signature, pointer_size);
+}
+
+ArtMethod* Class::FindClassMethod(ObjPtr<DexCache> dex_cache,
+ uint32_t dex_method_idx,
+ PointerSize pointer_size) {
+ // FIXME: Hijacking a proxy class by a custom class loader can break this assumption.
+ DCHECK(!IsProxyClass());
+
+ // First try to find a declared method by dex_method_idx if we have a dex_cache match.
+ ObjPtr<DexCache> this_dex_cache = GetDexCache();
+ if (this_dex_cache == dex_cache) {
+ // Lookup is always performed in the class referenced by the MethodId.
+ DCHECK_EQ(dex_type_idx_, GetDexFile().GetMethodId(dex_method_idx).class_idx_.index_);
+ for (ArtMethod& method : GetDeclaredMethodsSlice(pointer_size)) {
+ if (method.GetDexMethodIndex() == dex_method_idx) {
+ return &method;
+ }
+ }
+ }
+ // If not found, we need to search by name and signature.
+ const DexFile& dex_file = *dex_cache->GetDexFile();
+ const DexFile::MethodId& method_id = dex_file.GetMethodId(dex_method_idx);
+ const Signature signature = dex_file.GetMethodSignature(method_id);
+ StringPiece name; // Delay strlen() until actually needed.
+ // If we do not have a dex_cache match, try to find the declared method in this class now.
+ if (this_dex_cache != dex_cache && !GetDeclaredMethodsSlice(pointer_size).empty()) {
+ DCHECK(name.empty());
+ name = dex_file.StringDataByIdx(method_id.name_idx_);
+ for (ArtMethod& method : GetDeclaredMethodsSlice(pointer_size)) {
+ if (method.GetName() == name && method.GetSignature() == signature) {
+ return &method;
+ }
+ }
+ }
+
+ // Then search the superclass chain. If we find an inherited method, return it.
+ // If we find a method that's not inherited because of access restrictions,
+ // try to find a method inherited from an interface in copied methods.
+ ArtMethod* uninherited_method = nullptr;
+ ObjPtr<Class> klass = GetSuperClass();
+ for (; klass != nullptr; klass = klass->GetSuperClass()) {
+ ArtMethod* candidate_method = nullptr;
+ ArraySlice<ArtMethod> declared_methods = klass->GetDeclaredMethodsSlice(pointer_size);
+ if (klass->GetDexCache() == dex_cache) {
+ // Matching dex_cache. We cannot compare the `dex_method_idx` anymore because
+ // the type index differs, so compare the name index and proto index.
+ for (ArtMethod& method : declared_methods) {
+ const DexFile::MethodId& cmp_method_id = dex_file.GetMethodId(method.GetDexMethodIndex());
+ if (cmp_method_id.name_idx_ == method_id.name_idx_ &&
+ cmp_method_id.proto_idx_ == method_id.proto_idx_) {
+ candidate_method = &method;
+ break;
+ }
+ }
+ } else {
+ if (!declared_methods.empty() && name.empty()) {
+ name = dex_file.StringDataByIdx(method_id.name_idx_);
+ }
+ for (ArtMethod& method : declared_methods) {
+ if (method.GetName() == name && method.GetSignature() == signature) {
+ candidate_method = &method;
+ break;
+ }
+ }
+ }
+ if (candidate_method != nullptr) {
+ if (IsInheritedMethod(this, klass, *candidate_method)) {
+ return candidate_method;
+ } else {
+ uninherited_method = candidate_method;
+ break;
+ }
+ }
+ }
+
+ // Then search copied methods.
+ // If we found a method that's not inherited, stop the search in its declaring class.
+ ObjPtr<Class> end_klass = klass;
+ DCHECK_EQ(uninherited_method != nullptr, end_klass != nullptr);
+ // After we have searched the declared methods of the super-class chain,
+ // search copied methods which can contain methods from interfaces.
+ for (klass = this; klass != end_klass; klass = klass->GetSuperClass()) {
+ ArraySlice<ArtMethod> copied_methods = klass->GetCopiedMethodsSlice(pointer_size);
+ if (!copied_methods.empty() && name.empty()) {
+ name = dex_file.StringDataByIdx(method_id.name_idx_);
+ }
+ for (ArtMethod& method : copied_methods) {
+ if (method.GetName() == name && method.GetSignature() == signature) {
+ return &method; // No further check needed, copied methods are inherited by definition.
+ }
+ }
+ }
+ return uninherited_method; // Return the `uninherited_method` if any.
+}
+
+ArtMethod* Class::FindConstructor(const StringPiece& signature, PointerSize pointer_size) {
+ // Internal helper, never called on proxy classes. We can skip GetInterfaceMethodIfProxy().
+ DCHECK(!IsProxyClass());
+ StringPiece name("<init>");
+ for (ArtMethod& method : GetDirectMethodsSliceUnchecked(pointer_size)) {
+ if (method.GetName() == name && method.GetSignature() == signature) {
+ return &method;
}
}
return nullptr;
@@ -539,47 +671,6 @@
return nullptr;
}
-// TODO These should maybe be changed to be named FindOwnedVirtualMethod or something similar
-// because they do not only find 'declared' methods and will return copied methods. This behavior is
-// desired and correct but the naming can lead to confusion because in the java language declared
-// excludes interface methods which might be found by this.
-ArtMethod* Class::FindDeclaredVirtualMethod(const StringPiece& name,
- const StringPiece& signature,
- PointerSize pointer_size) {
- for (auto& method : GetVirtualMethods(pointer_size)) {
- ArtMethod* const np_method = method.GetInterfaceMethodIfProxy(pointer_size);
- if (name == np_method->GetName() && np_method->GetSignature() == signature) {
- return &method;
- }
- }
- return nullptr;
-}
-
-ArtMethod* Class::FindDeclaredVirtualMethod(const StringPiece& name,
- const Signature& signature,
- PointerSize pointer_size) {
- for (auto& method : GetVirtualMethods(pointer_size)) {
- ArtMethod* const np_method = method.GetInterfaceMethodIfProxy(pointer_size);
- if (name == np_method->GetName() && signature == np_method->GetSignature()) {
- return &method;
- }
- }
- return nullptr;
-}
-
-ArtMethod* Class::FindDeclaredVirtualMethod(ObjPtr<DexCache> dex_cache,
- uint32_t dex_method_idx,
- PointerSize pointer_size) {
- if (GetDexCache() == dex_cache) {
- for (auto& method : GetDeclaredVirtualMethods(pointer_size)) {
- if (method.GetDexMethodIndex() == dex_method_idx) {
- return &method;
- }
- }
- }
- return nullptr;
-}
-
ArtMethod* Class::FindDeclaredVirtualMethodByName(const StringPiece& name,
PointerSize pointer_size) {
for (auto& method : GetVirtualMethods(pointer_size)) {
@@ -591,42 +682,6 @@
return nullptr;
}
-ArtMethod* Class::FindVirtualMethod(const StringPiece& name,
- const StringPiece& signature,
- PointerSize pointer_size) {
- for (ObjPtr<Class> klass = this; klass != nullptr; klass = klass->GetSuperClass()) {
- ArtMethod* method = klass->FindDeclaredVirtualMethod(name, signature, pointer_size);
- if (method != nullptr) {
- return method;
- }
- }
- return nullptr;
-}
-
-ArtMethod* Class::FindVirtualMethod(const StringPiece& name,
- const Signature& signature,
- PointerSize pointer_size) {
- for (ObjPtr<Class> klass = this; klass != nullptr; klass = klass->GetSuperClass()) {
- ArtMethod* method = klass->FindDeclaredVirtualMethod(name, signature, pointer_size);
- if (method != nullptr) {
- return method;
- }
- }
- return nullptr;
-}
-
-ArtMethod* Class::FindVirtualMethod(ObjPtr<DexCache> dex_cache,
- uint32_t dex_method_idx,
- PointerSize pointer_size) {
- for (ObjPtr<Class> klass = this; klass != nullptr; klass = klass->GetSuperClass()) {
- ArtMethod* method = klass->FindDeclaredVirtualMethod(dex_cache, dex_method_idx, pointer_size);
- if (method != nullptr) {
- return method;
- }
- }
- return nullptr;
-}
-
ArtMethod* Class::FindVirtualMethodForInterfaceSuper(ArtMethod* method, PointerSize pointer_size) {
DCHECK(method->GetDeclaringClass()->IsInterface());
DCHECK(IsInterface()) << "Should only be called on a interface class";