From eff0f5d9d52805abf825601960cc0c42dc8d7b5a Mon Sep 17 00:00:00 2001 From: Andreas Gampe Date: Wed, 13 Aug 2014 21:49:37 -0700 Subject: [PATCH] ART: Fix class-linker handling ResolveMethod did not account correctly for the mutual exclusivity of direct and static methods. In such a case we threw a NoSuchMethodError, while the correct behavior is to throw an IncompatibleClassChangeError. Bug: 16956477 (cherry picked from commit b5d1efa0012d31f7c52c0a2e2b70c77c8708c885) Change-Id: Id014affe0b8a43dbd75570b123b921d5853ab135 --- runtime/class_linker.cc | 161 ++++++++++++++++++++++++------------------------ 1 file changed, 82 insertions(+), 79 deletions(-) diff --git a/runtime/class_linker.cc b/runtime/class_linker.cc index db42146ff..91cd11b0e 100644 --- a/runtime/class_linker.cc +++ b/runtime/class_linker.cc @@ -4853,7 +4853,7 @@ mirror::ArtMethod* ClassLinker::ResolveMethod(const DexFile& dex_file, uint32_t Handle class_loader, Handle referrer, InvokeType type) { - DCHECK(dex_cache.Get() != NULL); + DCHECK(dex_cache.Get() != nullptr); // Check for hit in the dex cache. mirror::ArtMethod* resolved = dex_cache->GetResolvedMethod(method_idx); if (resolved != nullptr && !resolved->IsRuntimeMethod()) { @@ -4862,9 +4862,9 @@ mirror::ArtMethod* ClassLinker::ResolveMethod(const DexFile& dex_file, uint32_t // Fail, get the declaring class. const DexFile::MethodId& method_id = dex_file.GetMethodId(method_idx); mirror::Class* klass = ResolveType(dex_file, method_id.class_idx_, dex_cache, class_loader); - if (klass == NULL) { + if (klass == nullptr) { DCHECK(Thread::Current()->IsExceptionPending()); - return NULL; + return nullptr; } // Scan using method_idx, this saves string compares but will only hit for matching dex // caches/files. @@ -4875,7 +4875,7 @@ mirror::ArtMethod* ClassLinker::ResolveMethod(const DexFile& dex_file, uint32_t break; case kInterface: resolved = klass->FindInterfaceMethod(dex_cache.Get(), method_idx); - DCHECK(resolved == NULL || resolved->GetDeclaringClass()->IsInterface()); + DCHECK(resolved == nullptr || resolved->GetDeclaringClass()->IsInterface()); break; case kSuper: // Fall-through. case kVirtual: @@ -4884,7 +4884,7 @@ mirror::ArtMethod* ClassLinker::ResolveMethod(const DexFile& dex_file, uint32_t default: LOG(FATAL) << "Unreachable - invocation type: " << type; } - if (resolved == NULL) { + 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); @@ -4895,7 +4895,7 @@ mirror::ArtMethod* ClassLinker::ResolveMethod(const DexFile& dex_file, uint32_t break; case kInterface: resolved = klass->FindInterfaceMethod(name, signature); - DCHECK(resolved == NULL || resolved->GetDeclaringClass()->IsInterface()); + DCHECK(resolved == nullptr || resolved->GetDeclaringClass()->IsInterface()); break; case kSuper: // Fall-through. case kVirtual: @@ -4903,94 +4903,97 @@ mirror::ArtMethod* ClassLinker::ResolveMethod(const DexFile& dex_file, uint32_t break; } } - if (resolved != NULL) { - // We found a method, check for incompatible class changes. - if (resolved->CheckIncompatibleClassChange(type)) { - resolved = NULL; - } - } - if (resolved != NULL) { + // 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); return resolved; } 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. - 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); - break; - case kInterface: - case kVirtual: - case kSuper: - resolved = klass->FindDirectMethod(name, signature); - break; - } + // If we had a method, it's an incompatible-class-change error. + if (resolved != nullptr) { + ThrowIncompatibleClassChangeError(type, resolved->GetInvokeType(), resolved, referrer.Get()); + } 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. + 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); + // 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); + break; + } - // If we found something, check that it can be accessed by the referrer. - if (resolved != NULL && referrer.Get() != NULL) { - mirror::Class* methods_class = resolved->GetDeclaringClass(); - mirror::Class* referring_class = referrer->GetDeclaringClass(); - if (!referring_class->CanAccess(methods_class)) { - ThrowIllegalAccessErrorClassForMethodDispatch(referring_class, methods_class, - resolved, type); - return NULL; - } else if (!referring_class->CanAccessMember(methods_class, - resolved->GetAccessFlags())) { - ThrowIllegalAccessErrorMethod(referring_class, resolved); - return NULL; + // If we found something, check that it can be accessed by the referrer. + if (resolved != nullptr && referrer.Get() != nullptr) { + mirror::Class* methods_class = resolved->GetDeclaringClass(); + mirror::Class* referring_class = referrer->GetDeclaringClass(); + if (!referring_class->CanAccess(methods_class)) { + ThrowIllegalAccessErrorClassForMethodDispatch(referring_class, methods_class, + resolved, type); + return nullptr; + } else if (!referring_class->CanAccessMember(methods_class, + resolved->GetAccessFlags())) { + ThrowIllegalAccessErrorMethod(referring_class, resolved); + return nullptr; + } } - } - // 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 != NULL) { - ThrowIncompatibleClassChangeError(type, kVirtual, resolved, referrer.Get()); - } else { - resolved = klass->FindInterfaceMethod(name, signature); - if (resolved != NULL) { - ThrowIncompatibleClassChangeError(type, kInterface, resolved, referrer.Get()); + // 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.Get()); } else { - ThrowNoSuchMethodError(type, klass, name, signature); + resolved = klass->FindInterfaceMethod(name, signature); + if (resolved != nullptr) { + ThrowIncompatibleClassChangeError(type, kInterface, resolved, referrer.Get()); + } else { + ThrowNoSuchMethodError(type, klass, name, signature); + } } - } - break; - case kInterface: - if (resolved != NULL) { - ThrowIncompatibleClassChangeError(type, kDirect, resolved, referrer.Get()); - } else { - resolved = klass->FindVirtualMethod(name, signature); - if (resolved != NULL) { - ThrowIncompatibleClassChangeError(type, kVirtual, resolved, referrer.Get()); + break; + case kInterface: + if (resolved != nullptr) { + ThrowIncompatibleClassChangeError(type, kDirect, resolved, referrer.Get()); } else { - ThrowNoSuchMethodError(type, klass, name, signature); + resolved = klass->FindVirtualMethod(name, signature); + if (resolved != nullptr) { + ThrowIncompatibleClassChangeError(type, kVirtual, resolved, referrer.Get()); + } else { + ThrowNoSuchMethodError(type, klass, name, signature); + } } - } - break; - case kSuper: - ThrowNoSuchMethodError(type, klass, name, signature); - break; - case kVirtual: - if (resolved != NULL) { - ThrowIncompatibleClassChangeError(type, kDirect, resolved, referrer.Get()); - } else { - resolved = klass->FindInterfaceMethod(name, signature); - if (resolved != NULL) { - ThrowIncompatibleClassChangeError(type, kInterface, resolved, referrer.Get()); + break; + case kSuper: + ThrowNoSuchMethodError(type, klass, name, signature); + break; + case kVirtual: + if (resolved != nullptr) { + ThrowIncompatibleClassChangeError(type, kDirect, resolved, referrer.Get()); } else { - ThrowNoSuchMethodError(type, klass, name, signature); + resolved = klass->FindInterfaceMethod(name, signature); + if (resolved != nullptr) { + ThrowIncompatibleClassChangeError(type, kInterface, resolved, referrer.Get()); + } else { + ThrowNoSuchMethodError(type, klass, name, signature); + } } - } - break; + break; + } } DCHECK(Thread::Current()->IsExceptionPending()); - return NULL; + return nullptr; } } -- 2.11.0