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 ↗

(On Diff #185379)

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

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

include/lldb/Host/Symbols.h

 //===-- Symbols.h -----------------------------------------------*- 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
 //
 //===----------------------------------------------------------------------===//
 
 #ifndef liblldb_Symbols_h_
 #define liblldb_Symbols_h_
 
 #include <stdint.h>
 
+#include "lldb/Core/FileSpecList.h"
 #include "lldb/Utility/FileSpec.h"
 
 namespace lldb_private {
 
 class ArchSpec;
 class ModuleSpec;
 class UUID;
 
 class Symbols {
 public:
   //----------------------------------------------------------------------
   // Locate the executable file given a module specification.
   //
   // Locating the file should happen only on the local computer or using the
   // current computers global settings.
   //----------------------------------------------------------------------
   static ModuleSpec LocateExecutableObjectFile(const ModuleSpec &module_spec);
 
   //----------------------------------------------------------------------
   // Locate the symbol file given a module specification.
   //
   // Locating the file should happen only on the local computer or using the
   // current computers global settings.
   //----------------------------------------------------------------------
-  static FileSpec LocateExecutableSymbolFile(const ModuleSpec &module_spec);
+  static FileSpec
+  LocateExecutableSymbolFile(const ModuleSpec &module_spec,
+                             const FileSpecList &default_search_paths);
 
   static FileSpec FindSymbolFileInBundle(const FileSpec &dsym_bundle_fspec,
                                          const lldb_private::UUID *uuid,
                                          const ArchSpec *arch);
 
   //----------------------------------------------------------------------
   // Locate the object and symbol file given a module specification.
   //

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

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

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

source/Host/common/Symbols.cpp

 //===-- Symbols.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/Host/Symbols.h"
+#include "lldb/Core/ModuleList.h"
 #include "lldb/Core/ModuleSpec.h"
 #include "lldb/Host/FileSystem.h"
 #include "lldb/Symbol/ObjectFile.h"
-#include "lldb/Target/Target.h"
 #include "lldb/Utility/ArchSpec.h"
 #include "lldb/Utility/DataBuffer.h"
 #include "lldb/Utility/DataExtractor.h"
 #include "lldb/Utility/Log.h"
 #include "lldb/Utility/StreamString.h"
 #include "lldb/Utility/Timer.h"
 #include "lldb/Utility/UUID.h"
 
ModuleSpec Symbols::LocateExecutableObjectFile(const ModuleSpec &module_spec) {
   } else {
     LocateMacOSXFilesUsingDebugSymbols(module_spec, result);
   }
   return result;
 }
 
 // Keep "symbols.enable-external-lookup" description in sync with this function.
 
-FileSpec Symbols::LocateExecutableSymbolFile(const ModuleSpec &module_spec) {
+FileSpec
+Symbols::LocateExecutableSymbolFile(const ModuleSpec &module_spec,
+                                    const FileSpecList &default_search_paths) {
   FileSpec symbol_file_spec = module_spec.GetSymbolFileSpec();
   if (symbol_file_spec.IsAbsolute() &&
       FileSystem::Instance().Exists(symbol_file_spec))
     return symbol_file_spec;
 
   const char *symbol_filename = symbol_file_spec.GetFilename().AsCString();
   if (symbol_filename && symbol_filename[0]) {
-    FileSpecList debug_file_search_paths(
-        Target::GetDefaultDebugFileSearchPaths());
+    FileSpecList debug_file_search_paths = default_search_paths;
 
     // Add module directory.
     FileSpec module_file_spec = module_spec.GetFileSpec();
     // We keep the unresolved pathname if it fails.
     FileSystem::Instance().ResolveSymbolicLink(module_file_spec, module_file_spec);
 
     const ConstString &file_dir = module_file_spec.GetDirectory();
     {

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

source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp

 #include "lldb/Symbol/LineTable.h"
 #include "lldb/Symbol/ObjectFile.h"
 #include "lldb/Symbol/SymbolVendor.h"
 #include "lldb/Symbol/TypeMap.h"
 #include "lldb/Symbol/TypeSystem.h"
 #include "lldb/Symbol/VariableList.h"
 
 #include "lldb/Target/Language.h"
+#include "lldb/Target/Target.h"
 
 #include "AppleDWARFIndex.h"
 #include "DWARFASTParser.h"
 #include "DWARFASTParserClang.h"
 #include "DWARFDIECollection.h"
 #include "DWARFDebugAbbrev.h"
 #include "DWARFDebugAranges.h"
 #include "DWARFDebugInfo.h"

 }
 
 SymbolFileDWARFDwp *SymbolFileDWARF::GetDwpSymbolFile() {
   llvm::call_once(m_dwp_symfile_once_flag, [this]() {
     ModuleSpec module_spec;
     module_spec.GetFileSpec() = m_obj_file->GetFileSpec();
     module_spec.GetSymbolFileSpec() =
         FileSpec(m_obj_file->GetFileSpec().GetPath() + ".dwp");
-    FileSpec dwp_filespec = Symbols::LocateExecutableSymbolFile(module_spec);
+
+    FileSpecList search_paths = Target::GetDefaultDebugFileSearchPaths();
+    FileSpec dwp_filespec =
+        Symbols::LocateExecutableSymbolFile(module_spec, search_paths);
     if (FileSystem::Instance().Exists(dwp_filespec)) {
       m_dwp_symfile = SymbolFileDWARFDwp::Create(GetObjectFile()->GetModule(),
                                                  dwp_filespec);
     }
   });
   return m_dwp_symfile.get();
 }

source/Plugins/SymbolVendor/ELF/SymbolVendorELF.cpp

 
 #include "lldb/Core/Module.h"
 #include "lldb/Core/ModuleSpec.h"
 #include "lldb/Core/PluginManager.h"
 #include "lldb/Core/Section.h"
 #include "lldb/Host/Host.h"
 #include "lldb/Host/Symbols.h"
 #include "lldb/Symbol/ObjectFile.h"
+#include "lldb/Target/Target.h"
 #include "lldb/Utility/StreamString.h"
 #include "lldb/Utility/Timer.h"
 
 using namespace lldb;
 using namespace lldb_private;
 
 //----------------------------------------------------------------------
 // SymbolVendorELF constructor
SymbolVendorELF::CreateInstance(const lldb::ModuleSP &module_sp,
   for (size_t idx = 0; idx < file_spec_list.GetSize(); ++idx) {
     ModuleSpec module_spec;
     const FileSpec fspec = file_spec_list.GetFileSpecAtIndex(idx);
 
     module_spec.GetFileSpec() = obj_file->GetFileSpec();
     FileSystem::Instance().Resolve(module_spec.GetFileSpec());
     module_spec.GetSymbolFileSpec() = fspec;
     module_spec.GetUUID() = uuid;
-    FileSpec dsym_fspec = Symbols::LocateExecutableSymbolFile(module_spec);
+    FileSpecList search_paths = Target::GetDefaultDebugFileSearchPaths();
+    FileSpec dsym_fspec =
+        Symbols::LocateExecutableSymbolFile(module_spec, search_paths);
     if (dsym_fspec) {
       DataBufferSP dsym_file_data_sp;
       lldb::offset_t dsym_file_data_offset = 0;
       ObjectFileSP dsym_objfile_sp =
           ObjectFile::FindPlugin(module_sp, &dsym_fspec, 0,
                                  FileSystem::Instance().GetByteSize(dsym_fspec),
                                  dsym_file_data_sp, dsym_file_data_offset);
       if (dsym_objfile_sp) {

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

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;

unittests/Host/SymbolsTest.cpp

 //===-- SymbolsTest.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 "gtest/gtest.h"
 
 #include "lldb/Core/ModuleSpec.h"
 #include "lldb/Host/FileSystem.h"
 #include "lldb/Host/HostInfo.h"
 #include "lldb/Host/Symbols.h"
+#include "lldb/Target/Target.h"
 
 using namespace lldb_private;
 
 namespace {
 class SymbolsTest : public ::testing::Test {
 public:
   void SetUp() override {
     FileSystem::Initialize();
     HostInfo::Initialize();
   }
   void TearDown() override {
     HostInfo::Terminate();
     FileSystem::Terminate();
   }
 };
 } // namespace
 
 TEST_F(
     SymbolsTest,
     TerminateLocateExecutableSymbolFileForUnknownExecutableAndUnknownSymbolFile) {
   ModuleSpec module_spec;
-  FileSpec symbol_file_spec = Symbols::LocateExecutableSymbolFile(module_spec);
+  FileSpecList search_paths = Target::GetDefaultDebugFileSearchPaths();
+  FileSpec symbol_file_spec =
+      Symbols::LocateExecutableSymbolFile(module_spec, search_paths);
   EXPECT_TRUE(symbol_file_spec.GetFilename().IsEmpty());
 }
 
 TEST_F(SymbolsTest,
        LocateExecutableSymbolFileForUnknownExecutableAndMissingSymbolFile) {
   ModuleSpec module_spec;
   // using a GUID here because the symbol file shouldn't actually exist on disk
   module_spec.GetSymbolFileSpec().SetFile(
       "4A524676-B24B-4F4E-968A-551D465EBAF1.so", FileSpec::Style::native);
-  FileSpec symbol_file_spec = Symbols::LocateExecutableSymbolFile(module_spec);
+  FileSpecList search_paths = Target::GetDefaultDebugFileSearchPaths();
+  FileSpec symbol_file_spec =
+      Symbols::LocateExecutableSymbolFile(module_spec, search_paths);
   EXPECT_TRUE(symbol_file_spec.GetFilename().IsEmpty());
 }