[Lldb-commits] [lldb] [lldb] add a check using an MD5 hash for whether a file needs to be installed on the remote target (PR #108996)
via lldb-commits
lldb-commits at lists.llvm.org
Tue Sep 17 08:41:35 PDT 2024
https://github.com/dlav-sc created https://github.com/llvm/llvm-project/pull/108996
Lldb documentation says that during remote debugging lldb client should check the existence of the desired executable file on the remote target and only if the file is missing copy it there. There is no such check in fact, so during an attempt to copy the executable ETXTBSY error can occur. This patch adds the check using a MD5 file hash value.
Fixed tests:
TestProcessHandle.py
TestFdLeak.py
>From a647e415d2a2407f87d931ba968ffa6c4f81575e Mon Sep 17 00:00:00 2001
From: Daniil Avdeev <daniil.avdeev at syntacore.com>
Date: Tue, 23 Jul 2024 11:08:13 +0000
Subject: [PATCH] [lldb] add a check using an MD5 hash for whether a file needs
to be installed on the remote target
Lldb documentation says that during remote debugging lldb client
should check the existence of desired executable file on a remote
tagret and only if the file is missing copy it there. There is no
such check in fact, so during an attempt to copy the executable
ETXTBSY error can occur. This patch adds the check using a MD5 file
hash value.
Fixed tests:
TestProcessHandle.py
TestFdLeak.py
---
lldb/source/Target/Target.cpp | 165 +++++++++++++++++++++++++---------
1 file changed, 125 insertions(+), 40 deletions(-)
diff --git a/lldb/source/Target/Target.cpp b/lldb/source/Target/Target.cpp
index f1659aae0800db..8f1cfc7b4f0b67 100644
--- a/lldb/source/Target/Target.cpp
+++ b/lldb/source/Target/Target.cpp
@@ -76,6 +76,106 @@
using namespace lldb;
using namespace lldb_private;
+namespace {
+
+struct ExecutableInstaller {
+
+ ExecutableInstaller(PlatformSP platform, ModuleSP module)
+ : m_platform{platform}, m_module{module},
+ m_local_file{m_module->GetFileSpec()},
+ m_remote_file{m_module->GetRemoteInstallFileSpec()} {}
+
+ void setRemoteFile() const { m_module->SetPlatformFileSpec(m_remote_file); }
+
+ PlatformSP m_platform;
+ ModuleSP m_module;
+ const FileSpec m_local_file;
+ const FileSpec m_remote_file;
+};
+
+struct MainExecutableInstaller {
+
+ MainExecutableInstaller(PlatformSP platform, TargetSP target, ModuleSP module,
+ ProcessLaunchInfo *launch_info)
+ : m_platform{platform}, m_target{target}, m_module{module},
+ m_local_file{m_module->GetFileSpec()},
+ m_remote_file{
+ getRemoteFileSpec(m_platform, m_target, m_module, m_local_file)},
+ m_launch_info{launch_info} {}
+
+ void setRemoteFile() const {
+ m_module->SetPlatformFileSpec(m_remote_file);
+ m_launch_info->SetExecutableFile(m_remote_file,
+ false /*add_exe_file_as_first_arg*/);
+ m_platform->SetFilePermissions(m_remote_file, 0700 /*-rwx------*/);
+ }
+
+ PlatformSP m_platform;
+ TargetSP m_target;
+ ModuleSP m_module;
+ const FileSpec m_local_file;
+ const FileSpec m_remote_file;
+ ProcessLaunchInfo *m_launch_info;
+
+private:
+ // Creates a filename "<local_file_name>_<local_file_md5_hash>" for a file
+ // on a remote target. This is needed to check existance of the file on a
+ // remote machine and then install it if the file is missing.
+ static std::optional<std::string>
+ getRemoteFileName(const FileSpec &local_file) {
+ auto local_md5 = llvm::sys::fs::md5_contents(local_file.GetPath());
+ if (!local_md5)
+ return std::nullopt;
+
+ auto [high, low] = local_md5->words();
+
+ std::stringstream ss;
+ ss << local_file.GetFilename().GetCString() << "_" << high << low;
+ return ss.str();
+ }
+
+ static FileSpec getRemoteFileSpec(PlatformSP platform, TargetSP target,
+ ModuleSP module,
+ const FileSpec &local_file) {
+ FileSpec remote_file = module->GetRemoteInstallFileSpec();
+ if (remote_file || !target->GetAutoInstallMainExecutable())
+ return remote_file;
+
+ if (!local_file)
+ return {};
+
+ auto remote_filename = getRemoteFileName(local_file);
+ if (!remote_filename)
+ return {};
+
+ remote_file = platform->GetRemoteWorkingDirectory();
+ remote_file.AppendPathComponent(remote_filename.value());
+
+ return remote_file;
+ }
+};
+} // namespace
+
+template <typename Installer>
+static Status installExecutable(const Installer &installer) {
+ // Firstly check is the file already exists on a remote machine
+ if (installer.m_platform->GetFileExists(installer.m_remote_file)) {
+ installer.setRemoteFile();
+ return Status();
+ }
+
+ if (!installer.m_local_file)
+ return Status();
+
+ Status error = installer.m_platform->Install(installer.m_local_file,
+ installer.m_remote_file);
+ if (error.Fail())
+ return error;
+
+ installer.setRemoteFile();
+ return Status();
+}
+
constexpr std::chrono::milliseconds EvaluateExpressionOptions::default_timeout;
Target::Arch::Arch(const ArchSpec &spec)
@@ -3076,48 +3176,33 @@ TargetProperties &Target::GetGlobalProperties() {
Status Target::Install(ProcessLaunchInfo *launch_info) {
Status error;
PlatformSP platform_sp(GetPlatform());
- if (platform_sp) {
- if (platform_sp->IsRemote()) {
- if (platform_sp->IsConnected()) {
- // Install all files that have an install path when connected to a
- // remote platform. If target.auto-install-main-executable is set then
- // also install the main executable even if it does not have an explicit
- // install path specified.
- const ModuleList &modules = GetImages();
- const size_t num_images = modules.GetSize();
- for (size_t idx = 0; idx < num_images; ++idx) {
- ModuleSP module_sp(modules.GetModuleAtIndex(idx));
- if (module_sp) {
- const bool is_main_executable = module_sp == GetExecutableModule();
- FileSpec local_file(module_sp->GetFileSpec());
- if (local_file) {
- FileSpec remote_file(module_sp->GetRemoteInstallFileSpec());
- if (!remote_file) {
- if (is_main_executable && GetAutoInstallMainExecutable()) {
- // Automatically install the main executable.
- remote_file = platform_sp->GetRemoteWorkingDirectory();
- remote_file.AppendPathComponent(
- module_sp->GetFileSpec().GetFilename().GetCString());
- }
- }
- if (remote_file) {
- error = platform_sp->Install(local_file, remote_file);
- if (error.Success()) {
- module_sp->SetPlatformFileSpec(remote_file);
- if (is_main_executable) {
- platform_sp->SetFilePermissions(remote_file, 0700);
- if (launch_info)
- launch_info->SetExecutableFile(remote_file, false);
- }
- } else
- break;
- }
- }
- }
- }
- }
+ if (!platform_sp || !platform_sp->IsRemote() || !platform_sp->IsConnected())
+ return error;
+
+ // Install all files that have an install path when connected to a
+ // remote platform. If target.auto-install-main-executable is set then
+ // also install the main executable even if it does not have an explicit
+ // install path specified.
+
+ for (auto module_sp : GetImages().Modules()) {
+ if (module_sp == GetExecutableModule()) {
+ MainExecutableInstaller installer{platform_sp, shared_from_this(),
+ module_sp, launch_info};
+ if (installer.m_remote_file)
+ error = installExecutable(installer);
+ else if (GetAutoInstallMainExecutable())
+ error.SetErrorStringWithFormatv("Target::{0}() can't get a remote file",
+ __FUNCTION__);
+ } else {
+ ExecutableInstaller installer{platform_sp, module_sp};
+ if (installer.m_remote_file)
+ error = installExecutable(installer);
}
+
+ if (error.Fail())
+ return error;
}
+
return error;
}
More information about the lldb-commits
mailing list