https://github.com/DavidSpickett updated https://github.com/llvm/llvm-project/pull/213887
>From ebb61705af887e637253960527b1c0d1dd7db043 Mon Sep 17 00:00:00 2001 From: David Spickett <[email protected]> Date: Tue, 3 Sep 2024 10:46:24 +0000 Subject: [PATCH 1/4] [lldb] Store all XML register types in a single string map We are assuming that their ID's are unique, so there's no need to keep separate maps. We can do basic type checking by checking the kind of the type pointed to. A few more methods were added to the base RegisterType. GetSize() returns 0 for enums because enums don't have a size until they are used by a register. This is not ideal but it works for now. --- lldb/include/lldb/Utility/RegisterType.h | 6 + lldb/include/lldb/Utility/RegisterTypeFlags.h | 14 +- .../Process/gdb-remote/ProcessGDBRemote.cpp | 134 +++++++++--------- .../Process/gdb-remote/ProcessGDBRemote.h | 15 +- lldb/source/Utility/RegisterTypeFlags.cpp | 4 +- 5 files changed, 90 insertions(+), 83 deletions(-) diff --git a/lldb/include/lldb/Utility/RegisterType.h b/lldb/include/lldb/Utility/RegisterType.h index 9ecd6c1dfb639..96551262268dd 100644 --- a/lldb/include/lldb/Utility/RegisterType.h +++ b/lldb/include/lldb/Utility/RegisterType.h @@ -51,6 +51,12 @@ class RegisterType { m_dependencies = dependencies; } + virtual void DumpToLog(Log *log) const = 0; + + /// The size of the type in bytes. Return 0 if the size is unknown or context + /// specific. + virtual unsigned GetSize() const = 0; + private: const RegisterTypeKind m_kind; const std::string m_id; diff --git a/lldb/include/lldb/Utility/RegisterTypeFlags.h b/lldb/include/lldb/Utility/RegisterTypeFlags.h index b15e7e6999335..461c3e8e0e0e6 100644 --- a/lldb/include/lldb/Utility/RegisterTypeFlags.h +++ b/lldb/include/lldb/Utility/RegisterTypeFlags.h @@ -46,7 +46,15 @@ class RegisterTypeEnum : public RegisterType { const Enumerators &GetEnumerators() const { return m_enumerators; } - void DumpToLog(Log *log) const; + virtual void DumpToLog(Log *log) const override; + + virtual unsigned GetSize() const override { + // Enums don't have a size until they are used by a specific register, + // so we return 0 just to be sure they don't end up attached directly to a + // register. We expect them to only be used by flags, then the flags are + // attached to the register. + return 0; + } virtual void ToXMLElement(Stream &strm, const RegisterType *user = nullptr) const override; @@ -163,9 +171,9 @@ class RegisterTypeFlags : public RegisterType { } const std::vector<Field> &GetFields() const { return m_fields; } - unsigned GetSize() const { return m_size; } + virtual unsigned GetSize() const override { return m_size; } - void DumpToLog(Log *log) const; + virtual void DumpToLog(Log *log) const override; /// Produce a text table showing the layout of all the fields. Unnamed/padding /// fields will be included, with only their positions shown. diff --git a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp index bed8f936ec888..ba450ff771df2 100644 --- a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp +++ b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp @@ -5008,14 +5008,14 @@ ParseEnumEvalues(const XMLNode &enum_node) { return final_enumerators; } -static void ParseEnums( - XMLNode feature_node, - llvm::StringMap<std::unique_ptr<RegisterTypeEnum>> ®isters_enum_types) { +static void +ParseEnums(XMLNode feature_node, + llvm::StringMap<std::unique_ptr<RegisterType>> ®ister_types) { Log *log(GetLog(GDBRLog::Process)); // The top level element is "<enum...". feature_node.ForEachChildElementWithName( - "enum", [log, ®isters_enum_types](const XMLNode &enum_node) { + "enum", [log, ®ister_types](const XMLNode &enum_node) { std::string id; enum_node.ForEachAttribute([&id](const llvm::StringRef &attr_name, @@ -5043,7 +5043,7 @@ static void ParseEnums( LLDB_LOG(log, "ProcessGDBRemote::ParseEnums Found enum type \"{0}\"", id); - registers_enum_types.insert_or_assign( + register_types.insert_or_assign( id, std::make_unique<RegisterTypeEnum>(id, enumerators)); } } @@ -5053,17 +5053,16 @@ static void ParseEnums( }); } -static std::vector<RegisterTypeFlags::Field> -ParseFlagsFields(XMLNode flags_node, unsigned size, - const llvm::StringMap<std::unique_ptr<RegisterTypeEnum>> - ®isters_enum_types) { +static std::vector<RegisterTypeFlags::Field> ParseFlagsFields( + XMLNode flags_node, unsigned size, + const llvm::StringMap<std::unique_ptr<RegisterType>> ®ister_types) { Log *log(GetLog(GDBRLog::Process)); const unsigned max_start_bit = size * 8 - 1; // Process the fields of this set of flags. std::vector<RegisterTypeFlags::Field> fields; flags_node.ForEachChildElementWithName("field", [&fields, max_start_bit, &log, - ®isters_enum_types]( + ®ister_types]( const XMLNode &field_node) { std::optional<llvm::StringRef> name; @@ -5144,35 +5143,39 @@ ParseFlagsFields(XMLNode flags_node, unsigned size, "that has size > 64 bits, this is not supported", name->data()); else { - // A field's type may be set to the name of an enum type. + // A field's type may be set to another previously defined type. + // Right now we only support enum. const RegisterTypeEnum *enum_type = nullptr; if (type && !type->empty()) { - auto found = registers_enum_types.find(*type); - if (found != registers_enum_types.end()) { - enum_type = found->second.get(); - - // No enumerator can exceed the range of the field itself. - uint64_t max_value = - RegisterTypeFlags::Field::GetMaxValue(*start, *end); - for (const auto &enumerator : enum_type->GetEnumerators()) { - if (enumerator.m_value > max_value) { - enum_type = nullptr; - LLDB_LOG( - log, - "ProcessGDBRemote::ParseFlagsFields In enum \"{0}\" " - "evalue \"{1}\" with value {2} exceeds the maximum value " - "of field \"{3}\" ({4}), ignoring enum", - type->data(), enumerator.m_name, enumerator.m_value, - name->data(), max_value); - break; + auto found = register_types.find(*type); + if (found != register_types.end()) { + enum_type = llvm::dyn_cast<RegisterTypeEnum>(found->second.get()); + if (enum_type) { + // No enumerator can exceed the range of the field itself. + uint64_t max_value = + RegisterTypeFlags::Field::GetMaxValue(*start, *end); + for (const auto &enumerator : enum_type->GetEnumerators()) { + if (enumerator.m_value > max_value) { + enum_type = nullptr; + LLDB_LOG( + log, + "ProcessGDBRemote::ParseFlagsFields In enum \"{0}\" " + "evalue \"{1}\" with value {2} exceeds the maximum " + "value " + "of field \"{3}\" ({4}), ignoring enum", + type->data(), enumerator.m_name, enumerator.m_value, + name->data(), max_value); + break; + } } } } else { - LLDB_LOG(log, - "ProcessGDBRemote::ParseFlagsFields Could not find type " - "\"{0}\" " - "for field \"{1}\", ignoring", - type->data(), name->data()); + LLDB_LOG( + log, + "ProcessGDBRemote::ParseFlagsFields Could not find enum type " + "\"{0}\" " + "for field \"{1}\", ignoring", + type->data(), name->data()); } } @@ -5189,15 +5192,11 @@ ParseFlagsFields(XMLNode flags_node, unsigned size, void ParseFlags( XMLNode feature_node, - llvm::StringMap<std::unique_ptr<RegisterTypeFlags>> ®isters_flags_types, - const llvm::StringMap<std::unique_ptr<RegisterTypeEnum>> - ®isters_enum_types) { + llvm::StringMap<std::unique_ptr<RegisterType>> ®ister_types) { Log *log(GetLog(GDBRLog::Process)); feature_node.ForEachChildElementWithName( - "flags", - [&log, ®isters_flags_types, - ®isters_enum_types](const XMLNode &flags_node) -> bool { + "flags", [&log, ®ister_types](const XMLNode &flags_node) -> bool { LLDB_LOG(log, "ProcessGDBRemote::ParseFlags Found flags node \"{0}\"", flags_node.GetAttributeValue("id").c_str()); @@ -5230,7 +5229,7 @@ void ParseFlags( if (id && size) { // Process the fields of this set of flags. std::vector<RegisterTypeFlags::Field> fields = - ParseFlagsFields(flags_node, *size, registers_enum_types); + ParseFlagsFields(flags_node, *size, register_types); if (fields.size()) { // Sort so that the fields with the MSBs are first. std::sort(fields.rbegin(), fields.rend()); @@ -5243,26 +5242,27 @@ void ParseFlags( // If no fields overlap, use them. if (overlap == fields.end()) { - if (registers_flags_types.contains(*id)) { + if (register_types.contains(*id)) { // In theory you could define some flag set, use it with a - // register then redefine it. We do not know if anyone does + // register then reuse the ID. We do not know if anyone does // that, or what they would expect to happen in that case. // // LLDB chooses to take the first definition and ignore the rest // as waiting until everything has been processed is more - // expensive and difficult. This means that pointers to flag - // sets in the register info remain valid if later the flag set - // is redefined. If we allowed redefinitions, LLDB would crash + // expensive and difficult. This means that pointers to types + // in the register info remain valid if later the ID is reused. + // If we allowed redefinitions, LLDB would crash // when you tried to print a register that used the original // definition. LLDB_LOG( log, - "ProcessGDBRemote::ParseFlags Definition of flags " + "ProcessGDBRemote::ParseFlags Definition of flags with ID " "\"{0}\" shadows " - "previous definition, using original definition instead.", + "previous use of that ID, using original definition " + "instead.", id->data()); } else { - registers_flags_types.insert_or_assign( + register_types.insert_or_assign( *id, std::make_unique<RegisterTypeFlags>( id->str(), *size, std::move(fields))); } @@ -5295,25 +5295,21 @@ void ParseFlags( bool ParseRegisters( XMLNode feature_node, GdbServerTargetInfo &target_info, std::vector<DynamicRegisterInfo::Register> ®isters, - llvm::StringMap<std::unique_ptr<RegisterTypeFlags>> ®isters_flags_types, - llvm::StringMap<std::unique_ptr<RegisterTypeEnum>> ®isters_enum_types) { + llvm::StringMap<std::unique_ptr<RegisterType>> ®ister_types) { if (!feature_node) return false; Log *log(GetLog(GDBRLog::Process)); // Enums first because they are referenced by fields in the flags. - ParseEnums(feature_node, registers_enum_types); - for (const auto &enum_type : registers_enum_types) - enum_type.second->DumpToLog(log); - - ParseFlags(feature_node, registers_flags_types, registers_enum_types); - for (const auto &flags : registers_flags_types) - flags.second->DumpToLog(log); + ParseEnums(feature_node, register_types); + ParseFlags(feature_node, register_types); + for (const auto ®ister_type : register_types) + register_type.second->DumpToLog(log); feature_node.ForEachChildElementWithName( "reg", - [&target_info, ®isters, ®isters_flags_types, + [&target_info, ®isters, ®ister_types, log](const XMLNode ®_node) -> bool { std::string gdb_group; std::string gdb_type; @@ -5392,19 +5388,19 @@ bool ParseRegisters( if (!gdb_type.empty()) { // gdb_type could reference some flags type defined in XML. - llvm::StringMap<std::unique_ptr<RegisterTypeFlags>>::iterator it = - registers_flags_types.find(gdb_type); - if (it != registers_flags_types.end()) { - auto flags_type = it->second.get(); - if (reg_info.byte_size == flags_type->GetSize()) - reg_info.register_type = flags_type; + llvm::StringMap<std::unique_ptr<RegisterType>>::iterator it = + register_types.find(gdb_type); + if (it != register_types.end()) { + auto register_type = it->second.get(); + if (reg_info.byte_size == register_type->GetSize()) + reg_info.register_type = register_type; else LLDB_LOG( log, "ProcessGDBRemote::ParseRegisters Size of register flags {0} " "({1} bytes) for register {2} does not match the register " "size ({3} bytes). Ignoring this set of flags.", - flags_type->GetID().c_str(), flags_type->GetSize(), + register_type->GetID().c_str(), register_type->GetSize(), reg_info.name, reg_info.byte_size); } @@ -5574,8 +5570,7 @@ bool ProcessGDBRemote::GetGDBServerRegisterInfoXMLAndProcess( if (arch_to_use.IsValid()) { for (auto &feature_node : feature_nodes) { - ParseRegisters(feature_node, target_info, registers, - m_registers_flags_types, m_registers_enum_types); + ParseRegisters(feature_node, target_info, registers, m_register_types); } for (const auto &include : target_info.includes) { @@ -5651,8 +5646,7 @@ llvm::Error ProcessGDBRemote::GetGDBServerRegisterInfo(ArchSpec &arch_to_use) { // That's why we clear the cache here, and not in // GetGDBServerRegisterInfoXMLAndProcess. To prevent it being cleared on every // include read. - m_registers_flags_types.clear(); - m_registers_enum_types.clear(); + m_register_types.clear(); std::vector<DynamicRegisterInfo::Register> registers; if (GetGDBServerRegisterInfoXMLAndProcess(arch_to_use, "target.xml", registers) && diff --git a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h index 4b60f9c662910..89bd9605710bb 100644 --- a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h +++ b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h @@ -560,19 +560,18 @@ class ProcessGDBRemote : public Process, void ParseExpeditedRegisters(ExpeditedRegisterMap &expedited_register_map, lldb::ThreadSP thread_sp); - // Lists of register fields generated from the remote's target XML. - // Pointers to these RegisterTypeFlags will be set in the register info passed + // Lists of register types generated from the remote's target XML. + // Pointers to these RegisterTypes will be set in the register info passed // back to the upper levels of lldb. Doing so is safe because this class will // live at least as long as the debug session. We therefore do not store the // data directly in the map because the map may reallocate it's storage as new // entries are added. Which would invalidate any pointers set in the register // info up to that point. - llvm::StringMap<std::unique_ptr<RegisterTypeFlags>> m_registers_flags_types; - - // Enum types are referenced by register fields. This does not store the data - // directly because the map may reallocate. Pointers to these are contained - // within instances of RegisterTypeFlags. - llvm::StringMap<std::unique_ptr<RegisterTypeEnum>> m_registers_enum_types; + // The key is the XML ID of the type. The kind of element does not play a part + // here, the XML author should use unique global IDs. + // RegisterTypes may contain pointers to other RegisterTypes, but they will + // not attempt to destroy those types when they themselves destruct. + llvm::StringMap<std::unique_ptr<RegisterType>> m_register_types; }; } // namespace process_gdb_remote diff --git a/lldb/source/Utility/RegisterTypeFlags.cpp b/lldb/source/Utility/RegisterTypeFlags.cpp index 7c6ba6ef6d3ef..214c97855ba2a 100644 --- a/lldb/source/Utility/RegisterTypeFlags.cpp +++ b/lldb/source/Utility/RegisterTypeFlags.cpp @@ -162,7 +162,7 @@ RegisterTypeFlags::RegisterTypeFlags(std::string id, unsigned size, } void RegisterTypeFlags::DumpToLog(Log *log) const { - LLDB_LOG(log, "ID: \"{0}\" Size: {1}", GetID().c_str(), m_size); + LLDB_LOG(log, "flags ID: \"{0}\" Size: {1}", GetID().c_str(), m_size); for (const Field &field : m_fields) field.DumpToLog(log); } @@ -376,7 +376,7 @@ void RegisterTypeEnum::Enumerator::DumpToLog(Log *log) const { } void RegisterTypeEnum::DumpToLog(Log *log) const { - LLDB_LOG(log, "ID: \"{0}\"", GetID().c_str()); + LLDB_LOG(log, "enum ID: \"{0}\"", GetID().c_str()); for (const auto &enumerator : GetEnumerators()) enumerator.DumpToLog(log); } >From bffe5e560665405242f3ede5c47cfaadcc6e0a88 Mon Sep 17 00:00:00 2001 From: David Spickett <[email protected]> Date: Wed, 5 Aug 2026 14:11:04 +0000 Subject: [PATCH 2/4] use first instance of duplicated ID --- .../Process/gdb-remote/ProcessGDBRemote.cpp | 27 +++++++++++++------ .../gdb_remote_client/TestXMLRegisterFlags.py | 7 +++-- 2 files changed, 22 insertions(+), 12 deletions(-) diff --git a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp index ba450ff771df2..8b9d9bee8acb4 100644 --- a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp +++ b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp @@ -5037,14 +5037,25 @@ ParseEnums(XMLNode feature_node, }); if (!id.empty()) { - RegisterTypeEnum::Enumerators enumerators = - ParseEnumEvalues(enum_node); - if (!enumerators.empty()) { - LLDB_LOG(log, - "ProcessGDBRemote::ParseEnums Found enum type \"{0}\"", - id); - register_types.insert_or_assign( - id, std::make_unique<RegisterTypeEnum>(id, enumerators)); + // If there are multiple elements using the same ID, we will only + // use the first one seen. + if (register_types.contains(id)) { + LLDB_LOG( + log, + "ProcessGDBRemote::ParseEnums Definition of enum with " + "ID \"{0}\" shadows previous use of that ID, using original " + "definition instead.", + id); + } else { + RegisterTypeEnum::Enumerators enumerators = + ParseEnumEvalues(enum_node); + if (!enumerators.empty()) { + LLDB_LOG(log, + "ProcessGDBRemote::ParseEnums Found enum type \"{0}\"", + id); + register_types.insert_or_assign( + id, std::make_unique<RegisterTypeEnum>(id, enumerators)); + } } } diff --git a/lldb/test/API/functionalities/gdb_remote_client/TestXMLRegisterFlags.py b/lldb/test/API/functionalities/gdb_remote_client/TestXMLRegisterFlags.py index f600f807a787c..be0dc94eac04a 100644 --- a/lldb/test/API/functionalities/gdb_remote_client/TestXMLRegisterFlags.py +++ b/lldb/test/API/functionalities/gdb_remote_client/TestXMLRegisterFlags.py @@ -706,8 +706,7 @@ def test_enum_duplicated_evalue(self): @skipIfXmlSupportMissing @skipIfRemote def test_enum_duplicated(self): - """Check that lldb only uses the last instance of enums with the same - id.""" + """Check that lldb uses the first instance of enums with the same id.""" self.setup_register_test( """\ <enum id="some_enum" size="4"> @@ -724,8 +723,8 @@ def test_enum_duplicated(self): <reg name="cpsr" regnum="33" bitsize="32" type="cpsr_flags"/>""" ) - self.expect("register info cpsr", patterns=["E: 1 = def$"]) - self.expect("register read cpsr", patterns=[r"\(E = def\)$"]) + self.expect("register info cpsr", patterns=["E: 1 = abc$"]) + self.expect("register read cpsr", patterns=[r"\(E = abc\)$"]) @skipIfXmlSupportMissing @skipIfRemote >From a59bb7b6b35a78be85c202a853e56b45bfc977d6 Mon Sep 17 00:00:00 2001 From: David Spickett <[email protected]> Date: Wed, 5 Aug 2026 15:21:17 +0000 Subject: [PATCH 3/4] Add test for ID overlap --- .../Process/gdb-remote/ProcessGDBRemote.cpp | 4 +- .../gdb_remote_client/TestXMLRegisterFlags.py | 44 +++++++++++++++++++ 2 files changed, 46 insertions(+), 2 deletions(-) diff --git a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp index 8b9d9bee8acb4..521eee55fd213 100644 --- a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp +++ b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp @@ -5408,9 +5408,9 @@ bool ParseRegisters( else LLDB_LOG( log, - "ProcessGDBRemote::ParseRegisters Size of register flags {0} " + "ProcessGDBRemote::ParseRegisters Size of register type {0} " "({1} bytes) for register {2} does not match the register " - "size ({3} bytes). Ignoring this set of flags.", + "size ({3} bytes). Ignoring this register type.", register_type->GetID().c_str(), register_type->GetSize(), reg_info.name, reg_info.byte_size); } diff --git a/lldb/test/API/functionalities/gdb_remote_client/TestXMLRegisterFlags.py b/lldb/test/API/functionalities/gdb_remote_client/TestXMLRegisterFlags.py index be0dc94eac04a..a9223627e3494 100644 --- a/lldb/test/API/functionalities/gdb_remote_client/TestXMLRegisterFlags.py +++ b/lldb/test/API/functionalities/gdb_remote_client/TestXMLRegisterFlags.py @@ -726,6 +726,50 @@ def test_enum_duplicated(self): self.expect("register info cpsr", patterns=["E: 1 = abc$"]) self.expect("register read cpsr", patterns=[r"\(E = abc\)$"]) + @skipIfXmlSupportMissing + @skipIfRemote + def test_duplicate_enum_flags_id(self): + # If two elements are of the same type and have the same ID, the first + # one to be found wins. If they have different types, because we look + # for enums first, enums win over flags. + self.setup_register_test( + """\ + <enum id="duplicated_id" size="8"> + <evalue name="foo_1" value="1"/> + </enum> + <flags id="duplicated_id" size="4"> + <field name="incorrect" start="0" end="0"/> + </flags> + <flags id="duplicated_id_2" size="8"> + <field name="A" start="0" end="0" type="duplicated_id"/> + <field name="B" start="1" end="1" type="duplicated_id_2"/> + </flags> + <enum id="duplicated_id_2" size="8"> + <evalue name="pc_enum_0" value="0"/> + </enum> + <flags id="pc_flags" size="8"> + <field name="A" start="0" end="0" type="duplicated_id_2"/> + </flags> + <reg name="pc" bitsize="64" type="pc_flags"/> + <reg name="x0" regnum="0" bitsize="64" type="duplicated_id_2"/> + <reg name="cpsr" regnum="33" bitsize="32" type="duplicated_id"/>""" + ) + + # Check that a later flags is ignored in favour of an earlier enum. + # The "duplicated_id" enum wins over the "duplicated_id" flags. + # So cpsr does not have a flags type. + self.expect("register read cpsr", substrs=["(incorrect"], matching=False) + + # Check that an earlier flags is ignored in favour of a later enum. + # The "duplicated_id" enum is parsed and used by x0's field A, + # but x0's flags "duplicated_id_2" is ignored because of the + # "duplicated_id_2" enum found previously. So x0 has no flags. + self.expect("register read x0", substrs=["A = "], matching=False) + + # Prove we did parse the "duplicated_id_2" enum, which the PC's flags + # uses. + self.expect("register read pc", substrs=["(A = pc_enum_0)"]) + @skipIfXmlSupportMissing @skipIfRemote def test_enum_use_first_valid(self): >From c592f4a432742333c291488f82804bb06b036624 Mon Sep 17 00:00:00 2001 From: David Spickett <[email protected]> Date: Thu, 6 Aug 2026 12:45:08 +0000 Subject: [PATCH 4/4] Add tests to demonstrate our parsing mistakes. --- .../gdb_remote_client/TestXMLRegisterFlags.py | 74 +++++++++++++++++++ 1 file changed, 74 insertions(+) diff --git a/lldb/test/API/functionalities/gdb_remote_client/TestXMLRegisterFlags.py b/lldb/test/API/functionalities/gdb_remote_client/TestXMLRegisterFlags.py index a9223627e3494..d452992d28092 100644 --- a/lldb/test/API/functionalities/gdb_remote_client/TestXMLRegisterFlags.py +++ b/lldb/test/API/functionalities/gdb_remote_client/TestXMLRegisterFlags.py @@ -1097,3 +1097,77 @@ def test_fields_same_name_different_enum(self): ) self.expect("register read x0", patterns=[r"\(foo = foo_1, foo = foo_0\)$"]) + + @skipIfXmlSupportMissing + @skipIfRemote + def test_duplicate_ids_between_features(self): + self.setup_multidoc_test( + { + "target.xml": dedent( + """\ + <?xml version="1.0"?> + <target version="1.0"> + <architecture>aarch64</architecture> + <feature name="org.gnu.gdb.aarch64.other_feature"> + <flags id="x0_flags" size="8"> + <field name="first_flags_set" start="0" end="0"/> + </flags> + </feature> + <feature name="org.gnu.gdb.aarch64.core"> + <flags id="x0_flags" size="8"> + <field name="second_flags_set" start="0" end="0"/> + </flags> + <reg name="pc" bitsize="64"/> + <reg name="x0" regnum="0" bitsize="64" type="x0_flags"/> + </feature> + </target>""" + ) + } + ) + + # We incorrectly assume that ids are unique across all feature elements. + # https://github.com/llvm/llvm-project/issues/214444 + # This means that the x0_flags from other_feature makes us ignore the + # x0_flags in core. Which is the one that we should be using. + self.expect("register read x0", substrs=["(first_flags_set = 1)"]) + + @skipIfXmlSupportMissing + @skipIfRemote + def test_duplicate_ids_between_documents(self): + self.setup_multidoc_test( + { + "target.xml": dedent( + """\ + <?xml version="1.0"?> + <target version="1.0"> + <architecture>aarch64</architecture> + <xi:include href="flags.xml"/> + <feature name="org.gnu.gdb.aarch64.core"> + <flags id="x0_flags" size="8"> + <field name="core_flags_set" start="0" end="0"/> + </flags> + <reg name="pc" bitsize="64"/> + <reg name="x0" regnum="0" bitsize="64" type="x0_flags"/> + </feature> + </target>""" + ), + "flags.xml": dedent( + """\ + <feature name="org.gnu.gdb.aarch64.other_feature"> + <flags id="x0_flags" size="8"> + <field name="included_flags_set" start="0" end="0"/> + </flags> + </feature> + """ + ), + } + ) + + # Everything in the first document is parsed before the included document. + # This means that the flags in target.xml are seen first and the ones + # in flags.xml are ignored. + # FIXME: It is likely that we are supposed to parse the included file at + # the point where it is included, before parsing the rest of the current + # file. + # https://github.com/llvm/llvm-project/issues/214444 + self.expect("register read x0", substrs=["(core_flags_set = 1)"]) _______________________________________________ lldb-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
