ART: Fix up some multi-image cases

Change the auto-generated multi-image names to include the path
components from the first image, as well as prefix them with the
first image's name to disambiguate. This fixes vogar-style usage.

Fix an out-of-bounds issue in dex2oat when dex files are missing.

Forbid generating or patching multi-image parts when loading images.
Instead just fail loading them.

Remember ImageSpace instances that have been added while trying to
load a multi-image set. Remove all loaded instances when the overall
loading process fails.

Refactor the dex location adaptation into ImageSpace. Reuse the code
in the Runtime path for fallback, so that all dex files can be found
correctly.

Fix an out-of-bounds access in OatFileAssistant in fallback mode.

Partially reverts d895961d07a1d320b29f2045a48bc5a1944a4d3c. Push an
actual image name, that is, something with an art extension, to
the vogar scripts.

Partially reverts c525604b313bb77a2077e1fec43dfab76cb1b9b1. Test
119-noimage-patchoat works again.

Bug: 26317072
Bug: 26320300
Change-Id: I3f05fa77f22a2b9ca54c3105ffc53646c1928604
diff --git a/dex2oat/dex2oat.cc b/dex2oat/dex2oat.cc
index bb4224b..50480d9 100644
--- a/dex2oat/dex2oat.cc
+++ b/dex2oat/dex2oat.cc
@@ -854,6 +854,11 @@
       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];
@@ -861,9 +866,24 @@
       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.
@@ -882,14 +902,15 @@
           infix = dex_file.substr(strlen("core"));
         }
       }
-      // Use a counted loop to skip the first one.
+
+      // Now create the other names. 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], infix, ".art");
+        std::string image_name = CreateMultiImageName(dex_locations_[i], prefix, 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], infix, ".oat");
+        std::string oat_name = CreateMultiImageName(dex_locations_[i], prefix, infix, ".oat");
         char_backing_storage_.push_back(base_oat + oat_name);
         oat_filenames_.push_back((char_backing_storage_.end() - 1)->c_str());
       }
@@ -904,13 +925,23 @@
     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* suffix) {
+                                          const char* replace_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('.');
@@ -919,7 +950,8 @@
       }
     }
     if (EndsWith(in, ".jar")) {
-      in = in.substr(0, in.length() - strlen(".jar")) + (suffix != nullptr ? suffix : "");
+      in = in.substr(0, in.length() - strlen(".jar")) +
+          (replace_suffix != nullptr ? replace_suffix : "");
     }
     return in;
   }
@@ -1284,16 +1316,21 @@
 
   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 = { 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]);
+        std::vector<const DexFile*> dex_files;
+        if (index < dex_files_.size()) {
           dex_files.push_back(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++;
+          }
         }
         dex_files_per_oat_file_.push_back(std::move(dex_files));
       }
@@ -1384,8 +1421,9 @@
 
     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. 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, but
+      // need to strip all path components (they will be re-established when loading).
+      // 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) {
@@ -1396,14 +1434,24 @@
         std::string dex_loc = dex_locations_[i];
         std::string image_filename = image_filenames_[i];
 
-        // Use the dex_loc path, but the image_filename name.
+        // Use the dex_loc path, but the image_filename name (without path elements).
         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_slash + 1);
+          dex_loc = image_filename.substr(image_last_sep + 1);
         } else {
           dex_loc = dex_loc.substr(0, dex_last_slash + 1) +
-              image_filename.substr(image_last_slash + 1);
+              image_filename.substr(image_last_sep + 1);
         }
 
         // Image filenames already end with .art, no need to replace.
diff --git a/runtime/gc/heap.cc b/runtime/gc/heap.cc
index 3e432c7..7f67ae4 100644
--- a/runtime/gc/heap.cc
+++ b/runtime/gc/heap.cc
@@ -265,6 +265,9 @@
     // 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];
@@ -277,6 +280,7 @@
       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();
@@ -298,66 +302,18 @@
             continue;
           }
 
-          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);
-          }
+          space::ImageSpace::CreateMultiImageLocations(image_file_name,
+                                                       boot_classpath,
+                                                       &image_file_names);
         }
       } 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;
-        // TODO: Remove already loaded spaces.
+        // Remove already loaded spaces.
+        for (space::Space* loaded_space : added_image_spaces) {
+          RemoveSpace(loaded_space);
+        }
         break;
       }
     }
diff --git a/runtime/gc/space/image_space.cc b/runtime/gc/space/image_space.cc
index 952759c..dfdbd04 100644
--- a/runtime/gc/space/image_space.cc
+++ b/runtime/gc/space/image_space.cc
@@ -523,6 +523,9 @@
           } 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);
@@ -615,6 +618,9 @@
     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());
@@ -969,6 +975,67 @@
       << ",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 a54358a..b8ae4a0 100644
--- a/runtime/gc/space/image_space.h
+++ b/runtime/gc/space/image_space.h
@@ -122,6 +122,12 @@
                                 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 cb80cc9..8543ff4 100644
--- a/runtime/oat_file_assistant.cc
+++ b/runtime/oat_file_assistant.cc
@@ -846,7 +846,12 @@
 
 std::string OatFileAssistant::ImageLocation() {
   Runtime* runtime = Runtime::Current();
-  return runtime->GetHeap()->GetBootImageSpaces()[0]->GetImageLocation();
+  const std::vector<gc::space::ImageSpace*>& image_spaces =
+      runtime->GetHeap()->GetBootImageSpaces();
+  if (image_spaces.empty()) {
+    return "";
+  }
+  return image_spaces[0]->GetImageLocation();
 }
 
 const uint32_t* OatFileAssistant::GetRequiredDexChecksum() {
diff --git a/runtime/runtime.cc b/runtime/runtime.cc
index 98aa8d8..5c72629 100644
--- a/runtime/runtime.cc
+++ b/runtime/runtime.cc
@@ -827,13 +827,9 @@
       const OatHeader& boot_oat_header = oat_file->GetOatHeader();
       const char* boot_cp = boot_oat_header.GetStoreValueByKey(OatHeader::kBootClassPath);
       if (boot_cp != nullptr) {
-        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());
-        }
+        gc::space::ImageSpace::CreateMultiImageLocations(image_locations[0],
+                                                         boot_cp,
+                                                         &image_locations);
       }
     }
 
diff --git a/test/Android.run-test.mk b/test/Android.run-test.mk
index a3aeb6d..81cfb70 100644
--- a/test/Android.run-test.mk
+++ b/test/Android.run-test.mk
@@ -337,11 +337,6 @@
 
 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 840fffb..f29e51f 100755
--- a/tools/run-jdwp-tests.sh
+++ b/tools/run-jdwp-tests.sh
@@ -64,7 +64,8 @@
     # with mksh.
     art="bash ${OUT_DIR-out}/host/linux-x86/bin/art"
     art_debugee="bash ${OUT_DIR-out}/host/linux-x86/bin/art"
-    image="-Ximage:${ANDROID_BUILD_TOP}/${OUT_DIR-out}/host/linux-x86/framework/core-jit.art"
+    # We force generation of a new image to avoid build-time and run-time classpath differences.
+    image="-Ximage:/system/non/existent/vogar.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 3c93f19..11ed8b9 100755
--- a/tools/run-libcore-tests.sh
+++ b/tools/run-libcore-tests.sh
@@ -79,7 +79,11 @@
     vogar_args="$vogar_args --vm-arg -Ximage:/data/art-test/core-optimizing.art"
     shift
   elif [[ "$1" == "--mode=host" ]]; then
-    vogar_args="$vogar_args --vm-arg -Ximage:${ANDROID_BUILD_TOP}/${OUT_DIR-out}/host/linux-x86/framework/core-jit.art"
+    # 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"
     shift
   elif [[ "$1" == "--debug" ]]; then
     # Remove the --debug from the arguments.