Skip to content

Commit cae4d02

Browse files
labathrlavaee
authored andcommitted
[lldb] Extract debug server location code (llvm#145706)
.. 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).
1 parent 776589a commit cae4d02

File tree

8 files changed

+160
-133
lines changed

8 files changed

+160
-133
lines changed

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

Lines changed: 1 addition & 76 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,6 @@
1111
#include "lldb/Host/Config.h"
1212
#include "lldb/Host/FileSystem.h"
1313
#include "lldb/Host/Host.h"
14-
#include "lldb/Host/HostInfo.h"
1514
#include "lldb/Host/Pipe.h"
1615
#include "lldb/Host/ProcessLaunchInfo.h"
1716
#include "lldb/Host/Socket.h"
@@ -33,14 +32,6 @@
3332
#include <sys/stat.h>
3433
#include <variant>
3534

36-
#if defined(__APPLE__)
37-
#define DEBUGSERVER_BASENAME "debugserver"
38-
#elif defined(_WIN32)
39-
#define DEBUGSERVER_BASENAME "lldb-server.exe"
40-
#else
41-
#define DEBUGSERVER_BASENAME "lldb-server"
42-
#endif
43-
4435
#if HAVE_LIBCOMPRESSION
4536
#include <compression.h>
4637
#endif
@@ -836,77 +827,11 @@ GDBRemoteCommunication::CheckForPacket(const uint8_t *src, size_t src_len,
836827
return GDBRemoteCommunication::PacketType::Invalid;
837828
}
838829

839-
FileSpec GDBRemoteCommunication::GetDebugserverPath(Platform *platform) {
840-
Log *log = GetLog(GDBRLog::Process);
841-
// If we locate debugserver, keep that located version around
842-
static FileSpec g_debugserver_file_spec;
843-
FileSpec debugserver_file_spec;
844-
845-
Environment host_env = Host::GetEnvironment();
846-
847-
// Always check to see if we have an environment override for the path to the
848-
// debugserver to use and use it if we do.
849-
std::string env_debugserver_path = host_env.lookup("LLDB_DEBUGSERVER_PATH");
850-
if (!env_debugserver_path.empty()) {
851-
debugserver_file_spec.SetFile(env_debugserver_path,
852-
FileSpec::Style::native);
853-
LLDB_LOGF(log,
854-
"GDBRemoteCommunication::%s() gdb-remote stub exe path set "
855-
"from environment variable: %s",
856-
__FUNCTION__, env_debugserver_path.c_str());
857-
} else
858-
debugserver_file_spec = g_debugserver_file_spec;
859-
bool debugserver_exists =
860-
FileSystem::Instance().Exists(debugserver_file_spec);
861-
if (!debugserver_exists) {
862-
// The debugserver binary is in the LLDB.framework/Resources directory.
863-
debugserver_file_spec = HostInfo::GetSupportExeDir();
864-
if (debugserver_file_spec) {
865-
debugserver_file_spec.AppendPathComponent(DEBUGSERVER_BASENAME);
866-
debugserver_exists = FileSystem::Instance().Exists(debugserver_file_spec);
867-
if (debugserver_exists) {
868-
LLDB_LOGF(log,
869-
"GDBRemoteCommunication::%s() found gdb-remote stub exe '%s'",
870-
__FUNCTION__, debugserver_file_spec.GetPath().c_str());
871-
872-
g_debugserver_file_spec = debugserver_file_spec;
873-
} else {
874-
if (platform)
875-
debugserver_file_spec =
876-
platform->LocateExecutable(DEBUGSERVER_BASENAME);
877-
else
878-
debugserver_file_spec.Clear();
879-
if (debugserver_file_spec) {
880-
// Platform::LocateExecutable() wouldn't return a path if it doesn't
881-
// exist
882-
debugserver_exists = true;
883-
} else {
884-
LLDB_LOGF(log,
885-
"GDBRemoteCommunication::%s() could not find "
886-
"gdb-remote stub exe '%s'",
887-
__FUNCTION__, debugserver_file_spec.GetPath().c_str());
888-
}
889-
// Don't cache the platform specific GDB server binary as it could
890-
// change from platform to platform
891-
g_debugserver_file_spec.Clear();
892-
}
893-
}
894-
}
895-
return debugserver_file_spec;
896-
}
897-
898830
Status GDBRemoteCommunication::StartDebugserverProcess(
899-
std::variant<llvm::StringRef, shared_fd_t> comm, Platform *platform,
831+
std::variant<llvm::StringRef, shared_fd_t> comm,
900832
ProcessLaunchInfo &launch_info, const Args *inferior_args) {
901833
Log *log = GetLog(GDBRLog::Process);
902834

903-
FileSpec debugserver_file_spec = GetDebugserverPath(platform);
904-
if (!debugserver_file_spec)
905-
return Status::FromErrorString("unable to locate " DEBUGSERVER_BASENAME);
906-
907-
launch_info.SetExecutableFile(debugserver_file_spec,
908-
/*add_exe_file_as_first_arg=*/true);
909-
910835
Args &debugserver_args = launch_info.GetArguments();
911836

912837
#if !defined(__APPLE__)

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

Lines changed: 4 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -134,16 +134,12 @@ class GDBRemoteCommunication : public Communication {
134134

135135
std::chrono::seconds GetPacketTimeout() const { return m_packet_timeout; }
136136

137-
// Get the debugserver path and check that it exist.
138-
static FileSpec GetDebugserverPath(Platform *platform);
139-
140137
// Start a debugserver instance on the current host using the
141138
// supplied connection URL.
142-
static Status StartDebugserverProcess(
143-
std::variant<llvm::StringRef, shared_fd_t> comm,
144-
Platform *platform, // If non nullptr, then check with the platform for
145-
// the GDB server binary if it can't be located
146-
ProcessLaunchInfo &launch_info, const Args *inferior_args);
139+
static Status
140+
StartDebugserverProcess(std::variant<llvm::StringRef, shared_fd_t> comm,
141+
ProcessLaunchInfo &launch_info,
142+
const Args *inferior_args);
147143

148144
void DumpHistory(Stream &strm);
149145

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

Lines changed: 11 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -46,9 +46,10 @@ using namespace lldb_private;
4646

4747
// GDBRemoteCommunicationServerPlatform constructor
4848
GDBRemoteCommunicationServerPlatform::GDBRemoteCommunicationServerPlatform(
49-
const Socket::SocketProtocol socket_protocol, uint16_t gdbserver_port)
50-
: GDBRemoteCommunicationServerCommon(), m_socket_protocol(socket_protocol),
51-
m_gdbserver_port(gdbserver_port) {
49+
FileSpec debugserver_path, const Socket::SocketProtocol socket_protocol,
50+
uint16_t gdbserver_port)
51+
: m_debugserver_path(std::move(debugserver_path)),
52+
m_socket_protocol(socket_protocol), m_gdbserver_port(gdbserver_port) {
5253

5354
RegisterMemberFunctionHandler(
5455
StringExtractorGDBRemote::eServerPacketType_qC,
@@ -102,14 +103,15 @@ Status GDBRemoteCommunicationServerPlatform::LaunchGDBServer(
102103
debugserver_launch_info.SetLaunchInSeparateProcessGroup(false);
103104
debugserver_launch_info.SetMonitorProcessCallback(
104105
[](lldb::pid_t, int, int) {});
106+
if (!FileSystem::Instance().Exists(m_debugserver_path))
107+
return Status::FromErrorString("debugserver does not exist");
108+
debugserver_launch_info.SetExecutableFile(m_debugserver_path,
109+
/*add_exe_file_as_first_arg=*/true);
105110

106111
Status error;
107112
if (fd == SharedSocket::kInvalidFD) {
108113
if (m_socket_protocol == Socket::ProtocolTcp) {
109-
// Just check that GDBServer exists. GDBServer must be launched after
110-
// accepting the connection.
111-
if (!GetDebugserverPath(nullptr))
112-
return Status::FromErrorString("unable to locate debugserver");
114+
// The server will be launched after accepting the connection.
113115
return Status();
114116
}
115117

@@ -120,13 +122,11 @@ Status GDBRemoteCommunicationServerPlatform::LaunchGDBServer(
120122
#endif
121123
socket_name = GetDomainSocketPath("gdbserver").GetPath();
122124
url << socket_name;
123-
error = StartDebugserverProcess(url.str(), nullptr, debugserver_launch_info,
124-
&args);
125+
error = StartDebugserverProcess(url.str(), debugserver_launch_info, &args);
125126
} else {
126127
if (m_socket_protocol != Socket::ProtocolTcp)
127128
return Status::FromErrorString("protocol must be tcp");
128-
error =
129-
StartDebugserverProcess(fd, nullptr, debugserver_launch_info, &args);
129+
error = StartDebugserverProcess(fd, debugserver_launch_info, &args);
130130
}
131131

132132
if (error.Success()) {

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

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,8 @@ class GDBRemoteCommunicationServerPlatform
2626
: public GDBRemoteCommunicationServerCommon {
2727
public:
2828
GDBRemoteCommunicationServerPlatform(
29-
const Socket::SocketProtocol socket_protocol, uint16_t gdbserver_port);
29+
FileSpec debugserver_path, const Socket::SocketProtocol socket_protocol,
30+
uint16_t gdbserver_port);
3031

3132
~GDBRemoteCommunicationServerPlatform() override;
3233

@@ -40,6 +41,7 @@ class GDBRemoteCommunicationServerPlatform
4041
void SetPendingGdbServer(const std::string &socket_name);
4142

4243
protected:
44+
const FileSpec m_debugserver_path;
4345
const Socket::SocketProtocol m_socket_protocol;
4446
std::recursive_mutex m_spawned_pids_mutex;
4547
std::set<lldb::pid_t> m_spawned_pids;

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

Lines changed: 58 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@
3434
#include "lldb/DataFormatters/FormatManager.h"
3535
#include "lldb/Host/ConnectionFileDescriptor.h"
3636
#include "lldb/Host/FileSystem.h"
37+
#include "lldb/Host/HostInfo.h"
3738
#include "lldb/Host/HostThread.h"
3839
#include "lldb/Host/PosixApi.h"
3940
#include "lldb/Host/PseudoTerminal.h"
@@ -92,7 +93,14 @@
9293
#include "llvm/Support/Threading.h"
9394
#include "llvm/Support/raw_ostream.h"
9495

96+
#if defined(__APPLE__)
9597
#define DEBUGSERVER_BASENAME "debugserver"
98+
#elif defined(_WIN32)
99+
#define DEBUGSERVER_BASENAME "lldb-server.exe"
100+
#else
101+
#define DEBUGSERVER_BASENAME "lldb-server"
102+
#endif
103+
96104
using namespace lldb;
97105
using namespace lldb_private;
98106
using namespace lldb_private::process_gdb_remote;
@@ -3448,6 +3456,51 @@ ProcessGDBRemote::EstablishConnectionIfNeeded(const ProcessInfo &process_info) {
34483456
return error;
34493457
}
34503458

3459+
static FileSpec GetDebugserverPath(Platform &platform) {
3460+
Log *log = GetLog(GDBRLog::Process);
3461+
// If we locate debugserver, keep that located version around
3462+
static FileSpec g_debugserver_file_spec;
3463+
FileSpec debugserver_file_spec;
3464+
3465+
Environment host_env = Host::GetEnvironment();
3466+
3467+
// Always check to see if we have an environment override for the path to the
3468+
// debugserver to use and use it if we do.
3469+
std::string env_debugserver_path = host_env.lookup("LLDB_DEBUGSERVER_PATH");
3470+
if (!env_debugserver_path.empty()) {
3471+
debugserver_file_spec.SetFile(env_debugserver_path,
3472+
FileSpec::Style::native);
3473+
LLDB_LOG(log, "gdb-remote stub exe path set from environment variable: {0}",
3474+
env_debugserver_path);
3475+
} else
3476+
debugserver_file_spec = g_debugserver_file_spec;
3477+
if (FileSystem::Instance().Exists(debugserver_file_spec))
3478+
return debugserver_file_spec;
3479+
3480+
// The debugserver binary is in the LLDB.framework/Resources directory.
3481+
debugserver_file_spec = HostInfo::GetSupportExeDir();
3482+
if (debugserver_file_spec) {
3483+
debugserver_file_spec.AppendPathComponent(DEBUGSERVER_BASENAME);
3484+
if (FileSystem::Instance().Exists(debugserver_file_spec)) {
3485+
LLDB_LOG(log, "found gdb-remote stub exe '{0}'", debugserver_file_spec);
3486+
3487+
g_debugserver_file_spec = debugserver_file_spec;
3488+
} else {
3489+
debugserver_file_spec = platform.LocateExecutable(DEBUGSERVER_BASENAME);
3490+
if (!debugserver_file_spec) {
3491+
// Platform::LocateExecutable() wouldn't return a path if it doesn't
3492+
// exist
3493+
LLDB_LOG(log, "could not find gdb-remote stub exe '{0}'",
3494+
debugserver_file_spec);
3495+
}
3496+
// Don't cache the platform specific GDB server binary as it could
3497+
// change from platform to platform
3498+
g_debugserver_file_spec.Clear();
3499+
}
3500+
}
3501+
return debugserver_file_spec;
3502+
}
3503+
34513504
Status ProcessGDBRemote::LaunchAndConnectToDebugserver(
34523505
const ProcessInfo &process_info) {
34533506
using namespace std::placeholders; // For _1, _2, etc.
@@ -3466,6 +3519,8 @@ Status ProcessGDBRemote::LaunchAndConnectToDebugserver(
34663519
std::bind(MonitorDebugserverProcess, this_wp, _1, _2, _3));
34673520
debugserver_launch_info.SetUserID(process_info.GetUserID());
34683521

3522+
FileSpec debugserver_path = GetDebugserverPath(*GetTarget().GetPlatform());
3523+
34693524
#if defined(__APPLE__)
34703525
// On macOS 11, we need to support x86_64 applications translated to
34713526
// arm64. We check whether a binary is translated and spawn the correct
@@ -3478,12 +3533,12 @@ Status ProcessGDBRemote::LaunchAndConnectToDebugserver(
34783533
NULL, 0) == 0 &&
34793534
bufsize > 0) {
34803535
if (processInfo.kp_proc.p_flag & P_TRANSLATED) {
3481-
FileSpec rosetta_debugserver(
3482-
"/Library/Apple/usr/libexec/oah/debugserver");
3483-
debugserver_launch_info.SetExecutableFile(rosetta_debugserver, false);
3536+
debugserver_path = FileSpec("/Library/Apple/usr/libexec/oah/debugserver");
34843537
}
34853538
}
34863539
#endif
3540+
debugserver_launch_info.SetExecutableFile(debugserver_path,
3541+
/*add_exe_file_as_first_arg=*/true);
34873542

34883543
llvm::Expected<Socket::Pair> socket_pair = Socket::CreatePair();
34893544
if (!socket_pair)
@@ -3495,7 +3550,6 @@ Status ProcessGDBRemote::LaunchAndConnectToDebugserver(
34953550
return error;
34963551

34973552
error = m_gdb_comm.StartDebugserverProcess(shared_socket.GetSendableFD(),
3498-
GetTarget().GetPlatform().get(),
34993553
debugserver_launch_info, nullptr);
35003554

35013555
if (error.Fail()) {
Lines changed: 46 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,3 @@
1-
""" Check that errors while handling qLaunchGDBServer are reported to the user.
2-
Though this isn't a platform command in itself, the best way to test it is
3-
from Python because we can juggle multiple processes more easily.
4-
"""
5-
61
import os
72
import socket
83
import shutil
@@ -15,14 +10,7 @@
1510
class TestPlatformProcessLaunchGDBServer(TestBase):
1611
NO_DEBUG_INFO_TESTCASE = True
1712

18-
@skipIfRemote
19-
# Windows cannot delete the executable while it is running.
20-
# On Darwin we may be using debugserver.
21-
@skipUnlessPlatform(["linux"])
22-
@add_test_categories(["lldb-server"])
23-
def test_platform_process_launch_gdb_server(self):
24-
self.build()
25-
13+
def _launch_and_connect(self, exe):
2614
hostname = socket.getaddrinfo("localhost", 0, proto=socket.IPPROTO_TCP)[0][4][0]
2715
listen_url = "[%s]:0" % hostname
2816

@@ -33,16 +21,9 @@ def test_platform_process_launch_gdb_server(self):
3321
listen_url,
3422
"--socket-file",
3523
port_file,
36-
"--",
37-
self.getBuildArtifact("a.out"),
38-
"foo",
3924
]
4025

41-
# Run lldb-server from a new location.
42-
new_lldb_server = self.getBuildArtifact("lldb-server")
43-
shutil.copy(lldbgdbserverutils.get_lldb_server_exe(), new_lldb_server)
44-
45-
self.spawnSubprocess(new_lldb_server, commandline_args)
26+
self.spawnSubprocess(exe, commandline_args)
4627
socket_id = lldbutil.wait_for_file_on_target(self, port_file)
4728

4829
new_platform = lldb.SBPlatform("remote-" + self.getPlatform())
@@ -51,10 +32,52 @@ def test_platform_process_launch_gdb_server(self):
5132
connect_url = "connect://[%s]:%s" % (hostname, socket_id)
5233
self.runCmd("platform connect %s" % connect_url)
5334

54-
# First connect to lldb-server which spawn a process to handle the connection.
55-
# Then remove our new lldb-server so that when it tries to invoke itself as a
35+
wd = self.getBuildArtifact("wd")
36+
os.mkdir(wd)
37+
new_platform.SetWorkingDirectory(wd)
38+
39+
@skipIfRemote
40+
# Windows cannot delete the executable while it is running.
41+
# On Darwin we may be using debugserver.
42+
@skipUnlessPlatform(["linux"])
43+
@add_test_categories(["lldb-server"])
44+
def test_launch_error(self):
45+
"""
46+
Check that errors while handling qLaunchGDBServer are reported to the
47+
user. Though this isn't a platform command in itself, the best way to
48+
test it is from Python because we can juggle multiple processes more
49+
easily.
50+
"""
51+
52+
self.build()
53+
54+
# Run lldb-server from a new location.
55+
new_lldb_server = self.getBuildArtifact("lldb-server")
56+
shutil.copy(lldbgdbserverutils.get_lldb_server_exe(), new_lldb_server)
57+
self._launch_and_connect(new_lldb_server)
58+
59+
# Now, remove our new lldb-server so that when it tries to invoke itself as a
5660
# gdbserver, it fails.
5761
os.remove(new_lldb_server)
5862

5963
self.runCmd("target create {}".format(self.getBuildArtifact("a.out")))
6064
self.expect("run", substrs=["unable to launch a GDB server on"], error=True)
65+
66+
@skipIfRemote
67+
@skipIfDarwin # Uses debugserver for debugging
68+
@add_test_categories(["lldb-server"])
69+
def test_launch_with_unusual_process_name(self):
70+
"""
71+
Test that lldb-server can launch a debug session when running under an
72+
unusual name (or under a symlink which resolves to an unusal name).
73+
"""
74+
75+
self.build()
76+
77+
# Run lldb-server from a new location.
78+
new_lldb_server = self.getBuildArtifact("obfuscated-server")
79+
shutil.copy(lldbgdbserverutils.get_lldb_server_exe(), new_lldb_server)
80+
self._launch_and_connect(new_lldb_server)
81+
82+
self.runCmd("target create {}".format(self.getBuildArtifact("a.out")))
83+
self.expect("run", substrs=["exited with status = 0"])

0 commit comments

Comments
 (0)