Skip to content

[lldb] Unify CalculateMD5 return types #90921

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
May 3, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions lldb/include/lldb/Target/Platform.h
Original file line number Diff line number Diff line change
Expand Up @@ -649,8 +649,8 @@ class Platform : public PluginInterface {

virtual std::string GetPlatformSpecificConnectionInformation() { return ""; }

virtual bool CalculateMD5(const FileSpec &file_spec, uint64_t &low,
uint64_t &high);
virtual llvm::ErrorOr<llvm::MD5::MD5Result>
CalculateMD5(const FileSpec &file_spec);

virtual uint32_t GetResumeCountForLaunchInfo(ProcessLaunchInfo &launch_info) {
return 1;
Expand Down
4 changes: 2 additions & 2 deletions lldb/include/lldb/Target/RemoteAwarePlatform.h
Original file line number Diff line number Diff line change
Expand Up @@ -58,8 +58,8 @@ class RemoteAwarePlatform : public Platform {
Status SetFilePermissions(const FileSpec &file_spec,
uint32_t file_permissions) override;

bool CalculateMD5(const FileSpec &file_spec, uint64_t &low,
uint64_t &high) override;
llvm::ErrorOr<llvm::MD5::MD5Result>
CalculateMD5(const FileSpec &file_spec) override;

Status GetFileWithUUID(const FileSpec &platform_file, const UUID *uuid,
FileSpec &local_file) override;
Expand Down
16 changes: 10 additions & 6 deletions lldb/source/Plugins/Platform/MacOSX/PlatformDarwinDevice.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -405,17 +405,21 @@ lldb_private::Status PlatformDarwinDevice::GetSharedModuleWithLocalCache(
// when going over the *slow* GDB remote transfer mechanism we first
// check the hashes of the files - and only do the actual transfer if
// they differ
uint64_t high_local, high_remote, low_local, low_remote;
auto MD5 = llvm::sys::fs::md5_contents(module_cache_spec.GetPath());
if (!MD5)
return Status(MD5.getError());
std::tie(high_local, low_local) = MD5->words();

m_remote_platform_sp->CalculateMD5(module_spec.GetFileSpec(),
low_remote, high_remote);
if (low_local != low_remote || high_local != high_remote) {
Log *log = GetLog(LLDBLog::Platform);
bool requires_transfer = true;
llvm::ErrorOr<llvm::MD5::MD5Result> remote_md5 =
m_remote_platform_sp->CalculateMD5(module_spec.GetFileSpec());
if (std::error_code ec = remote_md5.getError())
LLDB_LOG(log, "couldn't get md5 sum from remote: {0}",
ec.message());
else
requires_transfer = *MD5 != *remote_md5;
if (requires_transfer) {
// bring in the remote file
Log *log = GetLog(LLDBLog::Platform);
LLDB_LOGF(log,
"[%s] module %s/%s needs to be replaced from remote copy",
(IsHost() ? "host" : "remote"),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -684,12 +684,12 @@ Status PlatformRemoteGDBServer::RunShellCommand(
signo_ptr, command_output, timeout);
}

bool PlatformRemoteGDBServer::CalculateMD5(const FileSpec &file_spec,
uint64_t &low, uint64_t &high) {
llvm::ErrorOr<llvm::MD5::MD5Result>
PlatformRemoteGDBServer::CalculateMD5(const FileSpec &file_spec) {
if (!IsConnected())
return false;
return std::make_error_code(std::errc::not_connected);

return m_gdb_client_up->CalculateMD5(file_spec, low, high);
return m_gdb_client_up->CalculateMD5(file_spec);
}

void PlatformRemoteGDBServer::CalculateTrapHandlerSymbolNames() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -146,8 +146,8 @@ class PlatformRemoteGDBServer : public Platform, private UserIDResolver {

void CalculateTrapHandlerSymbolNames() override;

bool CalculateMD5(const FileSpec &file_spec, uint64_t &low,
uint64_t &high) override;
llvm::ErrorOr<llvm::MD5::MD5Result>
CalculateMD5(const FileSpec &file_spec) override;

const lldb::UnixSignalsSP &GetRemoteUnixSignals() override;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3418,8 +3418,8 @@ bool GDBRemoteCommunicationClient::GetFileExists(
return true;
}

bool GDBRemoteCommunicationClient::CalculateMD5(
const lldb_private::FileSpec &file_spec, uint64_t &low, uint64_t &high) {
llvm::ErrorOr<llvm::MD5::MD5Result> GDBRemoteCommunicationClient::CalculateMD5(
const lldb_private::FileSpec &file_spec) {
std::string path(file_spec.GetPath(false));
lldb_private::StreamString stream;
stream.PutCString("vFile:MD5:");
Expand All @@ -3428,11 +3428,11 @@ bool GDBRemoteCommunicationClient::CalculateMD5(
if (SendPacketAndWaitForResponse(stream.GetString(), response) ==
PacketResult::Success) {
if (response.GetChar() != 'F')
return false;
return std::make_error_code(std::errc::illegal_byte_sequence);
if (response.GetChar() != ',')
return false;
return std::make_error_code(std::errc::illegal_byte_sequence);
if (response.Peek() && *response.Peek() == 'x')
return false;
return std::make_error_code(std::errc::no_such_file_or_directory);

// GDBRemoteCommunicationServerCommon::Handle_vFile_MD5 concatenates low and
// high hex strings. We can't use response.GetHexMaxU64 because that can't
Expand All @@ -3455,25 +3455,33 @@ bool GDBRemoteCommunicationClient::CalculateMD5(
auto part =
response.GetStringRef().substr(response.GetFilePos(), MD5_HALF_LENGTH);
if (part.size() != MD5_HALF_LENGTH)
return false;
return std::make_error_code(std::errc::illegal_byte_sequence);
response.SetFilePos(response.GetFilePos() + part.size());

uint64_t low;
if (part.getAsInteger(/*radix=*/16, low))
return false;
return std::make_error_code(std::errc::illegal_byte_sequence);

// Get high part
part =
response.GetStringRef().substr(response.GetFilePos(), MD5_HALF_LENGTH);
if (part.size() != MD5_HALF_LENGTH)
return false;
return std::make_error_code(std::errc::illegal_byte_sequence);
response.SetFilePos(response.GetFilePos() + part.size());

uint64_t high;
if (part.getAsInteger(/*radix=*/16, high))
return false;
return std::make_error_code(std::errc::illegal_byte_sequence);

return true;
llvm::MD5::MD5Result result;
llvm::support::endian::write<uint64_t, llvm::endianness::little>(
result.data(), low);
llvm::support::endian::write<uint64_t, llvm::endianness::little>(
result.data() + 8, high);

return result;
}
return false;
return std::make_error_code(std::errc::operation_canceled);
}

bool GDBRemoteCommunicationClient::AvoidGPackets(ProcessGDBRemote *process) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 &low, uint64_t &high);
llvm::ErrorOr<llvm::MD5::MD5Result> CalculateMD5(const FileSpec &file_spec);

lldb::DataBufferSP ReadRegister(
lldb::tid_t tid,
Expand Down
36 changes: 16 additions & 20 deletions lldb/source/Target/Platform.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1199,22 +1199,22 @@ Status Platform::PutFile(const FileSpec &source, const FileSpec &destination,
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");
llvm::ErrorOr<llvm::MD5::MD5Result> remote_md5 = CalculateMD5(destination);
if (std::error_code ec = remote_md5.getError()) {
LLDB_LOG(log, "[PutFile] couldn't get md5 sum of destination: {0}",
ec.message());
} 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");
llvm::ErrorOr<llvm::MD5::MD5Result> local_md5 =
llvm::sys::fs::md5_contents(source.GetPath());
if (std::error_code ec = local_md5.getError()) {
LLDB_LOG(log, "[PutFile] couldn't get md5 sum of source: {0}",
ec.message());
} 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);
remote_md5->high(), remote_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;
local_md5->high(), local_md5->low());
requires_upload = *remote_md5 != *local_md5;
}
}

Expand Down Expand Up @@ -1339,15 +1339,11 @@ lldb_private::Status Platform::RunShellCommand(
return Status("unable to run a remote command without a platform");
}

bool Platform::CalculateMD5(const FileSpec &file_spec, uint64_t &low,
uint64_t &high) {
llvm::ErrorOr<llvm::MD5::MD5Result>
Platform::CalculateMD5(const FileSpec &file_spec) {
if (!IsHost())
return false;
auto Result = llvm::sys::fs::md5_contents(file_spec.GetPath());
if (!Result)
return false;
std::tie(high, low) = Result->words();
return true;
return std::make_error_code(std::errc::not_supported);
return llvm::sys::fs::md5_contents(file_spec.GetPath());
}

void Platform::SetLocalCacheDirectory(const char *local) {
Expand Down
8 changes: 4 additions & 4 deletions lldb/source/Target/RemoteAwarePlatform.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -266,11 +266,11 @@ Status RemoteAwarePlatform::Unlink(const FileSpec &file_spec) {
return Platform::Unlink(file_spec);
}

bool RemoteAwarePlatform::CalculateMD5(const FileSpec &file_spec, uint64_t &low,
uint64_t &high) {
llvm::ErrorOr<llvm::MD5::MD5Result>
RemoteAwarePlatform::CalculateMD5(const FileSpec &file_spec) {
if (m_remote_platform_sp)
return m_remote_platform_sp->CalculateMD5(file_spec, low, high);
return Platform::CalculateMD5(file_spec, low, high);
return m_remote_platform_sp->CalculateMD5(file_spec);
return Platform::CalculateMD5(file_spec);
}

FileSpec RemoteAwarePlatform::GetRemoteWorkingDirectory() {
Expand Down