Merge "Revert "ART: Fix up some multi-image cases""

9 files changed, 87 insertions, 165 deletions

diff --git a/dex2oat/dex2oat.cc b/dex2oat/dex2oat.cc
index 50480d9043..bb4224b7c1 100644
--- a/dex2oat/dex2oat.cc
+++ b/dex2oat/dex2oat.cc
@@ -854,11 +854,6 @@ class Dex2Oat FINAL {
       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 @@ class Dex2Oat FINAL {
       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 @@ class Dex2Oat FINAL {
           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 @@ class Dex2Oat FINAL {
     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 @@ class Dex2Oat FINAL {
       }
     }
     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 @@ class Dex2Oat FINAL {
 
   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 @@ class Dex2Oat FINAL {
 
     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 @@ class Dex2Oat FINAL {
         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 7f67ae4f08..3e432c7c9b 100644
--- a/runtime/gc/heap.cc
+++ b/runtime/gc/heap.cc
@@ -265,9 +265,6 @@ Heap::Heap(size_t initial_size,
     // 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 @@ Heap::Heap(size_t initial_size,
       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 @@ Heap::Heap(size_t initial_size,
             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 dfdbd04427..952759ca7d 100644
--- a/runtime/gc/space/image_space.cc
+++ b/runtime/gc/space/image_space.cc
@@ -523,9 +523,6 @@ ImageSpace* ImageSpace::Create(const char* image_location,
           } 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 @@ ImageSpace* ImageSpace::Create(const char* image_location,
     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 @@ void ImageSpace::Dump(std::ostream& os) const {
       << ",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 b8ae4a033a..a54358a7eb 100644
--- a/runtime/gc/space/image_space.h
+++ b/runtime/gc/space/image_space.h
@@ -122,12 +122,6 @@ class ImageSpace : public MemMapSpace {
                                 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 8543ff4ef9..cb80cc91a4 100644
--- a/runtime/oat_file_assistant.cc
+++ b/runtime/oat_file_assistant.cc
@@ -846,12 +846,7 @@ std::string OatFileAssistant::OldProfileFileName() {
 
 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 5c72629320..98aa8d8c03 100644
--- a/runtime/runtime.cc
+++ b/runtime/runtime.cc
@@ -827,9 +827,13 @@ static bool OpenDexFilesFromImage(const std::string& image_location,
       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 81cfb7003a..a3aeb6d9d5 100644
--- a/test/Android.run-test.mk
+++ b/test/Android.run-test.mk
@@ -337,6 +337,11 @@ endif
 
 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 f29e51f047..840fffbd20 100755
--- a/tools/run-jdwp-tests.sh
+++ b/tools/run-jdwp-tests.sh
@@ -64,8 +64,7 @@ while true; do
     # 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 11ed8b94be..3c93f196d0 100755
--- a/tools/run-libcore-tests.sh
+++ b/tools/run-libcore-tests.sh
@@ -79,11 +79,7 @@ while true; do
     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.