[llvm-branch-commits] [lldb] [lldb] Refactor RegisterTypeBuilder (PR #213897)

David Spickett via llvm-branch-commits llvm-branch-commits at lists.llvm.org
Thu Aug 6 08:41:31 PDT 2026


https://github.com/DavidSpickett updated https://github.com/llvm/llvm-project/pull/213897

>From 6db948af80adbb4e74a92d21b7feff576bd2a257 Mon Sep 17 00:00:00 2001
From: David Spickett <david.spickett at arm.com>
Date: Thu, 6 Aug 2026 14:19:16 +0000
Subject: [PATCH 1/2] [lldb][AArch64] Use unique_ptr instead of statics in
 RegisterTypeDetector

Fixes #214264.

I used static variables for the created types, on the assumption
that only one detector would be used and that the host's
features would not change.

That is true for an lldb-server on a real Linux/FreeBSD system.
It is not true when we use the detector with core files. In the
same LLDB session you might load several files that came from
systems with different features.

The result was that the first detection sets up the static variables
and future detections do not update them. So subsequent core
files can have incorrect types.

(and in future if types vary per-process, we could have the same
issue in lldb-server)

To address this I am changing where the types are stored so that
each instance of the detector has its own set of types for which
it manages the lifetime.

* There is a vector of unique pointers to types. This vector
  is a member, so is per instance of the detector class.
* As new types are added to this vector, it may reallocate,
  but the location of the RegisterTypes themselves will stay
  the same, this is important.
* Detector functions use MakeType to create types, and MakeType
  handles managing the vector. Nothing else accesses the vector
  directly.
* MakeType returns a raw pointer to the type.
* Detector functions return a raw pointer to the top level
  type for the register. For example if it has flags that
  have enums for their fields, the flags type is the top level
  type.
* These top level raw pointers are given to the rest of LLDB.
* We assume that the lifetime of the dector is > that of anyone
  using the raw pointers.
* When the detector destructs, the unique pointers destruct and
  the RegisterTypes are freed.

This handing out of raw pointers is likely a bad idea, but I
want to keep the changes here within the detector.

All existing tests pass, and I have added a test that loads 2
core files with different features. That test fails without
these changes because the STORE_ONLY feature is detected
incorrectly.
---
 .../Utility/RegisterTypeDetector_arm64.cpp    | 142 ++++++++----------
 .../Utility/RegisterTypeDetector_arm64.h      |  84 +++++++----
 .../TestAArch64LinuxMTEMemoryTagCoreFile.py   |  19 +++
 3 files changed, 142 insertions(+), 103 deletions(-)

diff --git a/lldb/source/Plugins/Process/Utility/RegisterTypeDetector_arm64.cpp b/lldb/source/Plugins/Process/Utility/RegisterTypeDetector_arm64.cpp
index 8e7ddca73b27b..24b1bd28b4d9c 100644
--- a/lldb/source/Plugins/Process/Utility/RegisterTypeDetector_arm64.cpp
+++ b/lldb/source/Plugins/Process/Utility/RegisterTypeDetector_arm64.cpp
@@ -9,6 +9,8 @@
 #include "RegisterTypeDetector_arm64.h"
 #include "lldb/Utility/RegisterTypeFlags.h"
 
+#include <functional>
+
 // This file is built on all systems because it is used by native processes and
 // core files, so we manually define the needed HWCAP values here.
 // These values are the same for Linux and FreeBSD.
@@ -38,10 +40,10 @@ Arm64RegisterTypeDetector::DetectPOREL0Type(uint64_t hwcap, uint64_t hwcap2,
   (void)hwcap3;
 
   if (!(hwcap2 & HWCAP2_POE))
-    return {};
+    return nullptr;
 
-  static const RegisterTypeEnum por_el0_perm_enum(
-      "por_el0_perm_enum", {
+  const RegisterTypeEnum *por_el0_perm_enum = MakeType<RegisterTypeEnum>(
+      "por_el0_perm_enum", RegisterTypeEnum::Enumerators{
                                {0b0000, "No Access"},
                                {0b0001, "Read"},
                                {0b0010, "Execute"},
@@ -52,28 +54,25 @@ Arm64RegisterTypeDetector::DetectPOREL0Type(uint64_t hwcap, uint64_t hwcap2,
                                {0b0111, "Read, Write, Execute"},
                            });
 
-  static const RegisterTypeFlags por_el0_flags(
-      "por_el0_flags", 8,
-      {
-          {"Perm15", 60, 63, &por_el0_perm_enum},
-          {"Perm14", 56, 59, &por_el0_perm_enum},
-          {"Perm13", 52, 55, &por_el0_perm_enum},
-          {"Perm12", 48, 51, &por_el0_perm_enum},
-          {"Perm11", 44, 47, &por_el0_perm_enum},
-          {"Perm10", 40, 43, &por_el0_perm_enum},
-          {"Perm9", 36, 39, &por_el0_perm_enum},
-          {"Perm8", 32, 35, &por_el0_perm_enum},
-          {"Perm7", 28, 31, &por_el0_perm_enum},
-          {"Perm6", 24, 27, &por_el0_perm_enum},
-          {"Perm5", 20, 23, &por_el0_perm_enum},
-          {"Perm4", 16, 19, &por_el0_perm_enum},
-          {"Perm3", 12, 15, &por_el0_perm_enum},
-          {"Perm2", 8, 11, &por_el0_perm_enum},
-          {"Perm1", 4, 7, &por_el0_perm_enum},
-          {"Perm0", 0, 3, &por_el0_perm_enum},
-      });
-
-  return &por_el0_flags;
+  return MakeType<RegisterTypeFlags>("por_el0_flags", 8,
+                                     std::vector<RegisterTypeFlags::Field>{
+                                         {"Perm15", 60, 63, por_el0_perm_enum},
+                                         {"Perm14", 56, 59, por_el0_perm_enum},
+                                         {"Perm13", 52, 55, por_el0_perm_enum},
+                                         {"Perm12", 48, 51, por_el0_perm_enum},
+                                         {"Perm11", 44, 47, por_el0_perm_enum},
+                                         {"Perm10", 40, 43, por_el0_perm_enum},
+                                         {"Perm9", 36, 39, por_el0_perm_enum},
+                                         {"Perm8", 32, 35, por_el0_perm_enum},
+                                         {"Perm7", 28, 31, por_el0_perm_enum},
+                                         {"Perm6", 24, 27, por_el0_perm_enum},
+                                         {"Perm5", 20, 23, por_el0_perm_enum},
+                                         {"Perm4", 16, 19, por_el0_perm_enum},
+                                         {"Perm3", 12, 15, por_el0_perm_enum},
+                                         {"Perm2", 8, 11, por_el0_perm_enum},
+                                         {"Perm1", 4, 7, por_el0_perm_enum},
+                                         {"Perm0", 0, 3, por_el0_perm_enum},
+                                     });
 }
 
 const RegisterType *Arm64RegisterTypeDetector::DetectFPMRType(uint64_t hwcap,
@@ -83,25 +82,24 @@ const RegisterType *Arm64RegisterTypeDetector::DetectFPMRType(uint64_t hwcap,
   (void)hwcap3;
 
   if (!(hwcap2 & HWCAP2_FPMR))
-    return {};
-
-  static const RegisterTypeEnum fp8_format_enum("fp8_format_enum",
-                                                {
-                                                    {0, "FP8_E5M2"},
-                                                    {1, "FP8_E4M3"},
-                                                });
-
-  static const RegisterTypeFlags fpmr_flags("fpmr_flags", 8,
-                                            {{"LSCALE2", 32, 37},
-                                             {"NSCALE", 24, 31},
-                                             {"LSCALE", 16, 22},
-                                             {"OSC", 15},
-                                             {"OSM", 14},
-                                             {"F8D", 6, 8, &fp8_format_enum},
-                                             {"F8S2", 3, 5, &fp8_format_enum},
-                                             {"F8S1", 0, 2, &fp8_format_enum}});
-
-  return &fpmr_flags;
+    return nullptr;
+
+  const RegisterTypeEnum *fp8_format_enum = MakeType<RegisterTypeEnum>(
+      "fp8_format_enum", RegisterTypeEnum::Enumerators{
+                             {0, "FP8_E5M2"},
+                             {1, "FP8_E4M3"},
+                         });
+
+  return MakeType<RegisterTypeFlags>(
+      "fpmr_flags", 8,
+      std::vector<RegisterTypeFlags::Field>{{"LSCALE2", 32, 37},
+                                            {"NSCALE", 24, 31},
+                                            {"LSCALE", 16, 22},
+                                            {"OSC", 15},
+                                            {"OSM", 14},
+                                            {"F8D", 6, 8, fp8_format_enum},
+                                            {"F8S2", 3, 5, fp8_format_enum},
+                                            {"F8S1", 0, 2, fp8_format_enum}});
 }
 
 const RegisterType *Arm64RegisterTypeDetector::DetectGCSFeaturesType(
@@ -110,12 +108,12 @@ const RegisterType *Arm64RegisterTypeDetector::DetectGCSFeaturesType(
   (void)hwcap3;
 
   if (!(hwcap & HWCAP_GCS))
-    return {};
-
-  static const RegisterTypeFlags gcs_features_flags(
-      "gcs_features_flags", 8, {{"PUSH", 2}, {"WRITE", 1}, {"ENABLE", 0}});
+    return nullptr;
 
-  return &gcs_features_flags;
+  return MakeType<RegisterTypeFlags>(
+      "gcs_features_flags", 8,
+      std::vector<RegisterTypeFlags::Field>{
+          {"PUSH", 2}, {"WRITE", 1}, {"ENABLE", 0}});
 }
 
 const RegisterType *Arm64RegisterTypeDetector::DetectSVCRType(uint64_t hwcap,
@@ -130,10 +128,9 @@ const RegisterType *Arm64RegisterTypeDetector::DetectSVCRType(uint64_t hwcap,
   // Represents the pseudo register that lldb-server builds, which itself
   // matches the architectural register SCVR. The fields match SVCR in the Arm
   // manual.
-  static const RegisterTypeFlags svcr_flags("svcr_flags", 8,
-                                            {{"ZA", 1}, {"SM", 0}});
-
-  return &svcr_flags;
+  return MakeType<RegisterTypeFlags>(
+      "svcr_flags", 8,
+      std::vector<RegisterTypeFlags::Field>{{"ZA", 1}, {"SM", 0}});
 }
 
 const RegisterType *
@@ -153,19 +150,19 @@ Arm64RegisterTypeDetector::DetectMTECtrlType(uint64_t hwcap, uint64_t hwcap2,
   if (hwcap3 & HWCAP3_MTE_STORE_ONLY)
     fields.push_back({"STORE_ONLY", 19});
 
-  static const RegisterTypeEnum tcf_enum(
-      "tcf_enum",
-      {{0, "TCF_NONE"}, {1, "TCF_SYNC"}, {2, "TCF_ASYNC"}, {3, "TCF_ASYMM"}});
+  const RegisterTypeEnum *tcf_enum = MakeType<RegisterTypeEnum>(
+      "tcf_enum", RegisterTypeEnum::Enumerators{{0, "TCF_NONE"},
+                                                {1, "TCF_SYNC"},
+                                                {2, "TCF_ASYNC"},
+                                                {3, "TCF_ASYMM"}});
 
   fields.insert(
       std::end(fields),
       {{"TAGS", 3, 18}, // 16 bit bitfield shifted up by PR_MTE_TAG_SHIFT.
-       {"TCF", 1, 2, &tcf_enum},
+       {"TCF", 1, 2, tcf_enum},
        {"TAGGED_ADDR_ENABLE", 0}});
 
-  static const RegisterTypeFlags mte_ctrl_flags("mte_ctrl_flags", 8, fields);
-
-  return &mte_ctrl_flags;
+  return MakeType<RegisterTypeFlags>("mte_ctrl_flags", 8, fields);
 }
 
 const RegisterType *Arm64RegisterTypeDetector::DetectFPCRType(uint64_t hwcap,
@@ -173,15 +170,15 @@ const RegisterType *Arm64RegisterTypeDetector::DetectFPCRType(uint64_t hwcap,
                                                               uint64_t hwcap3) {
   (void)hwcap3;
 
-  static const RegisterTypeEnum rmode_enum(
-      "rmode_enum", {{0, "RN"}, {1, "RP"}, {2, "RM"}, {3, "RZ"}});
-  static RegisterTypeFlags fpcr_flags("fpcr_flags", 4, {});
+  const RegisterTypeEnum *rmode_enum = MakeType<RegisterTypeEnum>(
+      "rmode_enum", RegisterTypeEnum::Enumerators{
+                        {0, "RN"}, {1, "RP"}, {2, "RM"}, {3, "RZ"}});
 
   std::vector<RegisterTypeFlags::Field> fpcr_fields{
       {"AHP", 26},
       {"DN", 25},
       {"FZ", 24},
-      {"RMode", 22, 23, &rmode_enum},
+      {"RMode", 22, 23, rmode_enum},
       // Bits 21-20 are "Stride" which is unused in AArch64 state.
   };
 
@@ -211,9 +208,7 @@ const RegisterType *Arm64RegisterTypeDetector::DetectFPCRType(uint64_t hwcap,
     fpcr_fields.push_back({"FIZ", 0});
   }
 
-  fpcr_flags.SetFields(fpcr_fields);
-
-  return &fpcr_flags;
+  return MakeType<RegisterTypeFlags>("fpcr_flags", 4, fpcr_fields);
 }
 
 const RegisterType *Arm64RegisterTypeDetector::DetectFPSRType(uint64_t hwcap,
@@ -224,9 +219,9 @@ const RegisterType *Arm64RegisterTypeDetector::DetectFPSRType(uint64_t hwcap,
   (void)hwcap2;
   (void)hwcap3;
 
-  static const RegisterTypeFlags fpsr_flags(
+  return MakeType<RegisterTypeFlags>(
       "fpsr_flags", 4,
-      {
+      std::vector<RegisterTypeFlags::Field>{
           // Bits 31-28 are N/Z/C/V, only used by AArch32.
           {"QC", 27},
           // Bits 26-8 reserved.
@@ -238,8 +233,6 @@ const RegisterType *Arm64RegisterTypeDetector::DetectFPSRType(uint64_t hwcap,
           {"DZC", 1},
           {"IOC", 0},
       });
-
-  return &fpsr_flags;
 }
 
 const RegisterType *Arm64RegisterTypeDetector::DetectCPSRType(uint64_t hwcap,
@@ -250,7 +243,6 @@ const RegisterType *Arm64RegisterTypeDetector::DetectCPSRType(uint64_t hwcap,
   // The fields here are a combination of the Arm manual's SPSR_EL1,
   // plus a few changes where Linux has decided not to make use of them at all,
   // or at least not from userspace.
-  static RegisterTypeFlags cpsr_flags("cpsr_flags", 4, {});
 
   // Status bits that are always present.
   std::vector<RegisterTypeFlags::Field> cpsr_fields{
@@ -292,15 +284,13 @@ const RegisterType *Arm64RegisterTypeDetector::DetectCPSRType(uint64_t hwcap,
   // Bit 1 is unused and expected to be 0.
   cpsr_fields.push_back({"SP", 0});
 
-  cpsr_flags.SetFields(cpsr_fields);
-
-  return &cpsr_flags;
+  return MakeType<RegisterTypeFlags>("cpsr_flags", 4, cpsr_fields);
 }
 
 void Arm64RegisterTypeDetector::DetectTypes(uint64_t hwcap, uint64_t hwcap2,
                                             uint64_t hwcap3) {
   for (auto &reg : m_registers)
-    reg.m_type = reg.m_detector(hwcap, hwcap2, hwcap3);
+    reg.m_type = std::invoke(reg.m_detector, this, hwcap, hwcap2, hwcap3);
   m_has_detected = true;
 }
 
diff --git a/lldb/source/Plugins/Process/Utility/RegisterTypeDetector_arm64.h b/lldb/source/Plugins/Process/Utility/RegisterTypeDetector_arm64.h
index 18c2f58bd4d4e..62eb494b3d390 100644
--- a/lldb/source/Plugins/Process/Utility/RegisterTypeDetector_arm64.h
+++ b/lldb/source/Plugins/Process/Utility/RegisterTypeDetector_arm64.h
@@ -51,47 +51,77 @@ class Arm64RegisterTypeDetector {
   bool HasDetected() const { return m_has_detected; }
 
 private:
-  using DetectorFn =
-      std::function<const RegisterType *(uint64_t, uint64_t, uint64_t)>;
+  // A detector function inspects the hwcaps and builds a type for that
+  // register. All types should be made using MakeType, and a raw pointer to
+  // the top level type must be returned.
+  using DetectorFn = const RegisterType *(Arm64RegisterTypeDetector::*)(
+      uint64_t, uint64_t, uint64_t);
 
-  static const RegisterType *DetectCPSRType(uint64_t hwcap, uint64_t hwcap2,
+  const RegisterType *DetectCPSRType(uint64_t hwcap, uint64_t hwcap2,
+                                     uint64_t hwcap3);
+  const RegisterType *DetectFPSRType(uint64_t hwcap, uint64_t hwcap2,
+                                     uint64_t hwcap3);
+  const RegisterType *DetectFPCRType(uint64_t hwcap, uint64_t hwcap2,
+                                     uint64_t hwcap3);
+  const RegisterType *DetectMTECtrlType(uint64_t hwcap, uint64_t hwcap2,
+                                        uint64_t hwcap3);
+  const RegisterType *DetectSVCRType(uint64_t hwcap, uint64_t hwcap2,
+                                     uint64_t hwcap3);
+  const RegisterType *DetectFPMRType(uint64_t hwcap, uint64_t hwcap2,
+                                     uint64_t hwcap3);
+  const RegisterType *DetectGCSFeaturesType(uint64_t hwcap, uint64_t hwcap2,
                                             uint64_t hwcap3);
-  static const RegisterType *DetectFPSRType(uint64_t hwcap, uint64_t hwcap2,
-                                            uint64_t hwcap3);
-  static const RegisterType *DetectFPCRType(uint64_t hwcap, uint64_t hwcap2,
-                                            uint64_t hwcap3);
-  static const RegisterType *DetectMTECtrlType(uint64_t hwcap, uint64_t hwcap2,
-                                               uint64_t hwcap3);
-  static const RegisterType *DetectSVCRType(uint64_t hwcap, uint64_t hwcap2,
-                                            uint64_t hwcap3);
-  static const RegisterType *DetectFPMRType(uint64_t hwcap, uint64_t hwcap2,
-                                            uint64_t hwcap3);
-  static const RegisterType *
-  DetectGCSFeaturesType(uint64_t hwcap, uint64_t hwcap2, uint64_t hwcap3);
-  static const RegisterType *DetectPOREL0Type(uint64_t hwcap, uint64_t hwcap2,
-                                              uint64_t hwcap3);
+  const RegisterType *DetectPOREL0Type(uint64_t hwcap, uint64_t hwcap2,
+                                       uint64_t hwcap3);
 
   struct RegisterEntry {
     RegisterEntry(llvm::StringRef name, unsigned size, DetectorFn detector)
-        : m_name(name), m_type(nullptr), m_detector(detector) {}
+        : m_name(name), m_detector(detector) {}
 
     llvm::StringRef m_name;
+    // A raw pointer to the top level type. This pointer's lifetime is managed
+    // by a unique pointer of the same value in m_detected_types.
     const RegisterType *m_type;
     DetectorFn m_detector;
   } m_registers[9] = {
-      RegisterEntry("cpsr", 4, DetectCPSRType),
-      RegisterEntry("fpsr", 4, DetectFPSRType),
-      RegisterEntry("fpcr", 4, DetectFPCRType),
-      RegisterEntry("mte_ctrl", 8, DetectMTECtrlType),
-      RegisterEntry("svcr", 8, DetectSVCRType),
-      RegisterEntry("fpmr", 8, DetectFPMRType),
-      RegisterEntry("gcs_features_enabled", 8, DetectGCSFeaturesType),
-      RegisterEntry("gcs_features_locked", 8, DetectGCSFeaturesType),
-      RegisterEntry("por_el0", 8, DetectPOREL0Type),
+      RegisterEntry("cpsr", 4, &Arm64RegisterTypeDetector::DetectCPSRType),
+      RegisterEntry("fpsr", 4, &Arm64RegisterTypeDetector::DetectFPSRType),
+      RegisterEntry("fpcr", 4, &Arm64RegisterTypeDetector::DetectFPCRType),
+      RegisterEntry("mte_ctrl", 8,
+                    &Arm64RegisterTypeDetector::DetectMTECtrlType),
+      RegisterEntry("svcr", 8, &Arm64RegisterTypeDetector::DetectSVCRType),
+      RegisterEntry("fpmr", 8, &Arm64RegisterTypeDetector::DetectFPMRType),
+      RegisterEntry("gcs_features_enabled", 8,
+                    &Arm64RegisterTypeDetector::DetectGCSFeaturesType),
+      RegisterEntry("gcs_features_locked", 8,
+                    &Arm64RegisterTypeDetector::DetectGCSFeaturesType),
+      RegisterEntry("por_el0", 8, &Arm64RegisterTypeDetector::DetectPOREL0Type),
   };
 
   // Becomes true once field detection has been run for all registers.
   bool m_has_detected = false;
+
+  template <typename T, typename... Args> const T *MakeType(Args &&...args) {
+    static_assert(std::is_base_of_v<RegisterType, T>);
+
+    auto type = std::make_unique<T>(std::forward<Args>(args)...);
+    const T *type_ptr = type.get();
+    m_detected_types.detected_types.push_back(std::move(type));
+    return type_ptr;
+  }
+
+  // This stores all the types created. There may be > 1 type per register,
+  // as a register may nest types (enums for fields for example).
+  // We do not use a vector of RegisterType, because the address of the types
+  // must remain the same as new types are created.
+  // Code other than MakeType should not use this vector directly, hence the
+  // class wrapper to enforce that.
+  class DetectedTypesHolder {
+    std::vector<std::unique_ptr<RegisterType>> detected_types;
+
+    template <typename T, typename... Args>
+    friend const T *Arm64RegisterTypeDetector::MakeType(Args &&...args);
+  } m_detected_types;
 };
 
 } // namespace lldb_private
diff --git a/lldb/test/API/linux/aarch64/mte_core_file/TestAArch64LinuxMTEMemoryTagCoreFile.py b/lldb/test/API/linux/aarch64/mte_core_file/TestAArch64LinuxMTEMemoryTagCoreFile.py
index ecda353a421e1..9462252b3a691 100644
--- a/lldb/test/API/linux/aarch64/mte_core_file/TestAArch64LinuxMTEMemoryTagCoreFile.py
+++ b/lldb/test/API/linux/aarch64/mte_core_file/TestAArch64LinuxMTEMemoryTagCoreFile.py
@@ -252,6 +252,25 @@ def test_mte_ctrl_register(self):
                 ],
             )
 
+    @skipIfLLVMTargetMissing("AArch64")
+    @skipIfXmlSupportMissing
+    def test_mte_ctrl_multiple_core_files(self):
+        """Core files from machines with different hardware features should
+        show different fields, even when loaded into the same session."""
+
+        # This core is from a machine with MTE but not store only MTE.
+        core_path = self.getSourcePath(
+            os.path.join("..", "non_address_bit_memory_access", "corefile")
+        )
+        self.runCmd(f"target create --core {core_path}")
+        self.expect(
+            "register read mte_ctrl", substrs=["STORE_ONLY = 0"], matching=False
+        )
+
+        # This core is from a machine with both MTE and store only MTE.
+        self.runCmd("target create --core core.mte")
+        self.expect("register read mte_ctrl", substrs=["STORE_ONLY = 0"])
+
     @skipIfLLVMTargetMissing("AArch64")
     def test_mte_no_tags(self):
         """Test that we handle there being a tag segment but that segment does

>From d08c8cbf0ba5fd485f7d1ef1b448a11971e0c522 Mon Sep 17 00:00:00 2001
From: David Spickett <david.spickett at linaro.org>
Date: Fri, 6 Sep 2024 13:17:43 +0000
Subject: [PATCH 2/2] [lldb] Refactor RegisterTypeBuilder

This prepares it for emitting union types. Major changes:
* Entry function is now a dispatcher to builder functions for each type.
* Name mangling is standardised.
* The register name parameter is no longer needed and so was removed.
---
 .../include/lldb/Target/RegisterTypeBuilder.h |   5 +-
 lldb/include/lldb/Target/Target.h             |   5 +-
 lldb/source/Core/DumpRegisterValue.cpp        |   4 +-
 .../RegisterTypeBuilderClang.cpp              | 193 ++++++++++--------
 .../RegisterTypeBuilderClang.h                |  15 +-
 lldb/source/Target/Target.cpp                 |   6 +-
 6 files changed, 127 insertions(+), 101 deletions(-)

diff --git a/lldb/include/lldb/Target/RegisterTypeBuilder.h b/lldb/include/lldb/Target/RegisterTypeBuilder.h
index c24d218962e39..5a37109661da0 100644
--- a/lldb/include/lldb/Target/RegisterTypeBuilder.h
+++ b/lldb/include/lldb/Target/RegisterTypeBuilder.h
@@ -19,9 +19,8 @@ class RegisterTypeBuilder : public PluginInterface {
   ~RegisterTypeBuilder() override = default;
 
   virtual CompilerType
-  GetRegisterType(const std::string &name,
-                  const lldb_private::RegisterType &type_info,
-                  uint32_t byte_size) = 0;
+  GetRegisterType(const lldb_private::RegisterType &type_info,
+                  uint32_t register_byte_size) = 0;
 
 protected:
   RegisterTypeBuilder() = default;
diff --git a/lldb/include/lldb/Target/Target.h b/lldb/include/lldb/Target/Target.h
index 39602421cfd96..3e1914513bad4 100644
--- a/lldb/include/lldb/Target/Target.h
+++ b/lldb/include/lldb/Target/Target.h
@@ -1564,9 +1564,8 @@ class Target : public std::enable_shared_from_this<Target>,
   ///     if none can be found.
   llvm::Expected<lldb_private::Address> GetEntryPointAddress();
 
-  CompilerType GetRegisterType(const std::string &name,
-                               const lldb_private::RegisterType &type_info,
-                               uint32_t byte_size);
+  CompilerType GetRegisterType(const lldb_private::RegisterType &type_info,
+                               uint32_t register_byte_size);
 
   /// Sends a breakpoint notification event.
   void NotifyBreakpointChanged(Breakpoint &bp,
diff --git a/lldb/source/Core/DumpRegisterValue.cpp b/lldb/source/Core/DumpRegisterValue.cpp
index 7096cfec5e11c..4ecaf0f08a693 100644
--- a/lldb/source/Core/DumpRegisterValue.cpp
+++ b/lldb/source/Core/DumpRegisterValue.cpp
@@ -129,8 +129,8 @@ void lldb_private::DumpRegisterValue(const RegisterValue &reg_val, Stream &s,
       (reg_info.byte_size != 4 && reg_info.byte_size != 8))
     return;
 
-  CompilerType register_compiler_type = target_sp->GetRegisterType(
-      reg_info.name, *reg_info.register_type, reg_info.byte_size);
+  CompilerType register_compiler_type =
+      target_sp->GetRegisterType(*reg_info.register_type, reg_info.byte_size);
   if (!register_compiler_type.IsValid())
     return;
 
diff --git a/lldb/source/Plugins/RegisterTypeBuilder/RegisterTypeBuilderClang.cpp b/lldb/source/Plugins/RegisterTypeBuilder/RegisterTypeBuilderClang.cpp
index d63c7e2e71bc1..9577a21077805 100644
--- a/lldb/source/Plugins/RegisterTypeBuilder/RegisterTypeBuilderClang.cpp
+++ b/lldb/source/Plugins/RegisterTypeBuilder/RegisterTypeBuilderClang.cpp
@@ -8,10 +8,8 @@
 
 #include "clang/AST/DeclCXX.h"
 
-#include "Plugins/TypeSystem/Clang/TypeSystemClang.h"
 #include "RegisterTypeBuilderClang.h"
 #include "lldb/Core/PluginManager.h"
-#include "lldb/Utility/RegisterTypeFlags.h"
 #include "lldb/lldb-enumerations.h"
 
 using namespace lldb_private;
@@ -35,94 +33,117 @@ RegisterTypeBuilderClang::CreateInstance(Target &target) {
 RegisterTypeBuilderClang::RegisterTypeBuilderClang(Target &target)
     : m_target(target) {}
 
+static std::string MakeTypeName(const RegisterType &type_info,
+                                uint32_t register_byte_size) {
+  std::string type_name = "__lldb_register_fields_";
+  switch (type_info.getKind()) {
+  case RegisterType::eRegisterTypeKindFlags:
+    type_name += "flags_";
+    break;
+  case RegisterType::eRegisterTypeKindEnum:
+    // Enums can be used by many registers and the size of each register
+    // may be different. The register size is used as the underlying size
+    // of the enumerators, so we must make one enum type per register size
+    // it is used with.
+    type_name += "enum_" + std::to_string(register_byte_size) + "_";
+    break;
+  }
+
+  return type_name + type_info.GetID();
+}
+
+CompilerType
+RegisterTypeBuilderClang::BuildEnumType(const RegisterTypeEnum &enum_type_info,
+                                        uint32_t register_byte_size,
+                                        lldb::TypeSystemClangSP type_system) {
+  std::string enum_type_name = MakeTypeName(enum_type_info, register_byte_size);
+
+  // Reuse existing type if we can.
+  if (CompilerType enum_type =
+          type_system->GetTypeForIdentifier<clang::EnumDecl>(
+              type_system->getASTContext(), enum_type_name))
+    return enum_type;
+
+  CompilerType register_uint_type =
+      type_system->GetBuiltinTypeForEncodingAndBitSize(lldb::eEncodingUint,
+                                                       register_byte_size * 8);
+  CompilerType enum_type = type_system->CreateEnumerationType(
+      enum_type_name, type_system->GetTranslationUnitDecl(),
+      OptionalClangModuleID(), Declaration(), register_uint_type, false);
+
+  type_system->StartTagDeclarationDefinition(enum_type);
+
+  Declaration decl;
+  for (const auto &enumerator : enum_type_info.GetEnumerators()) {
+    type_system->AddEnumerationValueToEnumerationType(
+        enum_type, decl, enumerator.m_name.c_str(), enumerator.m_value,
+        register_byte_size * 8);
+  }
+
+  type_system->CompleteTagDeclarationDefinition(enum_type);
+
+  return enum_type;
+}
+
+CompilerType RegisterTypeBuilderClang::BuildFlagsType(
+    const lldb_private::RegisterTypeFlags &flags_info,
+    uint32_t register_byte_size, lldb::TypeSystemClangSP type_system) {
+  std::string register_type_name = MakeTypeName(flags_info, register_byte_size);
+
+  // Reuse existing type if we can.
+  if (CompilerType flags_type =
+          type_system->GetTypeForIdentifier<clang::CXXRecordDecl>(
+              type_system->getASTContext(), register_type_name))
+    return flags_type;
+
+  // In most ABI, a change of field type means a change in storage unit.
+  // We want it all in one unit, so we use a field type the same as the
+  // register's size.
+  CompilerType field_uint_type =
+      type_system->GetBuiltinTypeForEncodingAndBitSize(lldb::eEncodingUint,
+                                                       register_byte_size * 8);
+
+  CompilerType flags_type = type_system->CreateRecordType(
+      nullptr, OptionalClangModuleID(), register_type_name,
+      llvm::to_underlying(clang::TagTypeKind::Struct), lldb::eLanguageTypeC);
+  type_system->StartTagDeclarationDefinition(flags_type);
+
+  for (auto field : flags_info.GetFields()) {
+    CompilerType field_type = field_uint_type;
+
+    if (const RegisterTypeEnum *enum_type_info = field.GetEnum())
+      if (!enum_type_info->GetEnumerators().empty())
+        field_type =
+            BuildEnumType(*enum_type_info, register_byte_size, type_system);
+
+    type_system->AddFieldToRecordType(flags_type, field.GetName(), field_type,
+                                      field.GetSizeInBits());
+  }
+
+  type_system->CompleteTagDeclarationDefinition(flags_type);
+  // So that the size of the type matches the size of the register.
+  type_system->SetIsPacked(flags_type);
+
+  // This should be true if RegisterTypeFlags padded correctly.
+  assert(
+      llvm::expectedToOptional(flags_type.GetByteSize(nullptr)).value_or(0) ==
+      flags_info.GetSize());
+
+  return flags_type;
+}
+
 CompilerType RegisterTypeBuilderClang::GetRegisterType(
-    const std::string &name, const lldb_private::RegisterType &type_info,
-    uint32_t byte_size) {
+    const lldb_private::RegisterType &type_info, uint32_t register_byte_size) {
   lldb::TypeSystemClangSP type_system =
       ScratchTypeSystemClang::GetForTarget(m_target);
   assert(type_system);
 
-  std::string register_type_name = "__lldb_register_fields_" + name;
-  // For now we can only build sets of flags.
-  const RegisterTypeFlags *flags =
-      llvm::dyn_cast<RegisterTypeFlags>(&type_info);
-  if (!flags)
-    return {};
-
-  // See if we have made this type before and can reuse it.
-  CompilerType fields_type =
-      type_system->GetTypeForIdentifier<clang::CXXRecordDecl>(
-          type_system->getASTContext(), register_type_name);
-
-  if (!fields_type) {
-    // In most ABI, a change of field type means a change in storage unit.
-    // We want it all in one unit, so we use a field type the same as the
-    // register's size.
-    CompilerType field_uint_type =
-        type_system->GetBuiltinTypeForEncodingAndBitSize(lldb::eEncodingUint,
-                                                         byte_size * 8);
-
-    fields_type = type_system->CreateRecordType(
-        nullptr, OptionalClangModuleID(), register_type_name,
-        llvm::to_underlying(clang::TagTypeKind::Struct), lldb::eLanguageTypeC);
-    type_system->StartTagDeclarationDefinition(fields_type);
-
-    // We assume that RegisterTypeFlags has padded and sorted the fields
-    // already.
-    for (const RegisterTypeFlags::Field &field : flags->GetFields()) {
-      CompilerType field_type = field_uint_type;
-
-      if (const RegisterTypeEnum *enum_type = field.GetEnum()) {
-        const RegisterTypeEnum::Enumerators &enumerators =
-            enum_type->GetEnumerators();
-        if (!enumerators.empty()) {
-          // Enums can be used by many registers and the size of each register
-          // may be different. The register size is used as the underlying size
-          // of the enumerators, so we must make one enum type per register size
-          // it is used with.
-          std::string enum_type_name = "__lldb_register_fields_enum_" +
-                                       enum_type->GetID() + "_" +
-                                       std::to_string(byte_size);
-
-          // Enums can be used by mutiple fields and multiple registers, so we
-          // may have built this one already.
-          CompilerType field_enum_type =
-              type_system->GetTypeForIdentifier<clang::EnumDecl>(
-                  type_system->getASTContext(), enum_type_name);
-
-          if (field_enum_type)
-            field_type = field_enum_type;
-          else {
-            field_type = type_system->CreateEnumerationType(
-                enum_type_name, type_system->GetTranslationUnitDecl(),
-                OptionalClangModuleID(), Declaration(), field_uint_type, false);
-
-            type_system->StartTagDeclarationDefinition(field_type);
-
-            Declaration decl;
-            for (auto enumerator : enumerators) {
-              type_system->AddEnumerationValueToEnumerationType(
-                  field_type, decl, enumerator.m_name.c_str(),
-                  enumerator.m_value, byte_size * 8);
-            }
-
-            type_system->CompleteTagDeclarationDefinition(field_type);
-          }
-        }
-      }
-
-      type_system->AddFieldToRecordType(fields_type, field.GetName(),
-                                        field_type, field.GetSizeInBits());
-    }
-
-    type_system->CompleteTagDeclarationDefinition(fields_type);
-    // So that the size of the type matches the size of the register.
-    type_system->SetIsPacked(fields_type);
-
-    // This should be true if RegisterTypeFlags padded correctly.
-    assert(llvm::expectedToOptional(fields_type.GetByteSize(nullptr))
-               .value_or(0) == flags->GetSize());
+  switch (type_info.getKind()) {
+  case RegisterType::eRegisterTypeKindFlags:
+    return BuildFlagsType(*llvm::dyn_cast<RegisterTypeFlags>(&type_info),
+                          register_byte_size, type_system);
+  case RegisterType::eRegisterTypeKindEnum:
+    return BuildEnumType(*llvm::dyn_cast<RegisterTypeEnum>(&type_info),
+                         register_byte_size, type_system);
   }
-
-  return fields_type;
 }
diff --git a/lldb/source/Plugins/RegisterTypeBuilder/RegisterTypeBuilderClang.h b/lldb/source/Plugins/RegisterTypeBuilder/RegisterTypeBuilderClang.h
index f00fdc1a5587d..47fff0699b03c 100644
--- a/lldb/source/Plugins/RegisterTypeBuilder/RegisterTypeBuilderClang.h
+++ b/lldb/source/Plugins/RegisterTypeBuilder/RegisterTypeBuilderClang.h
@@ -9,8 +9,10 @@
 #ifndef LLDB_SOURCE_PLUGINS_REGISTERTYPEBUILDER_REGISTERTYPEBUILDERCLANG_H
 #define LLDB_SOURCE_PLUGINS_REGISTERTYPEBUILDER_REGISTERTYPEBUILDERCLANG_H
 
+#include "Plugins/TypeSystem/Clang/TypeSystemClang.h"
 #include "lldb/Target/RegisterTypeBuilder.h"
 #include "lldb/Target/Target.h"
+#include "lldb/Utility/RegisterTypeFlags.h"
 
 namespace lldb_private {
 class RegisterTypeBuilderClang : public RegisterTypeBuilder {
@@ -28,11 +30,18 @@ class RegisterTypeBuilderClang : public RegisterTypeBuilder {
   }
   static lldb::RegisterTypeBuilderSP CreateInstance(Target &target);
 
-  CompilerType GetRegisterType(const std::string &name,
-                               const lldb_private::RegisterType &type_info,
-                               uint32_t byte_size) override;
+  CompilerType GetRegisterType(const lldb_private::RegisterType &type_info,
+                               uint32_t register_byte_size) override;
 
 private:
+  CompilerType BuildEnumType(const RegisterTypeEnum &enum_type_info,
+                             uint32_t register_byte_size,
+                             lldb::TypeSystemClangSP type_system);
+
+  CompilerType BuildFlagsType(const RegisterTypeFlags &flags_info,
+                              uint32_t register_byte_size,
+                              lldb::TypeSystemClangSP type_system);
+
   Target &m_target;
 };
 } // namespace lldb_private
diff --git a/lldb/source/Target/Target.cpp b/lldb/source/Target/Target.cpp
index b83d67bbf045e..f55ca3908ea34 100644
--- a/lldb/source/Target/Target.cpp
+++ b/lldb/source/Target/Target.cpp
@@ -2740,14 +2740,12 @@ Target::GetScratchTypeSystemForLanguage(lldb::LanguageType language,
 }
 
 CompilerType
-Target::GetRegisterType(const std::string &name,
-                        const lldb_private::RegisterType &type_info,
+Target::GetRegisterType(const lldb_private::RegisterType &type_info,
                         uint32_t byte_size) {
   if (!m_register_type_builder_sp)
     m_register_type_builder_sp = PluginManager::GetRegisterTypeBuilder(*this);
   assert(m_register_type_builder_sp);
-  return m_register_type_builder_sp->GetRegisterType(name, type_info,
-                                                     byte_size);
+  return m_register_type_builder_sp->GetRegisterType(type_info, byte_size);
 }
 
 std::vector<lldb::TypeSystemSP>



More information about the llvm-branch-commits mailing list