Simplify name verification in ClassLoaderContext.
Do not go through OatFile::ResolveRelativeEncodedDexLocation
to avoid unnecessary handling of multi-dex suffix. That was
broken anyway because `abs_dex_location` is not expected to
be a multi-dex location as demonstrated by an additional
test which would have previously failed.
Test: Improved test in class_loader_context_test.
Test: m test-art-host-gtest
Test: testrunner.py --host --optimizing
(cherry picked from commit 36ec598a4d887746291d003c97c2cb28b5987768)
Bug: 147478491
Change-Id: I85405ed0cab3bde4fc8bd7fb3c0913d5bc41c62c
diff --git a/runtime/class_loader_context.cc b/runtime/class_loader_context.cc
index 34fd146..925f25e 100644
--- a/runtime/class_loader_context.cc
+++ b/runtime/class_loader_context.cc
@@ -1163,6 +1163,18 @@
return VerificationResult::kVerifies;
}
+// Returns true if absolute `path` ends with relative `suffix` starting at
+// a directory name boundary, i.e. after a '/'. For example, "foo/bar"
+// is a valid suffix of "/data/foo/bar" but not "/data-foo/bar".
+static inline bool AbsolutePathHasRelativeSuffix(const std::string& path,
+ const std::string& suffix) {
+ DCHECK(IsAbsoluteLocation(path));
+ DCHECK(!IsAbsoluteLocation(suffix));
+ return (path.size() > suffix.size()) &&
+ (path[path.size() - suffix.size() - 1u] == '/') &&
+ (std::string_view(path).substr(/*pos*/ path.size() - suffix.size()) == suffix);
+}
+
bool ClassLoaderContext::ClassLoaderInfoMatch(
const ClassLoaderInfo& info,
const ClassLoaderInfo& expected_info,
@@ -1197,40 +1209,34 @@
// and the other as an absolute one.
bool is_dex_name_absolute = IsAbsoluteLocation(info.classpath[k]);
bool is_expected_dex_name_absolute = IsAbsoluteLocation(expected_info.classpath[k]);
- std::string dex_name;
- std::string expected_dex_name;
+ bool dex_names_match = false;
+
if (is_dex_name_absolute == is_expected_dex_name_absolute) {
// If both locations are absolute or relative then compare them as they are.
// This is usually the case for: shared libraries and secondary dex files.
- dex_name = info.classpath[k];
- expected_dex_name = expected_info.classpath[k];
+ dex_names_match = (info.classpath[k] == expected_info.classpath[k]);
} else if (is_dex_name_absolute) {
// The runtime name is absolute but the compiled name (the expected one) is relative.
// This is the case for split apks which depend on base or on other splits.
- dex_name = info.classpath[k];
- OatFile::ResolveRelativeEncodedDexLocation(info.classpath[k].c_str(),
- expected_info.classpath[k],
- &expected_dex_name);
+ dex_names_match =
+ AbsolutePathHasRelativeSuffix(info.classpath[k], expected_info.classpath[k]);
} else if (is_expected_dex_name_absolute) {
// The runtime name is relative but the compiled name is absolute.
// There is no expected use case that would end up here as dex files are always loaded
// with their absolute location. However, be tolerant and do the best effort (in case
// there are unexpected new use case...).
- OatFile::ResolveRelativeEncodedDexLocation(expected_info.classpath[k].c_str(),
- info.classpath[k],
- &dex_name);
- expected_dex_name = expected_info.classpath[k];
+ dex_names_match =
+ AbsolutePathHasRelativeSuffix(expected_info.classpath[k], info.classpath[k]);
} else {
// Both locations are relative. In this case there's not much we can be sure about
// except that the names are the same. The checksum will ensure that the files are
// are same. This should not happen outside testing and manual invocations.
- dex_name = info.classpath[k];
- expected_dex_name = expected_info.classpath[k];
+ dex_names_match = (info.classpath[k] == expected_info.classpath[k]);
}
// Compare the locations.
- if (dex_name != expected_dex_name) {
+ if (!dex_names_match) {
LOG(WARNING) << "ClassLoaderContext classpath element mismatch"
<< ". expected=" << expected_info.classpath[k]
<< ", found=" << info.classpath[k]
diff --git a/runtime/class_loader_context_test.cc b/runtime/class_loader_context_test.cc
index 0c36fd4..0083278 100644
--- a/runtime/class_loader_context_test.cc
+++ b/runtime/class_loader_context_test.cc
@@ -1402,7 +1402,18 @@
std::unique_ptr<ClassLoaderContext> context = CreateContextForClassLoader(class_loader);
- ASSERT_EQ(context->VerifyClassLoaderContextMatch(context->EncodeContextForOatFile("")),
+ std::string context_with_no_base_dir = context->EncodeContextForOatFile("");
+ ASSERT_EQ(context->VerifyClassLoaderContextMatch(context_with_no_base_dir),
+ ClassLoaderContext::VerificationResult::kVerifies);
+
+ std::string dex_location = GetTestDexFileName("MultiDex");
+ size_t pos = dex_location.rfind('/');
+ ASSERT_NE(std::string::npos, pos);
+ std::string parent = dex_location.substr(0, pos);
+
+ std::string context_with_base_dir = context->EncodeContextForOatFile(parent);
+ ASSERT_NE(context_with_base_dir, context_with_no_base_dir);
+ ASSERT_EQ(context->VerifyClassLoaderContextMatch(context_with_base_dir),
ClassLoaderContext::VerificationResult::kVerifies);
}
