Revert "Revert "Do not load app image for class collisions""
Fixed bug in oat file assistant to allow the special shared library
marker.
Bug: 77342775
Bug: 79200502
Bug: 79575750
Test: test-art-host
This reverts commit 2c7e13b120926d3c3c18d649cd9849ea31b81477.
Change-Id: I647f55a07e4aef8bef56fb1ad7ff23056174b135
diff --git a/runtime/Android.bp b/runtime/Android.bp
index 116453b..64e6796 100644
--- a/runtime/Android.bp
+++ b/runtime/Android.bp
@@ -422,6 +422,7 @@
srcs: [
"arch/instruction_set.h",
"base/mutex.h",
+ "class_loader_context.h",
"class_status.h",
"debugger.h",
"gc_root.h",
diff --git a/runtime/class_loader_context.cc b/runtime/class_loader_context.cc
index 9817414..2bd5411 100644
--- a/runtime/class_loader_context.cc
+++ b/runtime/class_loader_context.cc
@@ -672,9 +672,10 @@
return !location.empty() && location[0] == '/';
}
-bool ClassLoaderContext::VerifyClassLoaderContextMatch(const std::string& context_spec,
- bool verify_names,
- bool verify_checksums) const {
+ClassLoaderContext::VerificationResult ClassLoaderContext::VerifyClassLoaderContextMatch(
+ const std::string& context_spec,
+ bool verify_names,
+ bool verify_checksums) const {
if (verify_names || verify_checksums) {
DCHECK(dex_files_open_attempted_);
DCHECK(dex_files_open_result_);
@@ -683,15 +684,21 @@
ClassLoaderContext expected_context;
if (!expected_context.Parse(context_spec, verify_checksums)) {
LOG(WARNING) << "Invalid class loader context: " << context_spec;
- return false;
+ return VerificationResult::kMismatch;
}
// Special shared library contexts always match. They essentially instruct the runtime
// to ignore the class path check because the oat file is known to be loaded in different
// contexts. OatFileManager will further verify if the oat file can be loaded based on the
// collision check.
- if (special_shared_library_ || expected_context.special_shared_library_) {
- return true;
+ if (expected_context.special_shared_library_) {
+ // Special case where we are the only entry in the class path.
+ if (class_loader_chain_.size() == 1 && class_loader_chain_[0].classpath.size() == 0) {
+ return VerificationResult::kVerifies;
+ }
+ return VerificationResult::kForcedToSkipChecks;
+ } else if (special_shared_library_) {
+ return VerificationResult::kForcedToSkipChecks;
}
if (expected_context.class_loader_chain_.size() != class_loader_chain_.size()) {
@@ -699,7 +706,7 @@
<< expected_context.class_loader_chain_.size()
<< ", actual=" << class_loader_chain_.size()
<< " (" << context_spec << " | " << EncodeContextForOatFile("") << ")";
- return false;
+ return VerificationResult::kMismatch;
}
for (size_t i = 0; i < class_loader_chain_.size(); i++) {
@@ -710,14 +717,14 @@
<< ". expected=" << GetClassLoaderTypeName(expected_info.type)
<< ", found=" << GetClassLoaderTypeName(info.type)
<< " (" << context_spec << " | " << EncodeContextForOatFile("") << ")";
- return false;
+ return VerificationResult::kMismatch;
}
if (info.classpath.size() != expected_info.classpath.size()) {
LOG(WARNING) << "ClassLoaderContext classpath size mismatch for position " << i
<< ". expected=" << expected_info.classpath.size()
<< ", found=" << info.classpath.size()
<< " (" << context_spec << " | " << EncodeContextForOatFile("") << ")";
- return false;
+ return VerificationResult::kMismatch;
}
if (verify_checksums) {
@@ -772,7 +779,7 @@
<< ". expected=" << expected_info.classpath[k]
<< ", found=" << info.classpath[k]
<< " (" << context_spec << " | " << EncodeContextForOatFile("") << ")";
- return false;
+ return VerificationResult::kMismatch;
}
// Compare the checksums.
@@ -781,11 +788,11 @@
<< ". expected=" << expected_info.checksums[k]
<< ", found=" << info.checksums[k]
<< " (" << context_spec << " | " << EncodeContextForOatFile("") << ")";
- return false;
+ return VerificationResult::kMismatch;
}
}
}
- return true;
+ return VerificationResult::kVerifies;
}
jclass ClassLoaderContext::GetClassLoaderClass(ClassLoaderType type) {
diff --git a/runtime/class_loader_context.h b/runtime/class_loader_context.h
index 1c83007..a4268aa 100644
--- a/runtime/class_loader_context.h
+++ b/runtime/class_loader_context.h
@@ -22,8 +22,10 @@
#include "arch/instruction_set.h"
#include "base/dchecked_vector.h"
+#include "dex/dex_file.h"
#include "handle_scope.h"
#include "mirror/class_loader.h"
+#include "oat_file.h"
#include "scoped_thread_state_change.h"
namespace art {
@@ -34,6 +36,18 @@
// Utility class which holds the class loader context used during compilation/verification.
class ClassLoaderContext {
public:
+ enum class VerificationResult {
+ kVerifies,
+ kForcedToSkipChecks,
+ kMismatch,
+ };
+
+ enum ClassLoaderType {
+ kInvalidClassLoader = 0,
+ kPathClassLoader = 1,
+ kDelegateLastClassLoader = 2
+ };
+
~ClassLoaderContext();
// Opens requested class path files and appends them to ClassLoaderInfo::opened_dex_files.
@@ -109,7 +123,7 @@
// This should be called after OpenDexFiles().
// Names are only verified if verify_names is true.
// Checksums are only verified if verify_checksums is true.
- bool VerifyClassLoaderContextMatch(const std::string& context_spec,
+ VerificationResult VerifyClassLoaderContextMatch(const std::string& context_spec,
bool verify_names = true,
bool verify_checksums = true) const;
@@ -141,12 +155,6 @@
static std::unique_ptr<ClassLoaderContext> Default();
private:
- enum ClassLoaderType {
- kInvalidClassLoader = 0,
- kPathClassLoader = 1,
- kDelegateLastClassLoader = 2
- };
-
struct ClassLoaderInfo {
// The type of this class loader.
ClassLoaderType type;
diff --git a/runtime/class_loader_context_test.cc b/runtime/class_loader_context_test.cc
index 4689ae4..5e3f48c 100644
--- a/runtime/class_loader_context_test.cc
+++ b/runtime/class_loader_context_test.cc
@@ -608,6 +608,17 @@
VerifyClassLoaderPCLFromTestDex(context.get(), 3, "ForClassLoaderA");
}
+
+TEST_F(ClassLoaderContextTest, VerifyClassLoaderContextFirstElement) {
+ std::string context_spec = "PCL[]";
+ std::unique_ptr<ClassLoaderContext> context = ParseContextWithChecksums(context_spec);
+ ASSERT_TRUE(context != nullptr);
+ PretendContextOpenedDexFiles(context.get());
+ // Ensure that the special shared library marks as verified for the first thing in the class path.
+ ASSERT_EQ(context->VerifyClassLoaderContextMatch(OatFile::kSpecialSharedLibrary),
+ ClassLoaderContext::VerificationResult::kVerifies);
+}
+
TEST_F(ClassLoaderContextTest, VerifyClassLoaderContextMatch) {
std::string context_spec = "PCL[a.dex*123:b.dex*456];DLC[c.dex*890]";
std::unique_ptr<ClassLoaderContext> context = ParseContextWithChecksums(context_spec);
@@ -619,28 +630,36 @@
VerifyClassLoaderPCL(context.get(), 0, "a.dex:b.dex");
VerifyClassLoaderDLC(context.get(), 1, "c.dex");
- ASSERT_TRUE(context->VerifyClassLoaderContextMatch(context_spec));
+ ASSERT_EQ(context->VerifyClassLoaderContextMatch(context_spec),
+ ClassLoaderContext::VerificationResult::kVerifies);
std::string wrong_class_loader_type = "PCL[a.dex*123:b.dex*456];PCL[c.dex*890]";
- ASSERT_FALSE(context->VerifyClassLoaderContextMatch(wrong_class_loader_type));
+ ASSERT_EQ(context->VerifyClassLoaderContextMatch(wrong_class_loader_type),
+ ClassLoaderContext::VerificationResult::kMismatch);
std::string wrong_class_loader_order = "DLC[c.dex*890];PCL[a.dex*123:b.dex*456]";
- ASSERT_FALSE(context->VerifyClassLoaderContextMatch(wrong_class_loader_order));
+ ASSERT_EQ(context->VerifyClassLoaderContextMatch(wrong_class_loader_order),
+ ClassLoaderContext::VerificationResult::kMismatch);
std::string wrong_classpath_order = "PCL[b.dex*456:a.dex*123];DLC[c.dex*890]";
- ASSERT_FALSE(context->VerifyClassLoaderContextMatch(wrong_classpath_order));
+ ASSERT_EQ(context->VerifyClassLoaderContextMatch(wrong_classpath_order),
+ ClassLoaderContext::VerificationResult::kMismatch);
std::string wrong_checksum = "PCL[a.dex*999:b.dex*456];DLC[c.dex*890]";
- ASSERT_FALSE(context->VerifyClassLoaderContextMatch(wrong_checksum));
+ ASSERT_EQ(context->VerifyClassLoaderContextMatch(wrong_checksum),
+ ClassLoaderContext::VerificationResult::kMismatch);
std::string wrong_extra_class_loader = "PCL[a.dex*123:b.dex*456];DLC[c.dex*890];PCL[d.dex*321]";
- ASSERT_FALSE(context->VerifyClassLoaderContextMatch(wrong_extra_class_loader));
+ ASSERT_EQ(context->VerifyClassLoaderContextMatch(wrong_extra_class_loader),
+ ClassLoaderContext::VerificationResult::kMismatch);
std::string wrong_extra_classpath = "PCL[a.dex*123:b.dex*456];DLC[c.dex*890:d.dex*321]";
- ASSERT_FALSE(context->VerifyClassLoaderContextMatch(wrong_extra_classpath));
+ ASSERT_EQ(context->VerifyClassLoaderContextMatch(wrong_extra_classpath),
+ ClassLoaderContext::VerificationResult::kMismatch);
std::string wrong_spec = "PCL[a.dex*999:b.dex*456];DLC[";
- ASSERT_FALSE(context->VerifyClassLoaderContextMatch(wrong_spec));
+ ASSERT_EQ(context->VerifyClassLoaderContextMatch(wrong_spec),
+ ClassLoaderContext::VerificationResult::kMismatch);
}
TEST_F(ClassLoaderContextTest, VerifyClassLoaderContextMatchAfterEncoding) {
@@ -652,7 +671,8 @@
std::unique_ptr<ClassLoaderContext> context = CreateContextForClassLoader(class_loader_d);
std::string context_with_no_base_dir = context->EncodeContextForOatFile("");
- ASSERT_TRUE(context->VerifyClassLoaderContextMatch(context_with_no_base_dir));
+ ASSERT_EQ(context->VerifyClassLoaderContextMatch(context_with_no_base_dir),
+ ClassLoaderContext::VerificationResult::kVerifies);
std::string dex_location = GetTestDexFileName("ForClassLoaderA");
size_t pos = dex_location.rfind('/');
@@ -661,7 +681,8 @@
std::string context_with_base_dir = context->EncodeContextForOatFile(parent);
ASSERT_NE(context_with_base_dir, context_with_no_base_dir);
- ASSERT_TRUE(context->VerifyClassLoaderContextMatch(context_with_base_dir));
+ ASSERT_EQ(context->VerifyClassLoaderContextMatch(context_with_base_dir),
+ ClassLoaderContext::VerificationResult::kVerifies);
}
TEST_F(ClassLoaderContextTest, VerifyClassLoaderContextMatchAfterEncodingMultidex) {
@@ -669,7 +690,8 @@
std::unique_ptr<ClassLoaderContext> context = CreateContextForClassLoader(class_loader);
- ASSERT_TRUE(context->VerifyClassLoaderContextMatch(context->EncodeContextForOatFile("")));
+ ASSERT_EQ(context->VerifyClassLoaderContextMatch(context->EncodeContextForOatFile("")),
+ ClassLoaderContext::VerificationResult::kVerifies);
}
} // namespace art
diff --git a/runtime/oat_file_assistant.cc b/runtime/oat_file_assistant.cc
index 9c8b651..7d69927 100644
--- a/runtime/oat_file_assistant.cc
+++ b/runtime/oat_file_assistant.cc
@@ -1217,7 +1217,9 @@
return false;
}
- bool result = context->VerifyClassLoaderContextMatch(file->GetClassLoaderContext());
+
+ const bool result = context->VerifyClassLoaderContextMatch(file->GetClassLoaderContext()) !=
+ ClassLoaderContext::VerificationResult::kMismatch;
if (!result) {
VLOG(oat) << "ClassLoaderContext check failed. Context was "
<< file->GetClassLoaderContext()
diff --git a/runtime/oat_file_manager.cc b/runtime/oat_file_manager.cc
index 16e6cf3..59a1045 100644
--- a/runtime/oat_file_manager.cc
+++ b/runtime/oat_file_manager.cc
@@ -276,9 +276,19 @@
}
}
-static bool CollisionCheck(std::vector<const DexFile*>& dex_files_loaded,
- std::vector<const DexFile*>& dex_files_unloaded,
- std::string* error_msg /*out*/) {
+static bool CheckClassCollision(const OatFile* oat_file,
+ const ClassLoaderContext* context,
+ std::string* error_msg /*out*/) {
+ std::vector<const DexFile*> dex_files_loaded = context->FlattenOpenedDexFiles();
+
+ // Vector that holds the newly opened dex files live, this is done to prevent leaks.
+ std::vector<std::unique_ptr<const DexFile>> opened_dex_files;
+
+ ScopedTrace st("Collision check");
+ // Add dex files from the oat file to check.
+ std::vector<const DexFile*> dex_files_unloaded;
+ AddDexFilesFromOat(oat_file, &dex_files_unloaded, &opened_dex_files);
+
// Generate type index information for each dex file.
std::vector<TypeIndexInfo> loaded_types;
for (const DexFile* dex_file : dex_files_loaded) {
@@ -355,9 +365,10 @@
// against the following top element. If the descriptor is the same, it is now checked whether
// the two elements agree on whether their dex file was from an already-loaded oat-file or the
// new oat file. Any disagreement indicates a collision.
-bool OatFileManager::HasCollisions(const OatFile* oat_file,
- const ClassLoaderContext* context,
- std::string* error_msg /*out*/) const {
+OatFileManager::CheckCollisionResult OatFileManager::CheckCollision(
+ const OatFile* oat_file,
+ const ClassLoaderContext* context,
+ /*out*/ std::string* error_msg) const {
DCHECK(oat_file != nullptr);
DCHECK(error_msg != nullptr);
@@ -367,28 +378,59 @@
// Note that this has correctness implications as we cannot guarantee that the class resolution
// used during compilation is OK (b/37777332).
if (context == nullptr) {
- LOG(WARNING) << "Skipping duplicate class check due to unsupported classloader";
- return false;
+ LOG(WARNING) << "Skipping duplicate class check due to unsupported classloader";
+ return CheckCollisionResult::kSkippedUnsupportedClassLoader;
}
- // If the pat file loading context matches the context used during compilation then we accept
+ // If the oat file loading context matches the context used during compilation then we accept
// the oat file without addition checks
- if (context->VerifyClassLoaderContextMatch(oat_file->GetClassLoaderContext())) {
- return false;
+ ClassLoaderContext::VerificationResult result = context->VerifyClassLoaderContextMatch(
+ oat_file->GetClassLoaderContext(),
+ /*verify_names*/ true,
+ /*verify_checksums*/ true);
+ switch (result) {
+ case ClassLoaderContext::VerificationResult::kForcedToSkipChecks:
+ return CheckCollisionResult::kSkippedClassLoaderContextSharedLibrary;
+ case ClassLoaderContext::VerificationResult::kMismatch:
+ // Mismatched context, do the actual collision check.
+ break;
+ case ClassLoaderContext::VerificationResult::kVerifies:
+ return CheckCollisionResult::kNoCollisions;
}
// The class loader context does not match. Perform a full duplicate classes check.
+ return CheckClassCollision(oat_file, context, error_msg)
+ ? CheckCollisionResult::kPerformedHasCollisions : CheckCollisionResult::kNoCollisions;
+}
- std::vector<const DexFile*> dex_files_loaded = context->FlattenOpenedDexFiles();
+bool OatFileManager::AcceptOatFile(CheckCollisionResult result) const {
+ // Take the file only if it has no collisions, or we must take it because of preopting.
+ // Also accept oat files for shared libraries and unsupported class loaders.
+ return result != CheckCollisionResult::kPerformedHasCollisions;
+}
- // Vector that holds the newly opened dex files live, this is done to prevent leaks.
- std::vector<std::unique_ptr<const DexFile>> opened_dex_files;
-
- ScopedTrace st("Collision check");
- // Add dex files from the oat file to check.
- std::vector<const DexFile*> dex_files_unloaded;
- AddDexFilesFromOat(oat_file, &dex_files_unloaded, &opened_dex_files);
- return CollisionCheck(dex_files_loaded, dex_files_unloaded, error_msg);
+bool OatFileManager::ShouldLoadAppImage(CheckCollisionResult check_collision_result,
+ const OatFile* source_oat_file,
+ ClassLoaderContext* context,
+ std::string* error_msg) {
+ Runtime* const runtime = Runtime::Current();
+ if (kEnableAppImage && (!runtime->IsJavaDebuggable() || source_oat_file->IsDebuggable())) {
+ // If we verified the class loader context (skipping due to the special marker doesn't
+ // count), then also avoid the collision check.
+ bool load_image = check_collision_result == CheckCollisionResult::kNoCollisions;
+ // If we skipped the collision check, we need to reverify to be sure its OK to load the
+ // image.
+ if (!load_image &&
+ check_collision_result ==
+ CheckCollisionResult::kSkippedClassLoaderContextSharedLibrary) {
+ // We can load the app image only if there are no collisions. If we know the
+ // class loader but didn't do the full collision check in HasCollisions(),
+ // do it now. b/77342775
+ load_image = !CheckClassCollision(source_oat_file, context, error_msg);
+ }
+ return load_image;
+ }
+ return false;
}
std::vector<std::unique_ptr<const DexFile>> OatFileManager::OpenDexFilesFromOat(
@@ -473,16 +515,17 @@
<< reinterpret_cast<uintptr_t>(oat_file.get())
<< " (executable=" << (oat_file != nullptr ? oat_file->IsExecutable() : false) << ")";
+ CheckCollisionResult check_collision_result = CheckCollisionResult::kPerformedHasCollisions;
if ((class_loader != nullptr || dex_elements != nullptr) && oat_file != nullptr) {
// Prevent oat files from being loaded if no class_loader or dex_elements are provided.
// This can happen when the deprecated DexFile.<init>(String) is called directly, and it
// could load oat files without checking the classpath, which would be incorrect.
// Take the file only if it has no collisions, or we must take it because of preopting.
- bool accept_oat_file =
- !HasCollisions(oat_file.get(), context.get(), /*out*/ &error_msg);
+ check_collision_result = CheckCollision(oat_file.get(), context.get(), /*out*/ &error_msg);
+ bool accept_oat_file = AcceptOatFile(check_collision_result);
if (!accept_oat_file) {
// Failed the collision check. Print warning.
- if (Runtime::Current()->IsDexFileFallbackEnabled()) {
+ if (runtime->IsDexFileFallbackEnabled()) {
if (!oat_file_assistant.HasOriginalDexFiles()) {
// We need to fallback but don't have original dex files. We have to
// fallback to opening the existing oat file. This is potentially
@@ -529,10 +572,11 @@
// We need to throw away the image space if we are debuggable but the oat-file source of the
// image is not otherwise we might get classes with inlined methods or other such things.
std::unique_ptr<gc::space::ImageSpace> image_space;
- if (kEnableAppImage && (!runtime->IsJavaDebuggable() || source_oat_file->IsDebuggable())) {
+ if (ShouldLoadAppImage(check_collision_result,
+ source_oat_file,
+ context.get(),
+ &error_msg)) {
image_space = oat_file_assistant.OpenImageSpace(source_oat_file);
- } else {
- image_space = nullptr;
}
if (image_space != nullptr) {
ScopedObjectAccess soa(self);
diff --git a/runtime/oat_file_manager.h b/runtime/oat_file_manager.h
index 038474e..80456e9 100644
--- a/runtime/oat_file_manager.h
+++ b/runtime/oat_file_manager.h
@@ -108,23 +108,39 @@
void SetOnlyUseSystemOatFiles();
private:
+ enum class CheckCollisionResult {
+ kSkippedUnsupportedClassLoader,
+ kSkippedClassLoaderContextSharedLibrary,
+ kNoCollisions,
+ kPerformedHasCollisions,
+ };
+
// Check that the class loader context of the given oat file matches the given context.
// This will perform a check that all class loaders in the chain have the same type and
// classpath.
// If the context is null (which means the initial class loader was null or unsupported)
- // this returns false.
+ // this returns kSkippedUnsupportedClassLoader.
// If the context does not validate the method will check for duplicate class definitions of
// the given oat file against the oat files (either from the class loaders if possible or all
// non-boot oat files otherwise).
- // Return true if there are any class definition collisions in the oat_file.
- bool HasCollisions(const OatFile* oat_file,
- const ClassLoaderContext* context,
- /*out*/ std::string* error_msg) const
+ // Return kPerformedHasCollisions if there are any class definition collisions in the oat_file.
+ CheckCollisionResult CheckCollision(const OatFile* oat_file,
+ const ClassLoaderContext* context,
+ /*out*/ std::string* error_msg) const
REQUIRES(!Locks::oat_file_manager_lock_);
const OatFile* FindOpenedOatFileFromOatLocationLocked(const std::string& oat_location) const
REQUIRES(Locks::oat_file_manager_lock_);
+ // Return true if we should accept the oat file.
+ bool AcceptOatFile(CheckCollisionResult result) const;
+
+ // Return true if we should attempt to load the app image.
+ bool ShouldLoadAppImage(CheckCollisionResult check_collision_result,
+ const OatFile* source_oat_file,
+ ClassLoaderContext* context,
+ std::string* error_msg);
+
std::set<std::unique_ptr<const OatFile>> oat_files_ GUARDED_BY(Locks::oat_file_manager_lock_);
bool have_non_pic_oat_file_;