[lldb] Extract debug server location code

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__)

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);
 

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()) {

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;

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()) {

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"])