Merge "Improved side-effects/can-throw information on intrinsics."
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/imgdiag/imgdiag.cc b/imgdiag/imgdiag.cc
index b8a72af..93a0974 100644
--- a/imgdiag/imgdiag.cc
+++ b/imgdiag/imgdiag.cc
@@ -49,7 +49,7 @@
public:
explicit ImgDiagDumper(std::ostream* os,
const ImageHeader& image_header,
- const char* image_location,
+ const std::string& image_location,
pid_t image_diff_pid)
: os_(os),
image_header_(image_header),
@@ -163,7 +163,7 @@
std::string error_msg;
// Walk the bytes and diff against our boot image
- const ImageHeader& boot_image_header = GetBootImageHeader();
+ const ImageHeader& boot_image_header = image_header_;
os << "\nObserving boot image header at address "
<< reinterpret_cast<const void*>(&boot_image_header)
@@ -812,14 +812,6 @@
return page_frame_number != page_frame_number_clean;
}
- static const ImageHeader& GetBootImageHeader() {
- gc::Heap* heap = Runtime::Current()->GetHeap();
- std::vector<gc::space::ImageSpace*> image_spaces = heap->GetBootImageSpaces();
- CHECK(!image_spaces.empty());
- const ImageHeader& image_header = image_spaces[0]->GetImageHeader();
- return image_header;
- }
-
private:
// Return the image location, stripped of any directories, e.g. "boot.art" or "core.art"
std::string GetImageLocationBaseName() const {
@@ -828,7 +820,7 @@
std::ostream* os_;
const ImageHeader& image_header_;
- const char* image_location_;
+ const std::string image_location_;
pid_t image_diff_pid_; // Dump image diff against boot.art if pid is non-negative
DISALLOW_COPY_AND_ASSIGN(ImgDiagDumper);
@@ -847,7 +839,7 @@
}
ImgDiagDumper img_diag_dumper(
- os, image_header, image_space->GetImageLocation().c_str(), image_diff_pid);
+ os, image_header, image_space->GetImageLocation(), image_diff_pid);
if (!img_diag_dumper.Dump()) {
return EXIT_FAILURE;
}
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.