Pass OatDexFile by reference in FindDexCache
This is more robust as we were passing pointers before and we could
potentially get an unintended dex cache.
As a drive-by, improve logging until bug 206992606 has been fixed.
Bug: 206992606
Test: ART tests
Change-Id: I98f309a27fd1c502199513f657c0cef65a1f0eb3
diff --git a/runtime/class_linker.cc b/runtime/class_linker.cc
index 93c5358..5de7adf 100644
--- a/runtime/class_linker.cc
+++ b/runtime/class_linker.cc
@@ -4117,8 +4117,7 @@
UNREACHABLE();
}
-ObjPtr<mirror::DexCache> ClassLinker::FindDexCache(Thread* self,
- const OatDexFile* const oat_dex_file) {
+ObjPtr<mirror::DexCache> ClassLinker::FindDexCache(Thread* self, const OatDexFile& oat_dex_file) {
ReaderMutexLock mu(self, *Locks::dex_lock_);
const DexCacheData* dex_cache_data = FindDexCacheDataLocked(oat_dex_file);
ObjPtr<mirror::DexCache> dex_cache = DecodeDexCacheLocked(self, dex_cache_data);
@@ -4132,7 +4131,7 @@
LOG(FATAL_WITHOUT_ABORT) << "Registered dex file " << entry.first->GetLocation();
}
}
- LOG(FATAL) << "Failed to find DexCache for OatDexFile " << oat_dex_file->GetDexFileLocation()
+ LOG(FATAL) << "Failed to find DexCache for OatDexFile " << oat_dex_file.GetDexFileLocation()
<< " " << &oat_dex_file;
UNREACHABLE();
}
@@ -4154,12 +4153,9 @@
}
const ClassLinker::DexCacheData* ClassLinker::FindDexCacheDataLocked(
- const OatDexFile* const oat_dex_file) {
- // DexFiles are not guaranteed to have an non-null OatDexFile*. If we pass a nullptr as parameter,
- // we might not get back the DexCacheData we are expecting.
- DCHECK_NE(oat_dex_file, nullptr);
- auto it = std::find_if(dex_caches_.begin(), dex_caches_.end(), [oat_dex_file](const auto& entry) {
- return entry.first->GetOatDexFile() == oat_dex_file;
+ const OatDexFile& oat_dex_file) {
+ auto it = std::find_if(dex_caches_.begin(), dex_caches_.end(), [&](const auto& entry) {
+ return entry.first->GetOatDexFile() == &oat_dex_file;
});
return it != dex_caches_.end() ? &it->second : nullptr;
}
diff --git a/runtime/class_linker.h b/runtime/class_linker.h
index bf791b2..d72e058 100644
--- a/runtime/class_linker.h
+++ b/runtime/class_linker.h
@@ -502,7 +502,7 @@
ObjPtr<mirror::DexCache> FindDexCache(Thread* self, const DexFile& dex_file)
REQUIRES(!Locks::dex_lock_)
REQUIRES_SHARED(Locks::mutator_lock_);
- ObjPtr<mirror::DexCache> FindDexCache(Thread* self, const OatDexFile* const oat_dex_file)
+ ObjPtr<mirror::DexCache> FindDexCache(Thread* self, const OatDexFile& oat_dex_file)
REQUIRES(!Locks::dex_lock_) REQUIRES_SHARED(Locks::mutator_lock_);
ClassTable* FindClassTable(Thread* self, ObjPtr<mirror::DexCache> dex_cache)
REQUIRES(!Locks::dex_lock_)
@@ -1123,7 +1123,7 @@
REQUIRES_SHARED(Locks::mutator_lock_);
const DexCacheData* FindDexCacheDataLocked(const DexFile& dex_file)
REQUIRES_SHARED(Locks::dex_lock_);
- const DexCacheData* FindDexCacheDataLocked(const OatDexFile* const oat_dex_file)
+ const DexCacheData* FindDexCacheDataLocked(const OatDexFile& oat_dex_file)
REQUIRES_SHARED(Locks::dex_lock_);
static ObjPtr<mirror::DexCache> DecodeDexCacheLocked(Thread* self, const DexCacheData* data)
REQUIRES_SHARED(Locks::dex_lock_, Locks::mutator_lock_);
diff --git a/runtime/entrypoints/entrypoint_utils-inl.h b/runtime/entrypoints/entrypoint_utils-inl.h
index fd6bf1f..a160a7b 100644
--- a/runtime/entrypoints/entrypoint_utils-inl.h
+++ b/runtime/entrypoints/entrypoint_utils-inl.h
@@ -19,6 +19,8 @@
#include "entrypoint_utils.h"
+#include <sstream>
+
#include "art_field-inl.h"
#include "art_method-inl.h"
#include "base/enums.h"
@@ -87,12 +89,38 @@
ObjPtr<mirror::DexCache> dex_cache;
if (method_info.HasDexFileIndex()) {
if (method_info.GetDexFileIndexKind() == MethodInfo::kKindBCP) {
- const DexFile* dex_file = class_linker->GetBootClassPath()[method_info.GetDexFileIndex()];
+ ArrayRef<const DexFile* const> bcp_dex_files(class_linker->GetBootClassPath());
+ const DexFile* dex_file = bcp_dex_files[method_info.GetDexFileIndex()];
dex_cache = class_linker->FindDexCache(Thread::Current(), *dex_file);
} else {
- auto oat_dex_files = method->GetDexFile()->GetOatDexFile()->GetOatFile()->GetOatDexFiles();
+ ArrayRef<const OatDexFile* const> oat_dex_files(
+ method->GetDexFile()->GetOatDexFile()->GetOatFile()->GetOatDexFiles());
+ // TODO(solanes, 206992606): Remove check when bug is solved.
+ const uint32_t index = method_info.GetDexFileIndex();
+ if (UNLIKELY(index >= oat_dex_files.size())) {
+ std::stringstream error_ss;
+
+ error_ss << "Wanted to retrieve DexFile index " << method_info.GetDexFileIndex()
+ << " from the oat_dex_files vector {";
+ for (const OatDexFile* odf_value : oat_dex_files) {
+ error_ss << odf_value << ",";
+ }
+ error_ss << "}. The outer method is: " << method->PrettyMethod() << " ("
+ << method->GetDexFile()->GetLocation() << "/"
+ << static_cast<const void*>(method->GetDexFile())
+ << "). The outermost method in the chain is: " << outer_method->PrettyMethod()
+ << " (" << outer_method->GetDexFile()->GetLocation() << "/"
+ << static_cast<const void*>(outer_method->GetDexFile())
+ << "). MethodInfo: method_index=" << std::dec << method_index
+ << ", is_in_bootclasspath=" << std::boolalpha
+ << (method_info.GetDexFileIndexKind() == MethodInfo::kKindBCP)
+ << ", dex_file_index=" << std::dec << method_info.GetDexFileIndex() << ".";
+ LOG(FATAL) << error_ss.str();
+ UNREACHABLE();
+ }
const OatDexFile* odf = oat_dex_files[method_info.GetDexFileIndex()];
- dex_cache = class_linker->FindDexCache(Thread::Current(), odf);
+ DCHECK(odf != nullptr);
+ dex_cache = class_linker->FindDexCache(Thread::Current(), *odf);
}
} else {
dex_cache = outer_method->GetDexCache();