Fix race in artInvokeInterfaceTrampoline().
When artInvokeInterfaceTrampoline() didn't get a resolved
interface method, it was calling FindMethodFromCode<>()
where the interface method was resolved and then a virtual
method for it was looked up. It then tried to retrieve the
interface method from the DexCache where it would usualy be
found as it was just stored there by the method resolution.
However, another thread could have just evicted that entry.
Instead of adding another call to LookupResolvedMethod(),
use the fact that artInvokeInterfaceTrampoline() already
explicitly inlines all the FindMethodFromCode<>() except
for the interface method resolution, so we can simply do
the resolution and then take the existing path for the rest
of the work.
Test: m test-art-host-gtest
Test: testrunner.py --host
Bug: 30627598
(cherry picked from commit 302f69cf7172d057c3ad9a8d4125a38e9dab8a71)
Change-Id: Ibdf254ae26cc3191d41a5021ae832288bf847ef4
diff --git a/runtime/entrypoints/entrypoint_utils-inl.h b/runtime/entrypoints/entrypoint_utils-inl.h
index a6c5d6c..be3e4f8 100644
--- a/runtime/entrypoints/entrypoint_utils-inl.h
+++ b/runtime/entrypoints/entrypoint_utils-inl.h
@@ -476,7 +476,7 @@
case kDirect:
return resolved_method;
case kVirtual: {
- mirror::Class* klass = (*this_object)->GetClass();
+ ObjPtr<mirror::Class> klass = (*this_object)->GetClass();
uint16_t vtable_index = resolved_method->GetMethodIndex();
if (access_check &&
(!klass->HasVTable() ||
@@ -509,7 +509,7 @@
// It is not an interface. If the referring class is in the class hierarchy of the
// referenced class in the bytecode, we use its super class. Otherwise, we throw
// a NoSuchMethodError.
- mirror::Class* super_class = nullptr;
+ ObjPtr<mirror::Class> super_class = nullptr;
if (method_reference_class->IsAssignableFrom(h_referring_class.Get())) {
super_class = h_referring_class->GetSuperClass();
}
@@ -554,11 +554,10 @@
case kInterface: {
uint32_t imt_index = ImTable::GetImtIndex(resolved_method);
PointerSize pointer_size = class_linker->GetImagePointerSize();
- ArtMethod* imt_method = (*this_object)->GetClass()->GetImt(pointer_size)->
- Get(imt_index, pointer_size);
+ ObjPtr<mirror::Class> klass = (*this_object)->GetClass();
+ ArtMethod* imt_method = klass->GetImt(pointer_size)->Get(imt_index, pointer_size);
if (!imt_method->IsRuntimeMethod()) {
if (kIsDebugBuild) {
- mirror::Class* klass = (*this_object)->GetClass();
ArtMethod* method = klass->FindVirtualMethodForInterface(
resolved_method, class_linker->GetImagePointerSize());
CHECK_EQ(imt_method, method) << ArtMethod::PrettyMethod(resolved_method) << " / "
@@ -568,7 +567,7 @@
}
return imt_method;
} else {
- ArtMethod* interface_method = (*this_object)->GetClass()->FindVirtualMethodForInterface(
+ ArtMethod* interface_method = klass->FindVirtualMethodForInterface(
resolved_method, class_linker->GetImagePointerSize());
if (UNLIKELY(interface_method == nullptr)) {
ThrowIncompatibleClassChangeErrorClassForInterfaceDispatch(resolved_method,
diff --git a/runtime/entrypoints/quick/quick_trampoline_entrypoints.cc b/runtime/entrypoints/quick/quick_trampoline_entrypoints.cc
index 3061365..6d4f77b 100644
--- a/runtime/entrypoints/quick/quick_trampoline_entrypoints.cc
+++ b/runtime/entrypoints/quick/quick_trampoline_entrypoints.cc
@@ -2483,43 +2483,16 @@
Thread* self,
ArtMethod** sp)
REQUIRES_SHARED(Locks::mutator_lock_) {
- ObjPtr<mirror::Object> this_object(raw_this_object);
ScopedQuickEntrypointChecks sqec(self);
- StackHandleScope<1> hs(self);
- Handle<mirror::Class> cls(hs.NewHandle(this_object->GetClass()));
+ StackHandleScope<2> hs(self);
+ Handle<mirror::Object> this_object = hs.NewHandle(raw_this_object);
+ Handle<mirror::Class> cls = hs.NewHandle(this_object->GetClass());
ArtMethod* caller_method = QuickArgumentVisitor::GetCallingMethod(sp);
ArtMethod* method = nullptr;
ImTable* imt = cls->GetImt(kRuntimePointerSize);
- if (LIKELY(interface_method != nullptr)) {
- DCHECK_NE(interface_method->GetDexMethodIndex(), DexFile::kDexNoIndex);
- // If the interface method is already resolved, look whether we have a match in the
- // ImtConflictTable.
- ArtMethod* conflict_method = imt->Get(ImTable::GetImtIndex(interface_method),
- kRuntimePointerSize);
- if (LIKELY(conflict_method->IsRuntimeMethod())) {
- ImtConflictTable* current_table = conflict_method->GetImtConflictTable(kRuntimePointerSize);
- DCHECK(current_table != nullptr);
- method = current_table->Lookup(interface_method, kRuntimePointerSize);
- } else {
- // It seems we aren't really a conflict method!
- method = cls->FindVirtualMethodForInterface(interface_method, kRuntimePointerSize);
- }
- if (method != nullptr) {
- return GetTwoWordSuccessValue(
- reinterpret_cast<uintptr_t>(method->GetEntryPointFromQuickCompiledCode()),
- reinterpret_cast<uintptr_t>(method));
- }
-
- // No match, use the IfTable.
- method = cls->FindVirtualMethodForInterface(interface_method, kRuntimePointerSize);
- if (UNLIKELY(method == nullptr)) {
- ThrowIncompatibleClassChangeErrorClassForInterfaceDispatch(
- interface_method, this_object, caller_method);
- return GetTwoWordFailureValue(); // Failure.
- }
- } else {
+ if (UNLIKELY(interface_method == nullptr)) {
// The interface method is unresolved, so resolve it in the dex file of the caller.
// Fetch the dex_method_idx of the target interface method from the caller.
uint32_t dex_method_idx;
@@ -2538,50 +2511,74 @@
dex_method_idx = instr->VRegB_3rc();
}
- const DexFile* dex_file = caller_method->GetDeclaringClass()->GetDexCache()
- ->GetDexFile();
+ const DexFile& dex_file = caller_method->GetDeclaringClass()->GetDexFile();
uint32_t shorty_len;
- const char* shorty = dex_file->GetMethodShorty(dex_file->GetMethodId(dex_method_idx),
- &shorty_len);
+ const char* shorty = dex_file.GetMethodShorty(dex_file.GetMethodId(dex_method_idx),
+ &shorty_len);
{
- // Remember the args in case a GC happens in FindMethodFromCode.
+ // Remember the args in case a GC happens in ClassLinker::ResolveMethod().
ScopedObjectAccessUnchecked soa(self->GetJniEnv());
RememberForGcArgumentVisitor visitor(sp, false, shorty, shorty_len, &soa);
visitor.VisitArguments();
- method = FindMethodFromCode<kInterface, false>(dex_method_idx,
- &this_object,
- caller_method,
- self);
+ ClassLinker* class_linker = Runtime::Current()->GetClassLinker();
+ interface_method = class_linker->ResolveMethod<ClassLinker::ResolveMode::kNoChecks>(
+ self, dex_method_idx, caller_method, kInterface);
visitor.FixupReferences();
}
- if (UNLIKELY(method == nullptr)) {
+ if (UNLIKELY(interface_method == nullptr)) {
CHECK(self->IsExceptionPending());
return GetTwoWordFailureValue(); // Failure.
}
- interface_method =
- caller_method->GetDexCacheResolvedMethod(dex_method_idx, kRuntimePointerSize);
- DCHECK(!interface_method->IsRuntimeMethod());
+ }
+
+ DCHECK(!interface_method->IsRuntimeMethod());
+ // Look whether we have a match in the ImtConflictTable.
+ uint32_t imt_index = ImTable::GetImtIndex(interface_method);
+ ArtMethod* conflict_method = imt->Get(imt_index, kRuntimePointerSize);
+ if (LIKELY(conflict_method->IsRuntimeMethod())) {
+ ImtConflictTable* current_table = conflict_method->GetImtConflictTable(kRuntimePointerSize);
+ DCHECK(current_table != nullptr);
+ method = current_table->Lookup(interface_method, kRuntimePointerSize);
+ } else {
+ // It seems we aren't really a conflict method!
+ if (kIsDebugBuild) {
+ ArtMethod* m = cls->FindVirtualMethodForInterface(interface_method, kRuntimePointerSize);
+ CHECK_EQ(conflict_method, m)
+ << interface_method->PrettyMethod() << " / " << conflict_method->PrettyMethod() << " / "
+ << " / " << ArtMethod::PrettyMethod(m) << " / " << cls->PrettyClass();
+ }
+ method = conflict_method;
+ }
+ if (method != nullptr) {
+ return GetTwoWordSuccessValue(
+ reinterpret_cast<uintptr_t>(method->GetEntryPointFromQuickCompiledCode()),
+ reinterpret_cast<uintptr_t>(method));
+ }
+
+ // No match, use the IfTable.
+ method = cls->FindVirtualMethodForInterface(interface_method, kRuntimePointerSize);
+ if (UNLIKELY(method == nullptr)) {
+ ThrowIncompatibleClassChangeErrorClassForInterfaceDispatch(
+ interface_method, this_object.Get(), caller_method);
+ return GetTwoWordFailureValue(); // Failure.
}
// We arrive here if we have found an implementation, and it is not in the ImtConflictTable.
// We create a new table with the new pair { interface_method, method }.
- uint32_t imt_index = ImTable::GetImtIndex(interface_method);
- ArtMethod* conflict_method = imt->Get(imt_index, kRuntimePointerSize);
- if (conflict_method->IsRuntimeMethod()) {
- ArtMethod* new_conflict_method = Runtime::Current()->GetClassLinker()->AddMethodToConflictTable(
- cls.Get(),
- conflict_method,
- interface_method,
- method,
- /*force_new_conflict_method*/false);
- if (new_conflict_method != conflict_method) {
- // Update the IMT if we create a new conflict method. No fence needed here, as the
- // data is consistent.
- imt->Set(imt_index,
- new_conflict_method,
- kRuntimePointerSize);
- }
+ DCHECK(conflict_method->IsRuntimeMethod());
+ ArtMethod* new_conflict_method = Runtime::Current()->GetClassLinker()->AddMethodToConflictTable(
+ cls.Get(),
+ conflict_method,
+ interface_method,
+ method,
+ /*force_new_conflict_method*/false);
+ if (new_conflict_method != conflict_method) {
+ // Update the IMT if we create a new conflict method. No fence needed here, as the
+ // data is consistent.
+ imt->Set(imt_index,
+ new_conflict_method,
+ kRuntimePointerSize);
}
const void* code = method->GetEntryPointFromQuickCompiledCode();
