https://github.com/DavidSpickett updated https://github.com/llvm/llvm-project/pull/214515
>From 78ee0e6b63a292e40627cc3cf4bcc93904b48142 Mon Sep 17 00:00:00 2001 From: David Spickett <[email protected]> Date: Thu, 6 Aug 2026 14:19:16 +0000 Subject: [PATCH 1/3] [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 ® : 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 ac04848f69ba5e9b82e427258518c69224113943 Mon Sep 17 00:00:00 2001 From: David Spickett <[email protected]> Date: Thu, 6 Aug 2026 15:46:18 +0000 Subject: [PATCH 2/3] put back initialiser --- .../source/Plugins/Process/Utility/RegisterTypeDetector_arm64.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lldb/source/Plugins/Process/Utility/RegisterTypeDetector_arm64.h b/lldb/source/Plugins/Process/Utility/RegisterTypeDetector_arm64.h index 62eb494b3d390..745fd3dddd8b6 100644 --- a/lldb/source/Plugins/Process/Utility/RegisterTypeDetector_arm64.h +++ b/lldb/source/Plugins/Process/Utility/RegisterTypeDetector_arm64.h @@ -76,7 +76,7 @@ class Arm64RegisterTypeDetector { struct RegisterEntry { RegisterEntry(llvm::StringRef name, unsigned size, DetectorFn detector) - : m_name(name), m_detector(detector) {} + : m_name(name), m_type(nullptr), m_detector(detector) {} llvm::StringRef m_name; // A raw pointer to the top level type. This pointer's lifetime is managed >From 90f29b318eabd655aff2043406e8543abb167fbf Mon Sep 17 00:00:00 2001 From: David Spickett <[email protected]> Date: Fri, 7 Aug 2026 12:46:55 +0000 Subject: [PATCH 3/3] formatting --- .../Plugins/Process/Utility/RegisterTypeDetector_arm64.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lldb/source/Plugins/Process/Utility/RegisterTypeDetector_arm64.h b/lldb/source/Plugins/Process/Utility/RegisterTypeDetector_arm64.h index 745fd3dddd8b6..b27bff750c607 100644 --- a/lldb/source/Plugins/Process/Utility/RegisterTypeDetector_arm64.h +++ b/lldb/source/Plugins/Process/Utility/RegisterTypeDetector_arm64.h @@ -54,8 +54,8 @@ class Arm64RegisterTypeDetector { // 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); + using DetectorFn = const RegisterType *( + Arm64RegisterTypeDetector::*)(uint64_t, uint64_t, uint64_t); const RegisterType *DetectCPSRType(uint64_t hwcap, uint64_t hwcap2, uint64_t hwcap3); _______________________________________________ lldb-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
