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/class_linker.cc b/runtime/class_linker.cc
index 41adae4..07a40c4 100644
--- a/runtime/class_linker.cc
+++ b/runtime/class_linker.cc
@@ -149,8 +149,8 @@
return false;
}
- ArtMethod* exception_init_method = exception_class->FindDeclaredDirectMethod(
- "<init>", "(Ljava/lang/String;)V", class_linker->GetImagePointerSize());
+ ArtMethod* exception_init_method = exception_class->FindConstructor(
+ "(Ljava/lang/String;)V", class_linker->GetImagePointerSize());
return exception_init_method != nullptr;
}
@@ -4658,10 +4658,8 @@
// Find the <init>(InvocationHandler)V method. The exact method offset varies depending
// on which front-end compiler was used to build the libcore DEX files.
- ArtMethod* proxy_constructor = GetClassRoot(kJavaLangReflectProxy)->
- FindDeclaredDirectMethod("<init>",
- "(Ljava/lang/reflect/InvocationHandler;)V",
- image_pointer_size_);
+ ArtMethod* proxy_constructor = GetClassRoot(kJavaLangReflectProxy)->FindConstructor(
+ "(Ljava/lang/reflect/InvocationHandler;)V", image_pointer_size_);
DCHECK(proxy_constructor != nullptr)
<< "Could not find <init> method in java.lang.reflect.Proxy";
@@ -4673,8 +4671,9 @@
// code_ too)
DCHECK(out != nullptr);
out->CopyFrom(proxy_constructor, image_pointer_size_);
- // Make this constructor public and fix the class to be our Proxy version
+ // Make this constructor public and fix the class to be our Proxy version.
// Mark kAccCompileDontBother so that we don't take JIT samples for the method. b/62349349
+ // Note that the compiler calls a ResolveMethod() overload that does not handle a Proxy referrer.
out->SetAccessFlags((out->GetAccessFlags() & ~kAccProtected) |
kAccPublic |
kAccCompileDontBother);
@@ -7383,10 +7382,8 @@
// defaults. This means we don't need to do any trickery when creating the Miranda methods, since
// they will already be null. This has the additional benefit that the declarer of a miranda
// method will actually declare an abstract method.
- for (size_t i = ifcount; i != 0; ) {
+ for (size_t i = ifcount; i != 0u; ) {
--i;
-
- DCHECK_GE(i, 0u);
DCHECK_LT(i, ifcount);
size_t num_methods = iftable->GetInterface(i)->NumDeclaredVirtualMethods();
@@ -7967,201 +7964,95 @@
ArtMethod* referrer,
InvokeType type) {
DCHECK(dex_cache != nullptr);
+ DCHECK(referrer == nullptr || !referrer->IsProxyMethod());
// Check for hit in the dex cache.
- ArtMethod* resolved = dex_cache->GetResolvedMethod(method_idx, image_pointer_size_);
+ PointerSize pointer_size = image_pointer_size_;
+ ArtMethod* resolved = dex_cache->GetResolvedMethod(method_idx, pointer_size);
Thread::PoisonObjectPointersIfDebug();
- if (resolved != nullptr && !resolved->IsRuntimeMethod()) {
+ bool valid_dex_cache_method = resolved != nullptr && !resolved->IsRuntimeMethod();
+ if (kResolveMode == ResolveMode::kNoChecks && valid_dex_cache_method) {
+ // We have a valid method from the DexCache and no checks to perform.
DCHECK(resolved->GetDeclaringClassUnchecked() != nullptr) << resolved->GetDexMethodIndex();
- if (kResolveMode == ClassLinker::kForceICCECheck) {
- if (resolved->CheckIncompatibleClassChange(type)) {
- ThrowIncompatibleClassChangeError(type, resolved->GetInvokeType(), resolved, referrer);
- return nullptr;
- }
- }
return resolved;
}
- // Fail, get the declaring class.
const DexFile::MethodId& method_id = dex_file.GetMethodId(method_idx);
- ObjPtr<mirror::Class> klass = ResolveType(dex_file, method_id.class_idx_, dex_cache, class_loader);
- if (klass == nullptr) {
+ ObjPtr<mirror::Class> klass = nullptr;
+ if (valid_dex_cache_method) {
+ // We have a valid method from the DexCache but we need to perform ICCE and IAE checks.
+ DCHECK(resolved->GetDeclaringClassUnchecked() != nullptr) << resolved->GetDexMethodIndex();
+ klass = LookupResolvedType(dex_file, method_id.class_idx_, dex_cache.Get(), class_loader.Get());
+ DCHECK(klass != nullptr);
+ } else {
+ // The method was not in the DexCache, resolve the declaring class.
+ klass = ResolveType(dex_file, method_id.class_idx_, dex_cache, class_loader);
+ if (klass == nullptr) {
+ DCHECK(Thread::Current()->IsExceptionPending());
+ return nullptr;
+ }
+ }
+
+ // Check if the invoke type matches the class type.
+ if (kResolveMode == ResolveMode::kCheckICCEAndIAE &&
+ CheckInvokeClassMismatch</* kThrow */ true>(
+ dex_cache.Get(), type, [klass]() { return klass; })) {
DCHECK(Thread::Current()->IsExceptionPending());
return nullptr;
}
- // Scan using method_idx, this saves string compares but will only hit for matching dex
- // caches/files.
- switch (type) {
- case kDirect: // Fall-through.
- case kStatic:
- resolved = klass->FindDirectMethod(dex_cache.Get(), method_idx, image_pointer_size_);
- DCHECK(resolved == nullptr || resolved->GetDeclaringClassUnchecked() != nullptr);
- break;
- case kInterface:
- // We have to check whether the method id really belongs to an interface (dex static bytecode
- // constraint A15). Otherwise you must not invoke-interface on it.
- //
- // This is not symmetric to A12-A14 (direct, static, virtual), as using FindInterfaceMethod
- // assumes that the given type is an interface, and will check the interface table if the
- // method isn't declared in the class. So it may find an interface method (usually by name
- // in the handling below, but we do the constraint check early). In that case,
- // CheckIncompatibleClassChange will succeed (as it is called on an interface method)
- // unexpectedly.
- // Example:
- // interface I {
- // foo()
- // }
- // class A implements I {
- // ...
- // }
- // class B extends A {
- // ...
- // }
- // invoke-interface B.foo
- // -> FindInterfaceMethod finds I.foo (interface method), not A.foo (miranda method)
- if (UNLIKELY(!klass->IsInterface())) {
- ThrowIncompatibleClassChangeError(klass,
- "Found class %s, but interface was expected",
- klass->PrettyDescriptor().c_str());
- return nullptr;
- } else {
- resolved = klass->FindInterfaceMethod(dex_cache.Get(), method_idx, image_pointer_size_);
- DCHECK(resolved == nullptr || resolved->GetDeclaringClass()->IsInterface());
- }
- break;
- case kSuper:
- if (klass->IsInterface()) {
- resolved = klass->FindInterfaceMethod(dex_cache.Get(), method_idx, image_pointer_size_);
- } else {
- resolved = klass->FindVirtualMethod(dex_cache.Get(), method_idx, image_pointer_size_);
- }
- break;
- case kVirtual:
- resolved = klass->FindVirtualMethod(dex_cache.Get(), method_idx, image_pointer_size_);
- break;
- default:
- LOG(FATAL) << "Unreachable - invocation type: " << type;
- UNREACHABLE();
- }
- if (resolved == nullptr) {
- // Search by name, which works across dex files.
- const char* name = dex_file.StringDataByIdx(method_id.name_idx_);
- const Signature signature = dex_file.GetMethodSignature(method_id);
- switch (type) {
- case kDirect: // Fall-through.
- case kStatic:
- resolved = klass->FindDirectMethod(name, signature, image_pointer_size_);
- DCHECK(resolved == nullptr || resolved->GetDeclaringClassUnchecked() != nullptr);
- break;
- case kInterface:
- resolved = klass->FindInterfaceMethod(name, signature, image_pointer_size_);
- DCHECK(resolved == nullptr || resolved->GetDeclaringClass()->IsInterface());
- break;
- case kSuper:
- if (klass->IsInterface()) {
- resolved = klass->FindInterfaceMethod(name, signature, image_pointer_size_);
- } else {
- resolved = klass->FindVirtualMethod(name, signature, image_pointer_size_);
- }
- break;
- case kVirtual:
- resolved = klass->FindVirtualMethod(name, signature, image_pointer_size_);
- break;
+
+ if (!valid_dex_cache_method) {
+ // Search for the method using dex_cache and method_idx. The Class::Find*Method()
+ // functions can optimize the search if the dex_cache is the same as the DexCache
+ // of the class, with fall-back to name and signature search otherwise.
+ if (klass->IsInterface()) {
+ resolved = klass->FindInterfaceMethod(dex_cache.Get(), method_idx, pointer_size);
+ } else {
+ resolved = klass->FindClassMethod(dex_cache.Get(), method_idx, pointer_size);
+ }
+ DCHECK(resolved == nullptr || resolved->GetDeclaringClassUnchecked() != nullptr);
+ if (resolved != nullptr) {
+ // Be a good citizen and update the dex cache to speed subsequent calls.
+ dex_cache->SetResolvedMethod(method_idx, resolved, pointer_size);
}
}
+
+ // Note: We can check for IllegalAccessError only if we have a referrer.
+ if (kResolveMode == ResolveMode::kCheckICCEAndIAE && resolved != nullptr && referrer != nullptr) {
+ ObjPtr<mirror::Class> methods_class = resolved->GetDeclaringClass();
+ ObjPtr<mirror::Class> referring_class = referrer->GetDeclaringClass();
+ if (!referring_class->CheckResolvedMethodAccess(methods_class,
+ resolved,
+ dex_cache.Get(),
+ method_idx,
+ type)) {
+ DCHECK(Thread::Current()->IsExceptionPending());
+ return nullptr;
+ }
+ }
+
// If we found a method, check for incompatible class changes.
- if (LIKELY(resolved != nullptr && !resolved->CheckIncompatibleClassChange(type))) {
- // Be a good citizen and update the dex cache to speed subsequent calls.
- dex_cache->SetResolvedMethod(method_idx, resolved, image_pointer_size_);
+ if (LIKELY(resolved != nullptr) &&
+ LIKELY(kResolveMode == ResolveMode::kNoChecks ||
+ !resolved->CheckIncompatibleClassChange(type))) {
return resolved;
} else {
- // If we had a method, it's an incompatible-class-change error.
+ // If we had a method, or if we can find one with another lookup type,
+ // it's an incompatible-class-change error.
+ if (resolved == nullptr) {
+ if (klass->IsInterface()) {
+ resolved = klass->FindClassMethod(dex_cache.Get(), method_idx, pointer_size);
+ } else {
+ // If there was an interface method with the same signature,
+ // we would have found it also in the "copied" methods.
+ DCHECK(klass->FindInterfaceMethod(dex_cache.Get(), method_idx, pointer_size) == nullptr);
+ }
+ }
if (resolved != nullptr) {
ThrowIncompatibleClassChangeError(type, resolved->GetInvokeType(), resolved, referrer);
} else {
- // We failed to find the method which means either an access error, an incompatible class
- // change, or no such method. First try to find the method among direct and virtual methods.
+ // We failed to find the method (using all lookup types), so throw a NoSuchMethodError.
const char* name = dex_file.StringDataByIdx(method_id.name_idx_);
const Signature signature = dex_file.GetMethodSignature(method_id);
- switch (type) {
- case kDirect:
- case kStatic:
- resolved = klass->FindVirtualMethod(name, signature, image_pointer_size_);
- // Note: kDirect and kStatic are also mutually exclusive, but in that case we would
- // have had a resolved method before, which triggers the "true" branch above.
- break;
- case kInterface:
- case kVirtual:
- case kSuper:
- resolved = klass->FindDirectMethod(name, signature, image_pointer_size_);
- break;
- }
-
- // If we found something, check that it can be accessed by the referrer.
- bool exception_generated = false;
- if (resolved != nullptr && referrer != nullptr) {
- ObjPtr<mirror::Class> methods_class = resolved->GetDeclaringClass();
- ObjPtr<mirror::Class> referring_class = referrer->GetDeclaringClass();
- if (!referring_class->CanAccess(methods_class)) {
- ThrowIllegalAccessErrorClassForMethodDispatch(referring_class,
- methods_class,
- resolved,
- type);
- exception_generated = true;
- } else if (!referring_class->CanAccessMember(methods_class, resolved->GetAccessFlags())) {
- ThrowIllegalAccessErrorMethod(referring_class, resolved);
- exception_generated = true;
- }
- }
- if (!exception_generated) {
- // Otherwise, throw an IncompatibleClassChangeError if we found something, and check
- // interface methods and throw if we find the method there. If we find nothing, throw a
- // NoSuchMethodError.
- switch (type) {
- case kDirect:
- case kStatic:
- if (resolved != nullptr) {
- ThrowIncompatibleClassChangeError(type, kVirtual, resolved, referrer);
- } else {
- resolved = klass->FindInterfaceMethod(name, signature, image_pointer_size_);
- if (resolved != nullptr) {
- ThrowIncompatibleClassChangeError(type, kInterface, resolved, referrer);
- } else {
- ThrowNoSuchMethodError(type, klass, name, signature);
- }
- }
- break;
- case kInterface:
- if (resolved != nullptr) {
- ThrowIncompatibleClassChangeError(type, kDirect, resolved, referrer);
- } else {
- resolved = klass->FindVirtualMethod(name, signature, image_pointer_size_);
- if (resolved != nullptr) {
- ThrowIncompatibleClassChangeError(type, kVirtual, resolved, referrer);
- } else {
- ThrowNoSuchMethodError(type, klass, name, signature);
- }
- }
- break;
- case kSuper:
- if (resolved != nullptr) {
- ThrowIncompatibleClassChangeError(type, kDirect, resolved, referrer);
- } else {
- ThrowNoSuchMethodError(type, klass, name, signature);
- }
- break;
- case kVirtual:
- if (resolved != nullptr) {
- ThrowIncompatibleClassChangeError(type, kDirect, resolved, referrer);
- } else {
- resolved = klass->FindInterfaceMethod(name, signature, image_pointer_size_);
- if (resolved != nullptr) {
- ThrowIncompatibleClassChangeError(type, kInterface, resolved, referrer);
- } else {
- ThrowNoSuchMethodError(type, klass, name, signature);
- }
- }
- break;
- }
- }
+ ThrowNoSuchMethodError(type, klass, name, signature);
}
Thread::Current()->AssertPendingException();
return nullptr;
@@ -8180,21 +8071,16 @@
}
// Fail, get the declaring class.
const DexFile::MethodId& method_id = dex_file.GetMethodId(method_idx);
- ObjPtr<mirror::Class> klass = ResolveType(dex_file, method_id.class_idx_, dex_cache, class_loader);
+ ObjPtr<mirror::Class> klass =
+ ResolveType(dex_file, method_id.class_idx_, dex_cache, class_loader);
if (klass == nullptr) {
Thread::Current()->AssertPendingException();
return nullptr;
}
if (klass->IsInterface()) {
- LOG(FATAL) << "ResolveAmbiguousMethod: unexpected method in interface: "
- << klass->PrettyClass();
- return nullptr;
- }
-
- // Search both direct and virtual methods
- resolved = klass->FindDirectMethod(dex_cache.Get(), method_idx, image_pointer_size_);
- if (resolved == nullptr) {
- resolved = klass->FindVirtualMethod(dex_cache.Get(), method_idx, image_pointer_size_);
+ resolved = klass->FindInterfaceMethod(dex_cache.Get(), method_idx, image_pointer_size_);
+ } else {
+ resolved = klass->FindClassMethod(dex_cache.Get(), method_idx, image_pointer_size_);
}
return resolved;
@@ -8509,19 +8395,19 @@
case DexFile::MethodHandleType::kInvokeStatic: {
kind = mirror::MethodHandle::Kind::kInvokeStatic;
receiver_count = 0;
- target_method = ResolveMethod<kNoICCECheckForCache>(self,
- method_handle.field_or_method_idx_,
- referrer,
- InvokeType::kStatic);
+ target_method = ResolveMethod<ResolveMode::kNoChecks>(self,
+ method_handle.field_or_method_idx_,
+ referrer,
+ InvokeType::kStatic);
break;
}
case DexFile::MethodHandleType::kInvokeInstance: {
kind = mirror::MethodHandle::Kind::kInvokeVirtual;
receiver_count = 1;
- target_method = ResolveMethod<kNoICCECheckForCache>(self,
- method_handle.field_or_method_idx_,
- referrer,
- InvokeType::kVirtual);
+ target_method = ResolveMethod<ResolveMode::kNoChecks>(self,
+ method_handle.field_or_method_idx_,
+ referrer,
+ InvokeType::kVirtual);
break;
}
case DexFile::MethodHandleType::kInvokeConstructor: {
@@ -8529,10 +8415,10 @@
// are special cased later in this method.
kind = mirror::MethodHandle::Kind::kInvokeTransform;
receiver_count = 0;
- target_method = ResolveMethod<kNoICCECheckForCache>(self,
- method_handle.field_or_method_idx_,
- referrer,
- InvokeType::kDirect);
+ target_method = ResolveMethod<ResolveMode::kNoChecks>(self,
+ method_handle.field_or_method_idx_,
+ referrer,
+ InvokeType::kDirect);
break;
}
case DexFile::MethodHandleType::kInvokeDirect: {
@@ -8555,16 +8441,16 @@
if (target_method->IsPrivate()) {
kind = mirror::MethodHandle::Kind::kInvokeDirect;
- target_method = ResolveMethod<kNoICCECheckForCache>(self,
- method_handle.field_or_method_idx_,
- referrer,
- InvokeType::kDirect);
+ target_method = ResolveMethod<ResolveMode::kNoChecks>(self,
+ method_handle.field_or_method_idx_,
+ referrer,
+ InvokeType::kDirect);
} else {
kind = mirror::MethodHandle::Kind::kInvokeSuper;
- target_method = ResolveMethod<kNoICCECheckForCache>(self,
- method_handle.field_or_method_idx_,
- referrer,
- InvokeType::kSuper);
+ target_method = ResolveMethod<ResolveMode::kNoChecks>(self,
+ method_handle.field_or_method_idx_,
+ referrer,
+ InvokeType::kSuper);
if (UNLIKELY(target_method == nullptr)) {
break;
}
@@ -8580,10 +8466,10 @@
case DexFile::MethodHandleType::kInvokeInterface: {
kind = mirror::MethodHandle::Kind::kInvokeInterface;
receiver_count = 1;
- target_method = ResolveMethod<kNoICCECheckForCache>(self,
- method_handle.field_or_method_idx_,
- referrer,
- InvokeType::kInterface);
+ target_method = ResolveMethod<ResolveMode::kNoChecks>(self,
+ method_handle.field_or_method_idx_,
+ referrer,
+ InvokeType::kInterface);
break;
}
}
@@ -9200,14 +9086,14 @@
}
// Instantiate ResolveMethod.
-template ArtMethod* ClassLinker::ResolveMethod<ClassLinker::kForceICCECheck>(
+template ArtMethod* ClassLinker::ResolveMethod<ClassLinker::ResolveMode::kCheckICCEAndIAE>(
const DexFile& dex_file,
uint32_t method_idx,
Handle<mirror::DexCache> dex_cache,
Handle<mirror::ClassLoader> class_loader,
ArtMethod* referrer,
InvokeType type);
-template ArtMethod* ClassLinker::ResolveMethod<ClassLinker::kNoICCECheckForCache>(
+template ArtMethod* ClassLinker::ResolveMethod<ClassLinker::ResolveMode::kNoChecks>(
const DexFile& dex_file,
uint32_t method_idx,
Handle<mirror::DexCache> dex_cache,