Don't include UnixSignals.h from Host

[zturner]

Summary

This file lives in Target, which makes sense since it is supposed to represent the various types of signals that can occur on any platform (since you might be remote debugging a platform with different signals than the host).

However, Host had a method called GetUnixSignals which just returned an instance of this class initialized for the Host's signals. Rather than have this functionality in Host, we can just provide a static factory method on UnixSignals called CreateForHost. This brings us one step closer to breaking the Target -> Host -> Target cycle.

[labath]

The rationale you give for this class being in Target is sound from the POV of lldb client. (the CreateForHost function is slightly fishy, but not too much.) However, it starts to break down when you start to consider lldb-server, whose large chunk of responsibility revolves around signals, but it doesn't need anything else in the Target module. For that, I think the ultimate place of UnixSignals should be somewhere else (I was planning to try to move it to Utility, which would mean that GetHostSignals could stay where it is).

However, we still have a long way to go before we can make lldb-server more independent, and the changes you make here are fairly small and easy to back out if needed. Since they solve the immediate problems with the Host module, I think it's fine to do this change now, and then revisit the placement of this class later.

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

401

Despite being burried in Plugins/Process/gdb-remote, this code is actually part of lldb-server (and only lldb-server).

lldb/include/lldb/Host/Host.h

static uint32_t FindProcesses(const ProcessInstanceInfoMatch &match_info,
                                 ProcessInstanceInfoList &proc_infos);
 
   typedef std::map<lldb::pid_t, bool> TidMap;
   typedef std::pair<lldb::pid_t, bool> TidPair;
   static bool FindProcessThreads(const lldb::pid_t pid, TidMap &tids_to_attach);
 
   static bool GetProcessInfo(lldb::pid_t pid, ProcessInstanceInfo &proc_info);
 
-  static const lldb::UnixSignalsSP &GetUnixSignals();
-
   /// Launch the process specified in launch_info. The monitoring callback in
   /// launch_info must be set, and it will be called when the process
   /// terminates.
   static Status LaunchProcess(ProcessLaunchInfo &launch_info);
 
   //------------------------------------------------------------------
   /// Perform expansion of the command-line for this launch info This can
   /// potentially involve wildcard expansion

lldb/include/lldb/Target/Platform.h

virtual bool CalculateMD5(const FileSpec &file_spec, uint64_t &low,
                             uint64_t &high);
 
   virtual int32_t GetResumeCountForLaunchInfo(ProcessLaunchInfo &launch_info) {
     return 1;
   }
 
   virtual const lldb::UnixSignalsSP &GetRemoteUnixSignals();
 
-  const lldb::UnixSignalsSP &GetUnixSignals();
+  lldb::UnixSignalsSP GetUnixSignals();
 
   //------------------------------------------------------------------
   /// Locate a queue name given a thread's qaddr
   ///
   /// On a system using libdispatch ("Grand Central Dispatch") style queues, a
   /// thread may be associated with a GCD queue or not, and a queue may be
   /// associated with multiple threads. The process/thread must provide a way
   /// to find the "dispatch_qaddr" for each thread, and from that

lldb/include/lldb/Target/UnixSignals.h

 #include "lldb/lldb-private.h"
 #include "llvm/ADT/Optional.h"
 
 namespace lldb_private {
 
 class UnixSignals {
 public:
   static lldb::UnixSignalsSP Create(const ArchSpec &arch);
+  static lldb::UnixSignalsSP CreateForHost();
 
   //------------------------------------------------------------------
   // Constructors and Destructors
   //------------------------------------------------------------------
   UnixSignals();
 
   virtual ~UnixSignals();
 

lldb/source/Host/common/Host.cpp

 #include "lldb/Host/Host.h"
 #include "lldb/Host/HostInfo.h"
 #include "lldb/Host/HostProcess.h"
 #include "lldb/Host/MonitoringProcessLauncher.h"
 #include "lldb/Host/ProcessLaunchInfo.h"
 #include "lldb/Host/ProcessLauncher.h"
 #include "lldb/Host/ThreadLauncher.h"
 #include "lldb/Host/posix/ConnectionFileDescriptorPosix.h"
-#include "lldb/Target/UnixSignals.h"
 #include "lldb/Utility/DataBufferLLVM.h"
 #include "lldb/Utility/FileSpec.h"
 #include "lldb/Utility/Log.h"
 #include "lldb/Utility/Predicate.h"
 #include "lldb/Utility/Status.h"
 #include "lldb/lldb-private-forward.h"
 #include "llvm/ADT/SmallString.h"
 #include "llvm/ADT/StringSwitch.h"

 #if !defined(__APPLE__)
 bool Host::OpenFileInExternalEditor(const FileSpec &file_spec,
                                     uint32_t line_no) {
   return false;
 }
 
 #endif
 
-const UnixSignalsSP &Host::GetUnixSignals() {
-  static const auto s_unix_signals_sp =
-      UnixSignals::Create(HostInfo::GetArchitecture());
-  return s_unix_signals_sp;
-}
-
 std::unique_ptr<Connection> Host::CreateDefaultConnection(llvm::StringRef url) {
 #if defined(_WIN32)
   if (url.startswith("file://"))
     return std::unique_ptr<Connection>(new ConnectionGenericFile());
 #endif
   return std::unique_ptr<Connection>(new ConnectionFileDescriptor());
 }
 

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

GDBRemoteCommunicationServerPlatform::Handle_qC(
   return SendPacketNoLock(response.GetString());
 }
 
 GDBRemoteCommunication::PacketResult
 GDBRemoteCommunicationServerPlatform::Handle_jSignalsInfo(
     StringExtractorGDBRemote &packet) {
   StructuredData::Array signal_array;
 
-  const auto &signals = Host::GetUnixSignals();
+  lldb::UnixSignalsSP signals = UnixSignals::CreateForHost();
   for (auto signo = signals->GetFirstSignalNumber();
        signo != LLDB_INVALID_SIGNAL_NUMBER;
        signo = signals->GetNextSignalNumber(signo)) {
     auto dictionary = std::make_shared<StructuredData::Dictionary>();
 
     dictionary->AddIntegerItem("signo", signo);
     dictionary->AddStringItem("name", signals->GetSignalAsCString(signo));
 

lldb/source/Target/Platform.cpp

 
 const char *Platform::GetCacheHostname() { return GetHostname(); }
 
 const UnixSignalsSP &Platform::GetRemoteUnixSignals() {
   static const auto s_default_unix_signals_sp = std::make_shared<UnixSignals>();
   return s_default_unix_signals_sp;
 }
 
-const UnixSignalsSP &Platform::GetUnixSignals() {
+UnixSignalsSP Platform::GetUnixSignals() {
   if (IsHost())
-    return Host::GetUnixSignals();
+    return UnixSignals::CreateForHost();
   return GetRemoteUnixSignals();
 }
 
 uint32_t Platform::LoadImage(lldb_private::Process *process,
                              const lldb_private::FileSpec &local_file,
                              const lldb_private::FileSpec &remote_file,
                              lldb_private::Status &error) {
   if (local_file && remote_file) {

lldb/source/Target/UnixSignals.cpp

 //===-- UnixSignals.cpp -----------------------------------------*- C++ -*-===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
 #include "lldb/Target/UnixSignals.h"
 #include "Plugins/Process/Utility/FreeBSDSignals.h"
 #include "Plugins/Process/Utility/LinuxSignals.h"
 #include "Plugins/Process/Utility/MipsLinuxSignals.h"
 #include "Plugins/Process/Utility/NetBSDSignals.h"
+#include "lldb/Host/HostInfo.h"
 #include "lldb/Host/StringConvert.h"
 #include "lldb/Utility/ArchSpec.h"
 
 using namespace lldb_private;
 
 UnixSignals::Signal::Signal(const char *name, bool default_suppress,
                             bool default_stop, bool default_notify,
                             const char *description, const char *alias)
case llvm::Triple::OpenBSD:
     return std::make_shared<FreeBSDSignals>();
   case llvm::Triple::NetBSD:
     return std::make_shared<NetBSDSignals>();
   default:
     return std::make_shared<UnixSignals>();
   }
 }
 
+lldb::UnixSignalsSP UnixSignals::CreateForHost() {
+  static lldb::UnixSignalsSP s_unix_signals_sp =
+      Create(HostInfo::GetArchitecture());
+  return s_unix_signals_sp;
+}
+
 //----------------------------------------------------------------------
 // UnixSignals constructor
 //----------------------------------------------------------------------
 UnixSignals::UnixSignals() { Reset(); }
 
 UnixSignals::UnixSignals(const UnixSignals &rhs) : m_signals(rhs.m_signals) {}
 
 UnixSignals::~UnixSignals() = default;