https://github.com/DavidSpickett updated https://github.com/llvm/llvm-project/pull/196960
>From 86fd8304177f17d69c76d82f447eca376cfb65da Mon Sep 17 00:00:00 2001 From: David Spickett <[email protected]> Date: Fri, 31 Jul 2026 10:58:05 +0000 Subject: [PATCH] [lldb] Introduce RegisterType base class for all register type classes This is refactoring to prepare for https://github.com/llvm/llvm-project/issues/87471. Where we will be adding support for describing registers as unions and vectors. See: https://sourceware.org/gdb/current/onlinedocs/gdb.html/Target-Description-Format.html A union is like a C union and references other types defined in the XML. Just like a set of register flags might reference an enum for one of those flags. By introducing this base class I'm making the treatment of all these different types generic. So that when encoding them as XML we can emit the type's dependencies recursively, and then emit the type itself. This strategy will also be used later in RegisterTypeBuilderClang to generate AST to represent these types. As GDB decided to include size in enums, whenever we emit something it will get a "user" pointer. This allows an enum type to read the size of the register it's being attached to. No other type class requires this. I would call this "parent" but it is not usually the parent. The heirarchy is: * A RegisterFlags type contains many flags. * One of those flags has an enum as its type. * That enum needs to query two levels up to get the RegisterFlag's size. LLDB does not care about this enum size attribute, but GDB does so we emit it for compatibility. I don't expect anything other than a RegisterFlags to reference an enum at this time. In theory, a vector's element could be an enum but I do not know of anything available today that does this. I'd like to support arbitrary nesting of these types, but only later once known use cases work well. For the time being, the generic RegisterType pointer is cast into a RegisterFlags before use. In future this will become a switch over the possible register types we support. --- lldb/include/lldb/Utility/RegisterFlags.h | 42 +++--- lldb/include/lldb/Utility/RegisterInfo.h | 8 +- lldb/include/lldb/Utility/RegisterType.h | 62 +++++++++ lldb/source/Core/DumpRegisterInfo.cpp | 6 +- lldb/source/Core/DumpRegisterValue.cpp | 26 ++-- .../Utility/RegisterFlagsDetector_arm64.cpp | 2 +- .../RegisterContextPOSIXCore_riscv32.cpp | 3 +- .../GDBRemoteCommunicationServerLLGS.cpp | 15 +-- lldb/source/Utility/CMakeLists.txt | 1 + lldb/source/Utility/RegisterFlags.cpp | 54 ++++---- lldb/source/Utility/RegisterType.cpp | 26 ++++ lldb/unittests/Target/RegisterFlagsTest.cpp | 122 ++++++++++++------ 12 files changed, 256 insertions(+), 111 deletions(-) create mode 100644 lldb/include/lldb/Utility/RegisterType.h create mode 100644 lldb/source/Utility/RegisterType.cpp diff --git a/lldb/include/lldb/Utility/RegisterFlags.h b/lldb/include/lldb/Utility/RegisterFlags.h index 1fa9794e80365..be9eb22fdef46 100644 --- a/lldb/include/lldb/Utility/RegisterFlags.h +++ b/lldb/include/lldb/Utility/RegisterFlags.h @@ -9,6 +9,8 @@ #ifndef LLDB_UTILITY_REGISTERFLAGS_H #define LLDB_UTILITY_REGISTERFLAGS_H +#include "lldb/Utility/RegisterType.h" + #include <stdint.h> #include <string> #include <vector> @@ -20,7 +22,7 @@ namespace lldb_private { class Stream; class Log; -class FieldEnum { +class FieldEnum : public RegisterType { public: struct Enumerator { uint64_t m_value; @@ -31,9 +33,9 @@ class FieldEnum { Enumerator(uint64_t value, std::string name) : m_value(value), m_name(std::move(name)) {} - void ToXML(Stream &strm) const; - void DumpToLog(Log *log) const; + + void ToXMLElement(Stream &strm) const; }; typedef std::vector<Enumerator> Enumerators; @@ -45,18 +47,22 @@ class FieldEnum { const Enumerators &GetEnumerators() const { return m_enumerators; } - const std::string &GetID() const { return m_id; } - void ToXML(Stream &strm, unsigned size) const; void DumpToLog(Log *log) const; + virtual void ToXMLElement(Stream &strm, + const RegisterType *user = nullptr) const override; + + static bool classof(const RegisterType *register_type) { + return register_type->getKind() == RegisterType::eRegisterTypeKindEnum; + } + private: - std::string m_id; Enumerators m_enumerators; }; -class RegisterFlags { +class RegisterFlags : public RegisterType { public: class Field { public: @@ -102,10 +108,7 @@ class RegisterFlags { /// covered by either field. unsigned PaddingDistance(const Field &other) const; - /// Output XML that describes this field, to be inserted into a target XML - /// file. Reserved characters in field names like "<" are replaced with - /// their XML safe equivalents like ">". - void ToXML(Stream &strm) const; + void ToXMLElement(Stream &strm) const; bool operator<(const Field &rhs) const { return GetStart() < rhs.GetStart(); @@ -163,7 +166,6 @@ class RegisterFlags { } const std::vector<Field> &GetFields() const { return m_fields; } - const std::string &GetID() const { return m_id; } unsigned GetSize() const { return m_size; } void DumpToLog(Log *log) const; @@ -174,20 +176,14 @@ class RegisterFlags { /// be split into many tables as needed. std::string AsTable(uint32_t max_width) const; - /// Output XML that describes this set of flags. - /// EnumsToXML should have been called before this. - void ToXML(Stream &strm) const; + virtual void ToXMLElement(Stream &strm, + const RegisterType *user = nullptr) const override; - /// Enum types must be defined before use, and - /// GDBRemoteCommunicationServerLLGS view of the register types is based only - /// on the registers. So this method emits any enum types that the upcoming - /// set of fields may need. "seen" is a set of Enum IDs that we have already - /// printed, that is updated with any printed by this call. This prevents us - /// printing the same enum multiple times. - void EnumsToXML(Stream &strm, llvm::StringSet<> &seen) const; + static bool classof(const RegisterType *register_type) { + return register_type->getKind() == RegisterType::eRegisterTypeKindFlags; + } private: - const std::string m_id; /// Size in bytes const unsigned m_size; std::vector<Field> m_fields; diff --git a/lldb/include/lldb/Utility/RegisterInfo.h b/lldb/include/lldb/Utility/RegisterInfo.h index 8c903138e2b4b..811cd0b4e0419 100644 --- a/lldb/include/lldb/Utility/RegisterInfo.h +++ b/lldb/include/lldb/Utility/RegisterInfo.h @@ -9,7 +9,7 @@ #ifndef LLDB_UTILITY_REGISTERINFO_H #define LLDB_UTILITY_REGISTERINFO_H -#include "lldb/Utility/RegisterFlags.h" +#include "lldb/Utility/RegisterType.h" #include "lldb/lldb-enumerations.h" #include "llvm/ADT/ArrayRef.h" @@ -52,10 +52,10 @@ struct RegisterInfo { uint32_t *invalidate_regs; /// If not nullptr, a type defined by XML descriptions. /// Register info tables are constructed as const, but this field may need to - /// be updated if a specific target OS has a different layout. To enable that, + /// be updated if a specific target OS has a different type. To enable that, /// this is mutable. The data pointed to is still const, so you must swap a - /// whole set of flags for another. - mutable const RegisterFlags *flags_type; + /// whole type for another whole type. + mutable const RegisterType *register_type; llvm::ArrayRef<uint8_t> data(const uint8_t *context_base) const { return llvm::ArrayRef<uint8_t>(context_base + byte_offset, byte_size); diff --git a/lldb/include/lldb/Utility/RegisterType.h b/lldb/include/lldb/Utility/RegisterType.h new file mode 100644 index 0000000000000..4afb304e8bb5a --- /dev/null +++ b/lldb/include/lldb/Utility/RegisterType.h @@ -0,0 +1,62 @@ +//===------------------------------------------------------------*- C++ -*-===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===----------------------------------------------------------------------===// + +#ifndef LLDB_UTILITY_REGISTERTYPE_H +#define LLDB_UTILITY_REGISTERTYPE_H + +#include <string> +#include <unordered_set> +#include <vector> + +namespace lldb_private { + +class Stream; +class Log; + +class RegisterType { +public: + enum RegisterTypeKind { + eRegisterTypeKindFlags, + eRegisterTypeKindEnum, + }; + + RegisterTypeKind getKind() const { return m_kind; } + + RegisterType(RegisterTypeKind kind, std::string id) + : m_kind(kind), m_id(std::move(id)) {} + + /// Output XML that describes this type, to be inserted into a target XML + /// file. Reserved characters like "<" are replaced with their XML safe + /// equivalents like ">". + void ToXML(Stream &strm, + std::unordered_set<const RegisterType *> &previously_emitted, + const RegisterType *user = nullptr) const; + + virtual ~RegisterType() = default; + + /// Output the register type as an XML element. That is, "<foo ...>" until the + /// closing </foo>, including any child types in between. For example the + /// flags in a register flag set. + virtual void ToXMLElement(Stream &strm, + const RegisterType *user = nullptr) const = 0; + + const std::string &GetID() const { return m_id; } + + void SetDependencies(std::vector<const RegisterType *> dependencies) { + m_dependencies = dependencies; + } + +private: + const RegisterTypeKind m_kind; + const std::string m_id; + std::vector<const RegisterType *> m_dependencies; +}; + +} // namespace lldb_private + +#endif // LLDB_UTILITY_REGISTERTYPE_H \ No newline at end of file diff --git a/lldb/source/Core/DumpRegisterInfo.cpp b/lldb/source/Core/DumpRegisterInfo.cpp index 23946cf5428ba..9aaf611b18d63 100644 --- a/lldb/source/Core/DumpRegisterInfo.cpp +++ b/lldb/source/Core/DumpRegisterInfo.cpp @@ -11,6 +11,8 @@ #include "lldb/Utility/RegisterFlags.h" #include "lldb/Utility/Stream.h" +#include "llvm/Support/Casting.h" + using namespace lldb; using namespace lldb_private; @@ -62,7 +64,9 @@ void lldb_private::DumpRegisterInfo(Stream &strm, RegisterContext &ctx, } DoDumpRegisterInfo(strm, info.name, info.alt_name, info.byte_size, - invalidates, read_from, in_sets, info.flags_type, + invalidates, read_from, in_sets, + llvm::dyn_cast_if_present<lldb_private::RegisterFlags>( + info.register_type), terminal_width); } diff --git a/lldb/source/Core/DumpRegisterValue.cpp b/lldb/source/Core/DumpRegisterValue.cpp index 6bd6a2e9545d2..237798346346d 100644 --- a/lldb/source/Core/DumpRegisterValue.cpp +++ b/lldb/source/Core/DumpRegisterValue.cpp @@ -22,9 +22,10 @@ using namespace lldb; template <typename T> -static void dump_type_value(lldb_private::CompilerType &fields_type, T value, +static void dump_type_value(const lldb_private::RegisterFlags &flags_type, + lldb_private::CompilerType &fields_compiler_type, + T value, lldb_private::ExecutionContextScope *exe_scope, - const lldb_private::RegisterInfo ®_info, lldb_private::Stream &strm) { lldb::ByteOrder target_order = exe_scope->CalculateProcess()->GetByteOrder(); @@ -34,7 +35,7 @@ static void dump_type_value(lldb_private::CompilerType &fields_type, T value, // them. On a big endian host this all matches up, for a little endian // host we have to swap the order of the fields before display. if (target_order == lldb::ByteOrder::eByteOrderLittle) { - value = reg_info.flags_type->ReverseFieldOrder(value); + value = flags_type.ReverseFieldOrder(value); } // Then we need to match the target's endian on a byte level as well. @@ -45,7 +46,8 @@ static void dump_type_value(lldb_private::CompilerType &fields_type, T value, &value, sizeof(T), lldb_private::endian::InlHostByteOrder(), 8}; lldb::ValueObjectSP vobj_sp = lldb_private::ValueObjectConstResult::Create( - exe_scope, fields_type, lldb_private::ConstString(), data_extractor); + exe_scope, fields_compiler_type, lldb_private::ConstString(), + data_extractor); lldb_private::DumpValueObjectOptions dump_options; lldb_private::DumpValueObjectOptions::ChildPrintingDecider decider = [](lldb_private::ConstString varname) { @@ -121,22 +123,24 @@ void lldb_private::DumpRegisterValue(const RegisterValue ®_val, Stream &s, 0, // item_bit_offset exe_scope); - if (!print_flags || !reg_info.flags_type || !exe_scope || !target_sp || + const RegisterFlags *flags_type = + llvm::dyn_cast_if_present<RegisterFlags>(reg_info.register_type); + if (!print_flags || !flags_type || !exe_scope || !target_sp || (reg_info.byte_size != 4 && reg_info.byte_size != 8)) return; - CompilerType fields_type = target_sp->GetRegisterType( - reg_info.name, *reg_info.flags_type, reg_info.byte_size); + CompilerType fields_compiler_type = target_sp->GetRegisterType( + reg_info.name, *flags_type, reg_info.byte_size); // Use a new stream so we can remove a trailing newline later. StreamString fields_stream; if (reg_info.byte_size == 4) { - dump_type_value(fields_type, reg_val.GetAsUInt32(), exe_scope, reg_info, - fields_stream); + dump_type_value(*flags_type, fields_compiler_type, reg_val.GetAsUInt32(), + exe_scope, fields_stream); } else { - dump_type_value(fields_type, reg_val.GetAsUInt64(), exe_scope, reg_info, - fields_stream); + dump_type_value(*flags_type, fields_compiler_type, reg_val.GetAsUInt64(), + exe_scope, fields_stream); } // Registers are indented like: diff --git a/lldb/source/Plugins/Process/Utility/RegisterFlagsDetector_arm64.cpp b/lldb/source/Plugins/Process/Utility/RegisterFlagsDetector_arm64.cpp index 403d10f8ffed2..40343b4238265 100644 --- a/lldb/source/Plugins/Process/Utility/RegisterFlagsDetector_arm64.cpp +++ b/lldb/source/Plugins/Process/Utility/RegisterFlagsDetector_arm64.cpp @@ -314,7 +314,7 @@ void Arm64RegisterFlagsDetector::UpdateRegisterInfo( if (reg_it != search_registers.end()) { // Attach the field information. - reg_info->flags_type = reg_it->second; + reg_info->register_type = reg_it->second; // We do not expect to see this name again so don't look for it again. search_registers.erase(reg_it); } diff --git a/lldb/source/Plugins/Process/elf-core/RegisterContextPOSIXCore_riscv32.cpp b/lldb/source/Plugins/Process/elf-core/RegisterContextPOSIXCore_riscv32.cpp index 87fe98e2efe30..62a7bfd727926 100644 --- a/lldb/source/Plugins/Process/elf-core/RegisterContextPOSIXCore_riscv32.cpp +++ b/lldb/source/Plugins/Process/elf-core/RegisterContextPOSIXCore_riscv32.cpp @@ -336,5 +336,6 @@ RegisterContextCorePOSIX_riscv32::BuildDynamicRegister( CopyRegisterListToVector(reg_info.value_regs), CopyRegisterListToVector(reg_info.invalidate_regs), /*value_reg_offset=*/0, - reg_info.flags_type}; + llvm::dyn_cast_if_present<lldb_private::RegisterFlags>( + reg_info.register_type)}; } diff --git a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp index 4f11cf8c5475e..e1b7e6addfcb2 100644 --- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp +++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp @@ -39,6 +39,7 @@ #include "lldb/Utility/LLDBAssert.h" #include "lldb/Utility/LLDBLog.h" #include "lldb/Utility/Log.h" +#include "lldb/Utility/RegisterType.h" #include "lldb/Utility/State.h" #include "lldb/Utility/StreamString.h" #include "lldb/Utility/UnimplementedError.h" @@ -3326,7 +3327,7 @@ GDBRemoteCommunicationServerLLGS::BuildTargetXml() { if (registers_count) response.IndentMore(); - llvm::StringSet<> field_enums_seen; + std::unordered_set<const RegisterType *> register_types_emitted; for (int reg_index = 0; reg_index < registers_count; reg_index++) { const RegisterInfo *reg_info = reg_context.GetRegisterInfoAtIndex(reg_index); @@ -3338,12 +3339,8 @@ GDBRemoteCommunicationServerLLGS::BuildTargetXml() { continue; } - if (reg_info->flags_type) { - response.IndentMore(); - reg_info->flags_type->EnumsToXML(response, field_enums_seen); - reg_info->flags_type->ToXML(response); - response.IndentLess(); - } + if (reg_info->register_type) + reg_info->register_type->ToXML(response, register_types_emitted); response.Indent(); response.Printf("<reg name=\"%s\" bitsize=\"%" PRIu32 @@ -3364,8 +3361,8 @@ GDBRemoteCommunicationServerLLGS::BuildTargetXml() { if (!format.empty()) response << "format=\"" << format << "\" "; - if (reg_info->flags_type) - response << "type=\"" << reg_info->flags_type->GetID() << "\" "; + if (reg_info->register_type) + response << "type=\"" << reg_info->register_type->GetID() << "\" "; const char *const register_set_name = reg_context.GetRegisterSetNameForRegisterAtIndex(reg_index); diff --git a/lldb/source/Utility/CMakeLists.txt b/lldb/source/Utility/CMakeLists.txt index c0d0a42367b26..8efcbe47dd19b 100644 --- a/lldb/source/Utility/CMakeLists.txt +++ b/lldb/source/Utility/CMakeLists.txt @@ -54,6 +54,7 @@ add_lldb_library(lldbUtility NO_INTERNAL_DEPENDENCIES ProcessInfo.cpp RealpathPrefixes.cpp RegisterFlags.cpp + RegisterType.cpp RegisterValue.cpp RegularExpression.cpp Instrumentation.cpp diff --git a/lldb/source/Utility/RegisterFlags.cpp b/lldb/source/Utility/RegisterFlags.cpp index b0a0d597c1cde..8ef216abb88d1 100644 --- a/lldb/source/Utility/RegisterFlags.cpp +++ b/lldb/source/Utility/RegisterFlags.cpp @@ -12,6 +12,7 @@ #include "llvm/ADT/MapVector.h" #include "llvm/ADT/StringExtras.h" +#include "llvm/Support/Casting.h" #include <algorithm> #include <limits> @@ -146,16 +147,22 @@ void RegisterFlags::SetFields(const std::vector<Field> &fields) { // The last field may not extend all the way to bit 0. if (previous_field && previous_field->GetStart() != 0) m_fields.push_back(Field("", 0, previous_field->GetStart() - 1)); + + std::vector<const RegisterType *> dependencies; + for (const auto &field : m_fields) + if (auto enum_type = field.GetEnum()) + dependencies.push_back(dynamic_cast<const RegisterType *>(enum_type)); + SetDependencies(std::move(dependencies)); } RegisterFlags::RegisterFlags(std::string id, unsigned size, const std::vector<Field> &fields) - : m_id(std::move(id)), m_size(size) { + : RegisterType(RegisterType::eRegisterTypeKindFlags, id), m_size(size) { SetFields(fields); } void RegisterFlags::DumpToLog(Log *log) const { - LLDB_LOG(log, "ID: \"{0}\" Size: {1}", m_id.c_str(), m_size); + LLDB_LOG(log, "ID: \"{0}\" Size: {1}", GetID().c_str(), m_size); for (const Field &field : m_fields) field.DumpToLog(log); } @@ -318,18 +325,7 @@ std::string RegisterFlags::DumpEnums(uint32_t max_width) const { return strm.GetString().str(); } -void RegisterFlags::EnumsToXML(Stream &strm, llvm::StringSet<> &seen) const { - for (const Field &field : m_fields) - if (const FieldEnum *enum_type = field.GetEnum()) { - const std::string &id = enum_type->GetID(); - if (!seen.contains(id)) { - enum_type->ToXML(strm, GetSize()); - seen.insert(id); - } - } -} - -void FieldEnum::ToXML(Stream &strm, unsigned size) const { +void FieldEnum::ToXMLElement(Stream &strm, const RegisterType *user) const { // Example XML: // <enum id="foo" size="4"> // <evalue name="bar" value="1"/> @@ -338,10 +334,16 @@ void FieldEnum::ToXML(Stream &strm, unsigned size) const { // it. strm.Indent(); - strm << "<enum id=\"" << GetID() << "\" "; - // This is the size of the underlying enum type if this were a C type. - // In other words, the size of the register in bytes. - strm.Printf("size=\"%d\"", size); + strm << "<enum id=\"" << GetID() << "\""; + + // We don't expect the user of an enum type to be anything but a register, + // but we cannot crash if that isn't true. + if (const RegisterFlags *flags_type = + llvm::dyn_cast_if_present<RegisterFlags>(user)) { + // This is the size of the underlying enum type if this were a C type. + // In other words, the size of the register in bytes. + strm.Printf(" size=\"%d\"", flags_type->GetSize()); + } const Enumerators &enumerators = GetEnumerators(); if (enumerators.empty()) { @@ -353,14 +355,14 @@ void FieldEnum::ToXML(Stream &strm, unsigned size) const { strm.IndentMore(); for (const auto &enumerator : enumerators) { strm.Indent(); - enumerator.ToXML(strm); + enumerator.ToXMLElement(strm); strm.PutChar('\n'); } strm.IndentLess(); strm.Indent("</enum>\n"); } -void FieldEnum::Enumerator::ToXML(Stream &strm) const { +void FieldEnum::Enumerator::ToXMLElement(Stream &strm) const { std::string escaped_name; llvm::raw_string_ostream escape_strm(escaped_name); llvm::printHTMLEscaped(m_name, escape_strm); @@ -373,12 +375,13 @@ void FieldEnum::Enumerator::DumpToLog(Log *log) const { } void FieldEnum::DumpToLog(Log *log) const { - LLDB_LOG(log, "ID: \"{0}\"", m_id.c_str()); + LLDB_LOG(log, "ID: \"{0}\"", GetID().c_str()); for (const auto &enumerator : GetEnumerators()) enumerator.DumpToLog(log); } -void RegisterFlags::ToXML(Stream &strm) const { +void RegisterFlags::ToXMLElement(Stream &strm, const RegisterType *user) const { + (void)user; // Example XML: // <flags id="cpsr_flags" size="4"> // <field name="incorrect" start="0" end="0"/> @@ -394,14 +397,14 @@ void RegisterFlags::ToXML(Stream &strm) const { strm << "\n"; strm.IndentMore(); - field.ToXML(strm); + field.ToXMLElement(strm); strm.IndentLess(); } strm.PutChar('\n'); strm.Indent("</flags>\n"); } -void RegisterFlags::Field::ToXML(Stream &strm) const { +void RegisterFlags::Field::ToXMLElement(Stream &strm) const { // Example XML with an enum: // <field name="correct" start="0" end="0" type="some_enum"> // Without: @@ -423,7 +426,8 @@ void RegisterFlags::Field::ToXML(Stream &strm) const { } FieldEnum::FieldEnum(std::string id, const Enumerators &enumerators) - : m_id(id), m_enumerators(enumerators) { + : RegisterType(RegisterType::eRegisterTypeKindEnum, id), + m_enumerators(enumerators) { for (const auto &enumerator : m_enumerators) { UNUSED_IF_ASSERT_DISABLED(enumerator); assert(enumerator.m_name.size() && "Enumerator name cannot be empty"); diff --git a/lldb/source/Utility/RegisterType.cpp b/lldb/source/Utility/RegisterType.cpp new file mode 100644 index 0000000000000..67184d91c0522 --- /dev/null +++ b/lldb/source/Utility/RegisterType.cpp @@ -0,0 +1,26 @@ +//===----------------------------------------------------------------------===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===----------------------------------------------------------------------===// + +#include "lldb/Utility/RegisterType.h" + +using namespace lldb_private; + +void RegisterType::ToXML( + Stream &strm, std::unordered_set<const RegisterType *> &previously_emitted, + const RegisterType *user) const { + // If we already emitted this, don't emit it again. + if (!previously_emitted.insert(this).second) + return; + + // Emit this type's dependencies first. + for (auto dep : m_dependencies) + dep->ToXML(strm, previously_emitted, this); + + // Finally emit this type. + ToXMLElement(strm, user); +} \ No newline at end of file diff --git a/lldb/unittests/Target/RegisterFlagsTest.cpp b/lldb/unittests/Target/RegisterFlagsTest.cpp index ebd4009c2965c..40d29a820c5c3 100644 --- a/lldb/unittests/Target/RegisterFlagsTest.cpp +++ b/lldb/unittests/Target/RegisterFlagsTest.cpp @@ -11,6 +11,8 @@ #include "gmock/gmock.h" #include "gtest/gtest.h" +#include "llvm/Support/Casting.h" + using namespace lldb_private; using namespace lldb; @@ -354,7 +356,7 @@ TEST(RegisterFlagsTest, DumpEnums) { "C: 0 = Cdef_enumerator_1, 1 = Cdef_enumerator_2"); } -TEST(RegisterFieldsTest, FlagsToXML) { +TEST(RegisterFieldsTest, FlagsToXMLElementElement) { StreamString strm; // RegisterFlags requires that some fields be given, so no testing of empty @@ -362,12 +364,13 @@ TEST(RegisterFieldsTest, FlagsToXML) { // Unnamed fields are padding that are ignored. This applies to fields passed // in, and those generated to fill the other bits (31-1 here). - RegisterFlags("Foo", 4, {RegisterFlags::Field("", 0, 0)}).ToXML(strm); + RegisterFlags("Foo", 4, {RegisterFlags::Field("", 0, 0)}).ToXMLElement(strm); ASSERT_EQ(strm.GetString(), "<flags id=\"Foo\" size=\"4\">\n" "</flags>\n"); strm.Clear(); - RegisterFlags("Foo", 4, {RegisterFlags::Field("abc", 0, 0)}).ToXML(strm); + RegisterFlags("Foo", 4, {RegisterFlags::Field("abc", 0, 0)}) + .ToXMLElement(strm); ASSERT_EQ(strm.GetString(), "<flags id=\"Foo\" size=\"4\">\n" " <field name=\"abc\" start=\"0\" end=\"0\"/>\n" "</flags>\n"); @@ -378,7 +381,7 @@ TEST(RegisterFieldsTest, FlagsToXML) { RegisterFlags( "Bar", 5, {RegisterFlags::Field("f1", 25, 32), RegisterFlags::Field("f2", 10, 24)}) - .ToXML(strm); + .ToXMLElement(strm); ASSERT_EQ(strm.GetString(), " <flags id=\"Bar\" size=\"5\">\n" " <field name=\"f1\" start=\"25\" end=\"32\"/>\n" @@ -392,7 +395,7 @@ TEST(RegisterFieldsTest, FlagsToXML) { {RegisterFlags::Field("A<", 4), RegisterFlags::Field("B>", 3), RegisterFlags::Field("C'", 2), RegisterFlags::Field("D\"", 1), RegisterFlags::Field("E&", 0)}) - .ToXML(strm); + .ToXMLElement(strm); ASSERT_EQ(strm.GetString(), "<flags id=\"Safe\" size=\"8\">\n" " <field name=\"A<\" start=\"4\" end=\"4\"/>\n" @@ -408,7 +411,7 @@ TEST(RegisterFieldsTest, FlagsToXML) { RegisterFlags("Enumerators", 8, {RegisterFlags::Field("NoEnumerators", 4), RegisterFlags::Field("OneEnumerator", 3, 3, &enum_single)}) - .ToXML(strm); + .ToXMLElement(strm); ASSERT_EQ(strm.GetString(), "<flags id=\"Enumerators\" size=\"8\">\n" " <field name=\"NoEnumerators\" start=\"4\" end=\"4\"/>\n" @@ -417,10 +420,10 @@ TEST(RegisterFieldsTest, FlagsToXML) { "</flags>\n"); } -TEST(RegisterFlagsTest, EnumeratorToXML) { +TEST(RegisterFlagsTest, EnumeratorToXMLElement) { StreamString strm; - FieldEnum::Enumerator(1234, "test").ToXML(strm); + FieldEnum::Enumerator(1234, "test").ToXMLElement(strm); ASSERT_EQ(strm.GetString(), "<evalue name=\"test\" value=\"1234\"/>"); // Special XML chars in names must be escaped. @@ -439,58 +442,105 @@ TEST(RegisterFlagsTest, EnumeratorToXML) { for (const auto &[enumerator, expected] : special_names) { strm.Clear(); - enumerator.ToXML(strm); + enumerator.ToXMLElement(strm); ASSERT_EQ(strm.GetString(), expected); } } -TEST(RegisterFlagsTest, EnumToXML) { +TEST(RegisterFlagsTest, EnumToXMLElement) { StreamString strm; - FieldEnum("empty_enum", {}).ToXML(strm, 4); + RegisterFlags user_4("Foo", 4, {RegisterFlags::Field("", 0, 0)}); + FieldEnum("empty_enum", {}) + .ToXMLElement(strm, llvm::dyn_cast<const RegisterType>(&user_4)); ASSERT_EQ(strm.GetString(), "<enum id=\"empty_enum\" size=\"4\"/>\n"); strm.Clear(); + RegisterFlags user_5("Foo", 5, {RegisterFlags::Field("", 0, 0)}); FieldEnum("single_enumerator", {FieldEnum::Enumerator(0, "zero")}) - .ToXML(strm, 5); + .ToXMLElement(strm, llvm::dyn_cast<const RegisterType>(&user_5)); ASSERT_EQ(strm.GetString(), "<enum id=\"single_enumerator\" size=\"5\">\n" " <evalue name=\"zero\" value=\"0\"/>\n" "</enum>\n"); + // Currently we don't emit size if the user of this type is not a flags. + // We don't expect to see this situation in real use. strm.Clear(); FieldEnum("multiple_enumerator", {FieldEnum::Enumerator(0, "zero"), FieldEnum::Enumerator(1, "one")}) - .ToXML(strm, 8); - ASSERT_EQ(strm.GetString(), "<enum id=\"multiple_enumerator\" size=\"8\">\n" + .ToXMLElement(strm, nullptr); + ASSERT_EQ(strm.GetString(), "<enum id=\"multiple_enumerator\">\n" " <evalue name=\"zero\" value=\"0\"/>\n" " <evalue name=\"one\" value=\"1\"/>\n" "</enum>\n"); } -TEST(RegisterFlagsTest, EnumsToXML) { +TEST(RegisterFlagsTest, RegisterFlagsToXML) { // This method should output all the enums used by the register flag set, - // only once. + // then the flags set itself. There should only be one definition of each + // enum, even if it is used by multiple fields. + + // In the server we assume that each type has a unqiue address and use + // that to deduplicate them. So here we heap allocate them to simulate that. StreamString strm; - FieldEnum enum_a("enum_a", {FieldEnum::Enumerator(0, "zero")}); - FieldEnum enum_b("enum_b", {FieldEnum::Enumerator(1, "one")}); - FieldEnum enum_c("enum_c", {FieldEnum::Enumerator(2, "two")}); - llvm::StringSet<> seen; + auto enum_a = std::make_shared<FieldEnum>( + "enum_a", FieldEnum::Enumerators{FieldEnum::Enumerator(0, "zero")}); + auto enum_b = std::make_shared<FieldEnum>( + "enum_b", FieldEnum::Enumerators{FieldEnum::Enumerator(1, "one")}); + auto enum_c = std::make_shared<FieldEnum>( + "enum_c", FieldEnum::Enumerators{FieldEnum::Enumerator(2, "two")}); + + std::unordered_set<const RegisterType *> previously_emitted; // Pretend that enum_c was already emitted for a different flag set. - seen.insert("enum_c"); - - RegisterFlags("Test", 4, - { - RegisterFlags::Field("f1", 31, 31, &enum_a), - RegisterFlags::Field("f2", 30, 30, &enum_a), - RegisterFlags::Field("f3", 29, 29, &enum_b), - RegisterFlags::Field("f4", 27, 28, &enum_c), - }) - .EnumsToXML(strm, seen); - ASSERT_EQ(strm.GetString(), "<enum id=\"enum_a\" size=\"4\">\n" - " <evalue name=\"zero\" value=\"0\"/>\n" - "</enum>\n" - "<enum id=\"enum_b\" size=\"4\">\n" - " <evalue name=\"one\" value=\"1\"/>\n" - "</enum>\n"); + previously_emitted.insert(enum_c.get()); + + std::vector<RegisterFlags::Field> fields{ + RegisterFlags::Field("f1", 31, 31, enum_a.get()), + RegisterFlags::Field("f2", 30, 30, enum_a.get()), + RegisterFlags::Field("f3", 29, 29, enum_b.get()), + RegisterFlags::Field("f4", 27, 28, enum_c.get()), + }; + + auto TestFlags = std::make_shared<RegisterFlags>("Test", 4, fields); + TestFlags->ToXML(strm, previously_emitted); + ASSERT_EQ(strm.GetString(), + "<enum id=\"enum_a\" size=\"4\">\n" + " <evalue name=\"zero\" value=\"0\"/>\n" + "</enum>\n" + "<enum id=\"enum_b\" size=\"4\">\n" + " <evalue name=\"one\" value=\"1\"/>\n" + "</enum>\n" + "<flags id=\"Test\" size=\"4\">\n" + " <field name=\"f1\" start=\"31\" end=\"31\" type=\"enum_a\"/>\n" + " <field name=\"f2\" start=\"30\" end=\"30\" type=\"enum_a\"/>\n" + " <field name=\"f3\" start=\"29\" end=\"29\" type=\"enum_b\"/>\n" + " <field name=\"f4\" start=\"27\" end=\"28\" type=\"enum_c\"/>\n" + "</flags>\n"); + + // If another flag set were to use the same enums we should not output them + // again. Only output new things. + auto enum_d = std::make_shared<FieldEnum>( + "enum_d", FieldEnum::Enumerators{FieldEnum::Enumerator(3, "three")}); + fields.push_back(RegisterFlags::Field("f5", 25, 26, enum_d.get())); + auto TestFlags2 = std::make_shared<RegisterFlags>("Test", 4, fields); + + strm.Clear(); + TestFlags2->ToXML(strm, previously_emitted); + ASSERT_EQ(strm.GetString(), + "<enum id=\"enum_d\" size=\"4\">\n" + " <evalue name=\"three\" value=\"3\"/>\n" + "</enum>\n" + "<flags id=\"Test\" size=\"4\">\n" + " <field name=\"f1\" start=\"31\" end=\"31\" type=\"enum_a\"/>\n" + " <field name=\"f2\" start=\"30\" end=\"30\" type=\"enum_a\"/>\n" + " <field name=\"f3\" start=\"29\" end=\"29\" type=\"enum_b\"/>\n" + " <field name=\"f4\" start=\"27\" end=\"28\" type=\"enum_c\"/>\n" + " <field name=\"f5\" start=\"25\" end=\"26\" type=\"enum_d\"/>\n" + "</flags>\n"); + + // If we have already emitted this set of flags, don't emit it again. + strm.Clear(); + TestFlags2->ToXML(strm, previously_emitted); + ASSERT_EQ(strm.GetString(), ""); } _______________________________________________ lldb-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
