commit a27af08ee0c5f307366be73363369cc1d95d6d4c
author Mathieu Chartier <mathieuc@google.com> Sat Feb 03 16:12:23 2018 -0800
committer Mathieu Chartier <mathieuc@google.com> Mon Feb 05 14:30:15 2018 -0800
tree b419ec0515085f2a362da9e123f6531b4d1162b0
parent ca6f1c0cb7655e90237583c1b6b771d3c8aae95d

Handle code item overruns in cdex code item write

For the case where code items overrun, use a safe iterator to not
crash.

Bug: 72315996
Bug: 63756964
Test: test-art-host-gtest

Change-Id: I2aa8d127aad09ed3c8e6c9f9cc71c4b5646a5307

dexlayout/dexlayout_test.cc

diff --git a/dexlayout/dexlayout_test.cc b/dexlayout/dexlayout_test.cc
index be272fc..db35a03 100644
--- a/dexlayout/dexlayout_test.cc
+++ b/dexlayout/dexlayout_test.cc
@@ -221,6 +221,12 @@
     "AHAAAAACAAAAAwAAAIwAAAADAAAAAQAAAJgAAAAFAAAABAAAAKQAAAAGAAAAAQAAAMQAAAABIAAA"
     "AwAAAOQAAAACIAAABwAAACQBAAADIAAAAwAAAFYBAAAAIAAAAQAAAGUBAAAAEAAAAQAAAHgBAAA=";
 
+// Returns the default compact dex option for dexlayout based on kDefaultCompactDexLevel.
+static std::vector<std::string> DefaultCompactDexOption() {
+  return (kDefaultCompactDexLevel == CompactDexLevel::kCompactDexLevelFast) ?
+      std::vector<std::string>{"-x", "fast"} : std::vector<std::string>{"-x", "none"};
+}
+
 static void WriteBase64ToFile(const char* base64, File* file) {
   // Decode base64.
   CHECK(base64 != nullptr);
@@ -289,7 +295,7 @@
     for (const std::string &dex_file : GetLibCoreDexFileNames()) {
       std::vector<std::string> dexlayout_args =
           { "-w", tmp_dir, "-o", tmp_name, dex_file };
-      if (!DexLayoutExec(dexlayout_args, error_msg)) {
+      if (!DexLayoutExec(dexlayout_args, error_msg, /*pass_default_cdex_option*/ false)) {
         return false;
       }
       size_t dex_file_last_slash = dex_file.rfind('/');
@@ -467,7 +473,7 @@
     // -v makes sure that the layout did not corrupt the dex file.
     std::vector<std::string> dexlayout_args =
         { "-i", "-v", "-w", tmp_dir, "-o", tmp_name, "-p", profile_file, dex_file };
-    if (!DexLayoutExec(dexlayout_args, error_msg)) {
+    if (!DexLayoutExec(dexlayout_args, error_msg, /*pass_default_cdex_option*/ false)) {
       return false;
     }
 
@@ -479,7 +485,7 @@
     // -i since the checksum won't match from the first layout.
     std::vector<std::string> second_dexlayout_args =
         { "-i", "-v", "-w", tmp_dir, "-o", tmp_name, "-p", profile_file, output_dex };
-    if (!DexLayoutExec(second_dexlayout_args, error_msg)) {
+    if (!DexLayoutExec(second_dexlayout_args, error_msg, /*pass_default_cdex_option*/ false)) {
       return false;
     }
 
@@ -512,7 +518,7 @@
     std::string output_dex = tmp_dir + "classes.dex.new";
 
     std::vector<std::string> dexlayout_args = { "-w", tmp_dir, "-o", "/dev/null", input_dex };
-    if (!DexLayoutExec(dexlayout_args, error_msg)) {
+    if (!DexLayoutExec(dexlayout_args, error_msg, /*pass_default_cdex_option*/ false)) {
       return false;
     }
 
@@ -550,12 +556,18 @@
     return true;
   }
 
-  bool DexLayoutExec(const std::vector<std::string>& dexlayout_args, std::string* error_msg) {
+  bool DexLayoutExec(const std::vector<std::string>& dexlayout_args,
+                     std::string* error_msg,
+                     bool pass_default_cdex_option = true) {
     std::vector<std::string> argv;
 
     std::string dexlayout = GetDexLayoutPath();
     CHECK(OS::FileExists(dexlayout.c_str())) << dexlayout << " should be a valid file path";
     argv.push_back(dexlayout);
+    if (pass_default_cdex_option) {
+      std::vector<std::string> cdex_level = DefaultCompactDexOption();
+      argv.insert(argv.end(), cdex_level.begin(), cdex_level.end());
+    }
 
     argv.insert(argv.end(), dexlayout_args.begin(), dexlayout_args.end());
 
@@ -730,11 +742,33 @@
     CHECK(mutated_successfully)
         << "Failed to find candidate code item with only one code unit in last instruction.";
   });
-  std::vector<std::string> dexlayout_args = { "-i", "-o", "/dev/null", temp_dex.GetFilename() };
+
+  std::string error_msg;
+
+  ScratchFile tmp_file;
+  const std::string& tmp_name = tmp_file.GetFilename();
+  size_t tmp_last_slash = tmp_name.rfind('/');
+  std::string tmp_dir = tmp_name.substr(0, tmp_last_slash + 1);
+  ScratchFile profile_file;
+
+  std::vector<std::string> dexlayout_args =
+      { "-i",
+        "-v",
+        "-w", tmp_dir,
+        "-o", tmp_name,
+        "-p", profile_file.GetFilename(),
+        temp_dex.GetFilename()
+      };
+  // -v makes sure that the layout did not corrupt the dex file.
   ASSERT_TRUE(DexLayoutExec(&temp_dex,
                             /*dex_filename*/ nullptr,
-                            nullptr /* profile_file */,
+                            &profile_file,
                             dexlayout_args));
+
+  std::string output_dex = temp_dex.GetFilename() + ".new";
+  std::vector<std::string> rm_exec_argv =
+      { "/bin/rm", output_dex };
+  ASSERT_TRUE(::art::Exec(rm_exec_argv, &error_msg));
 }
 
 // Test that link data is written out (or at least the header is updated).