https://github.com/DavidSpickett updated https://github.com/llvm/llvm-project/pull/213897
>From b004aeaa7f1b78213fd36dda800233279fa2b45a Mon Sep 17 00:00:00 2001 From: David Spickett <[email protected]> Date: Fri, 6 Sep 2024 13:17:43 +0000 Subject: [PATCH 1/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 ®_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> >From f39c68c7cde79503117a0e64ace7431b86633d4c Mon Sep 17 00:00:00 2001 From: David Spickett <[email protected]> Date: Tue, 11 Aug 2026 13:08:51 +0000 Subject: [PATCH 2/2] pass register info object around instead --- .../include/lldb/Target/RegisterTypeBuilder.h | 4 +--- lldb/include/lldb/Target/Target.h | 3 +-- lldb/source/Core/DumpRegisterValue.cpp | 3 +-- .../RegisterTypeBuilderClang.cpp | 19 ++++++++++++------- .../RegisterTypeBuilderClang.h | 3 +-- lldb/source/Target/Target.cpp | 6 ++---- 6 files changed, 18 insertions(+), 20 deletions(-) diff --git a/lldb/include/lldb/Target/RegisterTypeBuilder.h b/lldb/include/lldb/Target/RegisterTypeBuilder.h index 5a37109661da0..26b97b6194d49 100644 --- a/lldb/include/lldb/Target/RegisterTypeBuilder.h +++ b/lldb/include/lldb/Target/RegisterTypeBuilder.h @@ -18,9 +18,7 @@ class RegisterTypeBuilder : public PluginInterface { public: ~RegisterTypeBuilder() override = default; - virtual CompilerType - GetRegisterType(const lldb_private::RegisterType &type_info, - uint32_t register_byte_size) = 0; + virtual CompilerType GetRegisterType(const RegisterInfo ®_info) = 0; protected: RegisterTypeBuilder() = default; diff --git a/lldb/include/lldb/Target/Target.h b/lldb/include/lldb/Target/Target.h index 3e1914513bad4..a5247376cbdf5 100644 --- a/lldb/include/lldb/Target/Target.h +++ b/lldb/include/lldb/Target/Target.h @@ -1564,8 +1564,7 @@ class Target : public std::enable_shared_from_this<Target>, /// if none can be found. llvm::Expected<lldb_private::Address> GetEntryPointAddress(); - CompilerType GetRegisterType(const lldb_private::RegisterType &type_info, - uint32_t register_byte_size); + CompilerType GetRegisterType(const RegisterInfo ®_info); /// Sends a breakpoint notification event. void NotifyBreakpointChanged(Breakpoint &bp, diff --git a/lldb/source/Core/DumpRegisterValue.cpp b/lldb/source/Core/DumpRegisterValue.cpp index 4ecaf0f08a693..0a3c8eabf8b9a 100644 --- a/lldb/source/Core/DumpRegisterValue.cpp +++ b/lldb/source/Core/DumpRegisterValue.cpp @@ -129,8 +129,7 @@ void lldb_private::DumpRegisterValue(const RegisterValue ®_val, Stream &s, (reg_info.byte_size != 4 && reg_info.byte_size != 8)) return; - CompilerType register_compiler_type = - target_sp->GetRegisterType(*reg_info.register_type, reg_info.byte_size); + CompilerType register_compiler_type = target_sp->GetRegisterType(reg_info); if (!register_compiler_type.IsValid()) return; diff --git a/lldb/source/Plugins/RegisterTypeBuilder/RegisterTypeBuilderClang.cpp b/lldb/source/Plugins/RegisterTypeBuilder/RegisterTypeBuilderClang.cpp index 9577a21077805..d5221953bf10e 100644 --- a/lldb/source/Plugins/RegisterTypeBuilder/RegisterTypeBuilderClang.cpp +++ b/lldb/source/Plugins/RegisterTypeBuilder/RegisterTypeBuilderClang.cpp @@ -132,18 +132,23 @@ CompilerType RegisterTypeBuilderClang::BuildFlagsType( return flags_type; } -CompilerType RegisterTypeBuilderClang::GetRegisterType( - const lldb_private::RegisterType &type_info, uint32_t register_byte_size) { +CompilerType +RegisterTypeBuilderClang::GetRegisterType(const RegisterInfo ®_info) { lldb::TypeSystemClangSP type_system = ScratchTypeSystemClang::GetForTarget(m_target); assert(type_system); - switch (type_info.getKind()) { + if (!reg_info.register_type) + return CompilerType(); + + switch (reg_info.register_type->getKind()) { case RegisterType::eRegisterTypeKindFlags: - return BuildFlagsType(*llvm::dyn_cast<RegisterTypeFlags>(&type_info), - register_byte_size, type_system); + return BuildFlagsType( + *llvm::dyn_cast<RegisterTypeFlags>(reg_info.register_type), + reg_info.byte_size, type_system); case RegisterType::eRegisterTypeKindEnum: - return BuildEnumType(*llvm::dyn_cast<RegisterTypeEnum>(&type_info), - register_byte_size, type_system); + return BuildEnumType( + *llvm::dyn_cast<RegisterTypeEnum>(reg_info.register_type), + reg_info.byte_size, type_system); } } diff --git a/lldb/source/Plugins/RegisterTypeBuilder/RegisterTypeBuilderClang.h b/lldb/source/Plugins/RegisterTypeBuilder/RegisterTypeBuilderClang.h index 47fff0699b03c..ae2bea69c70a8 100644 --- a/lldb/source/Plugins/RegisterTypeBuilder/RegisterTypeBuilderClang.h +++ b/lldb/source/Plugins/RegisterTypeBuilder/RegisterTypeBuilderClang.h @@ -30,8 +30,7 @@ class RegisterTypeBuilderClang : public RegisterTypeBuilder { } static lldb::RegisterTypeBuilderSP CreateInstance(Target &target); - CompilerType GetRegisterType(const lldb_private::RegisterType &type_info, - uint32_t register_byte_size) override; + CompilerType GetRegisterType(const RegisterInfo ®_info) override; private: CompilerType BuildEnumType(const RegisterTypeEnum &enum_type_info, diff --git a/lldb/source/Target/Target.cpp b/lldb/source/Target/Target.cpp index f55ca3908ea34..8770ffb6dd0bd 100644 --- a/lldb/source/Target/Target.cpp +++ b/lldb/source/Target/Target.cpp @@ -2739,13 +2739,11 @@ Target::GetScratchTypeSystemForLanguage(lldb::LanguageType language, create_on_demand); } -CompilerType -Target::GetRegisterType(const lldb_private::RegisterType &type_info, - uint32_t byte_size) { +CompilerType Target::GetRegisterType(const RegisterInfo ®_info) { 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(type_info, byte_size); + return m_register_type_builder_sp->GetRegisterType(reg_info); } std::vector<lldb::TypeSystemSP> _______________________________________________ lldb-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
