[adb] Fix incremental installation on Windows

Use only the syscalls that work with the wrapped ADB fds, or
extract the native handles for the case when need to call one
not wrapped.

Bug: 151239696
Test: adb install --incremental <apk> on Windows
Change-Id: Ia6de620171ab696b8136dcb60a2b63af6f86419f

adb/client/commandline.cpp

@@ -1423,13 +1423,13 @@ static bool _is_valid_ack_reply_fd(const int ack_reply_fd) {
 #endif
 }
 
-static bool _is_valid_fd(int fd) {
+static bool _is_valid_os_fd(int fd) {
     // Disallow invalid FDs and stdin/out/err as well.
     if (fd < 3) {
         return false;
     }
 #ifdef _WIN32
-    HANDLE handle = adb_get_os_handle(fd);
+    auto handle = (HANDLE)fd;
     DWORD info = 0;
     if (GetHandleInformation(handle, &info) == 0) {
         return false;
@@ -2005,7 +2005,7 @@ int adb_commandline(int argc, const char** argv) {
 #endif
         }
         int connection_fd = atoi(argv[1]);
-        if (!_is_valid_fd(connection_fd)) {
+        if (!_is_valid_os_fd(connection_fd)) {
             error_exit("Invalid connection_fd number given: %d", connection_fd);
         }
 
@@ -2013,7 +2013,7 @@ int adb_commandline(int argc, const char** argv) {
         close_on_exec(connection_fd);
 
         int output_fd = atoi(argv[2]);
-        if (!_is_valid_fd(output_fd)) {
+        if (!_is_valid_os_fd(output_fd)) {
             error_exit("Invalid output_fd number given: %d", output_fd);
         }
         output_fd = adb_register_socket(output_fd);

adb/client/incremental.cpp

@@ -41,8 +41,7 @@ using Size = int64_t;
 
 static inline int32_t read_int32(borrowed_fd fd) {
     int32_t result;
-    ReadFully(fd, &result, sizeof(result));
-    return result;
+    return ReadFdExactly(fd, &result, sizeof(result)) ? result : -1;
 }
 
 static inline void append_int(borrowed_fd fd, std::vector<char>* bytes) {
@@ -54,11 +53,14 @@ static inline void append_int(borrowed_fd fd, std::vector<char>* bytes) {
 
 static inline void append_bytes_with_size(borrowed_fd fd, std::vector<char>* bytes) {
     int32_t le_size = read_int32(fd);
+    if (le_size < 0) {
+        return;
+    }
     int32_t size = int32_t(le32toh(le_size));
     auto old_size = bytes->size();
     bytes->resize(old_size + sizeof(le_size) + size);
     memcpy(bytes->data() + old_size, &le_size, sizeof(le_size));
-    ReadFully(fd, bytes->data() + old_size + sizeof(le_size), size);
+    ReadFdExactly(fd, bytes->data() + old_size + sizeof(le_size), size);
 }
 
 static inline std::pair<std::vector<char>, int32_t> read_id_sig_headers(borrowed_fd fd) {
@@ -200,7 +202,7 @@ std::optional<Process> install(std::vector<std::string> files) {
         return {};
     }
     auto [pipe_read_fd, pipe_write_fd] = print_fds;
-    auto pipe_write_fd_param = std::to_string(pipe_write_fd);
+    auto pipe_write_fd_param = std::to_string(intptr_t(adb_get_os_handle(pipe_write_fd)));
     close_on_exec(pipe_read_fd);
 
     std::vector<std::string> args(std::move(files));

adb/client/incremental_utils.cpp

@@ -18,6 +18,7 @@
 
 #include "incremental_utils.h"
 
+#include <android-base/mapped_file.h>
 #include <android-base/strings.h>
 #include <ziparchive/zip_archive.h>
 #include <ziparchive/zip_writer.h>
@@ -26,6 +27,7 @@
 #include <numeric>
 #include <unordered_set>
 
+#include "adb_trace.h"
 #include "sysdeps.h"
 
 static constexpr int kBlockSize = 4096;
@@ -155,18 +157,81 @@ static std::vector<int32_t> ZipPriorityBlocks(off64_t signerBlockOffset, int64_t
     return zipPriorityBlocks;
 }
 
-// TODO(b/151676293): avoid using OpenArchiveFd that reads local file headers
+[[maybe_unused]] static ZipArchiveHandle openZipArchiveFd(int fd) {
+    bool transferFdOwnership = false;
+#ifdef _WIN32
+    //
+    // Need to create a special CRT FD here as the current one is not compatible with
+    // normal read()/write() calls that libziparchive uses.
+    // To make this work we have to create a copy of the file handle, as CRT doesn't care
+    // and closes it together with the new descriptor.
+    //
+    // Note: don't move this into a helper function, it's better to be hard to reuse because
+    //       the code is ugly and won't work unless it's a last resort.
+    //
+    auto handle = adb_get_os_handle(fd);
+    HANDLE dupedHandle;
+    if (!::DuplicateHandle(::GetCurrentProcess(), handle, ::GetCurrentProcess(), &dupedHandle, 0,
+                           false, DUPLICATE_SAME_ACCESS)) {
+        D("%s failed at DuplicateHandle: %d", __func__, (int)::GetLastError());
+        return {};
+    }
+    fd = _open_osfhandle((intptr_t)dupedHandle, _O_RDONLY | _O_BINARY);
+    if (fd < 0) {
+        D("%s failed at _open_osfhandle: %d", __func__, errno);
+        ::CloseHandle(handle);
+        return {};
+    }
+    transferFdOwnership = true;
+#endif
+    ZipArchiveHandle zip;
+    if (OpenArchiveFd(fd, "apk_fd", &zip, transferFdOwnership) != 0) {
+        D("%s failed at OpenArchiveFd: %d", __func__, errno);
+#ifdef _WIN32
+        // "_close()" is a secret WinCRT name for the regular close() function.
+        _close(fd);
+#endif
+        return {};
+    }
+    return zip;
+}
+
+static std::pair<ZipArchiveHandle, std::unique_ptr<android::base::MappedFile>> openZipArchive(
+        int fd, int64_t fileSize) {
+#ifndef __LP64__
+    if (fileSize >= INT_MAX) {
+        return {openZipArchiveFd(fd), nullptr};
+    }
+#endif
+    auto mapping =
+            android::base::MappedFile::FromOsHandle(adb_get_os_handle(fd), 0, fileSize, PROT_READ);
+    if (!mapping) {
+        D("%s failed at FromOsHandle: %d", __func__, errno);
+        return {};
+    }
+    ZipArchiveHandle zip;
+    if (OpenArchiveFromMemory(mapping->data(), mapping->size(), "apk_mapping", &zip) != 0) {
+        D("%s failed at OpenArchiveFromMemory: %d", __func__, errno);
+        return {};
+    }
+    return {zip, std::move(mapping)};
+}
+
+// TODO(b/151676293): avoid using libziparchive as it reads local file headers
 // which causes additional performance cost. Instead, only read from central directory.
 static std::vector<int32_t> InstallationPriorityBlocks(int fd, int64_t fileSize) {
-    std::vector<int32_t> installationPriorityBlocks;
-    ZipArchiveHandle zip;
-    if (OpenArchiveFd(fd, "", &zip, false) != 0) {
+    auto [zip, _] = openZipArchive(fd, fileSize);
+    if (!zip) {
         return {};
     }
+
     void* cookie = nullptr;
     if (StartIteration(zip, &cookie) != 0) {
+        D("%s failed at StartIteration: %d", __func__, errno);
         return {};
     }
+
+    std::vector<int32_t> installationPriorityBlocks;
     ZipEntry entry;
     std::string_view entryName;
     while (Next(cookie, &entry, &entryName) == 0) {
@@ -183,6 +248,7 @@ static std::vector<int32_t> InstallationPriorityBlocks(int fd, int64_t fileSize)
             int32_t endBlockIndex = offsetToBlockIndex(entryEndOffset);
             int32_t numNewBlocks = endBlockIndex - startBlockIndex + 1;
             appendBlocks(startBlockIndex, numNewBlocks, &installationPriorityBlocks);
+            D("\tadding to priority blocks: '%.*s'", (int)entryName.size(), entryName.data());
         } else if (entryName == "classes.dex") {
             // Only the head is needed for installation
             int32_t startBlockIndex = offsetToBlockIndex(entry.offset);
@@ -213,4 +279,4 @@ std::vector<int32_t> PriorityBlocksForFile(const std::string& filepath, int fd,
     unduplicate(priorityBlocks);
     return priorityBlocks;
 }
-}  // namespace incremental
+}  // namespace incremental

adb/sysdeps.h

@@ -276,6 +276,7 @@ inline void seekdir(DIR*, long) {
 class Process {
   public:
     constexpr explicit Process(HANDLE h = nullptr) : h_(h) {}
+    constexpr Process(Process&& other) : h_(std::exchange(other.h_, nullptr)) {}
     ~Process() { close(); }
     constexpr explicit operator bool() const { return h_ != nullptr; }
 
@@ -292,6 +293,8 @@ class Process {
     }
 
   private:
+    DISALLOW_COPY_AND_ASSIGN(Process);
+
     void close() {
         if (*this) {
             ::CloseHandle(h_);
@@ -666,6 +669,8 @@ static __inline__ int adb_get_os_handle(borrowed_fd fd) {
 class Process {
   public:
     constexpr explicit Process(pid_t pid) : pid_(pid) {}
+    constexpr Process(Process&& other) : pid_(std::exchange(other.pid_, -1)) {}
+
     constexpr explicit operator bool() const { return pid_ >= 0; }
 
     void wait() {
@@ -682,6 +687,8 @@ class Process {
     }
 
   private:
+    DISALLOW_COPY_AND_ASSIGN(Process);
+
     pid_t pid_;
 };