Revert "Revert "runtime: Mmap uncompressed dex files (in zip) as clean memory""

This reverts commit 960b2af8a05f0844e78004e2d0d3ae6ab058d430.

Fix failing 071-dex-file-map-clean on target
which was due to an IO race in the run-test script.

Test: make test-art-target
Bug: 27650033
Original-Change-Id: I18efbd392c5980ffe0d983833b6cc581e0237b92
Change-Id: I6f4ff1e85f8326916c2ae0842a32f53fb7901639
diff --git a/runtime/dex_file.cc b/runtime/dex_file.cc
index 9ad4063..973ac83 100644
--- a/runtime/dex_file.cc
+++ b/runtime/dex_file.cc
@@ -331,7 +331,32 @@
     *error_code = ZipOpenErrorCode::kDexFileError;
     return nullptr;
   }
-  std::unique_ptr<MemMap> map(zip_entry->ExtractToMemMap(location.c_str(), entry_name, error_msg));
+
+  std::unique_ptr<MemMap> map;
+  if (zip_entry->IsUncompressed()) {
+    if (!zip_entry->IsAlignedTo(alignof(Header))) {
+      // Do not mmap unaligned ZIP entries because
+      // doing so would fail dex verification which requires 4 byte alignment.
+      LOG(WARNING) << "Can't mmap dex file " << location << "!" << entry_name << " directly; "
+                   << "please zipalign to " << alignof(Header) << " bytes. "
+                   << "Falling back to extracting file.";
+    } else {
+      // Map uncompressed files within zip as file-backed to avoid a dirty copy.
+      map.reset(zip_entry->MapDirectlyFromFile(location.c_str(), /*out*/error_msg));
+      if (map == nullptr) {
+        LOG(WARNING) << "Can't mmap dex file " << location << "!" << entry_name << " directly; "
+                     << "is your ZIP file corrupted? Falling back to extraction.";
+        // Try again with Extraction which still has a chance of recovery.
+      }
+    }
+  }
+
+  if (map == nullptr) {
+    // Default path for compressed ZIP entries,
+    // and fallback for stored ZIP entries.
+    map.reset(zip_entry->ExtractToMemMap(location.c_str(), entry_name, error_msg));
+  }
+
   if (map == nullptr) {
     *error_msg = StringPrintf("Failed to extract '%s' from '%s': %s", entry_name, location.c_str(),
                               error_msg->c_str());
@@ -502,6 +527,11 @@
       oat_dex_file_(oat_dex_file) {
   CHECK(begin_ != nullptr) << GetLocation();
   CHECK_GT(size_, 0U) << GetLocation();
+  // Check base (=header) alignment.
+  // Must be 4-byte aligned to avoid undefined behavior when accessing
+  // any of the sections via a pointer.
+  CHECK_ALIGNED(begin_, alignof(Header));
+
   InitializeSectionsFromMapList();
 }
 
diff --git a/runtime/zip_archive.cc b/runtime/zip_archive.cc
index cd79bb6..416873f 100644
--- a/runtime/zip_archive.cc
+++ b/runtime/zip_archive.cc
@@ -23,10 +23,16 @@
 #include <unistd.h>
 #include <vector>
 
+#include "android-base/stringprintf.h"
 #include "base/unix_file/fd_file.h"
 
 namespace art {
 
+// Log file contents and mmap info when mapping entries directly.
+static constexpr const bool kDebugZipMapDirectly = false;
+
+using android::base::StringPrintf;
+
 uint32_t ZipEntry::GetUncompressedLength() {
   return zip_entry_->uncompressed_length;
 }
@@ -35,6 +41,15 @@
   return zip_entry_->crc32;
 }
 
+bool ZipEntry::IsUncompressed() {
+  return zip_entry_->method == kCompressStored;
+}
+
+bool ZipEntry::IsAlignedTo(size_t alignment) {
+  DCHECK(IsPowerOfTwo(alignment)) << alignment;
+  return IsAlignedParam(zip_entry_->offset, static_cast<int>(alignment));
+}
+
 ZipEntry::~ZipEntry() {
   delete zip_entry_;
 }
@@ -73,6 +88,102 @@
   return map.release();
 }
 
+MemMap* ZipEntry::MapDirectlyFromFile(const char* zip_filename, std::string* error_msg) {
+  const int zip_fd = GetFileDescriptor(handle_);
+  const char* entry_filename = entry_name_.c_str();
+
+  // Should not happen since we don't have a memory ZipArchive constructor.
+  // However the underlying ZipArchive isn't required to have an FD,
+  // so check to be sure.
+  CHECK_GE(zip_fd, 0) <<
+      StringPrintf("Cannot map '%s' (in zip '%s') directly because the zip archive "
+                   "is not file backed.",
+                   entry_filename,
+                   zip_filename);
+
+  if (!IsUncompressed()) {
+    *error_msg = StringPrintf("Cannot map '%s' (in zip '%s') directly because it is compressed.",
+                              entry_filename,
+                              zip_filename);
+    return nullptr;
+  } else if (zip_entry_->uncompressed_length != zip_entry_->compressed_length) {
+    *error_msg = StringPrintf("Cannot map '%s' (in zip '%s') directly because "
+                              "entry has bad size (%u != %u).",
+                              entry_filename,
+                              zip_filename,
+                              zip_entry_->uncompressed_length,
+                              zip_entry_->compressed_length);
+    return nullptr;
+  }
+
+  std::string name(entry_filename);
+  name += " mapped directly in memory from ";
+  name += zip_filename;
+
+  const off_t offset = zip_entry_->offset;
+
+  if (kDebugZipMapDirectly) {
+    LOG(INFO) << "zip_archive: " << "make mmap of " << name << " @ offset = " << offset;
+  }
+
+  std::unique_ptr<MemMap> map(
+      MemMap::MapFileAtAddress(nullptr,  // Expected pointer address
+                               GetUncompressedLength(),  // Byte count
+                               PROT_READ | PROT_WRITE,
+                               MAP_PRIVATE,
+                               zip_fd,
+                               offset,
+                               false,  // Don't restrict allocation to lower4GB
+                               false,  // Doesn't overlap existing map (reuse=false)
+                               name.c_str(),
+                               /*out*/error_msg));
+
+  if (map == nullptr) {
+    DCHECK(!error_msg->empty());
+  }
+
+  if (kDebugZipMapDirectly) {
+    // Dump contents of file, same format as using this shell command:
+    // $> od -j <offset> -t x1 <zip_filename>
+    static constexpr const int kMaxDumpChars = 15;
+    lseek(zip_fd, 0, SEEK_SET);
+
+    int count = offset + kMaxDumpChars;
+
+    std::string tmp;
+    char buf;
+
+    // Dump file contents.
+    int i = 0;
+    while (read(zip_fd, &buf, 1) > 0 && i < count) {
+      tmp += StringPrintf("%3d ", (unsigned int)buf);
+      ++i;
+    }
+
+    LOG(INFO) << "map_fd raw bytes starting at 0";
+    LOG(INFO) << "" << tmp;
+    LOG(INFO) << "---------------------------";
+
+    // Dump map contents.
+    if (map != nullptr) {
+      tmp = "";
+
+      count = kMaxDumpChars;
+
+      uint8_t* begin = map->Begin();
+      for (i = 0; i < count; ++i) {
+        tmp += StringPrintf("%3d ", (unsigned int)begin[i]);
+      }
+
+      LOG(INFO) << "map address " << StringPrintf("%p", begin);
+      LOG(INFO) << "map first " << kMaxDumpChars << " chars:";
+      LOG(INFO) << tmp;
+    }
+  }
+
+  return map.release();
+}
+
 static void SetCloseOnExec(int fd) {
   // This dance is more portable than Linux's O_CLOEXEC open(2) flag.
   int flags = fcntl(fd, F_GETFD);
@@ -129,7 +240,7 @@
     return nullptr;
   }
 
-  return new ZipEntry(handle_, zip_entry.release());
+  return new ZipEntry(handle_, zip_entry.release(), name);
 }
 
 ZipArchive::~ZipArchive() {
diff --git a/runtime/zip_archive.h b/runtime/zip_archive.h
index 42bf55c..1858444 100644
--- a/runtime/zip_archive.h
+++ b/runtime/zip_archive.h
@@ -37,19 +37,35 @@
 class ZipEntry {
  public:
   bool ExtractToFile(File& file, std::string* error_msg);
+  // Extract this entry to anonymous memory (R/W).
+  // Returns null on failure and sets error_msg.
   MemMap* ExtractToMemMap(const char* zip_filename, const char* entry_filename,
                           std::string* error_msg);
+  // Create a file-backed private (clean, R/W) memory mapping to this entry.
+  // 'zip_filename' is used for diagnostics only,
+  //   the original file that the ZipArchive was open with is used
+  //   for the mapping.
+  //
+  // Will only succeed if the entry is stored uncompressed.
+  // Returns null on failure and sets error_msg.
+  MemMap* MapDirectlyFromFile(const char* zip_filename, /*out*/std::string* error_msg);
   virtual ~ZipEntry();
 
   uint32_t GetUncompressedLength();
   uint32_t GetCrc32();
 
+  bool IsUncompressed();
+  bool IsAlignedTo(size_t alignment);
+
  private:
   ZipEntry(ZipArchiveHandle handle,
-           ::ZipEntry* zip_entry) : handle_(handle), zip_entry_(zip_entry) {}
+           ::ZipEntry* zip_entry,
+           const std::string& entry_name)
+    : handle_(handle), zip_entry_(zip_entry), entry_name_(entry_name) {}
 
   ZipArchiveHandle handle_;
   ::ZipEntry* const zip_entry_;
+  std::string const entry_name_;
 
   friend class ZipArchive;
   DISALLOW_COPY_AND_ASSIGN(ZipEntry);
diff --git a/test/071-dexfile-map-clean/build b/test/071-dexfile-map-clean/build
new file mode 100755
index 0000000..a171fc3
--- /dev/null
+++ b/test/071-dexfile-map-clean/build
@@ -0,0 +1,21 @@
+#!/bin/bash
+#
+# Copyright 2017 The Android Open Source Project
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#      http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+# Any JAR files used by this test shall have their classes.dex be stored, NOT compressed.
+# This is needed for our test correctness which validates classes.dex are mapped file-backed.
+#
+# In addition, align to at least 4 bytes since that's the dex alignment requirement.
+./default-build "$@" --zip-compression-method store --zip-align 4
diff --git a/test/071-dexfile-map-clean/expected.txt b/test/071-dexfile-map-clean/expected.txt
new file mode 100644
index 0000000..af7fb28
--- /dev/null
+++ b/test/071-dexfile-map-clean/expected.txt
@@ -0,0 +1,3 @@
+Another
+Secondary dexfile mmap is clean
+Another Instance
diff --git a/test/071-dexfile-map-clean/info.txt b/test/071-dexfile-map-clean/info.txt
new file mode 100644
index 0000000..7e45808
--- /dev/null
+++ b/test/071-dexfile-map-clean/info.txt
@@ -0,0 +1,11 @@
+Exercise Dalvik-specific DEX file feature. Will not work on RI.
+
+If these conditions are met:
+* When we are loading in a secondary dex file
+* and when dex2oat is not used
+* and the dex file is stored uncompressed in a ZIP file
+
+Then check:
+* The dex file is memory-mapped file-backed as clean memory
+(i.e. there is no extraction step)
+
diff --git a/test/071-dexfile-map-clean/run b/test/071-dexfile-map-clean/run
new file mode 100755
index 0000000..9c100ec
--- /dev/null
+++ b/test/071-dexfile-map-clean/run
@@ -0,0 +1,31 @@
+#!/bin/bash
+#
+# Copyright 2017 The Android Open Source Project
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#      http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+# Run without dex2oat so that we don't create oat/vdex files
+# when trying to load the secondary dex file.
+
+# In this way, the secondary dex file will be forced to be
+# loaded directly.
+#
+# In addition, make sure we call 'sync'
+# before executing dalvikvm because otherwise
+# it's highly likely the pushed JAR files haven't
+# been committed to permanent storage yet,
+# and when we mmap them the kernel will think
+# the memory is dirty (despite being file-backed).
+# (Note: this was reproducible 100% of the time on
+# a target angler device).
+./default-run "$@" --no-dex2oat --sync
diff --git a/test/071-dexfile-map-clean/src-ex/Another.java b/test/071-dexfile-map-clean/src-ex/Another.java
new file mode 100644
index 0000000..58464a6
--- /dev/null
+++ b/test/071-dexfile-map-clean/src-ex/Another.java
@@ -0,0 +1,21 @@
+/*
+ * Copyright (C) 2017 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+public class Another {
+    public Another() {
+        System.out.println("Another Instance");
+    }
+}
diff --git a/test/071-dexfile-map-clean/src/Main.java b/test/071-dexfile-map-clean/src/Main.java
new file mode 100644
index 0000000..8a196dd
--- /dev/null
+++ b/test/071-dexfile-map-clean/src/Main.java
@@ -0,0 +1,147 @@
+/*
+ * Copyright (C) 2017 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+import java.lang.reflect.Method;
+import java.util.Enumeration;
+
+import java.nio.file.Files;
+import java.nio.file.Paths;
+
+/**
+ * DexFile tests (Dalvik-specific).
+ */
+public class Main {
+    private static final String CLASS_PATH =
+        System.getenv("DEX_LOCATION") + "/071-dexfile-map-clean-ex.jar";
+
+    /**
+     * Prep the environment then run the test.
+     */
+    public static void main(String[] args) throws Exception {
+        // Load the dex file, this is a pre-requisite to mmap-ing it in.
+        Class<?> AnotherClass = testDexFile();
+        // Check that the memory maps are clean.
+        testDexMemoryMaps();
+
+        // Prevent garbage collector from collecting our DexFile
+        // (and unmapping too early) by using it after we finish
+        // our verification.
+        AnotherClass.newInstance();
+    }
+
+    private static boolean checkSmapsEntry(String[] smapsLines, int offset) {
+      String nameDescription = smapsLines[offset];
+      String[] split = nameDescription.split(" ");
+
+      String permissions = split[1];
+      // Mapped as read-only + anonymous.
+      if (!permissions.startsWith("r--p")) {
+        return false;
+      }
+
+      boolean validated = false;
+
+      // We have the right entry, now make sure it's valid.
+      for (int i = offset; i < smapsLines.length; ++i) {
+        String line = smapsLines[i];
+
+        if (line.startsWith("Shared_Dirty") || line.startsWith("Private_Dirty")) {
+          String lineTrimmed = line.trim();
+          String[] lineSplit = lineTrimmed.split(" +");
+
+          String sizeUsuallyInKb = lineSplit[lineSplit.length - 2];
+
+          sizeUsuallyInKb = sizeUsuallyInKb.trim();
+
+          if (!sizeUsuallyInKb.equals("0")) {
+            System.out.println(
+                "ERROR: Memory mapping for " + CLASS_PATH + " is unexpectedly dirty");
+            System.out.println(line);
+          } else {
+            validated = true;
+          }
+        }
+
+        // VmFlags marks the "end" of an smaps entry.
+        if (line.startsWith("VmFlags")) {
+          break;
+        }
+      }
+
+      if (validated) {
+        System.out.println("Secondary dexfile mmap is clean");
+      } else {
+        System.out.println("ERROR: Memory mapping is missing Shared_Dirty/Private_Dirty entries");
+      }
+
+      return true;
+    }
+
+    // This test takes relies on dex2oat being skipped.
+    // (enforced in 'run' file by using '--no-dex2oat'
+    //
+    // This could happen in a non-test situation
+    // if a secondary dex file is loaded (but not yet maintenance-mode compiled)
+    // with JIT.
+    //
+    // Or it could also happen if a secondary dex file is loaded and forced
+    // into running into the interpreter (e.g. duplicate classes).
+    //
+    // Rather than relying on those weird fallbacks,
+    // we force the runtime not to dex2oat the dex file to ensure
+    // this test is repeatable and less brittle.
+    private static void testDexMemoryMaps() throws Exception {
+        // Ensure that the secondary dex file is mapped clean (directly from JAR file).
+        String smaps = new String(Files.readAllBytes(Paths.get("/proc/self/smaps")));
+
+        String[] smapsLines = smaps.split("\n");
+        boolean found = true;
+        for (int i = 0; i < smapsLines.length; ++i) {
+          if (smapsLines[i].contains(CLASS_PATH)) {
+            if (checkSmapsEntry(smapsLines, i)) {
+              return;
+            } // else we found the wrong one, keep going.
+          }
+        }
+
+        // Error case:
+        System.out.println("Could not find " + CLASS_PATH + " RO-anonymous smaps entry");
+        System.out.println(smaps);
+    }
+
+    private static Class<?> testDexFile() throws Exception {
+        ClassLoader classLoader = Main.class.getClassLoader();
+        Class<?> DexFile = classLoader.loadClass("dalvik.system.DexFile");
+        Method DexFile_loadDex = DexFile.getMethod("loadDex",
+                                                   String.class,
+                                                   String.class,
+                                                   Integer.TYPE);
+        Method DexFile_entries = DexFile.getMethod("entries");
+        Object dexFile = DexFile_loadDex.invoke(null, CLASS_PATH, null, 0);
+        Enumeration<String> e = (Enumeration<String>) DexFile_entries.invoke(dexFile);
+        while (e.hasMoreElements()) {
+            String className = e.nextElement();
+            System.out.println(className);
+        }
+
+        Method DexFile_loadClass = DexFile.getMethod("loadClass",
+                                                     String.class,
+                                                     ClassLoader.class);
+        Class<?> AnotherClass = (Class<?>)DexFile_loadClass.invoke(dexFile,
+            "Another", Main.class.getClassLoader());
+        return AnotherClass;
+    }
+}
diff --git a/test/etc/default-build b/test/etc/default-build
index e9e3886..330a32e 100755
--- a/test/etc/default-build
+++ b/test/etc/default-build
@@ -60,6 +60,12 @@
   HAS_SRC_DEX2OAT_UNRESOLVED=false
 fi
 
+# Allow overriding ZIP_COMPRESSION_METHOD with e.g. 'store'
+ZIP_COMPRESSION_METHOD="deflate"
+# Align every ZIP file made by calling $ZIPALIGN command?
+WITH_ZIP_ALIGN=false
+ZIP_ALIGN_BYTES="-1"
+
 DX_FLAGS=""
 SKIP_DX_MERGER="false"
 EXPERIMENTAL=""
@@ -118,6 +124,17 @@
     DEFAULT_EXPERIMENT=""
     EXPERIMENTAL="${EXPERIMENTAL} $1"
     shift
+  elif [ "x$1" = "x--zip-compression-method" ]; then
+    # Allow using different zip compression method, e.g. 'store'
+    shift
+    ZIP_COMPRESSION_METHOD="$1"
+    shift
+  elif [ "x$1" = "x--zip-align" ]; then
+    # Align ZIP entries to some # of bytes.
+    shift
+    WITH_ZIP_ALIGN=true
+    ZIP_ALIGN_BYTES="$1"
+    shift
   elif expr "x$1" : "x--" >/dev/null 2>&1; then
     echo "unknown $0 option: $1" 1>&2
     exit 1
@@ -146,6 +163,26 @@
   JAVAC_ARGS="${JAVAC_ARGS} ${JAVAC_EXPERIMENTAL_ARGS[${experiment}]}"
 done
 
+#########################################
+
+# Catch all commands to 'ZIP' and prepend extra flags.
+# Optionally, zipalign results to some alignment.
+function zip() {
+  local zip_target="$1"
+  local entry_src="$2"
+  shift 2
+
+  command zip --compression-method "$ZIP_COMPRESSION_METHOD" "$zip_target" "$entry_src" "$@"
+
+  if "$WITH_ZIP_ALIGN"; then
+    # zipalign does not operate in-place, so write results to a temp file.
+    local tmp_file="$(mktemp)"
+    "$ZIPALIGN" -f "$ZIP_ALIGN_BYTES" "$zip_target" "$tmp_file"
+    # replace original zip target with our temp file.
+    mv "$tmp_file" "$zip_target"
+  fi
+}
+
 if [ -e classes.dex ]; then
   zip $TEST_NAME.jar classes.dex
   exit 0
diff --git a/test/etc/run-test-jar b/test/etc/run-test-jar
index 186a151..dfc316a 100755
--- a/test/etc/run-test-jar
+++ b/test/etc/run-test-jar
@@ -64,6 +64,10 @@
 APP_IMAGE="y"
 VDEX_FILTER=""
 
+# if "y", run 'sync' before dalvikvm to make sure all files from
+# build step (e.g. dex2oat) were finished writing.
+SYNC_BEFORE_RUN="n"
+
 while true; do
     if [ "x$1" = "x--quiet" ]; then
         QUIET="y"
@@ -262,6 +266,9 @@
         option="$1"
         VDEX_FILTER="--compiler-filter=$option"
         shift
+    elif [ "x$1" = "x--sync" ]; then
+        SYNC_BEFORE_RUN="y"
+        shift
     elif expr "x$1" : "x--" >/dev/null 2>&1; then
         echo "unknown $0 option: $1" 1>&2
         exit 1
@@ -491,6 +498,7 @@
 vdex_cmdline="true"
 mkdir_locations="${DEX_LOCATION}/dalvik-cache/$ISA"
 strip_cmdline="true"
+sync_cmdline="true"
 
 
 if [ "$PREBUILD" = "y" ]; then
@@ -530,6 +538,10 @@
   strip_cmdline="zip --quiet --delete $DEX_LOCATION/$TEST_NAME.jar classes.dex"
 fi
 
+if [ "$SYNC_BEFORE_RUN" = "y" ]; then
+  sync_cmdline="sync"
+fi
+
 DALVIKVM_ISA_FEATURES_ARGS=""
 if [ "x$INSTRUCTION_SET_FEATURES" != "x" ] ; then
   DALVIKVM_ISA_FEATURES_ARGS="-Xcompiler-option --instruction-set-features=${INSTRUCTION_SET_FEATURES}"
@@ -607,6 +619,7 @@
              $dex2oat_cmdline && \
              $vdex_cmdline && \
              $strip_cmdline && \
+             $sync_cmdline && \
              $dalvikvm_cmdline"
 
     cmdfile=$(tempfile -p "cmd-" -s "-$TEST_NAME")
@@ -679,7 +692,7 @@
     fi
 
     if [ "$DEV_MODE" = "y" ]; then
-      echo "mkdir -p ${mkdir_locations} && $dex2oat_cmdline && $vdex_cmdline && $strip_cmdline && $cmdline"
+      echo "mkdir -p ${mkdir_locations} && $dex2oat_cmdline && $vdex_cmdline && $strip_cmdline && $sync_cmdline && $cmdline"
     fi
 
     cd $ANDROID_BUILD_TOP
@@ -689,6 +702,7 @@
     $dex2oat_cmdline || { echo "Dex2oat failed." >&2 ; exit 2; }
     $vdex_cmdline || { echo "Dex2oat failed." >&2 ; exit 2; }
     $strip_cmdline || { echo "Strip failed." >&2 ; exit 3; }
+    $sync_cmdline || { echo "Sync failed." >&2 ; exit 4; }
 
     # For running, we must turn off logging when dex2oat or patchoat are missing. Otherwise we use
     # the same defaults as for prebuilt: everything when --dev, otherwise errors and above only.
diff --git a/test/run-test b/test/run-test
index 27c700e..4936039 100755
--- a/test/run-test
+++ b/test/run-test
@@ -90,6 +90,22 @@
 
 export JACK="$JACK -g -cp $JACK_CLASSPATH"
 
+# Zipalign is not on the PATH in some configs, auto-detect it.
+if [ -z "$ZIPALIGN" ]; then
+  if which zipalign >/dev/null; then
+    ZIPALIGN="zipalign";
+  else
+    # TODO: Add a dependency for zipalign in Android.run-test.mk
+    # once it doesn't depend on libandroidfw (b/35246701)
+    case "$OSTYPE" in
+      darwin*)  ZIPALIGN="$ANDROID_BUILD_TOP/prebuilts/sdk/tools/darwin/bin/zipalign" ;;
+      linux*)   ZIPALIGN="$ANDROID_BUILD_TOP/prebuilts/sdk/tools/linux/bin/zipalign" ;;
+      *)        echo "Can't find zipalign: unknown: $OSTYPE" >&2;;
+    esac
+  fi
+fi
+export ZIPALIGN
+
 info="info.txt"
 build="build"
 run="run"