bulbazord created this revision.
bulbazord added reviewers: jasonmolenda, labath, DavidSpickett.
Herald added a project: All.
bulbazord requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

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


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D149697

Files:
  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

Index: lldb/source/Utility/ArchSpec.cpp
===================================================================
--- lldb/source/Utility/ArchSpec.cpp
+++ lldb/source/Utility/ArchSpec.cpp
@@ -543,7 +543,6 @@
   m_triple = llvm::Triple();
   m_core = kCore_invalid;
   m_byte_order = eByteOrderInvalid;
-  m_distribution_id.Clear();
   m_flags = 0;
 }
 
@@ -689,14 +688,6 @@
   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 @@
 }
 
 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;
Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp
===================================================================
--- lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp
+++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp
@@ -187,8 +187,8 @@
   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(";");
Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h
===================================================================
--- lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h
+++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h
@@ -583,6 +583,7 @@
   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;
Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
===================================================================
--- lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
+++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
@@ -69,10 +69,10 @@
       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 @@
     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 @@
         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 @@
             ++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 @@
                     m_host_arch.GetTriple().getTriple().c_str(),
                     triple.c_str());
         }
-        if (!distribution_id.empty())
-          m_host_arch.SetDistributionId(distribution_id.c_str());
       }
     }
   }
Index: lldb/source/Host/linux/HostInfoLinux.cpp
===================================================================
--- lldb/source/Host/linux/HostInfoLinux.cpp
+++ lldb/source/Host/linux/HostInfoLinux.cpp
@@ -200,17 +200,13 @@
                                                    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());
   }
Index: lldb/include/lldb/Utility/ArchSpec.h
===================================================================
--- lldb/include/lldb/Utility/ArchSpec.h
+++ 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 @@
   /// \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 @@
   // 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);
Index: lldb/include/lldb/Host/HostInfoBase.h
===================================================================
--- lldb/include/lldb/Host/HostInfoBase.h
+++ lldb/include/lldb/Host/HostInfoBase.h
@@ -121,6 +121,14 @@
     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);
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to