Fix dex location of boot oat files during preopt
Dex location should reflect the path on target when preopting
on host. This previously did not hold and this patch fixes the
issue. Other paths remain the same as before. A DCHECK is added
to class linker to guarantee that -Xboot-classpath-locations is
not ignored.
Simultaneously it refactors the logic for resolving a relative
path to make it clearer which path is used for opening files
(dex file name) and which reflects the location on target (dex
location), as these differ when preopting.
The patch also adds a missing dex2oat dependency for oat_file_test.
Test: test-art-gtest-{host,target}-oat_file_test
Test: compiles, no DCHECK crashes
Change-Id: I0629c7ee505b5fd50649800bb3e08efc1ee44102
diff --git a/build/Android.gtest.mk b/build/Android.gtest.mk
index b1e97d9..4720d6d 100644
--- a/build/Android.gtest.mk
+++ b/build/Android.gtest.mk
@@ -241,9 +241,11 @@
$(TARGET_CORE_IMAGE_interpreter_32)
ART_GTEST_oat_file_test_HOST_DEPS := \
- $(ART_GTEST_dex2oat_environment_tests_HOST_DEPS)
+ $(ART_GTEST_dex2oat_environment_tests_HOST_DEPS) \
+ $(HOST_OUT_EXECUTABLES)/dex2oatd
ART_GTEST_oat_file_test_TARGET_DEPS := \
- $(ART_GTEST_dex2oat_environment_tests_TARGET_DEPS)
+ $(ART_GTEST_dex2oat_environment_tests_TARGET_DEPS) \
+ $(TARGET_OUT_EXECUTABLES)/dex2oatd
ART_GTEST_oat_file_assistant_test_HOST_DEPS := \
$(ART_GTEST_dex2oat_environment_tests_HOST_DEPS)
diff --git a/libartbase/base/file_utils.h b/libartbase/base/file_utils.h
index 2406336..92b09c9 100644
--- a/libartbase/base/file_utils.h
+++ b/libartbase/base/file_utils.h
@@ -102,6 +102,9 @@
// dup(2), except setting the O_CLOEXEC flag atomically, when possible.
int DupCloexec(int fd);
+// Returns true if `path` begins with a slash.
+inline bool IsAbsoluteLocation(const std::string& path) { return !path.empty() && path[0] == '/'; }
+
} // namespace art
#endif // ART_LIBARTBASE_BASE_FILE_UTILS_H_
diff --git a/runtime/class_linker.cc b/runtime/class_linker.cc
index 6fdac3f..658fec3 100644
--- a/runtime/class_linker.cc
+++ b/runtime/class_linker.cc
@@ -1077,6 +1077,14 @@
error_msg)) {
return false;
}
+ // Assert that if absolute boot classpath locations were provided, they were
+ // assigned to the loaded dex files.
+ if (kIsDebugBuild && IsAbsoluteLocation(boot_class_path_locations[i])) {
+ for (const auto& dex_file : dex_files) {
+ DCHECK_EQ(DexFileLoader::GetBaseLocation(dex_file->GetLocation()),
+ boot_class_path_locations[i]);
+ }
+ }
// Append opened dex files at the end.
boot_dex_files_.insert(boot_dex_files_.end(),
std::make_move_iterator(dex_files.begin()),
@@ -2027,10 +2035,8 @@
for (int32_t i = 0; i < dex_caches->GetLength(); i++) {
ObjPtr<mirror::DexCache> dex_cache = dex_caches->Get(i);
- std::string dex_file_location(dex_cache->GetLocation()->ToModifiedUtf8());
- // TODO: Only store qualified paths.
- // If non qualified, qualify it.
- dex_file_location = OatFile::ResolveRelativeEncodedDexLocation(dex_location, dex_file_location);
+ std::string dex_file_location = dex_cache->GetLocation()->ToModifiedUtf8();
+ DCHECK_EQ(dex_location, DexFileLoader::GetBaseLocation(dex_file_location));
std::unique_ptr<const DexFile> dex_file = OpenOatDexFile(oat_file,
dex_file_location.c_str(),
error_msg);
diff --git a/runtime/class_loader_context.cc b/runtime/class_loader_context.cc
index 2b62d59..34fd146 100644
--- a/runtime/class_loader_context.cc
+++ b/runtime/class_loader_context.cc
@@ -1122,10 +1122,6 @@
return result;
}
-static bool IsAbsoluteLocation(const std::string& location) {
- return !location.empty() && location[0] == '/';
-}
-
ClassLoaderContext::VerificationResult ClassLoaderContext::VerifyClassLoaderContextMatch(
const std::string& context_spec,
bool verify_names,
@@ -1213,15 +1209,17 @@
// 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];
- expected_dex_name = OatFile::ResolveRelativeEncodedDexLocation(
- info.classpath[k].c_str(), expected_info.classpath[k]);
+ OatFile::ResolveRelativeEncodedDexLocation(info.classpath[k].c_str(),
+ expected_info.classpath[k],
+ &expected_dex_name);
} 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...).
- dex_name = OatFile::ResolveRelativeEncodedDexLocation(
- expected_info.classpath[k].c_str(), info.classpath[k]);
+ OatFile::ResolveRelativeEncodedDexLocation(expected_info.classpath[k].c_str(),
+ info.classpath[k],
+ &dex_name);
expected_dex_name = expected_info.classpath[k];
} else {
// Both locations are relative. In this case there's not much we can be sure about
diff --git a/runtime/oat_file.cc b/runtime/oat_file.cc
index 624a1de..b788825 100644
--- a/runtime/oat_file.cc
+++ b/runtime/oat_file.cc
@@ -583,8 +583,16 @@
// Location encoded in the oat file. We will use this for multidex naming,
// see ResolveRelativeEncodedDexLocation.
std::string oat_dex_file_location(dex_file_location_data, dex_file_location_size);
- std::string dex_file_location =
- ResolveRelativeEncodedDexLocation(abs_dex_location, oat_dex_file_location);
+ // If `oat_dex_file_location` is relative (so that the oat file can be moved to
+ // a different folder), resolve to absolute location. Also resolve the file name
+ // in case dex files need to be opened from disk. The file name and location
+ // differ when cross-compiling on host for target.
+ std::string dex_file_name;
+ std::string dex_file_location;
+ ResolveRelativeEncodedDexLocation(abs_dex_location,
+ oat_dex_file_location,
+ &dex_file_location,
+ &dex_file_name);
uint32_t dex_file_checksum;
if (UNLIKELY(!ReadOatDexFileData(*this, &oat, &dex_file_checksum))) {
@@ -639,7 +647,7 @@
error_msg,
uncompressed_dex_files_.get());
} else {
- loaded = dex_file_loader.Open(dex_file_location.c_str(),
+ loaded = dex_file_loader.Open(dex_file_name.c_str(),
dex_file_location,
/*verify=*/ false,
/*verify_checksum=*/ false,
@@ -813,30 +821,28 @@
return false;
}
- std::string canonical_location =
- DexFileLoader::GetDexCanonicalLocation(dex_file_location.c_str());
-
// Create the OatDexFile and add it to the owning container.
- OatDexFile* oat_dex_file = new OatDexFile(this,
- dex_file_location,
- canonical_location,
- dex_file_checksum,
- dex_file_pointer,
- lookup_table_data,
- method_bss_mapping,
- type_bss_mapping,
- string_bss_mapping,
- class_offsets_pointer,
- dex_layout_sections);
+ OatDexFile* oat_dex_file = new OatDexFile(
+ this,
+ dex_file_location,
+ DexFileLoader::GetDexCanonicalLocation(dex_file_name.c_str()),
+ dex_file_checksum,
+ dex_file_pointer,
+ lookup_table_data,
+ method_bss_mapping,
+ type_bss_mapping,
+ string_bss_mapping,
+ class_offsets_pointer,
+ dex_layout_sections);
oat_dex_files_storage_.push_back(oat_dex_file);
// Add the location and canonical location (if different) to the oat_dex_files_ table.
// Note: we use the dex_file_location_data storage for the view, as oat_dex_file_location
// is just a temporary string.
std::string_view key(dex_file_location_data, dex_file_location_size);
+ std::string_view canonical_key(oat_dex_file->GetCanonicalDexFileLocation());
oat_dex_files_.Put(key, oat_dex_file);
- if (canonical_location != oat_dex_file_location) {
- std::string_view canonical_key(oat_dex_file->GetCanonicalDexFileLocation());
+ if (canonical_key != key) {
oat_dex_files_.Put(canonical_key, oat_dex_file);
}
}
@@ -1491,29 +1497,70 @@
// General OatFile code //
//////////////////////////
-std::string OatFile::ResolveRelativeEncodedDexLocation(
- const char* abs_dex_location, const std::string& rel_dex_location) {
+static bool IsLocationSuffix(const char* abs_dex_location, const std::string& rel_dex_location) {
+ std::string_view abs_location(abs_dex_location);
+ std::string target_suffix = "/" + DexFileLoader::GetBaseLocation(rel_dex_location);
+ if (abs_location.size() <= target_suffix.size()) {
+ return false;
+ }
+ size_t pos = abs_location.size() - target_suffix.size();
+ return abs_location.compare(pos, std::string::npos, target_suffix) == 0;
+}
+
+static void MaybeResolveDexPath(const char* abs_dex_location,
+ const std::string& rel_dex_location,
+ bool resolve,
+ /* out */ std::string* out_location) {
+ DCHECK(!resolve || abs_dex_location != nullptr);
+ if (out_location != nullptr) {
+ *out_location = resolve
+ ? std::string(abs_dex_location) + DexFileLoader::GetMultiDexSuffix(rel_dex_location)
+ : rel_dex_location;
+ }
+}
+
+void OatFile::ResolveRelativeEncodedDexLocation(const char* abs_dex_location,
+ const std::string& rel_dex_location,
+ /* out */ std::string* dex_file_location,
+ /* out */ std::string* dex_file_name) {
+ // Note that in this context `abs_dex_location` may not always be absolute
+ // and `rel_dex_location` may not always be relative. It simply means that
+ // we will try to resolve `rel_dex_location` into an absolute location using
+ // `abs_dex_location` for the base directory if needed.
+
+ bool resolve_location = false;
+ bool resolve_filename = false;
+
if (abs_dex_location != nullptr) {
- std::string base = DexFileLoader::GetBaseLocation(rel_dex_location);
- // Add :classes<N>.dex used for secondary multidex files.
- std::string multidex_suffix = DexFileLoader::GetMultiDexSuffix(rel_dex_location);
- if (!kIsTargetBuild) {
- // For host, we still do resolution as the rel_dex_location might be absolute
- // for a target dex (for example /system/foo/foo.apk).
- return std::string(abs_dex_location) + multidex_suffix;
- } else if (rel_dex_location[0] != '/') {
- // Check if the base is a suffix of the provided abs_dex_location.
- std::string target_suffix = ((rel_dex_location[0] != '/') ? "/" : "") + base;
- std::string abs_location(abs_dex_location);
- if (abs_location.size() > target_suffix.size()) {
- size_t pos = abs_location.size() - target_suffix.size();
- if (abs_location.compare(pos, std::string::npos, target_suffix) == 0) {
- return abs_location + multidex_suffix;
- }
- }
+ if (!IsAbsoluteLocation(rel_dex_location) &&
+ IsLocationSuffix(abs_dex_location, rel_dex_location)) {
+ // The base location (w/o multidex suffix) of the relative `rel_dex_location` is a suffix
+ // of `abs_dex_location`. This typically happens for oat files which only encode the
+ // basename() so the oat and dex files can move to different directories.
+ // Example:
+ // abs_dex_location = "/data/app/myapp/MyApplication.apk"
+ // rel_dex_location = "MyApplication.apk!classes2.dex"
+ resolve_location = true;
+ resolve_filename = true;
+ } else {
+ // Case 1: `rel_dex_location` is absolute
+ // On target always use `rel_dex_location` for both dex file name and dex location.
+ // On host assume we're cross-compiling and use `abs_dex_location` as a file name
+ // (for loading files) and `rel_dex_location` as the dex location. If we're not
+ // cross-compiling, the two paths should be equal.
+ // Case 2: `rel_dex_location` is relative and not suffix of `abs_location`
+ // This should never happen outside of tests. On target always use `rel_dex_location`. On
+ // host use `abs_dex_location` with the appropriate multidex suffix because
+ // `rel_dex_location` might be the target path.
+ resolve_location = false;
+ resolve_filename = !kIsTargetBuild;
}
}
- return rel_dex_location;
+
+ // Construct dex file location and dex file name if the correspoding out-param pointers
+ // were provided by the caller.
+ MaybeResolveDexPath(abs_dex_location, rel_dex_location, resolve_location, dex_file_location);
+ MaybeResolveDexPath(abs_dex_location, rel_dex_location, resolve_filename, dex_file_name);
}
static void CheckLocation(const std::string& location) {
diff --git a/runtime/oat_file.h b/runtime/oat_file.h
index fbe596e..e8e7df0 100644
--- a/runtime/oat_file.h
+++ b/runtime/oat_file.h
@@ -334,7 +334,12 @@
// Initialize relocation sections (.data.bimg.rel.ro and .bss).
void InitializeRelocations() const;
- // Returns the absolute dex location for the encoded relative dex location.
+ // Constructs the absolute dex location and/or dex file name for the relative dex
+ // location (`rel_dex_location`) in the oat file, using the `abs_dex_location` of
+ // the dex file this oat belongs to.
+ //
+ // The dex file name and dex location differ when cross compiling where the dex file
+ // name is the host path (for opening files) and dex location is the future path on target.
//
// If not null, abs_dex_location is used to resolve the absolute dex
// location of relative dex locations encoded in the oat file.
@@ -343,8 +348,13 @@
// to "/data/app/foo/base.apk", "/data/app/foo/base.apk!classes2.dex", etc.
// Relative encoded dex locations that don't match the given abs_dex_location
// are left unchanged.
- static std::string ResolveRelativeEncodedDexLocation(
- const char* abs_dex_location, const std::string& rel_dex_location);
+ //
+ // Computation of both `dex_file_location` and `dex_file_name` can be skipped
+ // by setting the corresponding out parameter to `nullptr`.
+ static void ResolveRelativeEncodedDexLocation(const char* abs_dex_location,
+ const std::string& rel_dex_location,
+ /* out */ std::string* dex_file_location,
+ /* out */ std::string* dex_file_name = nullptr);
// Finds the associated oat class for a dex_file and descriptor. Returns an invalid OatClass on
// error and sets found to false.
diff --git a/runtime/oat_file_test.cc b/runtime/oat_file_test.cc
index ce09da4d..7a122ba 100644
--- a/runtime/oat_file_test.cc
+++ b/runtime/oat_file_test.cc
@@ -30,50 +30,105 @@
class OatFileTest : public DexoptTest {
};
-TEST_F(OatFileTest, ResolveRelativeEncodedDexLocation) {
- EXPECT_EQ(std::string("/data/app/foo/base.apk"),
- OatFile::ResolveRelativeEncodedDexLocation(
- nullptr, "/data/app/foo/base.apk"));
+TEST_F(OatFileTest, ResolveRelativeEncodedDexLocation_NullAbsLocation) {
+ std::string dex_location;
+ std::string dex_file_name;
+ OatFile::ResolveRelativeEncodedDexLocation(nullptr,
+ "/data/app/foo/base.apk",
+ &dex_location,
+ &dex_file_name);
+ ASSERT_EQ("/data/app/foo/base.apk", dex_file_name);
+ ASSERT_EQ("/data/app/foo/base.apk", dex_location);
+}
- EXPECT_EQ(std::string("/data/app/foo/base.apk"),
- OatFile::ResolveRelativeEncodedDexLocation(
- "/data/app/foo/base.apk", "base.apk"));
+TEST_F(OatFileTest, ResolveRelativeEncodedDexLocation_NullAbsLocation_Multidex) {
+ std::string dex_location;
+ std::string dex_file_name;
+ OatFile::ResolveRelativeEncodedDexLocation(nullptr,
+ "/data/app/foo/base.apk!classes2.dex",
+ &dex_location,
+ &dex_file_name);
+ ASSERT_EQ("/data/app/foo/base.apk!classes2.dex", dex_file_name);
+ ASSERT_EQ("/data/app/foo/base.apk!classes2.dex", dex_location);
+}
- EXPECT_EQ(std::string("/data/app/foo/base.apk"),
- OatFile::ResolveRelativeEncodedDexLocation(
- "/data/app/foo/base.apk", "foo/base.apk"));
+TEST_F(OatFileTest, ResolveRelativeEncodedDexLocation_RelLocationAbsolute) {
+ std::string dex_location;
+ std::string dex_file_name;
+ OatFile::ResolveRelativeEncodedDexLocation("base.apk",
+ "/system/framework/base.apk",
+ &dex_location,
+ &dex_file_name);
+ ASSERT_EQ(kIsTargetBuild ? "/system/framework/base.apk" : "base.apk", dex_file_name);
+ ASSERT_EQ("/system/framework/base.apk", dex_location);
+}
- EXPECT_EQ(std::string("/data/app/foo/base.apk!classes2.dex"),
- OatFile::ResolveRelativeEncodedDexLocation(
- "/data/app/foo/base.apk", "base.apk!classes2.dex"));
+TEST_F(OatFileTest, ResolveRelativeEncodedDexLocation_BothAbsoluteLocations) {
+ std::string dex_location;
+ std::string dex_file_name;
+ OatFile::ResolveRelativeEncodedDexLocation("/data/app/foo/base.apk",
+ "/system/framework/base.apk",
+ &dex_location,
+ &dex_file_name);
+ ASSERT_EQ(kIsTargetBuild ? "/system/framework/base.apk" : "/data/app/foo/base.apk",
+ dex_file_name);
+ ASSERT_EQ("/system/framework/base.apk", dex_location);
+}
- EXPECT_EQ(std::string("/data/app/foo/base.apk!classes11.dex"),
- OatFile::ResolveRelativeEncodedDexLocation(
- "/data/app/foo/base.apk", "base.apk!classes11.dex"));
+TEST_F(OatFileTest, ResolveRelativeEncodedDexLocation_RelSuffixOfAbsLocation1) {
+ std::string dex_location;
+ std::string dex_file_name;
+ OatFile::ResolveRelativeEncodedDexLocation("/data/app/foo/base.apk",
+ "base.apk",
+ &dex_location,
+ &dex_file_name);
+ ASSERT_EQ("/data/app/foo/base.apk", dex_file_name);
+ ASSERT_EQ("/data/app/foo/base.apk", dex_location);
+}
- // Host and target differ in their way of handling locations
- // that are prefix of one another, due to boot image files.
- if (kIsTargetBuild) {
- EXPECT_EQ(std::string("/system/framework/base.apk"),
- OatFile::ResolveRelativeEncodedDexLocation(
- "/data/app/foo/base.apk", "/system/framework/base.apk"));
- EXPECT_EQ(std::string("base.apk"),
- OatFile::ResolveRelativeEncodedDexLocation(
- "/data/app/foo/sludge.apk", "base.apk"));
- EXPECT_EQ(std::string("o/base.apk"),
- OatFile::ResolveRelativeEncodedDexLocation(
- "/data/app/foo/base.apk", "o/base.apk"));
- } else {
- EXPECT_EQ(std::string("/data/app/foo/base.apk"),
- OatFile::ResolveRelativeEncodedDexLocation(
- "/data/app/foo/base.apk", "/system/framework/base.apk"));
- EXPECT_EQ(std::string("/data/app/foo/sludge.apk"),
- OatFile::ResolveRelativeEncodedDexLocation(
- "/data/app/foo/sludge.apk", "base.apk"));
- EXPECT_EQ(std::string("/data/app/foo/base.apk"),
- OatFile::ResolveRelativeEncodedDexLocation(
- "/data/app/foo/base.apk", "o/base.apk"));
- }
+TEST_F(OatFileTest, ResolveRelativeEncodedDexLocation_RelSuffixOfAbsLocation2) {
+ std::string dex_location;
+ std::string dex_file_name;
+ OatFile::ResolveRelativeEncodedDexLocation("/data/app/foo/base.apk",
+ "foo/base.apk",
+ &dex_location,
+ &dex_file_name);
+ ASSERT_EQ("/data/app/foo/base.apk", dex_file_name);
+ ASSERT_EQ("/data/app/foo/base.apk", dex_location);
+}
+
+TEST_F(OatFileTest, ResolveRelativeEncodedDexLocation_RelSuffixOfAbsLocation_Multidex) {
+ std::string dex_location;
+ std::string dex_file_name;
+ OatFile::ResolveRelativeEncodedDexLocation("/data/app/foo/base.apk",
+ "base.apk!classes11.dex",
+ &dex_location,
+ &dex_file_name);
+ ASSERT_EQ("/data/app/foo/base.apk!classes11.dex", dex_file_name);
+ ASSERT_EQ("/data/app/foo/base.apk!classes11.dex", dex_location);
+}
+
+TEST_F(OatFileTest, ResolveRelativeEncodedDexLocation_RelNotSuffixOfAbsLocation1) {
+ std::string dex_location;
+ std::string dex_file_name;
+ OatFile::ResolveRelativeEncodedDexLocation("/data/app/foo/sludge.apk",
+ "base.apk!classes2.dex",
+ &dex_location,
+ &dex_file_name);
+ ASSERT_EQ(kIsTargetBuild ? "base.apk!classes2.dex" : "/data/app/foo/sludge.apk!classes2.dex",
+ dex_file_name);
+ ASSERT_EQ("base.apk!classes2.dex", dex_location);
+}
+
+TEST_F(OatFileTest, ResolveRelativeEncodedDexLocation_RelNotSuffixOfAbsLocation2) {
+ std::string dex_location;
+ std::string dex_file_name;
+ OatFile::ResolveRelativeEncodedDexLocation("/data/app/foo/sludge.apk",
+ "o/base.apk",
+ &dex_location,
+ &dex_file_name);
+ ASSERT_EQ(kIsTargetBuild ? "o/base.apk" : "/data/app/foo/sludge.apk", dex_file_name);
+ ASSERT_EQ("o/base.apk", dex_location);
}
TEST_F(OatFileTest, LoadOat) {