Prevent spurious dexopts in 32-64 builds.
When we're checking if a file needs to be dexopted, we
need to compare oat file checksums with the image checksum
for the oat file's target instruction set and not the current
runtime's target instruction set.
bug:14475807
Change-Id: Ib44d8e3c6cdf3a37fce6332c694a6602c658e925
diff --git a/runtime/class_linker.cc b/runtime/class_linker.cc
index 703229c..18afa02 100644
--- a/runtime/class_linker.cc
+++ b/runtime/class_linker.cc
@@ -803,13 +803,29 @@
bool ClassLinker::VerifyOatFileChecksums(const OatFile* oat_file,
const char* dex_location,
uint32_t dex_location_checksum,
+ const InstructionSet instruction_set,
std::string* error_msg) {
Runtime* runtime = Runtime::Current();
- const ImageHeader& image_header = runtime->GetHeap()->GetImageSpace()->GetImageHeader();
- uint32_t image_oat_checksum = image_header.GetOatChecksum();
- uintptr_t image_oat_data_begin = reinterpret_cast<uintptr_t>(image_header.GetOatDataBegin());
- bool image_check = ((oat_file->GetOatHeader().GetImageFileLocationOatChecksum() == image_oat_checksum)
- && (oat_file->GetOatHeader().GetImageFileLocationOatDataBegin() == image_oat_data_begin));
+ const gc::space::ImageSpace* image_space = runtime->GetHeap()->GetImageSpace();
+
+ // If the requested instruction set is the same as the current runtime,
+ // we can use the checksums directly. If it isn't, we'll have to read the
+ // image header from the image for the right instruction set.
+ uint32_t image_oat_checksum = 0;
+ uintptr_t image_oat_data_begin = 0;
+ if (instruction_set == kRuntimeISA) {
+ const ImageHeader& image_header = image_space->GetImageHeader();
+ image_oat_checksum = image_header.GetOatChecksum();
+ image_oat_data_begin = reinterpret_cast<uintptr_t>(image_header.GetOatDataBegin());
+ } else {
+ UniquePtr<ImageHeader> image_header(gc::space::ImageSpace::ReadImageHeaderOrDie(
+ image_space->GetImageLocation().c_str(), instruction_set));
+ image_oat_checksum = image_header->GetOatChecksum();
+ image_oat_data_begin = reinterpret_cast<uintptr_t>(image_header->GetOatDataBegin());
+ }
+ const OatHeader& oat_header = oat_file->GetOatHeader();
+ bool image_check = ((oat_header.GetImageFileLocationOatChecksum() == image_oat_checksum)
+ && (oat_header.GetImageFileLocationOatDataBegin() == image_oat_data_begin));
const OatFile::OatDexFile* oat_dex_file = oat_file->GetOatDexFile(dex_location, &dex_location_checksum);
if (oat_dex_file == NULL) {
@@ -873,7 +889,7 @@
dex_file = oat_dex_file->OpenDexFile(error_msg);
} else {
bool verified = VerifyOatFileChecksums(oat_file.get(), dex_location, dex_location_checksum,
- error_msg);
+ kRuntimeISA, error_msg);
if (!verified) {
return nullptr;
}
diff --git a/runtime/class_linker.h b/runtime/class_linker.h
index a23add0..2c6873e 100644
--- a/runtime/class_linker.h
+++ b/runtime/class_linker.h
@@ -288,6 +288,7 @@
static bool VerifyOatFileChecksums(const OatFile* oat_file,
const char* dex_location,
uint32_t dex_location_checksum,
+ const InstructionSet instruction_set,
std::string* error_msg);
// TODO: replace this with multiple methods that allocate the correct managed type.
diff --git a/runtime/gc/space/image_space.cc b/runtime/gc/space/image_space.cc
index 3de1ba4..858582e 100644
--- a/runtime/gc/space/image_space.cc
+++ b/runtime/gc/space/image_space.cc
@@ -33,15 +33,16 @@
Atomic<uint32_t> ImageSpace::bitmap_index_(0);
-ImageSpace::ImageSpace(const std::string& name, MemMap* mem_map,
- accounting::ContinuousSpaceBitmap* live_bitmap)
- : MemMapSpace(name, mem_map, mem_map->Begin(), mem_map->End(), mem_map->End(),
- kGcRetentionPolicyNeverCollect) {
+ImageSpace::ImageSpace(const std::string& image_filename, const char* image_location,
+ MemMap* mem_map, accounting::ContinuousSpaceBitmap* live_bitmap)
+ : MemMapSpace(image_filename, mem_map, mem_map->Begin(), mem_map->End(), mem_map->End(),
+ kGcRetentionPolicyNeverCollect),
+ image_location_(image_location) {
DCHECK(live_bitmap != nullptr);
live_bitmap_.reset(live_bitmap);
}
-static bool GenerateImage(const std::string& image_file_name, std::string* error_msg) {
+static bool GenerateImage(const std::string& image_filename, std::string* error_msg) {
const std::string boot_class_path_string(Runtime::Current()->GetBootClassPathString());
std::vector<std::string> boot_class_path;
Split(boot_class_path_string, ':', boot_class_path);
@@ -57,7 +58,7 @@
arg_vector.push_back(dex2oat);
std::string image_option_string("--image=");
- image_option_string += image_file_name;
+ image_option_string += image_filename;
arg_vector.push_back(image_option_string);
arg_vector.push_back("--runtime-arg");
@@ -72,7 +73,7 @@
}
std::string oat_file_option_string("--oat-file=");
- oat_file_option_string += image_file_name;
+ oat_file_option_string += image_filename;
oat_file_option_string.erase(oat_file_option_string.size() - 3);
oat_file_option_string += "oat";
arg_vector.push_back(oat_file_option_string);
@@ -98,37 +99,72 @@
return Exec(arg_vector, error_msg);
}
-ImageSpace* ImageSpace::Create(const char* original_image_file_name,
- const InstructionSet image_isa) {
- if (OS::FileExists(original_image_file_name)) {
- // If the /system file exists, it should be up-to-date, don't try to generate
- std::string error_msg;
- ImageSpace* space = ImageSpace::Init(original_image_file_name, false, &error_msg);
- if (space == nullptr) {
- LOG(FATAL) << "Failed to load image '" << original_image_file_name << "': " << error_msg;
- }
- return space;
+bool ImageSpace::FindImageFilename(const char* image_location,
+ const InstructionSet image_isa,
+ std::string* image_filename,
+ bool *is_system) {
+ if (OS::FileExists(image_location)) {
+ *image_filename = image_location;
+ *is_system = true;
+ return true;
}
- // If the /system file didn't exist, we need to use one from the dalvik-cache.
- // If the cache file exists, try to open, but if it fails, regenerate.
- // If it does not exist, generate.
+
const std::string dalvik_cache = GetDalvikCacheOrDie(GetInstructionSetString(image_isa));
- std::string image_file_name(GetDalvikCacheFilenameOrDie(original_image_file_name,
- dalvik_cache.c_str()));
+
+ // Always set output location even if it does not exist,
+ // so that the caller knows where to create the image.
+ *image_filename = GetDalvikCacheFilenameOrDie(image_location, dalvik_cache.c_str());
+ *is_system = false;
+ return OS::FileExists(image_filename->c_str());
+}
+
+ImageHeader* ImageSpace::ReadImageHeaderOrDie(const char* image_location,
+ const InstructionSet image_isa) {
+ std::string image_filename;
+ bool is_system = false;
+ if (FindImageFilename(image_location, image_isa, &image_filename, &is_system)) {
+ UniquePtr<File> image_file(OS::OpenFileForReading(image_filename.c_str()));
+ UniquePtr<ImageHeader> image_header(new ImageHeader);
+ const bool success = image_file->ReadFully(image_header.get(), sizeof(ImageHeader));
+ if (!success || !image_header->IsValid()) {
+ LOG(FATAL) << "Invalid Image header for: " << image_filename;
+ return nullptr;
+ }
+
+ return image_header.release();
+ }
+
+ LOG(FATAL) << "Unable to find image file for: " << image_location;
+ return nullptr;
+}
+
+ImageSpace* ImageSpace::Create(const char* image_location,
+ const InstructionSet image_isa) {
+ std::string image_filename;
std::string error_msg;
- if (OS::FileExists(image_file_name.c_str())) {
- space::ImageSpace* image_space = ImageSpace::Init(image_file_name.c_str(), true, &error_msg);
- if (image_space != nullptr) {
- return image_space;
+ bool is_system = false;
+ if (FindImageFilename(image_location, image_isa, &image_filename, &is_system)) {
+ ImageSpace* space = ImageSpace::Init(image_filename.c_str(), image_location,
+ !is_system, &error_msg);
+ if (space != nullptr) {
+ return space;
+ }
+
+ // If the /system file exists, it should be up-to-date, don't try to generate it.
+ // If it's not the /system file, log a warning and fall through to GenerateImage.
+ if (is_system) {
+ LOG(FATAL) << "Failed to load image '" << image_filename << "': " << error_msg;
+ return nullptr;
} else {
LOG(WARNING) << error_msg;
}
}
- CHECK(GenerateImage(image_file_name, &error_msg))
- << "Failed to generate image '" << image_file_name << "': " << error_msg;
- ImageSpace* space = ImageSpace::Init(image_file_name.c_str(), true, &error_msg);
+
+ CHECK(GenerateImage(image_filename, &error_msg))
+ << "Failed to generate image '" << image_filename << "': " << error_msg;
+ ImageSpace* space = ImageSpace::Init(image_filename.c_str(), image_location, true, &error_msg);
if (space == nullptr) {
- LOG(FATAL) << "Failed to load image '" << original_image_file_name << "': " << error_msg;
+ LOG(FATAL) << "Failed to load image '" << image_filename << "': " << error_msg;
}
return space;
}
@@ -147,25 +183,26 @@
}
}
-ImageSpace* ImageSpace::Init(const char* image_file_name, bool validate_oat_file,
- std::string* error_msg) {
- CHECK(image_file_name != nullptr);
+ImageSpace* ImageSpace::Init(const char* image_filename, const char* image_location,
+ bool validate_oat_file, std::string* error_msg) {
+ CHECK(image_filename != nullptr);
+ CHECK(image_location != nullptr);
uint64_t start_time = 0;
if (VLOG_IS_ON(heap) || VLOG_IS_ON(startup)) {
start_time = NanoTime();
- LOG(INFO) << "ImageSpace::Init entering image_file_name=" << image_file_name;
+ LOG(INFO) << "ImageSpace::Init entering image_filename=" << image_filename;
}
- UniquePtr<File> file(OS::OpenFileForReading(image_file_name));
+ UniquePtr<File> file(OS::OpenFileForReading(image_filename));
if (file.get() == NULL) {
- *error_msg = StringPrintf("Failed to open '%s'", image_file_name);
+ *error_msg = StringPrintf("Failed to open '%s'", image_filename);
return nullptr;
}
ImageHeader image_header;
bool success = file->ReadFully(&image_header, sizeof(image_header));
if (!success || !image_header.IsValid()) {
- *error_msg = StringPrintf("Invalid image header in '%s'", image_file_name);
+ *error_msg = StringPrintf("Invalid image header in '%s'", image_filename);
return nullptr;
}
@@ -177,7 +214,7 @@
file->Fd(),
0,
false,
- image_file_name,
+ image_filename,
error_msg));
if (map.get() == NULL) {
DCHECK(!error_msg->empty());
@@ -190,14 +227,14 @@
PROT_READ, MAP_PRIVATE,
file->Fd(), image_header.GetBitmapOffset(),
false,
- image_file_name,
+ image_filename,
error_msg));
if (image_map.get() == nullptr) {
*error_msg = StringPrintf("Failed to map image bitmap: %s", error_msg->c_str());
return nullptr;
}
uint32_t bitmap_index = bitmap_index_.FetchAndAdd(1);
- std::string bitmap_name(StringPrintf("imagespace %s live-bitmap %u", image_file_name,
+ std::string bitmap_name(StringPrintf("imagespace %s live-bitmap %u", image_filename,
bitmap_index));
UniquePtr<accounting::ContinuousSpaceBitmap> bitmap(
accounting::ContinuousSpaceBitmap::CreateFromMemMap(bitmap_name, image_map.release(),
@@ -223,12 +260,13 @@
callee_save_method = image_header.GetImageRoot(ImageHeader::kRefsAndArgsSaveMethod);
runtime->SetCalleeSaveMethod(down_cast<mirror::ArtMethod*>(callee_save_method), Runtime::kRefsAndArgs);
- UniquePtr<ImageSpace> space(new ImageSpace(image_file_name, map.release(), bitmap.release()));
+ UniquePtr<ImageSpace> space(new ImageSpace(image_filename, image_location,
+ map.release(), bitmap.release()));
if (kIsDebugBuild) {
space->VerifyImageAllocations();
}
- space->oat_file_.reset(space->OpenOatFile(image_file_name, error_msg));
+ space->oat_file_.reset(space->OpenOatFile(image_filename, error_msg));
if (space->oat_file_.get() == nullptr) {
DCHECK(!error_msg->empty());
return nullptr;
diff --git a/runtime/gc/space/image_space.h b/runtime/gc/space/image_space.h
index 1652ec9..622371f 100644
--- a/runtime/gc/space/image_space.h
+++ b/runtime/gc/space/image_space.h
@@ -46,6 +46,11 @@
static ImageSpace* Create(const char* image, const InstructionSet image_isa)
SHARED_LOCKS_REQUIRED(Locks::mutator_lock_);
+ // Reads the image header from the specified image location for the
+ // instruction set image_isa.
+ static ImageHeader* ReadImageHeaderOrDie(const char* image_location,
+ const InstructionSet image_isa);
+
// Releases the OatFile from the ImageSpace so it can be transfer to
// the caller, presumably the ClassLinker.
OatFile* ReleaseOatFile()
@@ -58,10 +63,18 @@
return *reinterpret_cast<ImageHeader*>(Begin());
}
+ // Actual filename where image was loaded from.
+ // For example: /data/dalvik-cache/arm/system@framework@boot.art
const std::string GetImageFilename() const {
return GetName();
}
+ // Symbolic location for image.
+ // For example: /system/framework/boot.art
+ const std::string GetImageLocation() const {
+ return image_location_;
+ }
+
accounting::ContinuousSpaceBitmap* GetLiveBitmap() const OVERRIDE {
return live_bitmap_.get();
}
@@ -90,9 +103,21 @@
// image's OatFile is up-to-date relative to its DexFile
// inputs. Otherwise (for /data), validate the inputs and generate
// the OatFile in /data/dalvik-cache if necessary.
- static ImageSpace* Init(const char* image, bool validate_oat_file, std::string* error_msg)
+ static ImageSpace* Init(const char* image_filename, const char* image_location,
+ bool validate_oat_file, std::string* error_msg)
SHARED_LOCKS_REQUIRED(Locks::mutator_lock_);
+ // Returns the filename of the image corresponding to
+ // requested image_location, or the filename where a new image
+ // should be written if one doesn't exist. Looks for a generated
+ // image in the specified location and then in the dalvik-cache.
+ //
+ // Returns true if an image was found, false otherwise.
+ static bool FindImageFilename(const char* image_location,
+ const InstructionSet image_isa,
+ std::string* location,
+ bool* is_system);
+
OatFile* OpenOatFile(const char* image, std::string* error_msg) const
SHARED_LOCKS_REQUIRED(Locks::mutator_lock_);
@@ -105,14 +130,16 @@
UniquePtr<accounting::ContinuousSpaceBitmap> live_bitmap_;
- ImageSpace(const std::string& name, MemMap* mem_map,
- accounting::ContinuousSpaceBitmap* live_bitmap);
+ ImageSpace(const std::string& name, const char* image_location,
+ MemMap* mem_map, accounting::ContinuousSpaceBitmap* live_bitmap);
// The OatFile associated with the image during early startup to
// reserve space contiguous to the image. It is later released to
// the ClassLinker during it's initialization.
UniquePtr<OatFile> oat_file_;
+ const std::string image_location_;
+
DISALLOW_COPY_AND_ASSIGN(ImageSpace);
};
diff --git a/runtime/native/dalvik_system_DexFile.cc b/runtime/native/dalvik_system_DexFile.cc
index d9c1309..ed1ee7a 100644
--- a/runtime/native/dalvik_system_DexFile.cc
+++ b/runtime/native/dalvik_system_DexFile.cc
@@ -377,6 +377,8 @@
}
}
+ const InstructionSet target_instruction_set = GetInstructionSetFromString(instruction_set);
+
// Check if we have an odex file next to the dex file.
std::string odex_filename(OatFile::DexFilenameToOdexFilename(filename));
std::string error_msg;
@@ -403,6 +405,7 @@
return JNI_FALSE;
}
if (ClassLinker::VerifyOatFileChecksums(oat_file.get(), filename, location_checksum,
+ target_instruction_set,
&error_msg)) {
if (kVerboseLogging) {
LOG(INFO) << "DexFile_isDexOptNeeded precompiled file " << odex_filename
@@ -433,33 +436,6 @@
return JNI_TRUE;
}
- for (const auto& space : runtime->GetHeap()->GetContinuousSpaces()) {
- if (space->IsImageSpace()) {
- // TODO: Ensure this works with multiple image spaces.
- const ImageHeader& image_header = space->AsImageSpace()->GetImageHeader();
- if (oat_file->GetOatHeader().GetImageFileLocationOatChecksum() !=
- image_header.GetOatChecksum()) {
- if (kReasonLogging) {
- ScopedObjectAccess soa(env);
- LOG(INFO) << "DexFile_isDexOptNeeded cache file " << cache_location
- << " has out-of-date oat checksum compared to "
- << oat_file->GetLocation();
- }
- return JNI_TRUE;
- }
- if (oat_file->GetOatHeader().GetImageFileLocationOatDataBegin()
- != reinterpret_cast<uintptr_t>(image_header.GetOatDataBegin())) {
- if (kReasonLogging) {
- ScopedObjectAccess soa(env);
- LOG(INFO) << "DexFile_isDexOptNeeded cache file " << cache_location
- << " has out-of-date oat begin compared to "
- << oat_file->GetLocation();
- }
- return JNI_TRUE;
- }
- }
- }
-
uint32_t location_checksum;
if (!DexFile::GetChecksum(filename, &location_checksum, &error_msg)) {
if (kReasonLogging) {
@@ -470,7 +446,7 @@
}
if (!ClassLinker::VerifyOatFileChecksums(oat_file.get(), filename, location_checksum,
- &error_msg)) {
+ target_instruction_set, &error_msg)) {
if (kReasonLogging) {
LOG(INFO) << "DexFile_isDexOptNeeded cache file " << cache_location
<< " has out-of-date checksum compared to " << filename