[lldb] Skip remote PutFile when MD5 hashes equal

lldb/source/Plugins/Platform/gdb-server/PlatformRemoteGDBServer.cpp

@@ -684,6 +684,14 @@ Status PlatformRemoteGDBServer::RunShellCommand(
                                           signo_ptr, command_output, timeout);
 }
 
+bool PlatformRemoteGDBServer::CalculateMD5(const FileSpec &file_spec,
+                                           uint64_t &low, uint64_t &high) {
+  if (!IsConnected())
+    return false;
+
+  return m_gdb_client_up->CalculateMD5(file_spec, low, high);
+}
+
 void PlatformRemoteGDBServer::CalculateTrapHandlerSymbolNames() {
   m_trap_handlers.push_back(ConstString("_sigtramp"));
 }

lldb/source/Plugins/Platform/gdb-server/PlatformRemoteGDBServer.h

@@ -146,6 +146,9 @@ class PlatformRemoteGDBServer : public Platform, private UserIDResolver {
 
   void CalculateTrapHandlerSymbolNames() override;
 
+  bool CalculateMD5(const FileSpec &file_spec, uint64_t &low,
+                    uint64_t &high) override;
+
   const lldb::UnixSignalsSP &GetRemoteUnixSignals() override;
 
   size_t ConnectToWaitingProcesses(lldb_private::Debugger &debugger,

lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp

@@ -3419,7 +3419,7 @@ bool GDBRemoteCommunicationClient::GetFileExists(
 }
 
 bool GDBRemoteCommunicationClient::CalculateMD5(
-    const lldb_private::FileSpec &file_spec, uint64_t &high, uint64_t &low) {
+    const lldb_private::FileSpec &file_spec, uint64_t &low, uint64_t &high) {
   std::string path(file_spec.GetPath(false));
   lldb_private::StreamString stream;
   stream.PutCString("vFile:MD5:");
@@ -3433,8 +3433,44 @@ bool GDBRemoteCommunicationClient::CalculateMD5(
       return false;
     if (response.Peek() && *response.Peek() == 'x')
       return false;
-    low = response.GetHexMaxU64(false, UINT64_MAX);
-    high = response.GetHexMaxU64(false, UINT64_MAX);
+
+    // GDBRemoteCommunicationServerCommon::Handle_vFile_MD5 concatenates low and
+    // high hex strings. We can't use response.GetHexMaxU64 because that can't
+    // handle the concatenated hex string. What would happen is parsing the low
+    // would consume the whole response packet which would give incorrect
+    // results. Instead, we get the byte string for each low and high hex
+    // separately, and parse them.
+    //
+    // An alternate way to handle this is to change the server to put a
+    // delimiter between the low/high parts, and change the client to parse the
+    // delimiter. However, we choose not to do this so existing lldb-servers
+    // don't have to be patched
+
+    // The checksum is 128 bits encoded as hex
+    // This means low/high are halves of 64 bits each, in otherwords, 8 bytes.
+    // Each byte takes 2 hex characters in the response.
+    const size_t MD5_HALF_LENGTH = sizeof(uint64_t) * 2;
+
+    // Get low part
+    auto part =
+        response.GetStringRef().substr(response.GetFilePos(), MD5_HALF_LENGTH);
+    if (part.size() != MD5_HALF_LENGTH)
+      return false;
+    response.SetFilePos(response.GetFilePos() + part.size());
+
+    if (part.getAsInteger(/*radix=*/16, low))
+      return false;
+
+    // Get high part
+    part =
+        response.GetStringRef().substr(response.GetFilePos(), MD5_HALF_LENGTH);
+    if (part.size() != MD5_HALF_LENGTH)
+      return false;
+    response.SetFilePos(response.GetFilePos() + part.size());
+
+    if (part.getAsInteger(/*radix=*/16, high))
+      return false;
+
     return true;
   }
   return false;

lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h

@@ -392,7 +392,7 @@ class GDBRemoteCommunicationClient : public GDBRemoteClientBase {
           *command_output, // Pass nullptr if you don't want the command output
       const Timeout<std::micro> &timeout);
 
-  bool CalculateMD5(const FileSpec &file_spec, uint64_t &high, uint64_t &low);
+  bool CalculateMD5(const FileSpec &file_spec, uint64_t &low, uint64_t &high);
 
   lldb::DataBufferSP ReadRegister(
       lldb::tid_t tid,

lldb/source/Target/Platform.cpp

@@ -1197,6 +1197,32 @@ Status Platform::PutFile(const FileSpec &source, const FileSpec &destination,
   if (!source_file)
     return Status(source_file.takeError());
   Status error;
+
+  bool requires_upload = true;
+  uint64_t dest_md5_low, dest_md5_high;
+  bool success = CalculateMD5(destination, dest_md5_low, dest_md5_high);
+  if (!success) {
+    LLDB_LOGF(log, "[PutFile] couldn't get md5 sum of destination");
+  } else {
+    auto local_md5 = llvm::sys::fs::md5_contents(source.GetPath());
+    if (!local_md5) {
+      LLDB_LOGF(log, "[PutFile] couldn't get md5 sum of source");
+    } else {
+      const auto [local_md5_high, local_md5_low] = local_md5->words();
+      LLDB_LOGF(log, "[PutFile] destination md5: %016" PRIx64 "%016" PRIx64,
+                dest_md5_high, dest_md5_low);
+      LLDB_LOGF(log, "[PutFile]       local md5: %016" PRIx64 "%016" PRIx64,
+                local_md5_high, local_md5_low);
+      requires_upload =
+          local_md5_high != dest_md5_high || local_md5_low != dest_md5_low;
+    }
+  }
+
+  if (!requires_upload) {
+    LLDB_LOGF(log, "[PutFile] skipping PutFile because md5sums match");
+    return error;
+  }
+
   uint32_t permissions = source_file.get()->GetPermissions(error);
   if (permissions == 0)
     permissions = lldb::eFilePermissionsFileDefault;

lldb/unittests/Process/gdb-remote/GDBRemoteCommunicationClientTest.cpp

@@ -592,3 +592,26 @@ TEST_F(GDBRemoteCommunicationClientTest, WriteMemoryTags) {
                  std::vector<uint8_t>{0x99}, "QMemTags:456789,0:80000000:99",
                  "E03", false);
 }
+
+TEST_F(GDBRemoteCommunicationClientTest, CalculateMD5) {
+  FileSpec file_spec("/foo/bar", FileSpec::Style::posix);
+  uint64_t low, high;
+  std::future<bool> async_result = std::async(std::launch::async, [&] {
+    return client.CalculateMD5(file_spec, low, high);
+  });
+
+  lldb_private::StreamString stream;
+  stream.PutCString("vFile:MD5:");
+  stream.PutStringAsRawHex8(file_spec.GetPath(false));
+  HandlePacket(server, stream.GetString().str(),
+               "F,"
+               "deadbeef01020304"
+               "05060708deadbeef");
+  ASSERT_TRUE(async_result.get());
+
+  // Server and client puts/parses low, and then high
+  const uint64_t expected_low = 0xdeadbeef01020304;
+  const uint64_t expected_high = 0x05060708deadbeef;
+  EXPECT_EQ(expected_low, low);
+  EXPECT_EQ(expected_high, high);
+}