[Lldb-commits] [lldb] 669e57e - [lldb] Simplify specifying of platform supported architectures

Pavel Labath via lldb-commits lldb-commits at lists.llvm.org
Tue Nov 16 02:47:57 PST 2021


Author: Pavel Labath
Date: 2021-11-16T11:43:48+01:00
New Revision: 669e57ebd1a7137aba7aab47d9d4dd67e1b0b9a0

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

LOG: [lldb] Simplify specifying of platform supported architectures

The GetSupportedArchitectureAtIndex pattern forces the use of
complicated patterns in both the implementations of the function and in
the various callers.

This patch creates a new method (GetSupportedArchitectures), which
returns a list (vector) of architectures. The
GetSupportedArchitectureAtIndex is kept in order to enable incremental
rollout. Base Platform class contains implementations of both of these
methods, using the other method as the source of truth. Platforms
without infinite stacks should implement at least one of them.

This patch also ports Linux, FreeBSD and NetBSD platforms to the new
API. A new helper function (CreateArchList) is added to simplify the
common task of creating a list of ArchSpecs with the same OS but
different architectures.

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

Added: 
    

Modified: 
    lldb/include/lldb/Target/Platform.h
    lldb/source/Plugins/Platform/FreeBSD/PlatformFreeBSD.cpp
    lldb/source/Plugins/Platform/FreeBSD/PlatformFreeBSD.h
    lldb/source/Plugins/Platform/Linux/PlatformLinux.cpp
    lldb/source/Plugins/Platform/Linux/PlatformLinux.h
    lldb/source/Plugins/Platform/NetBSD/PlatformNetBSD.cpp
    lldb/source/Plugins/Platform/NetBSD/PlatformNetBSD.h
    lldb/source/Target/Platform.cpp

Removed: 
    


################################################################################
diff  --git a/lldb/include/lldb/Target/Platform.h b/lldb/include/lldb/Target/Platform.h
index 86c62140ba1ff..f3d88d6397d28 100644
--- a/lldb/include/lldb/Target/Platform.h
+++ b/lldb/include/lldb/Target/Platform.h
@@ -322,7 +322,13 @@ class Platform : public PluginInterface {
   ///     \b true if \a arch was filled in and is valid, \b false
   ///     otherwise.
   virtual bool GetSupportedArchitectureAtIndex(uint32_t idx,
-                                               ArchSpec &arch) = 0;
+                                               ArchSpec &arch);
+
+  /// Get the platform's supported architectures in the order in which they
+  /// should be searched.
+  /// NB: This implementation is mutually recursive with
+  /// GetSupportedArchitectureAtIndex. Subclasses should implement one of them.
+  virtual std::vector<ArchSpec> GetSupportedArchitectures();
 
   virtual size_t GetSoftwareBreakpointTrapOpcode(Target &target,
                                                  BreakpointSite *bp_site);
@@ -876,6 +882,12 @@ class Platform : public PluginInterface {
   }
 
 protected:
+  /// Create a list of ArchSpecs with the given OS and a architectures. The
+  /// vendor field is left as an "unspecified unknown".
+  static std::vector<ArchSpec>
+  CreateArchList(llvm::ArrayRef<llvm::Triple::ArchType> archs,
+                 llvm::Triple::OSType os);
+
   /// Private implementation of connecting to a process. If the stream is set
   /// we connect synchronously.
   lldb::ProcessSP DoConnectProcess(llvm::StringRef connect_url,

diff  --git a/lldb/source/Plugins/Platform/FreeBSD/PlatformFreeBSD.cpp b/lldb/source/Plugins/Platform/FreeBSD/PlatformFreeBSD.cpp
index ffd92d4fb848d..754d06de7cb92 100644
--- a/lldb/source/Plugins/Platform/FreeBSD/PlatformFreeBSD.cpp
+++ b/lldb/source/Plugins/Platform/FreeBSD/PlatformFreeBSD.cpp
@@ -111,72 +111,27 @@ void PlatformFreeBSD::Terminate() {
 /// Default Constructor
 PlatformFreeBSD::PlatformFreeBSD(bool is_host)
     : PlatformPOSIX(is_host) // This is the local host platform
-{}
-
-bool PlatformFreeBSD::GetSupportedArchitectureAtIndex(uint32_t idx,
-                                                      ArchSpec &arch) {
-  if (IsHost()) {
+{
+  if (is_host) {
     ArchSpec hostArch = HostInfo::GetArchitecture(HostInfo::eArchKindDefault);
-    if (hostArch.GetTriple().isOSFreeBSD()) {
-      if (idx == 0) {
-        arch = hostArch;
-        return arch.IsValid();
-      } else if (idx == 1) {
-        // If the default host architecture is 64-bit, look for a 32-bit
-        // variant
-        if (hostArch.IsValid() && hostArch.GetTriple().isArch64Bit()) {
-          arch = HostInfo::GetArchitecture(HostInfo::eArchKind32);
-          return arch.IsValid();
-        }
-      }
+    m_supported_architectures.push_back(hostArch);
+    if (hostArch.GetTriple().isArch64Bit()) {
+      m_supported_architectures.push_back(
+          HostInfo::GetArchitecture(HostInfo::eArchKind32));
     }
   } else {
-    if (m_remote_platform_sp)
-      return m_remote_platform_sp->GetSupportedArchitectureAtIndex(idx, arch);
-
-    llvm::Triple triple;
-    // Set the OS to FreeBSD
-    triple.setOS(llvm::Triple::FreeBSD);
-    // Set the architecture
-    switch (idx) {
-    case 0:
-      triple.setArchName("x86_64");
-      break;
-    case 1:
-      triple.setArchName("i386");
-      break;
-    case 2:
-      triple.setArchName("aarch64");
-      break;
-    case 3:
-      triple.setArchName("arm");
-      break;
-    case 4:
-      triple.setArchName("mips64");
-      break;
-    case 5:
-      triple.setArchName("mips");
-      break;
-    case 6:
-      triple.setArchName("ppc64");
-      break;
-    case 7:
-      triple.setArchName("ppc");
-      break;
-    default:
-      return false;
-    }
-    // Leave the vendor as "llvm::Triple:UnknownVendor" and don't specify the
-    // vendor by calling triple.SetVendorName("unknown") so that it is a
-    // "unspecified unknown". This means when someone calls
-    // triple.GetVendorName() it will return an empty string which indicates
-    // that the vendor can be set when two architectures are merged
-
-    // Now set the triple into "arch" and return true
-    arch.SetTriple(triple);
-    return true;
+    m_supported_architectures = CreateArchList(
+        {llvm::Triple::x86_64, llvm::Triple::x86, llvm::Triple::aarch64,
+         llvm::Triple::arm, llvm::Triple::mips64, llvm::Triple::ppc64,
+         llvm::Triple::ppc},
+        llvm::Triple::FreeBSD);
   }
-  return false;
+}
+
+std::vector<ArchSpec> PlatformFreeBSD::GetSupportedArchitectures() {
+  if (m_remote_platform_sp)
+    return m_remote_platform_sp->GetSupportedArchitectures();
+  return m_supported_architectures;
 }
 
 void PlatformFreeBSD::GetStatus(Stream &strm) {

diff  --git a/lldb/source/Plugins/Platform/FreeBSD/PlatformFreeBSD.h b/lldb/source/Plugins/Platform/FreeBSD/PlatformFreeBSD.h
index b4d2f740bed9a..fd37b13de0177 100644
--- a/lldb/source/Plugins/Platform/FreeBSD/PlatformFreeBSD.h
+++ b/lldb/source/Plugins/Platform/FreeBSD/PlatformFreeBSD.h
@@ -42,7 +42,7 @@ class PlatformFreeBSD : public PlatformPOSIX {
 
   void GetStatus(Stream &strm) override;
 
-  bool GetSupportedArchitectureAtIndex(uint32_t idx, ArchSpec &arch) override;
+  std::vector<ArchSpec> GetSupportedArchitectures() override;
 
   bool CanDebugProcess() override;
 
@@ -52,6 +52,8 @@ class PlatformFreeBSD : public PlatformPOSIX {
                                   lldb::addr_t length, unsigned prot,
                                   unsigned flags, lldb::addr_t fd,
                                   lldb::addr_t offset) override;
+
+  std::vector<ArchSpec> m_supported_architectures;
 };
 
 } // namespace platform_freebsd

diff  --git a/lldb/source/Plugins/Platform/Linux/PlatformLinux.cpp b/lldb/source/Plugins/Platform/Linux/PlatformLinux.cpp
index 940ecc5c5e831..6278d51289d76 100644
--- a/lldb/source/Plugins/Platform/Linux/PlatformLinux.cpp
+++ b/lldb/source/Plugins/Platform/Linux/PlatformLinux.cpp
@@ -109,78 +109,28 @@ void PlatformLinux::Terminate() {
 /// Default Constructor
 PlatformLinux::PlatformLinux(bool is_host)
     : PlatformPOSIX(is_host) // This is the local host platform
-{}
-
-bool PlatformLinux::GetSupportedArchitectureAtIndex(uint32_t idx,
-                                                    ArchSpec &arch) {
-  if (IsHost()) {
+{
+  if (is_host) {
     ArchSpec hostArch = HostInfo::GetArchitecture(HostInfo::eArchKindDefault);
-    if (hostArch.GetTriple().isOSLinux()) {
-      if (idx == 0) {
-        arch = hostArch;
-        return arch.IsValid();
-      } else if (idx == 1) {
-        // If the default host architecture is 64-bit, look for a 32-bit
-        // variant
-        if (hostArch.IsValid() && hostArch.GetTriple().isArch64Bit()) {
-          arch = HostInfo::GetArchitecture(HostInfo::eArchKind32);
-          return arch.IsValid();
-        }
-      }
+    m_supported_architectures.push_back(hostArch);
+    if (hostArch.GetTriple().isArch64Bit()) {
+      m_supported_architectures.push_back(
+          HostInfo::GetArchitecture(HostInfo::eArchKind32));
     }
   } else {
-    if (m_remote_platform_sp)
-      return m_remote_platform_sp->GetSupportedArchitectureAtIndex(idx, arch);
-
-    llvm::Triple triple;
-    // Set the OS to linux
-    triple.setOS(llvm::Triple::Linux);
-    // Set the architecture
-    switch (idx) {
-    case 0:
-      triple.setArchName("x86_64");
-      break;
-    case 1:
-      triple.setArchName("i386");
-      break;
-    case 2:
-      triple.setArchName("arm");
-      break;
-    case 3:
-      triple.setArchName("aarch64");
-      break;
-    case 4:
-      triple.setArchName("mips64");
-      break;
-    case 5:
-      triple.setArchName("hexagon");
-      break;
-    case 6:
-      triple.setArchName("mips");
-      break;
-    case 7:
-      triple.setArchName("mips64el");
-      break;
-    case 8:
-      triple.setArchName("mipsel");
-      break;
-    case 9:
-      triple.setArchName("s390x");
-      break;
-    default:
-      return false;
-    }
-    // Leave the vendor as "llvm::Triple:UnknownVendor" and don't specify the
-    // vendor by calling triple.SetVendorName("unknown") so that it is a
-    // "unspecified unknown". This means when someone calls
-    // triple.GetVendorName() it will return an empty string which indicates
-    // that the vendor can be set when two architectures are merged
-
-    // Now set the triple into "arch" and return true
-    arch.SetTriple(triple);
-    return true;
+    m_supported_architectures = CreateArchList(
+        {llvm::Triple::x86_64, llvm::Triple::x86, llvm::Triple::arm,
+         llvm::Triple::aarch64, llvm::Triple::mips64, llvm::Triple::mips64,
+         llvm::Triple::hexagon, llvm::Triple::mips, llvm::Triple::mips64el,
+         llvm::Triple::mipsel, llvm::Triple::systemz},
+        llvm::Triple::Linux);
   }
-  return false;
+}
+
+std::vector<ArchSpec> PlatformLinux::GetSupportedArchitectures() {
+  if (m_remote_platform_sp)
+    return m_remote_platform_sp->GetSupportedArchitectures();
+  return m_supported_architectures;
 }
 
 void PlatformLinux::GetStatus(Stream &strm) {

diff  --git a/lldb/source/Plugins/Platform/Linux/PlatformLinux.h b/lldb/source/Plugins/Platform/Linux/PlatformLinux.h
index 516089cd783b1..13224901cd926 100644
--- a/lldb/source/Plugins/Platform/Linux/PlatformLinux.h
+++ b/lldb/source/Plugins/Platform/Linux/PlatformLinux.h
@@ -42,7 +42,7 @@ class PlatformLinux : public PlatformPOSIX {
 
   void GetStatus(Stream &strm) override;
 
-  bool GetSupportedArchitectureAtIndex(uint32_t idx, ArchSpec &arch) override;
+  std::vector<ArchSpec> GetSupportedArchitectures() override;
 
   uint32_t GetResumeCountForLaunchInfo(ProcessLaunchInfo &launch_info) override;
 
@@ -57,6 +57,8 @@ class PlatformLinux : public PlatformPOSIX {
                                   lldb::addr_t length, unsigned prot,
                                   unsigned flags, lldb::addr_t fd,
                                   lldb::addr_t offset) override;
+
+  std::vector<ArchSpec> m_supported_architectures;
 };
 
 } // namespace platform_linux

diff  --git a/lldb/source/Plugins/Platform/NetBSD/PlatformNetBSD.cpp b/lldb/source/Plugins/Platform/NetBSD/PlatformNetBSD.cpp
index 3432dcec650d7..552b3890615c7 100644
--- a/lldb/source/Plugins/Platform/NetBSD/PlatformNetBSD.cpp
+++ b/lldb/source/Plugins/Platform/NetBSD/PlatformNetBSD.cpp
@@ -100,54 +100,24 @@ void PlatformNetBSD::Terminate() {
 /// Default Constructor
 PlatformNetBSD::PlatformNetBSD(bool is_host)
     : PlatformPOSIX(is_host) // This is the local host platform
-{}
-
-bool PlatformNetBSD::GetSupportedArchitectureAtIndex(uint32_t idx,
-                                                     ArchSpec &arch) {
-  if (IsHost()) {
+{
+  if (is_host) {
     ArchSpec hostArch = HostInfo::GetArchitecture(HostInfo::eArchKindDefault);
-    if (hostArch.GetTriple().isOSNetBSD()) {
-      if (idx == 0) {
-        arch = hostArch;
-        return arch.IsValid();
-      } else if (idx == 1) {
-        // If the default host architecture is 64-bit, look for a 32-bit
-        // variant
-        if (hostArch.IsValid() && hostArch.GetTriple().isArch64Bit()) {
-          arch = HostInfo::GetArchitecture(HostInfo::eArchKind32);
-          return arch.IsValid();
-        }
-      }
+    m_supported_architectures.push_back(hostArch);
+    if (hostArch.GetTriple().isArch64Bit()) {
+      m_supported_architectures.push_back(
+          HostInfo::GetArchitecture(HostInfo::eArchKind32));
     }
   } else {
-    if (m_remote_platform_sp)
-      return m_remote_platform_sp->GetSupportedArchitectureAtIndex(idx, arch);
-
-    llvm::Triple triple;
-    // Set the OS to NetBSD
-    triple.setOS(llvm::Triple::NetBSD);
-    // Set the architecture
-    switch (idx) {
-    case 0:
-      triple.setArchName("x86_64");
-      break;
-    case 1:
-      triple.setArchName("i386");
-      break;
-    default:
-      return false;
-    }
-    // Leave the vendor as "llvm::Triple:UnknownVendor" and don't specify the
-    // vendor by calling triple.SetVendorName("unknown") so that it is a
-    // "unspecified unknown". This means when someone calls
-    // triple.GetVendorName() it will return an empty string which indicates
-    // that the vendor can be set when two architectures are merged
-
-    // Now set the triple into "arch" and return true
-    arch.SetTriple(triple);
-    return true;
+    m_supported_architectures = CreateArchList(
+        {llvm::Triple::x86_64, llvm::Triple::x86}, llvm::Triple::NetBSD);
   }
-  return false;
+}
+
+std::vector<ArchSpec> PlatformNetBSD::GetSupportedArchitectures() {
+  if (m_remote_platform_sp)
+    return m_remote_platform_sp->GetSupportedArchitectures();
+  return m_supported_architectures;
 }
 
 void PlatformNetBSD::GetStatus(Stream &strm) {

diff  --git a/lldb/source/Plugins/Platform/NetBSD/PlatformNetBSD.h b/lldb/source/Plugins/Platform/NetBSD/PlatformNetBSD.h
index 383965a72c38b..7158fbd26efbb 100644
--- a/lldb/source/Plugins/Platform/NetBSD/PlatformNetBSD.h
+++ b/lldb/source/Plugins/Platform/NetBSD/PlatformNetBSD.h
@@ -42,7 +42,7 @@ class PlatformNetBSD : public PlatformPOSIX {
 
   void GetStatus(Stream &strm) override;
 
-  bool GetSupportedArchitectureAtIndex(uint32_t idx, ArchSpec &arch) override;
+  std::vector<ArchSpec> GetSupportedArchitectures() override;
 
   uint32_t GetResumeCountForLaunchInfo(ProcessLaunchInfo &launch_info) override;
 
@@ -54,6 +54,8 @@ class PlatformNetBSD : public PlatformPOSIX {
                                   lldb::addr_t length, unsigned prot,
                                   unsigned flags, lldb::addr_t fd,
                                   lldb::addr_t offset) override;
+
+  std::vector<ArchSpec> m_supported_architectures;
 };
 
 } // namespace platform_netbsd

diff  --git a/lldb/source/Target/Platform.cpp b/lldb/source/Target/Platform.cpp
index 0ae56e6ff1615..0a84a37d9534a 100644
--- a/lldb/source/Target/Platform.cpp
+++ b/lldb/source/Target/Platform.cpp
@@ -1144,6 +1144,35 @@ Platform::GetPlatformForArchitecture(const ArchSpec &arch,
   return platform_sp;
 }
 
+std::vector<ArchSpec>
+Platform::CreateArchList(llvm::ArrayRef<llvm::Triple::ArchType> archs,
+                         llvm::Triple::OSType os) {
+  std::vector<ArchSpec> list;
+  for(auto arch : archs) {
+    llvm::Triple triple;
+    triple.setArch(arch);
+    triple.setOS(os);
+    list.push_back(ArchSpec(triple));
+  }
+  return list;
+}
+
+bool Platform::GetSupportedArchitectureAtIndex(uint32_t idx, ArchSpec &arch) {
+  const auto &archs = GetSupportedArchitectures();
+  if (idx >= archs.size())
+    return false;
+  arch = archs[idx];
+  return true;
+}
+
+std::vector<ArchSpec> Platform::GetSupportedArchitectures() {
+  std::vector<ArchSpec> result;
+  ArchSpec arch;
+  for (uint32_t idx = 0; GetSupportedArchitectureAtIndex(idx, arch); ++idx)
+    result.push_back(arch);
+  return result;
+}
+
 /// Lets a platform answer if it is compatible with a given
 /// architecture and the target triple contained within.
 bool Platform::IsCompatibleArchitecture(const ArchSpec &arch,


        


More information about the lldb-commits mailing list