Revert "ART: Fix up some multi-image cases"
Fails imgdiag_test on device.
Bug: 26317072
Bug: 26320300
This reverts commit 288b1e9a0dddfb91e85067fe81de55174f313c7c.
Change-Id: Iccd05827b0630281b6f959331eaa4202526df78e
diff --git a/dex2oat/dex2oat.cc b/dex2oat/dex2oat.cc
index 50480d9..bb4224b 100644
--- a/dex2oat/dex2oat.cc
+++ b/dex2oat/dex2oat.cc
@@ -854,11 +854,6 @@
if (last_oat_slash == std::string::npos) {
Usage("--multi-image used with unusable oat filename %s", base_oat.c_str());
}
- // We also need to honor path components that were encoded through '@'. Otherwise the loading
- // code won't be able to find the images.
- if (base_oat.find('@', last_oat_slash) != std::string::npos) {
- last_oat_slash = base_oat.rfind('@');
- }
base_oat = base_oat.substr(0, last_oat_slash + 1);
std::string base_img = image_filenames_[0];
@@ -866,24 +861,9 @@
if (last_img_slash == std::string::npos) {
Usage("--multi-image used with unusable image filename %s", base_img.c_str());
}
- // We also need to honor path components that were encoded through '@'. Otherwise the loading
- // code won't be able to find the images.
- if (base_img.find('@', last_img_slash) != std::string::npos) {
- last_img_slash = base_img.rfind('@');
- }
-
- // Get the prefix, which is the primary image name (without path components). Strip the
- // extension.
- std::string prefix = base_img.substr(last_img_slash + 1);
- if (prefix.rfind('.') != std::string::npos) {
- prefix = prefix.substr(0, prefix.rfind('.'));
- }
- if (!prefix.empty()) {
- prefix = prefix + "-";
- }
-
base_img = base_img.substr(0, last_img_slash + 1);
+ // Now create the other names.
// Note: we have some special case here for our testing. We have to inject the differentiating
// parts for the different core images.
std::string infix; // Empty infix by default.
@@ -902,15 +882,14 @@
infix = dex_file.substr(strlen("core"));
}
}
-
- // Now create the other names. Use a counted loop to skip the first one.
+ // Use a counted loop to skip the first one.
for (size_t i = 1; i < dex_locations_.size(); ++i) {
// TODO: Make everything properly std::string.
- std::string image_name = CreateMultiImageName(dex_locations_[i], prefix, infix, ".art");
+ std::string image_name = CreateMultiImageName(dex_locations_[i], infix, ".art");
char_backing_storage_.push_back(base_img + image_name);
image_filenames_.push_back((char_backing_storage_.end() - 1)->c_str());
- std::string oat_name = CreateMultiImageName(dex_locations_[i], prefix, infix, ".oat");
+ std::string oat_name = CreateMultiImageName(dex_locations_[i], infix, ".oat");
char_backing_storage_.push_back(base_oat + oat_name);
oat_filenames_.push_back((char_backing_storage_.end() - 1)->c_str());
}
@@ -925,23 +904,13 @@
key_value_store_.reset(new SafeMap<std::string, std::string>());
}
- // Modify the input string in the following way:
- // 0) Assume input is /a/b/c.d
- // 1) Strip the path -> c.d
- // 2) Inject prefix p -> pc.d
- // 3) Inject infix i -> pci.d
- // 4) Replace suffix with s if it's "jar" -> d == "jar" -> pci.s
static std::string CreateMultiImageName(std::string in,
- const std::string& prefix,
const std::string& infix,
- const char* replace_suffix) {
+ const char* suffix) {
size_t last_dex_slash = in.rfind('/');
if (last_dex_slash != std::string::npos) {
in = in.substr(last_dex_slash + 1);
}
- if (!prefix.empty()) {
- in = prefix + in;
- }
if (!infix.empty()) {
// Inject infix.
size_t last_dot = in.rfind('.');
@@ -950,8 +919,7 @@
}
}
if (EndsWith(in, ".jar")) {
- in = in.substr(0, in.length() - strlen(".jar")) +
- (replace_suffix != nullptr ? replace_suffix : "");
+ in = in.substr(0, in.length() - strlen(".jar")) + (suffix != nullptr ? suffix : "");
}
return in;
}
@@ -1316,21 +1284,16 @@
void CreateDexOatMappings() {
if (oat_files_.size() > 1) {
- // TODO: This needs to change, as it is not a stable mapping. If a dex file is missing,
- // the images will be out of whack. b/26317072
size_t index = 0;
for (size_t i = 0; i < oat_files_.size(); ++i) {
- std::vector<const DexFile*> dex_files;
- if (index < dex_files_.size()) {
- dex_files.push_back(dex_files_[index]);
+ std::vector<const DexFile*> dex_files = { 1, dex_files_[index] };
+ dex_file_oat_filename_map_.emplace(dex_files_[index], oat_filenames_[i]);
+ index++;
+ while (index < dex_files_.size() &&
+ (dex_files_[index]->GetBaseLocation() == dex_files_[index - 1]->GetBaseLocation())) {
dex_file_oat_filename_map_.emplace(dex_files_[index], oat_filenames_[i]);
+ dex_files.push_back(dex_files_[index]);
index++;
- while (index < dex_files_.size() &&
- (dex_files_[index]->GetBaseLocation() == dex_files_[index - 1]->GetBaseLocation())) {
- dex_file_oat_filename_map_.emplace(dex_files_[index], oat_filenames_[i]);
- dex_files.push_back(dex_files_[index]);
- index++;
- }
}
dex_files_per_oat_file_.push_back(std::move(dex_files));
}
@@ -1421,9 +1384,8 @@
if (IsBootImage() && image_filenames_.size() > 1) {
// If we're compiling the boot image, store the boot classpath into the Key-Value store. If
- // the image filename was adapted (e.g., for our tests), we need to change this here, too, but
- // need to strip all path components (they will be re-established when loading).
- // We need this for the multi-image case.
+ // the image filename was adapted (e.g., for our tests), we need to change this here, too. We
+ // need this for the multi-image case.
std::ostringstream bootcp_oss;
bool first_bootcp = true;
for (size_t i = 0; i < dex_locations_.size(); ++i) {
@@ -1434,24 +1396,14 @@
std::string dex_loc = dex_locations_[i];
std::string image_filename = image_filenames_[i];
- // Use the dex_loc path, but the image_filename name (without path elements).
+ // Use the dex_loc path, but the image_filename name.
size_t dex_last_slash = dex_loc.rfind('/');
-
- // npos is max(size_t). That makes this a bit ugly.
size_t image_last_slash = image_filename.rfind('/');
- size_t image_last_at = image_filename.rfind('@');
- size_t image_last_sep = (image_last_slash == std::string::npos)
- ? image_last_at
- : (image_last_at == std::string::npos)
- ? std::string::npos
- : std::max(image_last_slash, image_last_at);
- // Note: whenever image_last_sep == npos, +1 overflow means using the full string.
-
if (dex_last_slash == std::string::npos) {
- dex_loc = image_filename.substr(image_last_sep + 1);
+ dex_loc = image_filename.substr(image_last_slash + 1);
} else {
dex_loc = dex_loc.substr(0, dex_last_slash + 1) +
- image_filename.substr(image_last_sep + 1);
+ image_filename.substr(image_last_slash + 1);
}
// Image filenames already end with .art, no need to replace.
diff --git a/runtime/gc/heap.cc b/runtime/gc/heap.cc
index 7f67ae4..3e432c7 100644
--- a/runtime/gc/heap.cc
+++ b/runtime/gc/heap.cc
@@ -265,9 +265,6 @@
// For code reuse, handle this like a work queue.
std::vector<std::string> image_file_names;
image_file_names.push_back(image_file_name);
- // The loaded spaces. Secondary images may fail to load, in which case we need to remove
- // already added spaces.
- std::vector<space::Space*> added_image_spaces;
for (size_t index = 0; index < image_file_names.size(); ++index) {
std::string& image_name = image_file_names[index];
@@ -280,7 +277,6 @@
ATRACE_END();
if (boot_image_space != nullptr) {
AddSpace(boot_image_space);
- added_image_spaces.push_back(boot_image_space);
// Oat files referenced by image files immediately follow them in memory, ensure alloc space
// isn't going to get in the middle
uint8_t* oat_file_end_addr = boot_image_space->GetImageHeader().GetOatFileEnd();
@@ -302,18 +298,66 @@
continue;
}
- space::ImageSpace::CreateMultiImageLocations(image_file_name,
- boot_classpath,
- &image_file_names);
+ std::vector<std::string> images;
+ Split(boot_classpath, ':', &images);
+
+ // Add the rest into the list. We have to adjust locations, possibly:
+ //
+ // For example, image_file_name is /a/b/c/d/e.art
+ // images[0] is f/c/d/e.art
+ // ----------------------------------------------
+ // images[1] is g/h/i/j.art -> /a/b/h/i/j.art
+
+ // Derive pattern.
+ std::vector<std::string> left;
+ Split(image_file_name, '/', &left);
+ std::vector<std::string> right;
+ Split(images[0], '/', &right);
+
+ size_t common = 1;
+ while (common < left.size() && common < right.size()) {
+ if (left[left.size() - common - 1] != right[right.size() - common - 1]) {
+ break;
+ }
+ common++;
+ }
+
+ std::vector<std::string> prefix_vector(left.begin(), left.end() - common);
+ std::string common_prefix = Join(prefix_vector, '/');
+ if (!common_prefix.empty() && common_prefix[0] != '/' && image_file_name[0] == '/') {
+ common_prefix = "/" + common_prefix;
+ }
+
+ // Apply pattern to images[1] .. images[n].
+ for (size_t i = 1; i < images.size(); ++i) {
+ std::string image = images[i];
+
+ size_t rslash = std::string::npos;
+ for (size_t j = 0; j < common; ++j) {
+ if (rslash != std::string::npos) {
+ rslash--;
+ }
+
+ rslash = image.rfind('/', rslash);
+ if (rslash == std::string::npos) {
+ rslash = 0;
+ }
+ if (rslash == 0) {
+ break;
+ }
+ }
+ std::string image_part = image.substr(rslash);
+
+ std::string new_image = common_prefix + (StartsWith(image_part, "/") ? "" : "/") +
+ image_part;
+ image_file_names.push_back(new_image);
+ }
}
} else {
LOG(ERROR) << "Could not create image space with image file '" << image_file_name << "'. "
<< "Attempting to fall back to imageless running. Error was: " << error_msg
<< "\nAttempted image: " << image_name;
- // Remove already loaded spaces.
- for (space::Space* loaded_space : added_image_spaces) {
- RemoveSpace(loaded_space);
- }
+ // TODO: Remove already loaded spaces.
break;
}
}
diff --git a/runtime/gc/space/image_space.cc b/runtime/gc/space/image_space.cc
index dfdbd04..952759c 100644
--- a/runtime/gc/space/image_space.cc
+++ b/runtime/gc/space/image_space.cc
@@ -523,9 +523,6 @@
} else if (!ImageCreationAllowed(is_global_cache, &reason)) {
// Whether we can write to the cache.
success = false;
- } else if (secondary_image) {
- reason = "Should not have to patch secondary image.";
- success = false;
} else {
// Try to relocate.
success = RelocateImage(image_location, cache_filename.c_str(), image_isa, &reason);
@@ -618,9 +615,6 @@
return nullptr;
} else if (!ImageCreationAllowed(is_global_cache, error_msg)) {
return nullptr;
- } else if (secondary_image) {
- *error_msg = "Cannot compile a secondary image.";
- return nullptr;
} else if (!GenerateImage(cache_filename, image_isa, error_msg)) {
*error_msg = StringPrintf("Failed to generate image '%s': %s",
cache_filename.c_str(), error_msg->c_str());
@@ -975,67 +969,6 @@
<< ",name=\"" << GetName() << "\"]";
}
-void ImageSpace::CreateMultiImageLocations(const std::string& input_image_file_name,
- const std::string& boot_classpath,
- std::vector<std::string>* image_file_names) {
- DCHECK(image_file_names != nullptr);
-
- std::vector<std::string> images;
- Split(boot_classpath, ':', &images);
-
- // Add the rest into the list. We have to adjust locations, possibly:
- //
- // For example, image_file_name is /a/b/c/d/e.art
- // images[0] is f/c/d/e.art
- // ----------------------------------------------
- // images[1] is g/h/i/j.art -> /a/b/h/i/j.art
-
- // Derive pattern.
- std::vector<std::string> left;
- Split(input_image_file_name, '/', &left);
- std::vector<std::string> right;
- Split(images[0], '/', &right);
-
- size_t common = 1;
- while (common < left.size() && common < right.size()) {
- if (left[left.size() - common - 1] != right[right.size() - common - 1]) {
- break;
- }
- common++;
- }
-
- std::vector<std::string> prefix_vector(left.begin(), left.end() - common);
- std::string common_prefix = Join(prefix_vector, '/');
- if (!common_prefix.empty() && common_prefix[0] != '/' && input_image_file_name[0] == '/') {
- common_prefix = "/" + common_prefix;
- }
-
- // Apply pattern to images[1] .. images[n].
- for (size_t i = 1; i < images.size(); ++i) {
- std::string image = images[i];
-
- size_t rslash = std::string::npos;
- for (size_t j = 0; j < common; ++j) {
- if (rslash != std::string::npos) {
- rslash--;
- }
-
- rslash = image.rfind('/', rslash);
- if (rslash == std::string::npos) {
- rslash = 0;
- }
- if (rslash == 0) {
- break;
- }
- }
- std::string image_part = image.substr(rslash);
-
- std::string new_image = common_prefix + (StartsWith(image_part, "/") ? "" : "/") +
- image_part;
- image_file_names->push_back(new_image);
- }
-}
-
} // namespace space
} // namespace gc
} // namespace art
diff --git a/runtime/gc/space/image_space.h b/runtime/gc/space/image_space.h
index b8ae4a0..a54358a 100644
--- a/runtime/gc/space/image_space.h
+++ b/runtime/gc/space/image_space.h
@@ -122,12 +122,6 @@
bool* has_data,
bool *is_global_cache);
- // Use the input image filename to adapt the names in the given boot classpath to establish
- // complete locations for secondary images.
- static void CreateMultiImageLocations(const std::string& input_image_file_name,
- const std::string& boot_classpath,
- std::vector<std::string>* image_filenames);
-
// Return the end of the image which includes non-heap objects such as ArtMethods and ArtFields.
uint8_t* GetImageEnd() const {
return Begin() + GetImageHeader().GetImageSize();
diff --git a/runtime/oat_file_assistant.cc b/runtime/oat_file_assistant.cc
index 8543ff4..cb80cc9 100644
--- a/runtime/oat_file_assistant.cc
+++ b/runtime/oat_file_assistant.cc
@@ -846,12 +846,7 @@
std::string OatFileAssistant::ImageLocation() {
Runtime* runtime = Runtime::Current();
- const std::vector<gc::space::ImageSpace*>& image_spaces =
- runtime->GetHeap()->GetBootImageSpaces();
- if (image_spaces.empty()) {
- return "";
- }
- return image_spaces[0]->GetImageLocation();
+ return runtime->GetHeap()->GetBootImageSpaces()[0]->GetImageLocation();
}
const uint32_t* OatFileAssistant::GetRequiredDexChecksum() {
diff --git a/runtime/runtime.cc b/runtime/runtime.cc
index 5c72629..98aa8d8 100644
--- a/runtime/runtime.cc
+++ b/runtime/runtime.cc
@@ -827,9 +827,13 @@
const OatHeader& boot_oat_header = oat_file->GetOatHeader();
const char* boot_cp = boot_oat_header.GetStoreValueByKey(OatHeader::kBootClassPath);
if (boot_cp != nullptr) {
- gc::space::ImageSpace::CreateMultiImageLocations(image_locations[0],
- boot_cp,
- &image_locations);
+ std::vector<std::string> cp;
+ Split(boot_cp, ':', &cp);
+
+ if (cp.size() > 1) {
+ // More images, enqueue (skipping the first).
+ image_locations.insert(image_locations.end(), cp.begin() + 1, cp.end());
+ }
}
}
diff --git a/test/Android.run-test.mk b/test/Android.run-test.mk
index 81cfb70..a3aeb6d 100644
--- a/test/Android.run-test.mk
+++ b/test/Android.run-test.mk
@@ -337,6 +337,11 @@
TEST_ART_BROKEN_GCSTRESS_RUN_TESTS :=
+# b/26320300: multi image is broken and 119-noimage-patchoat fails because of it.
+ART_TEST_KNOWN_BROKEN += $(call all-run-test-names,target,$(RUN_TYPES),$(PREBUILD_TYPES),$(COMPILER_TYPES), \
+ $(RELOCATE_TYPES),$(TRACE_TYPES),$(GC_TYPES),$(JNI_TYPES),$(IMAGE_TYPES),$(PICTEST_TYPES),$(DEBUGGABLE_TYPES), 119-noimage-patchoat, \
+ $(ALL_ADDRESS_SIZES))
+
# 115-native-bridge setup is complicated. Need to implement it correctly for the target.
ART_TEST_KNOWN_BROKEN += $(call all-run-test-names,target,$(RUN_TYPES),$(PREBUILD_TYPES),$(COMPILER_TYPES), \
$(RELOCATE_TYPES),$(TRACE_TYPES),$(GC_TYPES),$(JNI_TYPES),$(IMAGE_TYPES),$(PICTEST_TYPES),$(DEBUGGABLE_TYPES), 115-native-bridge, \
diff --git a/tools/run-jdwp-tests.sh b/tools/run-jdwp-tests.sh
index f29e51f..840fffb 100755
--- a/tools/run-jdwp-tests.sh
+++ b/tools/run-jdwp-tests.sh
@@ -64,8 +64,7 @@
# with mksh.
art="bash ${OUT_DIR-out}/host/linux-x86/bin/art"
art_debugee="bash ${OUT_DIR-out}/host/linux-x86/bin/art"
- # We force generation of a new image to avoid build-time and run-time classpath differences.
- image="-Ximage:/system/non/existent/vogar.art"
+ image="-Ximage:${ANDROID_BUILD_TOP}/${OUT_DIR-out}/host/linux-x86/framework/core-jit.art"
# We do not need a device directory on host.
device_dir=""
# Vogar knows which VM to use on host.
diff --git a/tools/run-libcore-tests.sh b/tools/run-libcore-tests.sh
index 11ed8b9..3c93f19 100755
--- a/tools/run-libcore-tests.sh
+++ b/tools/run-libcore-tests.sh
@@ -79,11 +79,7 @@
vogar_args="$vogar_args --vm-arg -Ximage:/data/art-test/core-optimizing.art"
shift
elif [[ "$1" == "--mode=host" ]]; then
- # We explicitly give a wrong path for the image, to ensure vogar
- # will create a boot image with the default compiler. Note that
- # giving an existing image on host does not work because of
- # classpath/resources differences when compiling the boot image.
- vogar_args="$vogar_args --vm-arg -Ximage:/non/existent/vogar.art"
+ vogar_args="$vogar_args --vm-arg -Ximage:${ANDROID_BUILD_TOP}/${OUT_DIR-out}/host/linux-x86/framework/core-jit.art"
shift
elif [[ "$1" == "--debug" ]]; then
# Remove the --debug from the arguments.
