[Lldb-commits] [lldb] cd05ffd - [lldb] Remove distribution_id from ArchSpec

Alex Langford via lldb-commits lldb-commits at lists.llvm.org
Wed May 3 10:36:24 PDT 2023


Author: Alex Langford
Date: 2023-05-03T10:30:42-07:00
New Revision: cd05ffdbb2c303cbf476a3a48b3f67094d428e37

URL: https://github.com/llvm/llvm-project/commit/cd05ffdbb2c303cbf476a3a48b3f67094d428e37
DIFF: https://github.com/llvm/llvm-project/commit/cd05ffdbb2c303cbf476a3a48b3f67094d428e37.diff

LOG: [lldb] Remove distribution_id from ArchSpec

The qHostInfo packet in the gdb-remote communication protocol specifies
that distribution_id can be set, so lldb handles that. But we store that
in the ArchSpec representing the "Host" platform (whatever platform the
debug server is running on). This field is otherwise unused in ArchSpec,
so it would be a lot easier if we stored that information at the
gdb-remote communication layer.

Sidenote: The distribution_id field is currently unused but I did not
want to remove it in case some folks found it useful (e.g. in downstream
forks).

Differential Revision: https://reviews.llvm.org/D149697

Added: 
    

Modified: 
    lldb/include/lldb/Host/HostInfoBase.h
    lldb/include/lldb/Utility/ArchSpec.h
    lldb/source/Host/linux/HostInfoLinux.cpp
    lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
    lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h
    lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp
    lldb/source/Utility/ArchSpec.cpp

Removed: 
    


################################################################################
diff  --git a/lldb/include/lldb/Host/HostInfoBase.h b/lldb/include/lldb/Host/HostInfoBase.h
index 42f71d91f9bf9..6c86c71e552dc 100644
--- a/lldb/include/lldb/Host/HostInfoBase.h
+++ b/lldb/include/lldb/Host/HostInfoBase.h
@@ -121,6 +121,14 @@ class HostInfoBase {
     return {};
   }
 
+  /// Returns the distribution id of the host
+  ///
+  /// This will be something like "ubuntu", "fedora", etc. on Linux.
+  ///
+  /// \return Returns either std::nullopt or a reference to a const std::string
+  /// containing the distribution id
+  static llvm::StringRef GetDistributionId() { return llvm::StringRef(); }
+
 protected:
   static bool ComputeSharedLibraryDirectory(FileSpec &file_spec);
   static bool ComputeSupportExeDirectory(FileSpec &file_spec);

diff  --git a/lldb/include/lldb/Utility/ArchSpec.h b/lldb/include/lldb/Utility/ArchSpec.h
index 2de517d765b2a..a226a3a5a9b71 100644
--- a/lldb/include/lldb/Utility/ArchSpec.h
+++ b/lldb/include/lldb/Utility/ArchSpec.h
@@ -10,7 +10,6 @@
 #define LLDB_UTILITY_ARCHSPEC_H
 
 #include "lldb/Utility/CompletionRequest.h"
-#include "lldb/Utility/ConstString.h"
 #include "lldb/lldb-enumerations.h"
 #include "lldb/lldb-forward.h"
 #include "lldb/lldb-private-enumerations.h"
@@ -342,20 +341,6 @@ class ArchSpec {
   /// \return An LLVM arch type.
   llvm::Triple::ArchType GetMachine() const;
 
-  /// Returns the distribution id of the architecture.
-  ///
-  /// This will be something like "ubuntu", "fedora", etc. on Linux.
-  ///
-  /// \return A ConstString ref containing the distribution id,
-  ///         potentially empty.
-  ConstString GetDistributionId() const;
-
-  /// Set the distribution id of the architecture.
-  ///
-  /// This will be something like "ubuntu", "fedora", etc. on Linux. This
-  /// should be the same value returned by HostInfo::GetDistributionId ().
-  void SetDistributionId(const char *distribution_id);
-
   /// Tests if this ArchSpec is valid.
   ///
   /// \return True if the current architecture is valid, false
@@ -555,8 +540,6 @@ class ArchSpec {
   // these are application specific extensions like micromips, mips16 etc.
   uint32_t m_flags = 0;
 
-  ConstString m_distribution_id;
-
   // Called when m_def or m_entry are changed.  Fills in all remaining members
   // with default values.
   void CoreUpdated(bool update_triple);

diff  --git a/lldb/source/Host/linux/HostInfoLinux.cpp b/lldb/source/Host/linux/HostInfoLinux.cpp
index 018be54ecf7aa..c66f787db0cf9 100644
--- a/lldb/source/Host/linux/HostInfoLinux.cpp
+++ b/lldb/source/Host/linux/HostInfoLinux.cpp
@@ -200,17 +200,13 @@ void HostInfoLinux::ComputeHostArchitectureSupport(ArchSpec &arch_32,
                                                    ArchSpec &arch_64) {
   HostInfoPosix::ComputeHostArchitectureSupport(arch_32, arch_64);
 
-  const char *distribution_id = GetDistributionId().data();
-
   // On Linux, "unknown" in the vendor slot isn't what we want for the default
   // triple.  It's probably an artifact of config.guess.
   if (arch_32.IsValid()) {
-    arch_32.SetDistributionId(distribution_id);
     if (arch_32.GetTriple().getVendor() == llvm::Triple::UnknownVendor)
       arch_32.GetTriple().setVendorName(llvm::StringRef());
   }
   if (arch_64.IsValid()) {
-    arch_64.SetDistributionId(distribution_id);
     if (arch_64.GetTriple().getVendor() == llvm::Triple::UnknownVendor)
       arch_64.GetTriple().setVendorName(llvm::StringRef());
   }

diff  --git a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
index 18552ea66b029..0501c5bd0ce04 100644
--- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
+++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
@@ -69,10 +69,10 @@ GDBRemoteCommunicationClient::GDBRemoteCommunicationClient()
       m_supports_vFileSize(true), m_supports_vFileMode(true),
       m_supports_vFileExists(true), m_supports_vRun(true),
 
-      m_host_arch(), m_process_arch(), m_os_build(), m_os_kernel(),
-      m_hostname(), m_gdb_server_name(), m_default_packet_timeout(0),
-      m_qSupported_response(), m_supported_async_json_packets_sp(),
-      m_qXfer_memory_map() {}
+      m_host_arch(), m_host_distribution_id(), m_process_arch(), m_os_build(),
+      m_os_kernel(), m_hostname(), m_gdb_server_name(),
+      m_default_packet_timeout(0), m_qSupported_response(),
+      m_supported_async_json_packets_sp(), m_qXfer_memory_map() {}
 
 // Destructor
 GDBRemoteCommunicationClient::~GDBRemoteCommunicationClient() {
@@ -307,6 +307,7 @@ void GDBRemoteCommunicationClient::ResetDiscoverableSettings(bool did_exec) {
     m_qSymbol_requests_done = false;
     m_supports_qModuleInfo = true;
     m_host_arch.Clear();
+    m_host_distribution_id.clear();
     m_os_version = llvm::VersionTuple();
     m_os_build.clear();
     m_os_kernel.clear();
@@ -1206,7 +1207,6 @@ bool GDBRemoteCommunicationClient::GetHostInfo(bool force) {
         std::string environment;
         std::string vendor_name;
         std::string triple;
-        std::string distribution_id;
         uint32_t pointer_byte_size = 0;
         ByteOrder byte_order = eByteOrderInvalid;
         uint32_t num_keys_decoded = 0;
@@ -1228,7 +1228,7 @@ bool GDBRemoteCommunicationClient::GetHostInfo(bool force) {
             ++num_keys_decoded;
           } else if (name.equals("distribution_id")) {
             StringExtractor extractor(value);
-            extractor.GetHexByteString(distribution_id);
+            extractor.GetHexByteString(m_host_distribution_id);
             ++num_keys_decoded;
           } else if (name.equals("os_build")) {
             StringExtractor extractor(value);
@@ -1376,8 +1376,6 @@ bool GDBRemoteCommunicationClient::GetHostInfo(bool force) {
                     m_host_arch.GetTriple().getTriple().c_str(),
                     triple.c_str());
         }
-        if (!distribution_id.empty())
-          m_host_arch.SetDistributionId(distribution_id.c_str());
       }
     }
   }

diff  --git a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h
index b32e09cf8e76f..02185cf52e33e 100644
--- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h
+++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h
@@ -583,6 +583,7 @@ class GDBRemoteCommunicationClient : public GDBRemoteClientBase {
   uint32_t m_addressing_bits = 0;
 
   ArchSpec m_host_arch;
+  std::string m_host_distribution_id;
   ArchSpec m_process_arch;
   UUID m_process_standalone_uuid;
   lldb::addr_t m_process_standalone_value = LLDB_INVALID_ADDRESS;

diff  --git a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp
index 7b10fd4217e05..9e90e98b1f2ab 100644
--- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp
+++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp
@@ -187,8 +187,8 @@ GDBRemoteCommunicationServerCommon::Handle_qHostInfo(
   response.PutStringAsRawHex8(host_triple.getTriple());
   response.Printf(";ptrsize:%u;", host_arch.GetAddressByteSize());
 
-  const char *distribution_id = host_arch.GetDistributionId().AsCString();
-  if (distribution_id) {
+  llvm::StringRef distribution_id = HostInfo::GetDistributionId();
+  if (!distribution_id.empty()) {
     response.PutCString("distribution_id:");
     response.PutStringAsRawHex8(distribution_id);
     response.PutCString(";");

diff  --git a/lldb/source/Utility/ArchSpec.cpp b/lldb/source/Utility/ArchSpec.cpp
index 475d0c8a63a7b..fb0e985a0d565 100644
--- a/lldb/source/Utility/ArchSpec.cpp
+++ b/lldb/source/Utility/ArchSpec.cpp
@@ -543,7 +543,6 @@ void ArchSpec::Clear() {
   m_triple = llvm::Triple();
   m_core = kCore_invalid;
   m_byte_order = eByteOrderInvalid;
-  m_distribution_id.Clear();
   m_flags = 0;
 }
 
@@ -689,14 +688,6 @@ llvm::Triple::ArchType ArchSpec::GetMachine() const {
   return llvm::Triple::UnknownArch;
 }
 
-ConstString ArchSpec::GetDistributionId() const {
-  return m_distribution_id;
-}
-
-void ArchSpec::SetDistributionId(const char *distribution_id) {
-  m_distribution_id.SetCString(distribution_id);
-}
-
 uint32_t ArchSpec::GetAddressByteSize() const {
   const CoreDefinition *core_def = FindCoreDefinition(m_core);
   if (core_def) {
@@ -979,8 +970,6 @@ static bool IsCompatibleEnvironment(llvm::Triple::EnvironmentType lhs,
 }
 
 bool ArchSpec::IsMatch(const ArchSpec &rhs, MatchType match) const {
-  // explicitly ignoring m_distribution_id in this method.
-
   if (GetByteOrder() != rhs.GetByteOrder() ||
       !cores_match(GetCore(), rhs.GetCore(), true, match == ExactMatch))
     return false;


        


More information about the lldb-commits mailing list