Revert^2: InMemoryDexClassLoader in ClassLoaderContext follow-up
Address follow-up comments on Ic64065819018a1e56dee0f65405d26beb8fd7bbd.
In particular, the classpath elements of IMC are replaced with
"<unknown>" magic value to make it clear that the dex location is bogus,
and a clarifying comment is added.
Previously this CL would fail on target because IMC classpath was not
being replaced with "<unknown>" and context matching failed. This was
only a problem on target because
OatFile::ResolveRelativeEncodedDexLocation ignores non-absolute
locations on host.
This reverts commit 93d99f3665cbd890509f4c707e1a62c5f26d320e.
Test: m test-art-gtest-class_loader_context_test
Bug: 72131483
Change-Id: I27cbfa69c24d412cc1b6bcce88218cc95e324ef5
diff --git a/runtime/class_loader_context.cc b/runtime/class_loader_context.cc
index 428bf30..2b62d59 100644
--- a/runtime/class_loader_context.cc
+++ b/runtime/class_loader_context.cc
@@ -54,6 +54,7 @@
static constexpr char kClassLoaderSeparator = ';';
static constexpr char kClasspathSeparator = ':';
static constexpr char kDexFileChecksumSeparator = '*';
+static constexpr char kInMemoryDexClassLoaderDexLocationMagic[] = "<unknown>";
ClassLoaderContext::ClassLoaderContext()
: special_shared_library_(false),
@@ -170,6 +171,8 @@
// Checksums are not provided and dex locations themselves have no meaning
// (although we keep them in the spec to simplify parsing). Treat this as
// an unknown class loader.
+ // We can hit this case if dex2oat is invoked with a spec containing IMC.
+ // Because the dex file data is only available at runtime, we cannot proceed.
return nullptr;
}
}
@@ -198,6 +201,7 @@
std::unique_ptr<ClassLoaderInfo> info(new ClassLoaderInfo(class_loader_type));
if (!parse_checksums) {
+ DCHECK(class_loader_type != kInMemoryDexClassLoader);
Split(classpath, kClasspathSeparator, &info->classpath);
} else {
std::vector<std::string> classpath_elements;
@@ -212,6 +216,10 @@
if (!android::base::ParseUint(dex_file_with_checksum[1].c_str(), &checksum)) {
return nullptr;
}
+ if ((class_loader_type == kInMemoryDexClassLoader) &&
+ (dex_file_with_checksum[0] != kInMemoryDexClassLoaderDexLocationMagic)) {
+ return nullptr;
+ }
info->classpath.push_back(dex_file_with_checksum[0]);
info->checksums.push_back(checksum);
@@ -417,6 +425,8 @@
while (!work_list.empty()) {
ClassLoaderInfo* info = work_list.back();
work_list.pop_back();
+ DCHECK(info->type != kInMemoryDexClassLoader) << __FUNCTION__ << " not supported for IMC";
+
size_t opened_dex_files_index = info->opened_dex_files.size();
for (const std::string& cp_elem : info->classpath) {
// If path is relative, append it to the provided base directory.
@@ -625,8 +635,10 @@
if (k > 0) {
out << kClasspathSeparator;
}
- // Find paths that were relative and convert them back from absolute.
- if (!base_dir.empty() && location.substr(0, base_dir.length()) == base_dir) {
+ if (info.type == kInMemoryDexClassLoader) {
+ out << kInMemoryDexClassLoaderDexLocationMagic;
+ } else if (!base_dir.empty() && location.substr(0, base_dir.length()) == base_dir) {
+ // Find paths that were relative and convert them back from absolute.
out << location.substr(base_dir.length() + 1).c_str();
} else {
out << location.c_str();
@@ -1051,7 +1063,11 @@
// Now that `info` is in the chain, populate dex files.
for (const DexFile* dex_file : dex_files_loaded) {
- info->classpath.push_back(dex_file->GetLocation());
+ // Dex location of dex files loaded with InMemoryDexClassLoader is always bogus.
+ // Use a magic value for the classpath instead.
+ info->classpath.push_back((type == kInMemoryDexClassLoader)
+ ? kInMemoryDexClassLoaderDexLocationMagic
+ : dex_file->GetLocation());
info->checksums.push_back(dex_file->GetLocationChecksum());
info->opened_dex_files.emplace_back(dex_file);
}
diff --git a/runtime/class_loader_context_test.cc b/runtime/class_loader_context_test.cc
index 6d44b71..0c36fd4 100644
--- a/runtime/class_loader_context_test.cc
+++ b/runtime/class_loader_context_test.cc
@@ -122,7 +122,7 @@
size_t index,
const std::string& test_name) {
VerifyClassLoaderFromTestDex(
- context, index, ClassLoaderContext::kInMemoryDexClassLoader, test_name);
+ context, index, ClassLoaderContext::kInMemoryDexClassLoader, test_name, "<unknown>");
}
enum class LocationCheck {
@@ -141,7 +141,8 @@
void VerifyOpenDexFiles(
ClassLoaderContext* context,
size_t index,
- std::vector<std::unique_ptr<const DexFile>>* all_dex_files) {
+ std::vector<std::unique_ptr<const DexFile>>* all_dex_files,
+ bool classpath_matches_dex_location = true) {
ASSERT_TRUE(context != nullptr);
ASSERT_TRUE(context->dex_files_open_attempted_);
ASSERT_TRUE(context->dex_files_open_result_);
@@ -169,7 +170,9 @@
ASSERT_EQ(expected_location, opened_location);
}
ASSERT_EQ(expected_dex_file->GetLocationChecksum(), opened_dex_file->GetLocationChecksum());
- ASSERT_EQ(info.classpath[k], opened_location);
+ if (classpath_matches_dex_location) {
+ ASSERT_EQ(info.classpath[k], opened_location);
+ }
}
}
@@ -250,11 +253,20 @@
void VerifyClassLoaderFromTestDex(ClassLoaderContext* context,
size_t index,
ClassLoaderContext::ClassLoaderType type,
- const std::string& test_name) {
+ const std::string& test_name,
+ const std::string& classpath = "") {
std::vector<std::unique_ptr<const DexFile>> dex_files = OpenTestDexFiles(test_name.c_str());
- VerifyClassLoaderInfo(context, index, type, GetTestDexFileName(test_name.c_str()));
- VerifyOpenDexFiles(context, index, &dex_files);
+ // If `classpath` is set, override the expected value of ClassLoaderInfo::classpath.
+ // Otherwise assume it is equal to dex location (here test dex file name).
+ VerifyClassLoaderInfo(context,
+ index,
+ type,
+ classpath.empty() ? GetTestDexFileName(test_name.c_str()) : classpath);
+ VerifyOpenDexFiles(context,
+ index,
+ &dex_files,
+ /* classpath_matches_dex_location= */ classpath.empty());
}
};
@@ -283,10 +295,22 @@
VerifyClassLoaderDLC(context.get(), 0, "a.dex");
}
-TEST_F(ClassLoaderContextTest, ParseInvalidContextIMC) {
+TEST_F(ClassLoaderContextTest, ParseValidContextIMC) {
+ std::unique_ptr<ClassLoaderContext> context = ParseContextWithChecksums("IMC[<unknown>*111]");
+ ASSERT_FALSE(context == nullptr);
+}
+
+TEST_F(ClassLoaderContextTest, ParseInvalidContextIMCNoChecksum) {
// IMC is treated as an unknown class loader unless a checksum is provided.
// This is because the dex location is always bogus.
- std::unique_ptr<ClassLoaderContext> context = ClassLoaderContext::Create("IMC[a.dex]");
+ std::unique_ptr<ClassLoaderContext> context = ClassLoaderContext::Create("IMC[<unknown>]");
+ ASSERT_TRUE(context == nullptr);
+}
+
+TEST_F(ClassLoaderContextTest, ParseInvalidContextIMCWrongClasspathMagic) {
+ // IMC does not support arbitrary dex location. A magic marker must be used
+ // otherwise the spec should be rejected.
+ std::unique_ptr<ClassLoaderContext> context = ClassLoaderContext::Create("IMC[a.dex*111]");
ASSERT_TRUE(context == nullptr);
}
@@ -500,11 +524,7 @@
std::unique_ptr<ClassLoaderContext> context;
std::string dex_name = GetTestDexFileName("Main");
- context = ParseContextWithChecksums("PCL[" + dex_name + "*111]");
- VerifyContextSize(context.get(), 1);
- ASSERT_TRUE(context->OpenDexFiles(InstructionSet::kArm, "."));
-
- context = ParseContextWithChecksums("IMC[" + dex_name + "*111]");
+ context = ParseContextWithChecksums("IMC[<unknown>*111]");
VerifyContextSize(context.get(), 1);
ASSERT_FALSE(context->OpenDexFiles(InstructionSet::kArm, "."));
}
@@ -1075,6 +1095,23 @@
ASSERT_EQ(expected_encoding, context->EncodeContextForOatFile(""));
}
+TEST_F(ClassLoaderContextTest, EncodeInOatFileIMC) {
+ jobject class_loader_a = LoadDexInPathClassLoader("Main", nullptr);
+ jobject class_loader_b = LoadDexInInMemoryDexClassLoader("MyClass", class_loader_a);
+
+ std::unique_ptr<ClassLoaderContext> context = CreateContextForClassLoader(class_loader_b);
+ ASSERT_TRUE(context->OpenDexFiles(InstructionSet::kArm, ""));
+
+ std::vector<std::unique_ptr<const DexFile>> dex1 = OpenTestDexFiles("Main");
+ std::vector<std::unique_ptr<const DexFile>> dex2 = OpenTestDexFiles("MyClass");
+ ASSERT_EQ(dex2.size(), 1u);
+
+ std::string encoding = context->EncodeContextForOatFile("");
+ std::string expected_encoding = "IMC[<unknown>*" + std::to_string(dex2[0]->GetLocationChecksum())
+ + "];PCL[" + CreateClassPathWithChecksums(dex1) + "]";
+ ASSERT_EQ(expected_encoding, context->EncodeContextForOatFile(""));
+}
+
TEST_F(ClassLoaderContextTest, EncodeForDex2oat) {
std::string dex1_name = GetTestDexFileName("Main");
std::string dex2_name = GetTestDexFileName("MultiDex");
@@ -1082,13 +1119,23 @@
ClassLoaderContext::Create("PCL[" + dex1_name + ":" + dex2_name + "]");
ASSERT_TRUE(context->OpenDexFiles(InstructionSet::kArm, ""));
- std::vector<std::unique_ptr<const DexFile>> dex1 = OpenTestDexFiles("Main");
- std::vector<std::unique_ptr<const DexFile>> dex2 = OpenTestDexFiles("MultiDex");
std::string encoding = context->EncodeContextForDex2oat("");
std::string expected_encoding = "PCL[" + dex1_name + ":" + dex2_name + "]";
ASSERT_EQ(expected_encoding, context->EncodeContextForDex2oat(""));
}
+TEST_F(ClassLoaderContextTest, EncodeForDex2oatIMC) {
+ jobject class_loader_a = LoadDexInPathClassLoader("Main", nullptr);
+ jobject class_loader_b = LoadDexInInMemoryDexClassLoader("MyClass", class_loader_a);
+
+ std::unique_ptr<ClassLoaderContext> context = CreateContextForClassLoader(class_loader_b);
+ ASSERT_TRUE(context->OpenDexFiles(InstructionSet::kArm, ""));
+
+ std::string encoding = context->EncodeContextForDex2oat("");
+ std::string expected_encoding = "IMC[<unknown>];PCL[" + GetTestDexFileName("Main") + "]";
+ ASSERT_EQ(expected_encoding, context->EncodeContextForDex2oat(""));
+}
+
// TODO(calin) add a test which creates the context for a class loader together with dex_elements.
TEST_F(ClassLoaderContextTest, CreateContextForClassLoader) {
// The chain is
@@ -1202,7 +1249,7 @@
}
TEST_F(ClassLoaderContextTest, VerifyClassLoaderContextWithIMCMatch) {
- std::string context_spec = "PCL[a.dex*123:b.dex*456];DLC[c.dex*890];IMC[d.dex*111]";
+ std::string context_spec = "PCL[a.dex*123:b.dex*456];DLC[c.dex*890];IMC[<unknown>*111]";
std::unique_ptr<ClassLoaderContext> context = ParseContextWithChecksums(context_spec);
// Pretend that we successfully open the dex files to pass the DCHECKS.
// (as it's much easier to test all the corner cases without relying on actual dex files).
@@ -1211,7 +1258,7 @@
VerifyContextSize(context.get(), 3);
VerifyClassLoaderPCL(context.get(), 0, "a.dex:b.dex");
VerifyClassLoaderDLC(context.get(), 1, "c.dex");
- VerifyClassLoaderIMC(context.get(), 2, "d.dex");
+ VerifyClassLoaderIMC(context.get(), 2, "<unknown>");
ASSERT_EQ(context->VerifyClassLoaderContextMatch(context_spec),
ClassLoaderContext::VerificationResult::kVerifies);
@@ -1286,18 +1333,19 @@
TEST_F(ClassLoaderContextTest, VerifyClassLoaderContextMatchWithIMCSL) {
std::string context_spec =
- "IMC[a.dex*123:b.dex*456]{IMC[d.dex*321];IMC[e.dex*654]#IMC[f.dex*098:g.dex*999]}"
- ";DLC[c.dex*890]";
+ "IMC[<unknown>*123:<unknown>*456]"
+ "{IMC[<unknown>*321];IMC[<unknown>*654]#IMC[<unknown>*098:<unknown>*999]};"
+ "DLC[c.dex*890]";
std::unique_ptr<ClassLoaderContext> context = ParseContextWithChecksums(context_spec);
// Pretend that we successfully open the dex files to pass the DCHECKS.
// (as it's much easier to test all the corner cases without relying on actual dex files).
PretendContextOpenedDexFiles(context.get());
VerifyContextSize(context.get(), 2);
- VerifyClassLoaderIMC(context.get(), 0, "a.dex:b.dex");
+ VerifyClassLoaderIMC(context.get(), 0, "<unknown>:<unknown>");
VerifyClassLoaderDLC(context.get(), 1, "c.dex");
- VerifyClassLoaderSharedLibraryIMC(context.get(), 0, 0, "d.dex");
- VerifyClassLoaderSharedLibraryIMC(context.get(), 0, 1, "f.dex:g.dex");
+ VerifyClassLoaderSharedLibraryIMC(context.get(), 0, 0, "<unknown>");
+ VerifyClassLoaderSharedLibraryIMC(context.get(), 0, 1, "<unknown>:<unknown>");
ASSERT_EQ(context->VerifyClassLoaderContextMatch(context_spec),
ClassLoaderContext::VerificationResult::kVerifies);
