Tweaks to patchoat and other related things.
Removed some flags from patchoat that were poorly specified and fixed
some other issues with the relocation system.
Bug: 15358152
Change-Id: Ia6d47b1a008f02373307d833ba45f00ea408d76f
diff --git a/dex2oat/dex2oat.cc b/dex2oat/dex2oat.cc
index d6501a1..a78d3f7 100644
--- a/dex2oat/dex2oat.cc
+++ b/dex2oat/dex2oat.cc
@@ -849,7 +849,6 @@
bool dump_timing = false;
bool dump_passes = false;
bool include_patch_information = CompilerOptions::kDefaultIncludePatchInformation;
- bool explicit_include_patch_information = false;
bool include_debug_symbols = kIsDebugBuild;
bool dump_slow_timing = kIsDebugBuild;
bool watch_dog_enabled = !kIsTargetBuild;
@@ -1037,10 +1036,8 @@
PassDriverMEOpts::SetDumpPassList(dump_passes);
} else if (option == "--include-patch-information") {
include_patch_information = true;
- explicit_include_patch_information = true;
} else if (option == "--no-include-patch-information") {
include_patch_information = false;
- explicit_include_patch_information = true;
} else {
Usage("Unknown argument %s", option.data());
}
@@ -1167,11 +1164,6 @@
Usage("Unknown --compiler-filter value %s", compiler_filter_string);
}
- if (!explicit_include_patch_information) {
- include_patch_information =
- (compiler_kind == Compiler::kQuick && CompilerOptions::kDefaultIncludePatchInformation);
- }
-
// Set the compilation target's implicit checks options.
switch (instruction_set) {
case kArm:
diff --git a/patchoat/patchoat.cc b/patchoat/patchoat.cc
index 4c88c9e..72189c3 100644
--- a/patchoat/patchoat.cc
+++ b/patchoat/patchoat.cc
@@ -66,6 +66,47 @@
}
}
+static bool LocationToFilename(const std::string& location, InstructionSet isa,
+ std::string* filename) {
+ bool has_system = false;
+ bool has_cache = false;
+ // image_location = /system/framework/boot.art
+ // system_image_location = /system/framework/<image_isa>/boot.art
+ std::string system_filename(GetSystemImageFilename(location.c_str(), isa));
+ if (OS::FileExists(system_filename.c_str())) {
+ has_system = true;
+ }
+
+ bool have_android_data = false;
+ bool dalvik_cache_exists = false;
+ std::string dalvik_cache;
+ GetDalvikCache(GetInstructionSetString(isa), false, &dalvik_cache,
+ &have_android_data, &dalvik_cache_exists);
+
+ std::string cache_filename;
+ if (have_android_data && dalvik_cache_exists) {
+ // Always set output location even if it does not exist,
+ // so that the caller knows where to create the image.
+ //
+ // image_location = /system/framework/boot.art
+ // *image_filename = /data/dalvik-cache/<image_isa>/boot.art
+ std::string error_msg;
+ if (GetDalvikCacheFilename(location.c_str(), dalvik_cache.c_str(),
+ &cache_filename, &error_msg)) {
+ has_cache = true;
+ }
+ }
+ if (has_system) {
+ *filename = system_filename;
+ return true;
+ } else if (has_cache) {
+ *filename = cache_filename;
+ return true;
+ } else {
+ return false;
+ }
+}
+
bool PatchOat::Patch(const std::string& image_location, off_t delta,
File* output_image, InstructionSet isa,
TimingLogger* timings) {
@@ -77,10 +118,15 @@
TimingLogger::ScopedTiming t("Runtime Setup", timings);
const char *isa_name = GetInstructionSetString(isa);
- std::string image_filename(GetSystemImageFilename(image_location.c_str(), isa));
+ std::string image_filename;
+ if (!LocationToFilename(image_location, isa, &image_filename)) {
+ LOG(ERROR) << "Unable to find image at location " << image_location;
+ return false;
+ }
std::unique_ptr<File> input_image(OS::OpenFileForReading(image_filename.c_str()));
if (input_image.get() == nullptr) {
- LOG(ERROR) << "unable to open input image file.";
+ LOG(ERROR) << "unable to open input image file at " << image_filename
+ << " for location " << image_location;
return false;
}
int64_t image_len = input_image->GetLength();
@@ -162,10 +208,15 @@
isa = ElfISAToInstructionSet(elf_hdr.e_machine);
}
const char* isa_name = GetInstructionSetString(isa);
- std::string image_filename(GetSystemImageFilename(image_location.c_str(), isa));
+ std::string image_filename;
+ if (!LocationToFilename(image_location, isa, &image_filename)) {
+ LOG(ERROR) << "Unable to find image at location " << image_location;
+ return false;
+ }
std::unique_ptr<File> input_image(OS::OpenFileForReading(image_filename.c_str()));
if (input_image.get() == nullptr) {
- LOG(ERROR) << "unable to open input image file.";
+ LOG(ERROR) << "unable to open input image file at " << image_filename
+ << " for location " << image_location;
return false;
}
int64_t image_len = input_image->GetLength();
@@ -239,11 +290,6 @@
bool PatchOat::WriteElf(File* out) {
TimingLogger::ScopedTiming t("Writing Elf File", timings_);
- std::string error_msg;
-
- // Lock the output file.
- ScopedFlock flock;
- flock.Init(out, &error_msg);
CHECK(oat_file_.get() != nullptr);
CHECK(out != nullptr);
@@ -261,9 +307,8 @@
TimingLogger::ScopedTiming t("Writing image File", timings_);
std::string error_msg;
- // Lock the output file.
- ScopedFlock flock;
- flock.Init(out, &error_msg);
+ ScopedFlock img_flock;
+ img_flock.Init(out, &error_msg);
CHECK(image_ != nullptr);
CHECK(out != nullptr);
@@ -625,9 +670,6 @@
UsageError(" --output-oat-file=<file.oat>: Specifies the exact file to write the patched oat");
UsageError(" file to.");
UsageError("");
- UsageError(" --output-oat-location=<file.oat>: Specifies the 'location' to write the patched");
- UsageError(" oat file to. If used one must also specify the --instruction-set");
- UsageError("");
UsageError(" --output-oat-fd=<file-descriptor>: Specifies the file-descriptor to write the");
UsageError(" the patched oat file to.");
UsageError("");
@@ -637,9 +679,6 @@
UsageError(" --output-image-fd=<file-descriptor>: Specifies the file-descriptor to write the");
UsageError(" the patched image file to.");
UsageError("");
- UsageError(" --output-image-location=<file.art>: Specifies the 'location' to write the patched");
- UsageError(" image file to. If used one must also specify the --instruction-set");
- UsageError("");
UsageError(" --orig-base-offset=<original-base-offset>: Specify the base offset the input file");
UsageError(" was compiled with. This is needed if one is specifying a --base-offset");
UsageError("");
@@ -657,6 +696,10 @@
UsageError(" --instruction-set flag. It will search for this image in the same way that");
UsageError(" is done when loading one.");
UsageError("");
+ UsageError(" --lock-output: Obtain a flock on output oat file before starting.");
+ UsageError("");
+ UsageError(" --no-lock-output: Do not attempt to obtain a flock on output oat file.");
+ UsageError("");
UsageError(" --dump-timings: dump out patch timing information");
UsageError("");
UsageError(" --no-dump-timings: do not dump out patch timing information");
@@ -699,7 +742,15 @@
return OS::OpenFileReadWrite(name);
} else {
*created = true;
- return OS::CreateEmptyFile(name);
+ std::unique_ptr<File> f(OS::CreateEmptyFile(name));
+ if (f.get() != nullptr) {
+ if (fchmod(f->Fd(), 0644) != 0) {
+ PLOG(ERROR) << "Unable to make " << name << " world readable";
+ unlink(name);
+ return nullptr;
+ }
+ }
+ return f.release();
}
}
@@ -731,11 +782,9 @@
bool have_input_oat = false;
std::string input_image_location;
std::string output_oat_filename;
- std::string output_oat_location;
int output_oat_fd = -1;
bool have_output_oat = false;
std::string output_image_filename;
- std::string output_image_location;
int output_image_fd = -1;
bool have_output_image = false;
uintptr_t base_offset = 0;
@@ -747,6 +796,7 @@
std::string patched_image_filename;
std::string patched_image_location;
bool dump_timings = kIsDebugBuild;
+ bool lock_output = true;
for (int i = 0; i < argc; i++) {
const StringPiece option(argv[i]);
@@ -797,24 +847,15 @@
}
} else if (option.starts_with("--input-image-location=")) {
input_image_location = option.substr(strlen("--input-image-location=")).data();
- } else if (option.starts_with("--output-oat-location=")) {
- if (have_output_oat) {
- Usage("Only one of --output-oat-file, --output-oat-location and --output-oat-fd may "
- "be used.");
- }
- have_output_oat = true;
- output_oat_location = option.substr(strlen("--output-oat-location=")).data();
} else if (option.starts_with("--output-oat-file=")) {
if (have_output_oat) {
- Usage("Only one of --output-oat-file, --output-oat-location and --output-oat-fd may "
- "be used.");
+ Usage("Only one of --output-oat-file, and --output-oat-fd may be used.");
}
have_output_oat = true;
output_oat_filename = option.substr(strlen("--output-oat-file=")).data();
} else if (option.starts_with("--output-oat-fd=")) {
if (have_output_oat) {
- Usage("Only one of --output-oat-file, --output-oat-location and --output-oat-fd may "
- "be used.");
+ Usage("Only one of --output-oat-file, --output-oat-fd may be used.");
}
have_output_oat = true;
const char* oat_fd_str = option.substr(strlen("--output-oat-fd=")).data();
@@ -824,24 +865,15 @@
if (output_oat_fd < 0) {
Usage("--output-oat-fd pass a negative value %d", output_oat_fd);
}
- } else if (option.starts_with("--output-image-location=")) {
- if (have_output_image) {
- Usage("Only one of --output-image-file, --output-image-location and --output-image-fd may "
- "be used.");
- }
- have_output_image = true;
- output_image_location= option.substr(strlen("--output-image-location=")).data();
} else if (option.starts_with("--output-image-file=")) {
if (have_output_image) {
- Usage("Only one of --output-image-file, --output-image-location and --output-image-fd may "
- "be used.");
+ Usage("Only one of --output-image-file, and --output-image-fd may be used.");
}
have_output_image = true;
output_image_filename = option.substr(strlen("--output-image-file=")).data();
} else if (option.starts_with("--output-image-fd=")) {
if (have_output_image) {
- Usage("Only one of --output-image-file, --output-image-location and --output-image-fd "
- "may be used.");
+ Usage("Only one of --output-image-file, and --output-image-fd may be used.");
}
have_output_image = true;
const char* image_fd_str = option.substr(strlen("--output-image-fd=")).data();
@@ -874,6 +906,10 @@
patched_image_location = option.substr(strlen("--patched-image-location=")).data();
} else if (option.starts_with("--patched-image-file=")) {
patched_image_filename = option.substr(strlen("--patched-image-file=")).data();
+ } else if (option == "--lock-output") {
+ lock_output = true;
+ } else if (option == "--no-lock-output") {
+ lock_output = false;
} else if (option == "--dump-timings") {
dump_timings = true;
} else if (option == "--no-dump-timings") {
@@ -923,29 +959,13 @@
if (!isa_set) {
Usage("specifying a location requires specifying an instruction set");
}
- input_oat_filename = GetSystemImageFilename(input_oat_location.c_str(), isa);
+ if (!LocationToFilename(input_oat_location, isa, &input_oat_filename)) {
+ Usage("Unable to find filename for input oat location %s", input_oat_location.c_str());
+ }
if (debug) {
LOG(INFO) << "Using input-oat-file " << input_oat_filename;
}
}
- if (!output_oat_location.empty()) {
- if (!isa_set) {
- Usage("specifying a location requires specifying an instruction set");
- }
- output_oat_filename = GetSystemImageFilename(output_oat_location.c_str(), isa);
- if (debug) {
- LOG(INFO) << "Using output-oat-file " << output_oat_filename;
- }
- }
- if (!output_image_location.empty()) {
- if (!isa_set) {
- Usage("specifying a location requires specifying an instruction set");
- }
- output_image_filename = GetSystemImageFilename(output_image_location.c_str(), isa);
- if (debug) {
- LOG(INFO) << "Using output-image-file " << output_image_filename;
- }
- }
if (!patched_image_location.empty()) {
if (!isa_set) {
Usage("specifying a location requires specifying an instruction set");
@@ -1009,6 +1029,9 @@
CHECK(!input_image_location.empty());
if (output_image_fd != -1) {
+ if (output_image_filename.empty()) {
+ output_image_filename = "output-image-file";
+ }
output_image.reset(new File(output_image_fd, output_image_filename));
} else {
CHECK(!output_image_filename.empty());
@@ -1020,6 +1043,9 @@
if (have_oat_files) {
if (input_oat_fd != -1) {
+ if (input_oat_filename.empty()) {
+ input_oat_filename = "input-oat-file";
+ }
input_oat.reset(new File(input_oat_fd, input_oat_filename));
} else {
CHECK(!input_oat_filename.empty());
@@ -1030,15 +1056,10 @@
}
if (output_oat_fd != -1) {
- output_oat.reset(new File(output_oat_fd, output_oat_filename));
- } else if (output_oat_filename == input_oat_filename) {
- // This could be a wierd situation, since we'd be writting from an mmap'd copy of this file.
- // Lets just unlink it.
- if (0 != unlink(input_oat_filename.c_str())) {
- PLOG(ERROR) << "Could not unlink " << input_oat_filename << " to make room for output";
- return false;
+ if (output_oat_filename.empty()) {
+ output_oat_filename = "output-oat-file";
}
- output_oat.reset(OS::CreateEmptyFile(output_oat_filename.c_str()));
+ output_oat.reset(new File(output_oat_fd, output_oat_filename));
} else {
CHECK(!output_oat_filename.empty());
output_oat.reset(CreateOrOpen(output_oat_filename.c_str(), &new_oat_out));
@@ -1063,6 +1084,22 @@
}
};
+ if ((have_oat_files && (input_oat.get() == nullptr || output_oat.get() == nullptr)) ||
+ (have_image_files && output_image.get() == nullptr)) {
+ cleanup(false);
+ return EXIT_FAILURE;
+ }
+
+ ScopedFlock output_oat_lock;
+ if (lock_output) {
+ std::string error_msg;
+ if (have_oat_files && !output_oat_lock.Init(output_oat.get(), &error_msg)) {
+ LOG(ERROR) << "Unable to lock output oat " << output_image->GetPath() << ": " << error_msg;
+ cleanup(false);
+ return EXIT_FAILURE;
+ }
+ }
+
if (debug) {
LOG(INFO) << "moving offset by " << base_delta
<< " (0x" << std::hex << base_delta << ") bytes or "
@@ -1083,7 +1120,6 @@
ret = PatchOat::Patch(input_image_location, base_delta, output_image.get(), isa, &timings);
}
cleanup(ret);
- sync();
return (ret) ? EXIT_SUCCESS : EXIT_FAILURE;
}
diff --git a/runtime/gc/space/image_space.cc b/runtime/gc/space/image_space.cc
index 7a3cbf3..883e1c1f 100644
--- a/runtime/gc/space/image_space.cc
+++ b/runtime/gc/space/image_space.cc
@@ -45,6 +45,26 @@
live_bitmap_.reset(live_bitmap);
}
+static int32_t ChooseRelocationOffsetDelta(int32_t min_delta, int32_t max_delta) {
+ CHECK_ALIGNED(min_delta, kPageSize);
+ CHECK_ALIGNED(max_delta, kPageSize);
+ CHECK_LT(min_delta, max_delta);
+
+ std::default_random_engine generator;
+ generator.seed(NanoTime() * getpid());
+ std::uniform_int_distribution<int32_t> distribution(min_delta, max_delta);
+ int32_t r = distribution(generator);
+ if (r % 2 == 0) {
+ r = RoundUp(r, kPageSize);
+ } else {
+ r = RoundDown(r, kPageSize);
+ }
+ CHECK_LE(min_delta, r);
+ CHECK_GE(max_delta, r);
+ CHECK_ALIGNED(r, kPageSize);
+ return r;
+}
+
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;
@@ -75,7 +95,11 @@
Runtime::Current()->AddCurrentRuntimeFeaturesAsDex2OatArguments(&arg_vector);
- arg_vector.push_back(StringPrintf("--base=0x%x", ART_BASE_ADDRESS));
+ int32_t base_offset = ChooseRelocationOffsetDelta(ART_BASE_ADDRESS_MIN_DELTA,
+ ART_BASE_ADDRESS_MAX_DELTA);
+ LOG(INFO) << "Using an offset of 0x" << std::hex << base_offset << " from default "
+ << "art base address of 0x" << std::hex << ART_BASE_ADDRESS;
+ arg_vector.push_back(StringPrintf("--base=0x%x", ART_BASE_ADDRESS + base_offset));
if (kIsTargetBuild) {
arg_vector.push_back("--image-classes-zip=/system/framework/framework.jar");
@@ -145,26 +169,6 @@
return true;
}
-static int32_t ChooseRelocationOffsetDelta(int32_t min_delta, int32_t max_delta) {
- CHECK_ALIGNED(min_delta, kPageSize);
- CHECK_ALIGNED(max_delta, kPageSize);
- CHECK_LT(min_delta, max_delta);
-
- std::default_random_engine generator;
- generator.seed(NanoTime() * getpid());
- std::uniform_int_distribution<int32_t> distribution(min_delta, max_delta);
- int32_t r = distribution(generator);
- if (r % 2 == 0) {
- r = RoundUp(r, kPageSize);
- } else {
- r = RoundDown(r, kPageSize);
- }
- CHECK_LE(min_delta, r);
- CHECK_GE(max_delta, r);
- CHECK_ALIGNED(r, kPageSize);
- return r;
-}
-
bool ImageSpace::RelocateImage(const char* image_location, const char* dest_filename,
InstructionSet isa, std::string* error_msg) {
std::string patchoat(Runtime::Current()->GetPatchoatExecutable());
@@ -376,13 +380,6 @@
CHECK(dalvik_cache_exists) << "No place to put generated image.";
CHECK(GenerateImage(cache_filename, &error_msg))
<< "Failed to generate image '" << cache_filename << "': " << error_msg;
- // TODO Should I relocate this image? Sure
- if (relocate) {
- if (!RelocateImage(cache_filename.c_str(), cache_filename.c_str(), image_isa, &error_msg)) {
- LOG(FATAL) << "Failed to relocate newly created image " << cache_filename.c_str();
- return nullptr;
- }
- }
{
// Note that we must not use the file descriptor associated with
// ScopedFlock::GetFile to Init the image file. We want the file
