[lldb] Extract debug server location code #145706
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
.. from the guts of GDBRemoteCommunication to ~top level. This is motivated by llvm#131519 and by the fact that's impossible to guess whether the author of a symlink intended it to be a "convenience shortcut" -- meaning it should be resolved before looking for related files; or an "implementation detail" -- meaning the related files should be located near the symlink itself. This debate is particularly ridiculous when it comes to lldb-server running in platform mode, because it also functions as a debug server, so what we really just need to do is to pass /proc/self/exe in a platform-independent manner. Moving the location logic higher up achieves that as lldb-platform (on non-macos) can pass `HostInfo::GetProgramFileSpec`, while liblldb can use the existing complex logic (which only worked on liblldb anyway as lldb-platform doesn't have a lldb_private::Platform instance). Another benefit of this patch is a reduction in dependency from GDBRemoteCommunication to the rest of liblldb (achieved by avoiding the Platform dependency).
|
@llvm/pr-subscribers-lldb Author: Pavel Labath (labath) Changes.. from the guts of GDBRemoteCommunication to ~top level. This is motivated by #131519 and by the fact that's impossible to guess whether the author of a symlink intended it to be a "convenience shortcut" -- meaning it should be resolved before looking for related files; or an "implementation detail" -- meaning the related files should be located near the symlink itself. This debate is particularly ridiculous when it comes to lldb-server running in platform mode, because it also functions as a debug server, so what we really just need to do is to pass /proc/self/exe in a platform-independent manner. Moving the location logic higher up achieves that as lldb-platform (on non-macos) can pass Another benefit of this patch is a reduction in dependency from GDBRemoteCommunication to the rest of liblldb (achieved by avoiding the Platform dependency). Patch is 23.19 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/145706.diff 8 Files Affected:
diff --git a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp
index bea2faff2330e..cea553e0ac9a2 100644
--- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp
+++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp
@@ -11,7 +11,6 @@
#include "lldb/Host/Config.h"
#include "lldb/Host/FileSystem.h"
#include "lldb/Host/Host.h"
-#include "lldb/Host/HostInfo.h"
#include "lldb/Host/Pipe.h"
#include "lldb/Host/ProcessLaunchInfo.h"
#include "lldb/Host/Socket.h"
@@ -33,14 +32,6 @@
#include <sys/stat.h>
#include <variant>
-#if defined(__APPLE__)
-#define DEBUGSERVER_BASENAME "debugserver"
-#elif defined(_WIN32)
-#define DEBUGSERVER_BASENAME "lldb-server.exe"
-#else
-#define DEBUGSERVER_BASENAME "lldb-server"
-#endif
-
#if HAVE_LIBCOMPRESSION
#include <compression.h>
#endif
@@ -835,77 +826,11 @@ GDBRemoteCommunication::CheckForPacket(const uint8_t *src, size_t src_len,
return GDBRemoteCommunication::PacketType::Invalid;
}
-FileSpec GDBRemoteCommunication::GetDebugserverPath(Platform *platform) {
- Log *log = GetLog(GDBRLog::Process);
- // If we locate debugserver, keep that located version around
- static FileSpec g_debugserver_file_spec;
- FileSpec debugserver_file_spec;
-
- Environment host_env = Host::GetEnvironment();
-
- // Always check to see if we have an environment override for the path to the
- // debugserver to use and use it if we do.
- std::string env_debugserver_path = host_env.lookup("LLDB_DEBUGSERVER_PATH");
- if (!env_debugserver_path.empty()) {
- debugserver_file_spec.SetFile(env_debugserver_path,
- FileSpec::Style::native);
- LLDB_LOGF(log,
- "GDBRemoteCommunication::%s() gdb-remote stub exe path set "
- "from environment variable: %s",
- __FUNCTION__, env_debugserver_path.c_str());
- } else
- debugserver_file_spec = g_debugserver_file_spec;
- bool debugserver_exists =
- FileSystem::Instance().Exists(debugserver_file_spec);
- if (!debugserver_exists) {
- // The debugserver binary is in the LLDB.framework/Resources directory.
- debugserver_file_spec = HostInfo::GetSupportExeDir();
- if (debugserver_file_spec) {
- debugserver_file_spec.AppendPathComponent(DEBUGSERVER_BASENAME);
- debugserver_exists = FileSystem::Instance().Exists(debugserver_file_spec);
- if (debugserver_exists) {
- LLDB_LOGF(log,
- "GDBRemoteCommunication::%s() found gdb-remote stub exe '%s'",
- __FUNCTION__, debugserver_file_spec.GetPath().c_str());
-
- g_debugserver_file_spec = debugserver_file_spec;
- } else {
- if (platform)
- debugserver_file_spec =
- platform->LocateExecutable(DEBUGSERVER_BASENAME);
- else
- debugserver_file_spec.Clear();
- if (debugserver_file_spec) {
- // Platform::LocateExecutable() wouldn't return a path if it doesn't
- // exist
- debugserver_exists = true;
- } else {
- LLDB_LOGF(log,
- "GDBRemoteCommunication::%s() could not find "
- "gdb-remote stub exe '%s'",
- __FUNCTION__, debugserver_file_spec.GetPath().c_str());
- }
- // Don't cache the platform specific GDB server binary as it could
- // change from platform to platform
- g_debugserver_file_spec.Clear();
- }
- }
- }
- return debugserver_file_spec;
-}
-
Status GDBRemoteCommunication::StartDebugserverProcess(
- std::variant<llvm::StringRef, shared_fd_t> comm, Platform *platform,
+ std::variant<llvm::StringRef, shared_fd_t> comm,
ProcessLaunchInfo &launch_info, const Args *inferior_args) {
Log *log = GetLog(GDBRLog::Process);
- FileSpec debugserver_file_spec = GetDebugserverPath(platform);
- if (!debugserver_file_spec)
- return Status::FromErrorString("unable to locate " DEBUGSERVER_BASENAME);
-
- launch_info.SetExecutableFile(debugserver_file_spec,
- /*add_exe_file_as_first_arg=*/true);
-
Args &debugserver_args = launch_info.GetArguments();
#if !defined(__APPLE__)
diff --git a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.h b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.h
index 31f8edf715a3a..35bf5eb2e3f0d 100644
--- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.h
+++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.h
@@ -134,16 +134,12 @@ class GDBRemoteCommunication : public Communication {
std::chrono::seconds GetPacketTimeout() const { return m_packet_timeout; }
- // Get the debugserver path and check that it exist.
- static FileSpec GetDebugserverPath(Platform *platform);
-
// Start a debugserver instance on the current host using the
// supplied connection URL.
- static Status StartDebugserverProcess(
- std::variant<llvm::StringRef, shared_fd_t> comm,
- Platform *platform, // If non nullptr, then check with the platform for
- // the GDB server binary if it can't be located
- ProcessLaunchInfo &launch_info, const Args *inferior_args);
+ static Status
+ StartDebugserverProcess(std::variant<llvm::StringRef, shared_fd_t> comm,
+ ProcessLaunchInfo &launch_info,
+ const Args *inferior_args);
void DumpHistory(Stream &strm);
diff --git a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerPlatform.cpp b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerPlatform.cpp
index 7506cf64def38..5876c3a9434a1 100644
--- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerPlatform.cpp
+++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerPlatform.cpp
@@ -46,9 +46,10 @@ using namespace lldb_private;
// GDBRemoteCommunicationServerPlatform constructor
GDBRemoteCommunicationServerPlatform::GDBRemoteCommunicationServerPlatform(
- const Socket::SocketProtocol socket_protocol, uint16_t gdbserver_port)
- : GDBRemoteCommunicationServerCommon(), m_socket_protocol(socket_protocol),
- m_gdbserver_port(gdbserver_port) {
+ FileSpec debugserver_path, const Socket::SocketProtocol socket_protocol,
+ uint16_t gdbserver_port)
+ : m_debugserver_path(std::move(debugserver_path)),
+ m_socket_protocol(socket_protocol), m_gdbserver_port(gdbserver_port) {
RegisterMemberFunctionHandler(
StringExtractorGDBRemote::eServerPacketType_qC,
@@ -102,14 +103,15 @@ Status GDBRemoteCommunicationServerPlatform::LaunchGDBServer(
debugserver_launch_info.SetLaunchInSeparateProcessGroup(false);
debugserver_launch_info.SetMonitorProcessCallback(
[](lldb::pid_t, int, int) {});
+ if (!FileSystem::Instance().Exists(m_debugserver_path))
+ return Status::FromErrorString("debugserver does not exist");
+ debugserver_launch_info.SetExecutableFile(m_debugserver_path,
+ /*add_exe_file_as_first_arg=*/true);
Status error;
if (fd == SharedSocket::kInvalidFD) {
if (m_socket_protocol == Socket::ProtocolTcp) {
- // Just check that GDBServer exists. GDBServer must be launched after
- // accepting the connection.
- if (!GetDebugserverPath(nullptr))
- return Status::FromErrorString("unable to locate debugserver");
+ // The server will be launched after accepting the connection.
return Status();
}
@@ -120,13 +122,11 @@ Status GDBRemoteCommunicationServerPlatform::LaunchGDBServer(
#endif
socket_name = GetDomainSocketPath("gdbserver").GetPath();
url << socket_name;
- error = StartDebugserverProcess(url.str(), nullptr, debugserver_launch_info,
- &args);
+ error = StartDebugserverProcess(url.str(), debugserver_launch_info, &args);
} else {
if (m_socket_protocol != Socket::ProtocolTcp)
return Status::FromErrorString("protocol must be tcp");
- error =
- StartDebugserverProcess(fd, nullptr, debugserver_launch_info, &args);
+ error = StartDebugserverProcess(fd, debugserver_launch_info, &args);
}
if (error.Success()) {
diff --git a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerPlatform.h b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerPlatform.h
index beee61ea8048e..bcfb6d3f3ed02 100644
--- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerPlatform.h
+++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerPlatform.h
@@ -26,7 +26,8 @@ class GDBRemoteCommunicationServerPlatform
: public GDBRemoteCommunicationServerCommon {
public:
GDBRemoteCommunicationServerPlatform(
- const Socket::SocketProtocol socket_protocol, uint16_t gdbserver_port);
+ FileSpec debugserver_path, const Socket::SocketProtocol socket_protocol,
+ uint16_t gdbserver_port);
~GDBRemoteCommunicationServerPlatform() override;
@@ -40,6 +41,7 @@ class GDBRemoteCommunicationServerPlatform
void SetPendingGdbServer(const std::string &socket_name);
protected:
+ const FileSpec m_debugserver_path;
const Socket::SocketProtocol m_socket_protocol;
std::recursive_mutex m_spawned_pids_mutex;
std::set<lldb::pid_t> m_spawned_pids;
diff --git a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
index 3f9c4ddc60a25..a2c34ddfc252e 100644
--- a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
+++ b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
@@ -34,6 +34,7 @@
#include "lldb/DataFormatters/FormatManager.h"
#include "lldb/Host/ConnectionFileDescriptor.h"
#include "lldb/Host/FileSystem.h"
+#include "lldb/Host/HostInfo.h"
#include "lldb/Host/HostThread.h"
#include "lldb/Host/PosixApi.h"
#include "lldb/Host/PseudoTerminal.h"
@@ -92,7 +93,14 @@
#include "llvm/Support/Threading.h"
#include "llvm/Support/raw_ostream.h"
+#if defined(__APPLE__)
#define DEBUGSERVER_BASENAME "debugserver"
+#elif defined(_WIN32)
+#define DEBUGSERVER_BASENAME "lldb-server.exe"
+#else
+#define DEBUGSERVER_BASENAME "lldb-server"
+#endif
+
using namespace lldb;
using namespace lldb_private;
using namespace lldb_private::process_gdb_remote;
@@ -3448,6 +3456,51 @@ ProcessGDBRemote::EstablishConnectionIfNeeded(const ProcessInfo &process_info) {
return error;
}
+static FileSpec GetDebugserverPath(Platform &platform) {
+ Log *log = GetLog(GDBRLog::Process);
+ // If we locate debugserver, keep that located version around
+ static FileSpec g_debugserver_file_spec;
+ FileSpec debugserver_file_spec;
+
+ Environment host_env = Host::GetEnvironment();
+
+ // Always check to see if we have an environment override for the path to the
+ // debugserver to use and use it if we do.
+ std::string env_debugserver_path = host_env.lookup("LLDB_DEBUGSERVER_PATH");
+ if (!env_debugserver_path.empty()) {
+ debugserver_file_spec.SetFile(env_debugserver_path,
+ FileSpec::Style::native);
+ LLDB_LOG(log, "gdb-remote stub exe path set from environment variable: {0}",
+ env_debugserver_path);
+ } else
+ debugserver_file_spec = g_debugserver_file_spec;
+ if (FileSystem::Instance().Exists(debugserver_file_spec))
+ return debugserver_file_spec;
+
+ // The debugserver binary is in the LLDB.framework/Resources directory.
+ debugserver_file_spec = HostInfo::GetSupportExeDir();
+ if (debugserver_file_spec) {
+ debugserver_file_spec.AppendPathComponent(DEBUGSERVER_BASENAME);
+ if (FileSystem::Instance().Exists(debugserver_file_spec)) {
+ LLDB_LOG(log, "found gdb-remote stub exe '{0}'", debugserver_file_spec);
+
+ g_debugserver_file_spec = debugserver_file_spec;
+ } else {
+ debugserver_file_spec = platform.LocateExecutable(DEBUGSERVER_BASENAME);
+ if (!debugserver_file_spec) {
+ // Platform::LocateExecutable() wouldn't return a path if it doesn't
+ // exist
+ LLDB_LOG(log, "could not find gdb-remote stub exe '{0}'",
+ debugserver_file_spec);
+ }
+ // Don't cache the platform specific GDB server binary as it could
+ // change from platform to platform
+ g_debugserver_file_spec.Clear();
+ }
+ }
+ return debugserver_file_spec;
+}
+
Status ProcessGDBRemote::LaunchAndConnectToDebugserver(
const ProcessInfo &process_info) {
using namespace std::placeholders; // For _1, _2, etc.
@@ -3466,6 +3519,8 @@ Status ProcessGDBRemote::LaunchAndConnectToDebugserver(
std::bind(MonitorDebugserverProcess, this_wp, _1, _2, _3));
debugserver_launch_info.SetUserID(process_info.GetUserID());
+ FileSpec debugserver_path = GetDebugserverPath(*GetTarget().GetPlatform());
+
#if defined(__APPLE__)
// On macOS 11, we need to support x86_64 applications translated to
// arm64. We check whether a binary is translated and spawn the correct
@@ -3478,12 +3533,12 @@ Status ProcessGDBRemote::LaunchAndConnectToDebugserver(
NULL, 0) == 0 &&
bufsize > 0) {
if (processInfo.kp_proc.p_flag & P_TRANSLATED) {
- FileSpec rosetta_debugserver(
- "/Library/Apple/usr/libexec/oah/debugserver");
- debugserver_launch_info.SetExecutableFile(rosetta_debugserver, false);
+ debugserver_path = FileSpec("/Library/Apple/usr/libexec/oah/debugserver");
}
}
#endif
+ debugserver_launch_info.SetExecutableFile(debugserver_path,
+ /*add_exe_file_as_first_arg=*/true);
llvm::Expected<Socket::Pair> socket_pair = Socket::CreatePair();
if (!socket_pair)
@@ -3495,7 +3550,6 @@ Status ProcessGDBRemote::LaunchAndConnectToDebugserver(
return error;
error = m_gdb_comm.StartDebugserverProcess(shared_socket.GetSendableFD(),
- GetTarget().GetPlatform().get(),
debugserver_launch_info, nullptr);
if (error.Fail()) {
diff --git a/lldb/test/API/commands/platform/launchgdbserver/TestPlatformLaunchGDBServer.py b/lldb/test/API/commands/platform/launchgdbserver/TestPlatformLaunchGDBServer.py
index c365bc993e338..45d855f5c1938 100644
--- a/lldb/test/API/commands/platform/launchgdbserver/TestPlatformLaunchGDBServer.py
+++ b/lldb/test/API/commands/platform/launchgdbserver/TestPlatformLaunchGDBServer.py
@@ -1,8 +1,3 @@
-""" Check that errors while handling qLaunchGDBServer are reported to the user.
- Though this isn't a platform command in itself, the best way to test it is
- from Python because we can juggle multiple processes more easily.
-"""
-
import os
import socket
import shutil
@@ -15,14 +10,7 @@
class TestPlatformProcessLaunchGDBServer(TestBase):
NO_DEBUG_INFO_TESTCASE = True
- @skipIfRemote
- # Windows cannot delete the executable while it is running.
- # On Darwin we may be using debugserver.
- @skipUnlessPlatform(["linux"])
- @add_test_categories(["lldb-server"])
- def test_platform_process_launch_gdb_server(self):
- self.build()
-
+ def _launch_and_connect(self, exe):
hostname = socket.getaddrinfo("localhost", 0, proto=socket.IPPROTO_TCP)[0][4][0]
listen_url = "[%s]:0" % hostname
@@ -33,16 +21,9 @@ def test_platform_process_launch_gdb_server(self):
listen_url,
"--socket-file",
port_file,
- "--",
- self.getBuildArtifact("a.out"),
- "foo",
]
- # Run lldb-server from a new location.
- new_lldb_server = self.getBuildArtifact("lldb-server")
- shutil.copy(lldbgdbserverutils.get_lldb_server_exe(), new_lldb_server)
-
- self.spawnSubprocess(new_lldb_server, commandline_args)
+ self.spawnSubprocess(exe, commandline_args)
socket_id = lldbutil.wait_for_file_on_target(self, port_file)
new_platform = lldb.SBPlatform("remote-" + self.getPlatform())
@@ -51,10 +32,52 @@ def test_platform_process_launch_gdb_server(self):
connect_url = "connect://[%s]:%s" % (hostname, socket_id)
self.runCmd("platform connect %s" % connect_url)
- # First connect to lldb-server which spawn a process to handle the connection.
- # Then remove our new lldb-server so that when it tries to invoke itself as a
+ wd = self.getBuildArtifact("wd")
+ os.mkdir(wd)
+ new_platform.SetWorkingDirectory(wd)
+
+ @skipIfRemote
+ # Windows cannot delete the executable while it is running.
+ # On Darwin we may be using debugserver.
+ @skipUnlessPlatform(["linux"])
+ @add_test_categories(["lldb-server"])
+ def test_launch_error(self):
+ """
+ Check that errors while handling qLaunchGDBServer are reported to the
+ user. Though this isn't a platform command in itself, the best way to
+ test it is from Python because we can juggle multiple processes more
+ easily.
+ """
+
+ self.build()
+
+ # Run lldb-server from a new location.
+ new_lldb_server = self.getBuildArtifact("lldb-server")
+ shutil.copy(lldbgdbserverutils.get_lldb_server_exe(), new_lldb_server)
+ self._launch_and_connect(new_lldb_server)
+
+ # Now, remove our new lldb-server so that when it tries to invoke itself as a
# gdbserver, it fails.
os.remove(new_lldb_server)
self.runCmd("target create {}".format(self.getBuildArtifact("a.out")))
self.expect("run", substrs=["unable to launch a GDB server on"], error=True)
+
+ @skipIfRemote
+ @skipIfDarwin # Uses debugserver for debugging
+ @add_test_categories(["lldb-server"])
+ def test_launch_with_unusual_process_name(self):
+ """
+ Test that lldb-server can launch a debug session when running under an
+ unusual name (or under a symlink which resolves to an unusal name).
+ """
+
+ self.build()
+
+ # Run lldb-server from a new location.
+ new_lldb_server = self.getBuildArtifact("obfuscated-server")
+ shutil.copy(lldbgdbserverutils.get_lldb_server_exe(), new_lldb_server)
+ self._launch_and_connect(new_lldb_server)
+
+ self.runCmd("target create {}".format(self.getBuildArtifact("a.out")))
+ self.expect("run", substrs=["exited with status = 0"])
diff --git a/lldb/test/API/tools/lldb-dap/console/TestDAP_console.py b/lldb/test/API/tools/lldb-dap/console/TestDAP_console.py
index 7b4d1adbb2071..811843dfdf7af 100644
--- a/lldb/test/API/tools/lldb-dap/console/TestDAP_console.py
+++ b/lldb/test/API/tools/lldb-dap/console/TestDAP_console.py
@@ -144,9 +144,9 @@ def test_exit_status_message_sigterm(self):
)
# Verify the exit status message is printed.
- self.assertIn(
- "exited with status = -1 (0xffffffff) debugserver died with signal SIGTERM",
+ self.assertRegex(
console_output,
+ ".*exited with status = -1 .* died with signal SIGTERM.*",
"Exit status does not contain message 'exited with status'",
)
diff --git a/lldb/tools/lldb-server/lldb-platform.cpp b/lldb/tools/lldb-server/lldb-platform.cpp
index 3c79dc001f65e..0bd928507ba89 100644
--- a/lldb/tools/lldb-server/lldb-platform.cpp
+++ b/lldb/tools/lldb-server/lldb-platform.cpp
@@ -30,6 +30,7 @@
#include "Plugins/Process/gdb-remote/GDBRemoteCommunicationServerPlatform.h"
#include "Plugins/Process/gdb-remote/ProcessGDBRemoteLog.h"
#include "lldb/Host/ConnectionFileDescriptor.h"
+#include "lldb/Host/FileSystem.h"
#include "lldb/Host/HostGetOpt.h"
#include "lldb/Host/HostInfo.h"
#include "lldb/Host/MainLoop.h"
@@ -192,14 +193,15 @@ static Status ListenGdbConnectionsIfNeeded(
}
static llvm::Expected<std::vector<MainLoopBase::ReadHandleUP>>
-AcceptGdbConnectionsIfNeeded(const Socket::SocketProtocol protocol,
+AcceptGdbConnectionsIfNeeded(const FileSpec &debugserver_path,
+ const Socket::SocketProtocol protocol,
std::unique_ptr<TCPSocket> &gdb_sock,
MainLoop &main_loop, const uint16_t gdbserver_port,
const lldb_private::Args &args) {
if (protocol != Socket::ProtocolTcp)
return std::vector<MainLoopBase::ReadHandleUP>();
- return gdb_sock->Accept(main_loop, [gdbserve...
[truncated]
|
There was a problem hiding this comment.
Thanks, and sorry for not getting back to the Mac issue I said I'd debug, I've been preoccupied lately.
To make sure I understand, this change hoists the logic that discovers the location of lldb-server (the function GetDebugserverPath) out of the unified logic of launching a debug server and up to ProcessGDBRemote, allowing lldb-server platform mode to supply itself as the debug server path, whereas other use cases that go through ProcessGDBRemote still get the old location-discovery logic, right? (which use cases are those? launching a debug server from lldb itself?)
That's okay, no worries.
That's completely correct. There are two exactly two use cases for launching lldb-gdb-server, one from (lib)lldb, and one from lldb-server-platform. This lets each implement discovery on its own (among other things). |
rlavaee
pushed a commit
to rlavaee/llvm-project
that referenced
this pull request
Jul 1, 2025
.. from the guts of GDBRemoteCommunication to ~top level. This is motivated by llvm#131519 and by the fact that's impossible to guess whether the author of a symlink intended it to be a "convenience shortcut" -- meaning it should be resolved before looking for related files; or an "implementation detail" -- meaning the related files should be located near the symlink itself. This debate is particularly ridiculous when it comes to lldb-server running in platform mode, because it also functions as a debug server, so what we really just need to do is to pass /proc/self/exe in a platform-independent manner. Moving the location logic higher up achieves that as lldb-platform (on non-macos) can pass `HostInfo::GetProgramFileSpec`, while liblldb can use the existing complex logic (which only worked on liblldb anyway as lldb-platform doesn't have a lldb_private::Platform instance). Another benefit of this patch is a reduction in dependency from GDBRemoteCommunication to the rest of liblldb (achieved by avoiding the Platform dependency).
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
.. from the guts of GDBRemoteCommunication to ~top level.
This is motivated by #131519 and by the fact that's impossible to guess whether the author of a symlink intended it to be a "convenience shortcut" -- meaning it should be resolved before looking for related files; or an "implementation detail" -- meaning the related files should be located near the symlink itself.
This debate is particularly ridiculous when it comes to lldb-server running in platform mode, because it also functions as a debug server, so what we really just need to do is to pass /proc/self/exe in a platform-independent manner.
Moving the location logic higher up achieves that as lldb-platform (on non-macos) can pass
HostInfo::GetProgramFileSpec, while liblldb can use the existing complex logic (which only worked on liblldb anyway as lldb-platform doesn't have a lldb_private::Platform instance).Another benefit of this patch is a reduction in dependency from GDBRemoteCommunication to the rest of liblldb (achieved by avoiding the Platform dependency).