https://github.com/DavidSpickett updated https://github.com/llvm/llvm-project/pull/204818
>From 08a465bbd59b9c2f77d40487f4d1fcbd98747eee Mon Sep 17 00:00:00 2001 From: David Spickett <[email protected]> Date: Thu, 18 Jun 2026 14:34:18 +0000 Subject: [PATCH 1/3] [lldb] Print register enum types only once Even if multiple fields reference them. I found this debugging AArch64 POE which has a register that contains 167 permission fields. All of which use the same enum type. Before these changes: (lldb) register info por <...> Perm15: 0 = No Access, 1 = Read, 2 = Execute, 3 = Read, Execute, 4 = Write, 5 = Write, Read, 6 = Write, Execute, 7 = Read, Write, Execute <... 14 copies of the enum one per field...> Perm0: 0 = No Access, 1 = Read, 2 = Execute, 3 = Read, Execute, 4 = Write, 5 = Write, Read, 6 = Write, Execute, 7 = Read, Write, Execute After: (lldb) register info por_el0 <...> Perm15, Perm14, Perm13, Perm12, Perm11, Perm10, Perm9, Perm8, Perm7, Perm6, Perm5, Perm4, Perm3, Perm2, Perm1, Perm0: 0 = No Access, 1 = Read, 2 = Execute, 3 = Read, Execute, 4 = Write, 5 = Write, Read, 6 = Write, Execute, 7 = Read, Write, Execute Which looks weird because we indent the enum according to the list of fields still, but I will change that later. Anyway, this won't be a problem for most cases where 2 or 3 fields share a type. This register is an extreme example. A unit test has been added and a test for POE using the existing core file tests. Assisted by: Codex (gpt 5.5) --- lldb/source/Target/RegisterFlags.cpp | 37 +++++++++++++++---- .../gdb_remote_client/TestXMLRegisterFlags.py | 4 +- .../permission_overlay/TestAArch64LinuxPOE.py | 11 ++++++ lldb/unittests/Target/RegisterFlagsTest.cpp | 17 ++++++++- 4 files changed, 57 insertions(+), 12 deletions(-) diff --git a/lldb/source/Target/RegisterFlags.cpp b/lldb/source/Target/RegisterFlags.cpp index 976e03870ad9e..ba9de95c5dc0a 100644 --- a/lldb/source/Target/RegisterFlags.cpp +++ b/lldb/source/Target/RegisterFlags.cpp @@ -12,6 +12,7 @@ #include "llvm/ADT/StringExtras.h" +#include <algorithm> #include <limits> #include <numeric> #include <optional> @@ -286,9 +287,14 @@ static void DumpEnumerators(StreamString &strm, size_t indent, } std::string RegisterFlags::DumpEnums(uint32_t max_width) const { - StreamString strm; - bool printed_enumerators_once = false; - + struct TypeToFields { + const FieldEnum *enum_type; + std::string field_names; + }; + std::vector<TypeToFields> enum_types; + + // Accumulate all fields that use the same enum, so that each enum is only + // printed once. for (const auto &field : m_fields) { const FieldEnum *enum_type = field.GetEnum(); if (!enum_type) @@ -298,19 +304,36 @@ std::string RegisterFlags::DumpEnums(uint32_t max_width) const { if (enumerators.empty()) continue; - // Break between enumerators of different fields. + auto existing = std::find_if(enum_types.begin(), enum_types.end(), + [enum_type](const TypeToFields &entry) { + return entry.enum_type == enum_type; + }); + if (existing != enum_types.end()) { + existing->field_names += ", " + field.GetName(); + continue; + } + + enum_types.push_back({enum_type, field.GetName()}); + } + + StreamString strm; + bool printed_enumerators_once = false; + + for (const TypeToFields &entry : enum_types) { + // Break between unique enumerator types. if (printed_enumerators_once) strm << "\n\n"; else printed_enumerators_once = true; - std::string name_string = field.GetName() + ": "; + std::string name_string = entry.field_names + ": "; size_t indent = name_string.size(); size_t current_width = indent; strm << name_string; - DumpEnumerators(strm, indent, current_width, max_width, enumerators); + DumpEnumerators(strm, indent, current_width, max_width, + entry.enum_type->GetEnumerators()); } return strm.GetString().str(); @@ -426,4 +449,4 @@ FieldEnum::FieldEnum(std::string id, const Enumerators &enumerators) UNUSED_IF_ASSERT_DISABLED(enumerator); assert(enumerator.m_name.size() && "Enumerator name cannot be empty"); } -} \ No newline at end of file +} diff --git a/lldb/test/API/functionalities/gdb_remote_client/TestXMLRegisterFlags.py b/lldb/test/API/functionalities/gdb_remote_client/TestXMLRegisterFlags.py index 1d0fd00ede3f0..a2662f7b343fe 100644 --- a/lldb/test/API/functionalities/gdb_remote_client/TestXMLRegisterFlags.py +++ b/lldb/test/API/functionalities/gdb_remote_client/TestXMLRegisterFlags.py @@ -1005,9 +1005,7 @@ def test_many_fields_same_enum(self): expected_info = [ dedent( """\ - f2: 1 = valid - - f1: 1 = valid$""" + f2, f1: 1 = valid$""" ) ] self.expect("register info x0", patterns=expected_info) diff --git a/lldb/test/API/linux/aarch64/permission_overlay/TestAArch64LinuxPOE.py b/lldb/test/API/linux/aarch64/permission_overlay/TestAArch64LinuxPOE.py index 124ec08cb9972..34d2e10d3bb48 100644 --- a/lldb/test/API/linux/aarch64/permission_overlay/TestAArch64LinuxPOE.py +++ b/lldb/test/API/linux/aarch64/permission_overlay/TestAArch64LinuxPOE.py @@ -151,6 +151,17 @@ def test_poe_core(self): substrs=[f" {self.EXPECTED_POR_EL0}\n" + self.EXPECTED_POR_EL0_FIELDS], ) + # por_el0 is an unusal case where every field uses the same enum. + # We should print all the field names in a list, then the enum only + # once. Rather than printing the enum 15 times, once for each field. + self.expect( + "register info por_el0", + substrs=[ + ", ".join([f"Perm{n}" for n in range(15, -1, -1)]) + + ": 0 = No Access" + ], + ) + # Protection keys are listed in /proc/<pid>/smaps, which is not included # in core files. self.expect("memory region --all", substrs=["protection key:"], matching=False) diff --git a/lldb/unittests/Target/RegisterFlagsTest.cpp b/lldb/unittests/Target/RegisterFlagsTest.cpp index ecffdd0fe44e6..4765791ae59f2 100644 --- a/lldb/unittests/Target/RegisterFlagsTest.cpp +++ b/lldb/unittests/Target/RegisterFlagsTest.cpp @@ -336,9 +336,22 @@ TEST(RegisterFlagsTest, DumpEnums) { RegisterFlags::Field{"E", 0, 0}, }) .DumpEnums(80), - "B: 0 = an_enumerator, 1 = another_enumerator\n" + "B, D: 0 = an_enumerator, 1 = another_enumerator"); + + // Fields using the same enum should be grouped together. + FieldEnum repeated_enum("repeated_enum", + {{0, "zero"}, {1, "one"}, {2, "two"}}); + ASSERT_EQ(RegisterFlags("", 8, + { + RegisterFlags::Field{"A", 6, 7, &repeated_enum}, + RegisterFlags::Field{"B", 4, 5, &repeated_enum}, + RegisterFlags::Field{"C", 2, 3, &enum_2}, + RegisterFlags::Field{"D", 0, 1, &repeated_enum}, + }) + .DumpEnums(80), + "A, B, D: 0 = zero, 1 = one, 2 = two\n" "\n" - "D: 0 = an_enumerator, 1 = another_enumerator"); + "C: 0 = Cdef_enumerator_1, 1 = Cdef_enumerator_2"); } TEST(RegisterFieldsTest, FlagsToXML) { >From f00bf3b76af81807964dce80b26f0a3807fcf807 Mon Sep 17 00:00:00 2001 From: David Spickett <[email protected]> Date: Fri, 19 Jun 2026 12:50:13 +0000 Subject: [PATCH 2/3] remove whitespace change --- lldb/source/Target/RegisterFlags.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lldb/source/Target/RegisterFlags.cpp b/lldb/source/Target/RegisterFlags.cpp index ba9de95c5dc0a..bd0d66e452f78 100644 --- a/lldb/source/Target/RegisterFlags.cpp +++ b/lldb/source/Target/RegisterFlags.cpp @@ -449,4 +449,4 @@ FieldEnum::FieldEnum(std::string id, const Enumerators &enumerators) UNUSED_IF_ASSERT_DISABLED(enumerator); assert(enumerator.m_name.size() && "Enumerator name cannot be empty"); } -} +} \ No newline at end of file >From a935bb38778e96eca4c31bf8720f796b4680b6a6 Mon Sep 17 00:00:00 2001 From: David Spickett <[email protected]> Date: Wed, 1 Jul 2026 12:24:53 +0000 Subject: [PATCH 3/3] vectormap --- lldb/source/Target/RegisterFlags.cpp | 45 ++++++++-------------------- 1 file changed, 12 insertions(+), 33 deletions(-) diff --git a/lldb/source/Target/RegisterFlags.cpp b/lldb/source/Target/RegisterFlags.cpp index bd0d66e452f78..ee1bdd3426255 100644 --- a/lldb/source/Target/RegisterFlags.cpp +++ b/lldb/source/Target/RegisterFlags.cpp @@ -10,6 +10,7 @@ #include "lldb/Utility/Log.h" #include "lldb/Utility/StreamString.h" +#include "llvm/ADT/MapVector.h" #include "llvm/ADT/StringExtras.h" #include <algorithm> @@ -287,53 +288,31 @@ static void DumpEnumerators(StreamString &strm, size_t indent, } std::string RegisterFlags::DumpEnums(uint32_t max_width) const { - struct TypeToFields { - const FieldEnum *enum_type; - std::string field_names; - }; - std::vector<TypeToFields> enum_types; - // Accumulate all fields that use the same enum, so that each enum is only // printed once. - for (const auto &field : m_fields) { - const FieldEnum *enum_type = field.GetEnum(); - if (!enum_type) - continue; - - const FieldEnum::Enumerators &enumerators = enum_type->GetEnumerators(); - if (enumerators.empty()) - continue; - - auto existing = std::find_if(enum_types.begin(), enum_types.end(), - [enum_type](const TypeToFields &entry) { - return entry.enum_type == enum_type; - }); - if (existing != enum_types.end()) { - existing->field_names += ", " + field.GetName(); - continue; - } - - enum_types.push_back({enum_type, field.GetName()}); - } + llvm::MapVector<const FieldEnum *, std::vector<std::string>> enum_uses; + for (const auto &field : m_fields) + if (const FieldEnum *enum_type = field.GetEnum()) + enum_uses[enum_type].push_back(field.GetName()); StreamString strm; - bool printed_enumerators_once = false; + bool printed_one_enumerator = false; - for (const TypeToFields &entry : enum_types) { + for (const auto &[enum_type, field_names] : enum_uses) { // Break between unique enumerator types. - if (printed_enumerators_once) + if (printed_one_enumerator) strm << "\n\n"; - else - printed_enumerators_once = true; - std::string name_string = entry.field_names + ": "; + printed_one_enumerator = true; + + std::string name_string = llvm::join(field_names, ", ") + ": "; size_t indent = name_string.size(); size_t current_width = indent; strm << name_string; DumpEnumerators(strm, indent, current_width, max_width, - entry.enum_type->GetEnumerators()); + enum_type->GetEnumerators()); } return strm.GetString().str(); _______________________________________________ lldb-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
