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

1 files changed, 107 insertions, 71 deletions

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 InstructionSet ElfISAToInstructionSet(Elf32_Word isa) {
   }
 }
 
+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 @@ bool PatchOat::Patch(const std::string& image_location, off_t delta,
 
   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 @@ bool PatchOat::Patch(const File* input_oat, const std::string& image_location, o
     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::Patch(const File* input_oat, const std::string& image_location, o
 
 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 @@ bool PatchOat::WriteImage(File* out) {
   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 @@ static void Usage(const char *fmt, ...) {
   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 @@ static void Usage(const char *fmt, ...) {
   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 @@ static void Usage(const char *fmt, ...) {
   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 @@ static File* CreateOrOpen(const char* name, bool* created) {
     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 @@ static int patchoat(int argc, char **argv) {
   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 @@ static int patchoat(int argc, char **argv) {
   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 @@ static int patchoat(int argc, char **argv) {
       }
     } 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 @@ static int patchoat(int argc, char **argv) {
       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 @@ static int patchoat(int argc, char **argv) {
       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,27 +959,11 @@ static int patchoat(int argc, char **argv) {
     if (!isa_set) {
       Usage("specifying a location requires specifying an instruction set");
     }
-    input_oat_filename = GetSystemImageFilename(input_oat_location.c_str(), isa);
-    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");
+    if (!LocationToFilename(input_oat_location, isa, &input_oat_filename)) {
+      Usage("Unable to find filename for input oat location %s", input_oat_location.c_str());
     }
-    output_image_filename = GetSystemImageFilename(output_image_location.c_str(), isa);
     if (debug) {
-      LOG(INFO) << "Using output-image-file " << output_image_filename;
+      LOG(INFO) << "Using input-oat-file " << input_oat_filename;
     }
   }
   if (!patched_image_location.empty()) {
@@ -1009,6 +1029,9 @@ static int patchoat(int argc, char **argv) {
     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 @@ static int patchoat(int argc, char **argv) {
 
   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 @@ static int patchoat(int argc, char **argv) {
     }
 
     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 @@ static int patchoat(int argc, char **argv) {
     }
   };
 
+  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 @@ static int patchoat(int argc, char **argv) {
     ret = PatchOat::Patch(input_image_location, base_delta, output_image.get(), isa, &timings);
   }
   cleanup(ret);
-  sync();
   return (ret) ? EXIT_SUCCESS : EXIT_FAILURE;
 }