https://github.com/Michael137 updated https://github.com/llvm/llvm-project/pull/95078
>From 75d1bc01ca42e327594cceba753bec483228efbd Mon Sep 17 00:00:00 2001 From: Michael Buch <michaelbuc...@gmail.com> Date: Mon, 10 Jun 2024 17:46:31 +0100 Subject: [PATCH 1/5] [lldb][DWARFASTParser][NFC] Factor out CXX/ObjC method specifics out of ParseSubroutine This patch moves some of the `is_cxx_method`/`objc_method` logic out of `DWARFASTParserClang::ParseSubroutine` into their own functions. Mainly the purpose of this is to (hopefully) make this function more readable by turning the deeply nested if-statements into early-returns. This will be useful in an upcoming change where we remove some of the branches of said if-statement. --- .../SymbolFile/DWARF/DWARFASTParserClang.cpp | 447 +++++++++--------- .../SymbolFile/DWARF/DWARFASTParserClang.h | 15 + 2 files changed, 251 insertions(+), 211 deletions(-) diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp index 579a538af3634..585ae4e58cc1a 100644 --- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp +++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp @@ -975,6 +975,226 @@ ConvertDWARFCallingConventionToClang(const ParsedDWARFTypeAttributes &attrs) { return clang::CC_C; } +bool DWARFASTParserClang::HandleObjCMethod( + ObjCLanguage::MethodName const &objc_method, DWARFDIE const &die, + CompilerType clang_type, ParsedDWARFTypeAttributes const &attrs, + bool is_variadic) { + SymbolFileDWARF *dwarf = die.GetDWARF(); + const auto tag = die.Tag(); + ConstString class_name(objc_method.GetClassName()); + if (!class_name) + return false; + + TypeSP complete_objc_class_type_sp( + dwarf->FindCompleteObjCDefinitionTypeForDIE(DWARFDIE(), class_name, + false)); + + if (!complete_objc_class_type_sp) + return false; + + CompilerType type_clang_forward_type = + complete_objc_class_type_sp->GetForwardCompilerType(); + + if (!type_clang_forward_type) + return false; + + if (!TypeSystemClang::IsObjCObjectOrInterfaceType(type_clang_forward_type)) + return false; + + clang::ObjCMethodDecl *objc_method_decl = m_ast.AddMethodToObjCObjectType( + type_clang_forward_type, attrs.name.GetCString(), clang_type, + attrs.is_artificial, is_variadic, attrs.is_objc_direct_call); + + if (!objc_method_decl) { + dwarf->GetObjectFile()->GetModule()->ReportError( + "[{0:x16}]: invalid Objective-C method {1:x4} ({2}), " + "please file a bug and attach the file at the start of " + "this error message", + die.GetOffset(), tag, DW_TAG_value_to_name(tag)); + return false; + } + + LinkDeclContextToDIE(objc_method_decl, die); + m_ast.SetMetadataAsUserID(objc_method_decl, die.GetID()); + + return true; +} + +std::pair<bool, TypeSP> DWARFASTParserClang::HandleCXXMethod( + DWARFDIE const &die, CompilerType clang_type, + ParsedDWARFTypeAttributes const &attrs, DWARFDIE const &decl_ctx_die, + bool is_static, bool &ignore_containing_context) { + Log *log = GetLog(DWARFLog::TypeCompletion | DWARFLog::Lookups); + SymbolFileDWARF *dwarf = die.GetDWARF(); + // Look at the parent of this DIE and see if it is a class or + // struct and see if this is actually a C++ method + Type *class_type = dwarf->ResolveType(decl_ctx_die); + if (!class_type) + return {}; + + if (class_type->GetID() != decl_ctx_die.GetID() || + IsClangModuleFwdDecl(decl_ctx_die)) { + + // We uniqued the parent class of this function to another + // class so we now need to associate all dies under + // "decl_ctx_die" to DIEs in the DIE for "class_type"... + DWARFDIE class_type_die = dwarf->GetDIE(class_type->GetID()); + + if (class_type_die) { + std::vector<DWARFDIE> failures; + + CopyUniqueClassMethodTypes(decl_ctx_die, class_type_die, class_type, + failures); + + // FIXME do something with these failures that's + // smarter than just dropping them on the ground. + // Unfortunately classes don't like having stuff added + // to them after their definitions are complete... + + Type *type_ptr = dwarf->GetDIEToType()[die.GetDIE()]; + if (type_ptr && type_ptr != DIE_IS_BEING_PARSED) { + return {true, type_ptr->shared_from_this()}; + } + } + } + + if (attrs.specification.IsValid()) { + // We have a specification which we are going to base our + // function prototype off of, so we need this type to be + // completed so that the m_die_to_decl_ctx for the method in + // the specification has a valid clang decl context. + class_type->GetForwardCompilerType(); + // If we have a specification, then the function type should + // have been made with the specification and not with this + // die. + DWARFDIE spec_die = attrs.specification.Reference(); + clang::DeclContext *spec_clang_decl_ctx = + GetClangDeclContextForDIE(spec_die); + if (spec_clang_decl_ctx) { + LinkDeclContextToDIE(spec_clang_decl_ctx, die); + } else { + dwarf->GetObjectFile()->GetModule()->ReportWarning( + "{0:x8}: DW_AT_specification({1:x16}" + ") has no decl\n", + die.GetID(), spec_die.GetOffset()); + } + return {true, nullptr}; + } + + if (attrs.abstract_origin.IsValid()) { + // We have a specification which we are going to base our + // function prototype off of, so we need this type to be + // completed so that the m_die_to_decl_ctx for the method in + // the abstract origin has a valid clang decl context. + class_type->GetForwardCompilerType(); + + DWARFDIE abs_die = attrs.abstract_origin.Reference(); + clang::DeclContext *abs_clang_decl_ctx = GetClangDeclContextForDIE(abs_die); + if (abs_clang_decl_ctx) { + LinkDeclContextToDIE(abs_clang_decl_ctx, die); + } else { + dwarf->GetObjectFile()->GetModule()->ReportWarning( + "{0:x8}: DW_AT_abstract_origin({1:x16}" + ") has no decl\n", + die.GetID(), abs_die.GetOffset()); + } + + return {true, nullptr}; + } + + CompilerType class_opaque_type = class_type->GetForwardCompilerType(); + if (!TypeSystemClang::IsCXXClassType(class_opaque_type)) + return {}; + + if (class_opaque_type.IsBeingDefined()) { + if (!is_static && !die.HasChildren()) { + // We have a C++ member function with no children (this + // pointer!) and clang will get mad if we try and make + // a function that isn't well formed in the DWARF, so + // we will just skip it... + return {true, nullptr}; + } + + llvm::PrettyStackTraceFormat stack_trace( + "SymbolFileDWARF::ParseType() is adding a method " + "%s to class %s in DIE 0x%8.8" PRIx64 " from %s", + attrs.name.GetCString(), class_type->GetName().GetCString(), + die.GetID(), dwarf->GetObjectFile()->GetFileSpec().GetPath().c_str()); + + const bool is_attr_used = false; + // Neither GCC 4.2 nor clang++ currently set a valid + // accessibility in the DWARF for C++ methods... + // Default to public for now... + const auto accessibility = attrs.accessibility == eAccessNone + ? eAccessPublic + : attrs.accessibility; + + clang::CXXMethodDecl *cxx_method_decl = m_ast.AddMethodToCXXRecordType( + class_opaque_type.GetOpaqueQualType(), attrs.name.GetCString(), + attrs.mangled_name, clang_type, accessibility, attrs.is_virtual, + is_static, attrs.is_inline, attrs.is_explicit, is_attr_used, + attrs.is_artificial); + + if (cxx_method_decl) { + LinkDeclContextToDIE(cxx_method_decl, die); + + ClangASTMetadata metadata; + metadata.SetUserID(die.GetID()); + + char const *object_pointer_name = + attrs.object_pointer ? attrs.object_pointer.GetName() : nullptr; + if (object_pointer_name) { + metadata.SetObjectPtrName(object_pointer_name); + LLDB_LOGF(log, + "Setting object pointer name: %s on method " + "object %p.\n", + object_pointer_name, static_cast<void *>(cxx_method_decl)); + } + m_ast.SetMetadata(cxx_method_decl, metadata); + } else { + ignore_containing_context = true; + } + + // Artificial methods are always handled even when we + // don't create a new declaration for them. + const bool type_handled = cxx_method_decl != nullptr || attrs.is_artificial; + + return {type_handled, nullptr}; + } + + // We were asked to parse the type for a method in a + // class, yet the class hasn't been asked to complete + // itself through the clang::ExternalASTSource protocol, + // so we need to just have the class complete itself and + // do things the right way, then our + // DIE should then have an entry in the + // dwarf->GetDIEToType() map. First + // we need to modify the dwarf->GetDIEToType() so it + // doesn't think we are trying to parse this DIE + // anymore... + dwarf->GetDIEToType()[die.GetDIE()] = NULL; + + // Now we get the full type to force our class type to + // complete itself using the clang::ExternalASTSource + // protocol which will parse all base classes and all + // methods (including the method for this DIE). + class_type->GetFullCompilerType(); + + // The type for this DIE should have been filled in the + // function call above. + Type *type_ptr = dwarf->GetDIEToType()[die.GetDIE()]; + if (type_ptr && type_ptr != DIE_IS_BEING_PARSED) { + return {true, type_ptr->shared_from_this()}; + } + + // The previous comment isn't actually true if the class wasn't + // resolved using the current method's parent DIE as source + // data. We need to ensure that we look up the method correctly + // in the class and then link the method's DIE to the unique + // CXXMethodDecl appropriately. + return {true, nullptr}; +} + TypeSP DWARFASTParserClang::ParseSubroutine(const DWARFDIE &die, const ParsedDWARFTypeAttributes &attrs) { @@ -989,13 +1209,6 @@ DWARFASTParserClang::ParseSubroutine(const DWARFDIE &die, unsigned type_quals = 0; - std::string object_pointer_name; - if (attrs.object_pointer) { - const char *object_pointer_name_cstr = attrs.object_pointer.GetName(); - if (object_pointer_name_cstr) - object_pointer_name = object_pointer_name_cstr; - } - DEBUG_PRINTF("0x%8.8" PRIx64 ": %s (\"%s\")\n", die.GetID(), DW_TAG_value_to_name(tag), type_name_cstr); @@ -1066,208 +1279,19 @@ DWARFASTParserClang::ParseSubroutine(const DWARFDIE &die, if (attrs.name) { bool type_handled = false; if (tag == DW_TAG_subprogram || tag == DW_TAG_inlined_subroutine) { - std::optional<const ObjCLanguage::MethodName> objc_method = - ObjCLanguage::MethodName::Create(attrs.name.GetStringRef(), true); - if (objc_method) { - CompilerType class_opaque_type; - ConstString class_name(objc_method->GetClassName()); - if (class_name) { - TypeSP complete_objc_class_type_sp( - dwarf->FindCompleteObjCDefinitionTypeForDIE(DWARFDIE(), - class_name, false)); - - if (complete_objc_class_type_sp) { - CompilerType type_clang_forward_type = - complete_objc_class_type_sp->GetForwardCompilerType(); - if (TypeSystemClang::IsObjCObjectOrInterfaceType( - type_clang_forward_type)) - class_opaque_type = type_clang_forward_type; - } - } - - if (class_opaque_type) { - clang::ObjCMethodDecl *objc_method_decl = - m_ast.AddMethodToObjCObjectType( - class_opaque_type, attrs.name.GetCString(), clang_type, - attrs.is_artificial, is_variadic, attrs.is_objc_direct_call); - type_handled = objc_method_decl != nullptr; - if (type_handled) { - LinkDeclContextToDIE(objc_method_decl, die); - m_ast.SetMetadataAsUserID(objc_method_decl, die.GetID()); - } else { - dwarf->GetObjectFile()->GetModule()->ReportError( - "[{0:x16}]: invalid Objective-C method {1:x4} ({2}), " - "please file a bug and attach the file at the start of " - "this error message", - die.GetOffset(), tag, DW_TAG_value_to_name(tag)); - } - } + if (std::optional<const ObjCLanguage::MethodName> objc_method = + ObjCLanguage::MethodName::Create(attrs.name.GetStringRef(), + true)) { + type_handled = + HandleObjCMethod(*objc_method, die, clang_type, attrs, is_variadic); } else if (is_cxx_method) { - // Look at the parent of this DIE and see if it is a class or - // struct and see if this is actually a C++ method - Type *class_type = dwarf->ResolveType(decl_ctx_die); - if (class_type) { - if (class_type->GetID() != decl_ctx_die.GetID() || - IsClangModuleFwdDecl(decl_ctx_die)) { - - // We uniqued the parent class of this function to another - // class so we now need to associate all dies under - // "decl_ctx_die" to DIEs in the DIE for "class_type"... - DWARFDIE class_type_die = dwarf->GetDIE(class_type->GetID()); - - if (class_type_die) { - std::vector<DWARFDIE> failures; - - CopyUniqueClassMethodTypes(decl_ctx_die, class_type_die, - class_type, failures); - - // FIXME do something with these failures that's - // smarter than just dropping them on the ground. - // Unfortunately classes don't like having stuff added - // to them after their definitions are complete... - - Type *type_ptr = dwarf->GetDIEToType()[die.GetDIE()]; - if (type_ptr && type_ptr != DIE_IS_BEING_PARSED) { - return type_ptr->shared_from_this(); - } - } - } + auto [handled, type_sp] = + HandleCXXMethod(die, clang_type, attrs, decl_ctx_die, is_static, + ignore_containing_context); + if (type_sp) + return type_sp; - if (attrs.specification.IsValid()) { - // We have a specification which we are going to base our - // function prototype off of, so we need this type to be - // completed so that the m_die_to_decl_ctx for the method in - // the specification has a valid clang decl context. - class_type->GetForwardCompilerType(); - // If we have a specification, then the function type should - // have been made with the specification and not with this - // die. - DWARFDIE spec_die = attrs.specification.Reference(); - clang::DeclContext *spec_clang_decl_ctx = - GetClangDeclContextForDIE(spec_die); - if (spec_clang_decl_ctx) { - LinkDeclContextToDIE(spec_clang_decl_ctx, die); - } else { - dwarf->GetObjectFile()->GetModule()->ReportWarning( - "{0:x8}: DW_AT_specification({1:x16}" - ") has no decl\n", - die.GetID(), spec_die.GetOffset()); - } - type_handled = true; - } else if (attrs.abstract_origin.IsValid()) { - // We have a specification which we are going to base our - // function prototype off of, so we need this type to be - // completed so that the m_die_to_decl_ctx for the method in - // the abstract origin has a valid clang decl context. - class_type->GetForwardCompilerType(); - - DWARFDIE abs_die = attrs.abstract_origin.Reference(); - clang::DeclContext *abs_clang_decl_ctx = - GetClangDeclContextForDIE(abs_die); - if (abs_clang_decl_ctx) { - LinkDeclContextToDIE(abs_clang_decl_ctx, die); - } else { - dwarf->GetObjectFile()->GetModule()->ReportWarning( - "{0:x8}: DW_AT_abstract_origin({1:x16}" - ") has no decl\n", - die.GetID(), abs_die.GetOffset()); - } - type_handled = true; - } else { - CompilerType class_opaque_type = - class_type->GetForwardCompilerType(); - if (TypeSystemClang::IsCXXClassType(class_opaque_type)) { - if (class_opaque_type.IsBeingDefined()) { - if (!is_static && !die.HasChildren()) { - // We have a C++ member function with no children (this - // pointer!) and clang will get mad if we try and make - // a function that isn't well formed in the DWARF, so - // we will just skip it... - type_handled = true; - } else { - llvm::PrettyStackTraceFormat stack_trace( - "SymbolFileDWARF::ParseType() is adding a method " - "%s to class %s in DIE 0x%8.8" PRIx64 " from %s", - attrs.name.GetCString(), - class_type->GetName().GetCString(), die.GetID(), - dwarf->GetObjectFile()->GetFileSpec().GetPath().c_str()); - - const bool is_attr_used = false; - // Neither GCC 4.2 nor clang++ currently set a valid - // accessibility in the DWARF for C++ methods... - // Default to public for now... - const auto accessibility = attrs.accessibility == eAccessNone - ? eAccessPublic - : attrs.accessibility; - - clang::CXXMethodDecl *cxx_method_decl = - m_ast.AddMethodToCXXRecordType( - class_opaque_type.GetOpaqueQualType(), - attrs.name.GetCString(), attrs.mangled_name, - clang_type, accessibility, attrs.is_virtual, - is_static, attrs.is_inline, attrs.is_explicit, - is_attr_used, attrs.is_artificial); - - type_handled = cxx_method_decl != nullptr; - // Artificial methods are always handled even when we - // don't create a new declaration for them. - type_handled |= attrs.is_artificial; - - if (cxx_method_decl) { - LinkDeclContextToDIE(cxx_method_decl, die); - - ClangASTMetadata metadata; - metadata.SetUserID(die.GetID()); - - if (!object_pointer_name.empty()) { - metadata.SetObjectPtrName(object_pointer_name.c_str()); - LLDB_LOGF(log, - "Setting object pointer name: %s on method " - "object %p.\n", - object_pointer_name.c_str(), - static_cast<void *>(cxx_method_decl)); - } - m_ast.SetMetadata(cxx_method_decl, metadata); - } else { - ignore_containing_context = true; - } - } - } else { - // We were asked to parse the type for a method in a - // class, yet the class hasn't been asked to complete - // itself through the clang::ExternalASTSource protocol, - // so we need to just have the class complete itself and - // do things the right way, then our - // DIE should then have an entry in the - // dwarf->GetDIEToType() map. First - // we need to modify the dwarf->GetDIEToType() so it - // doesn't think we are trying to parse this DIE - // anymore... - dwarf->GetDIEToType()[die.GetDIE()] = NULL; - - // Now we get the full type to force our class type to - // complete itself using the clang::ExternalASTSource - // protocol which will parse all base classes and all - // methods (including the method for this DIE). - class_type->GetFullCompilerType(); - - // The type for this DIE should have been filled in the - // function call above. - Type *type_ptr = dwarf->GetDIEToType()[die.GetDIE()]; - if (type_ptr && type_ptr != DIE_IS_BEING_PARSED) { - return type_ptr->shared_from_this(); - } - - // The previous comment isn't actually true if the class wasn't - // resolved using the current method's parent DIE as source - // data. We need to ensure that we look up the method correctly - // in the class and then link the method's DIE to the unique - // CXXMethodDecl appropriately. - type_handled = true; - } - } - } - } + type_handled = handled; } } @@ -1356,13 +1380,14 @@ DWARFASTParserClang::ParseSubroutine(const DWARFDIE &die, ClangASTMetadata metadata; metadata.SetUserID(die.GetID()); - if (!object_pointer_name.empty()) { - metadata.SetObjectPtrName(object_pointer_name.c_str()); + char const *object_pointer_name = + attrs.object_pointer ? attrs.object_pointer.GetName() : nullptr; + if (object_pointer_name) { + metadata.SetObjectPtrName(object_pointer_name); LLDB_LOGF(log, "Setting object pointer name: %s on function " "object %p.", - object_pointer_name.c_str(), - static_cast<void *>(function_decl)); + object_pointer_name, static_cast<void *>(function_decl)); } m_ast.SetMetadata(function_decl, metadata); } diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h index 8d4af203bb287..c29a82437c8b1 100644 --- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h +++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h @@ -23,6 +23,7 @@ #include "lldb/Core/PluginInterface.h" #include "Plugins/ExpressionParser/Clang/ClangASTImporter.h" +#include "Plugins/Language/ObjC/ObjCLanguage.h" #include "Plugins/TypeSystem/Clang/TypeSystemClang.h" #include <optional> @@ -370,6 +371,20 @@ class DWARFASTParserClang : public lldb_private::plugin::dwarf::DWARFASTParser { ParsedDWARFTypeAttributes &attrs); lldb::TypeSP ParseSubroutine(const lldb_private::plugin::dwarf::DWARFDIE &die, const ParsedDWARFTypeAttributes &attrs); + + bool + HandleObjCMethod(lldb_private::ObjCLanguage::MethodName const &objc_method, + lldb_private::plugin::dwarf::DWARFDIE const &die, + lldb_private::CompilerType clang_type, + ParsedDWARFTypeAttributes const &attrs, bool is_variadic); + + std::pair<bool, lldb::TypeSP> + HandleCXXMethod(lldb_private::plugin::dwarf::DWARFDIE const &die, + lldb_private::CompilerType clang_type, + ParsedDWARFTypeAttributes const &attrs, + lldb_private::plugin::dwarf::DWARFDIE const &decl_ctx_die, + bool is_static, bool &ignore_containing_context); + lldb::TypeSP ParseArrayType(const lldb_private::plugin::dwarf::DWARFDIE &die, const ParsedDWARFTypeAttributes &attrs); lldb::TypeSP >From 7d614613f3bbe29f2fccc25b7d97f7ad812d4067 Mon Sep 17 00:00:00 2001 From: Michael Buch <michaelbuc...@gmail.com> Date: Tue, 11 Jun 2024 11:21:32 +0100 Subject: [PATCH 2/5] fixup! fix style, add doxygen comments, rename functions, remove PrettyStackTraceFormat, remove confusing comment --- .../SymbolFile/DWARF/DWARFASTParserClang.cpp | 25 ++++------- .../SymbolFile/DWARF/DWARFASTParserClang.h | 45 +++++++++++++++---- 2 files changed, 45 insertions(+), 25 deletions(-) diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp index 585ae4e58cc1a..8141bbaa8ad04 100644 --- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp +++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp @@ -975,9 +975,9 @@ ConvertDWARFCallingConventionToClang(const ParsedDWARFTypeAttributes &attrs) { return clang::CC_C; } -bool DWARFASTParserClang::HandleObjCMethod( - ObjCLanguage::MethodName const &objc_method, DWARFDIE const &die, - CompilerType clang_type, ParsedDWARFTypeAttributes const &attrs, +bool DWARFASTParserClang::ParseObjCMethod( + const ObjCLanguage::MethodName &objc_method, const DWARFDIE &die, + CompilerType clang_type, const ParsedDWARFTypeAttributes &attrs, bool is_variadic) { SymbolFileDWARF *dwarf = die.GetDWARF(); const auto tag = die.Tag(); @@ -1020,14 +1020,13 @@ bool DWARFASTParserClang::HandleObjCMethod( return true; } -std::pair<bool, TypeSP> DWARFASTParserClang::HandleCXXMethod( - DWARFDIE const &die, CompilerType clang_type, - ParsedDWARFTypeAttributes const &attrs, DWARFDIE const &decl_ctx_die, +std::pair<bool, TypeSP> DWARFASTParserClang::ParseCXXMethod( + const DWARFDIE &die, CompilerType clang_type, + const ParsedDWARFTypeAttributes &attrs, const DWARFDIE &decl_ctx_die, bool is_static, bool &ignore_containing_context) { Log *log = GetLog(DWARFLog::TypeCompletion | DWARFLog::Lookups); SymbolFileDWARF *dwarf = die.GetDWARF(); - // Look at the parent of this DIE and see if it is a class or - // struct and see if this is actually a C++ method + Type *class_type = dwarf->ResolveType(decl_ctx_die); if (!class_type) return {}; @@ -1115,12 +1114,6 @@ std::pair<bool, TypeSP> DWARFASTParserClang::HandleCXXMethod( return {true, nullptr}; } - llvm::PrettyStackTraceFormat stack_trace( - "SymbolFileDWARF::ParseType() is adding a method " - "%s to class %s in DIE 0x%8.8" PRIx64 " from %s", - attrs.name.GetCString(), class_type->GetName().GetCString(), - die.GetID(), dwarf->GetObjectFile()->GetFileSpec().GetPath().c_str()); - const bool is_attr_used = false; // Neither GCC 4.2 nor clang++ currently set a valid // accessibility in the DWARF for C++ methods... @@ -1283,10 +1276,10 @@ DWARFASTParserClang::ParseSubroutine(const DWARFDIE &die, ObjCLanguage::MethodName::Create(attrs.name.GetStringRef(), true)) { type_handled = - HandleObjCMethod(*objc_method, die, clang_type, attrs, is_variadic); + ParseObjCMethod(*objc_method, die, clang_type, attrs, is_variadic); } else if (is_cxx_method) { auto [handled, type_sp] = - HandleCXXMethod(die, clang_type, attrs, decl_ctx_die, is_static, + ParseCXXMethod(die, clang_type, attrs, decl_ctx_die, is_static, ignore_containing_context); if (type_sp) return type_sp; diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h index c29a82437c8b1..e8e3ca0b6a368 100644 --- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h +++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h @@ -372,18 +372,45 @@ class DWARFASTParserClang : public lldb_private::plugin::dwarf::DWARFASTParser { lldb::TypeSP ParseSubroutine(const lldb_private::plugin::dwarf::DWARFDIE &die, const ParsedDWARFTypeAttributes &attrs); + /// Helper function called by \ref ParseSubroutine when parsing ObjC-methods. + /// + /// \param[in] objc_method Name of the ObjC method being parsed. + /// \param[in] die The DIE that represents the ObjC method being parsed. + /// \param[in] clang_type The CompilerType representing the function prototype + /// of the ObjC method being parsed. + /// \param[in] attrs DWARF attributes for \ref die. + /// \param[in] is_variadic Is true iff we're parsing a variadic method. + /// + /// \returns true on success bool - HandleObjCMethod(lldb_private::ObjCLanguage::MethodName const &objc_method, - lldb_private::plugin::dwarf::DWARFDIE const &die, - lldb_private::CompilerType clang_type, - ParsedDWARFTypeAttributes const &attrs, bool is_variadic); + ParseObjCMethod(const lldb_private::ObjCLanguage::MethodName &objc_method, + const lldb_private::plugin::dwarf::DWARFDIE &die, + lldb_private::CompilerType clang_type, + const ParsedDWARFTypeAttributes &attrs, bool is_variadic); + /// Helper function called by \ref ParseSubroutine when parsing C++ methods. + /// + /// \param[in] die The DIE that represents the C++ method being parsed. + /// \param[in] clang_type The CompilerType representing the function prototype + /// of the C++ method being parsed. + /// \param[in] attrs DWARF attributes for \ref die. + /// \param[in] decl_ctx_die The DIE representing the DeclContext of the C++ + /// method being parsed. + /// \param[in] is_static Is true iff we're parsing a static method. + /// \param[out] ignore_containing_context Will get set to true by this function + /// if the caller should treat this C++ method as-if it was not + /// a C++ method. Currently used as a hack to work around templated + /// C++ methods causing class definitions to mismatch between CUs. + /// + /// \returns A pair of <bool, TypeSP>. The first element is 'true' on success. + /// The second element is non-null if we have previously parsed this + /// method (a null TypeSP does not indicate failure). std::pair<bool, lldb::TypeSP> - HandleCXXMethod(lldb_private::plugin::dwarf::DWARFDIE const &die, - lldb_private::CompilerType clang_type, - ParsedDWARFTypeAttributes const &attrs, - lldb_private::plugin::dwarf::DWARFDIE const &decl_ctx_die, - bool is_static, bool &ignore_containing_context); + ParseCXXMethod(const lldb_private::plugin::dwarf::DWARFDIE &die, + lldb_private::CompilerType clang_type, + const ParsedDWARFTypeAttributes &attrs, + const lldb_private::plugin::dwarf::DWARFDIE &decl_ctx_die, + bool is_static, bool &ignore_containing_context); lldb::TypeSP ParseArrayType(const lldb_private::plugin::dwarf::DWARFDIE &die, const ParsedDWARFTypeAttributes &attrs); >From eb8af81b3320710054ec2214e303a5e5cf2efc25 Mon Sep 17 00:00:00 2001 From: Michael Buch <michaelbuc...@gmail.com> Date: Tue, 11 Jun 2024 11:21:43 +0100 Subject: [PATCH 3/5] fixup! clang-format --- .../Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp | 2 +- .../source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h | 8 +++++--- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp index 8141bbaa8ad04..5511f2667f4a9 100644 --- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp +++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp @@ -1280,7 +1280,7 @@ DWARFASTParserClang::ParseSubroutine(const DWARFDIE &die, } else if (is_cxx_method) { auto [handled, type_sp] = ParseCXXMethod(die, clang_type, attrs, decl_ctx_die, is_static, - ignore_containing_context); + ignore_containing_context); if (type_sp) return type_sp; diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h index e8e3ca0b6a368..43e6aa8626160 100644 --- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h +++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h @@ -397,10 +397,12 @@ class DWARFASTParserClang : public lldb_private::plugin::dwarf::DWARFASTParser { /// \param[in] decl_ctx_die The DIE representing the DeclContext of the C++ /// method being parsed. /// \param[in] is_static Is true iff we're parsing a static method. - /// \param[out] ignore_containing_context Will get set to true by this function + /// \param[out] ignore_containing_context Will get set to true by this + /// function /// if the caller should treat this C++ method as-if it was not - /// a C++ method. Currently used as a hack to work around templated - /// C++ methods causing class definitions to mismatch between CUs. + /// a C++ method. Currently used as a hack to work around + /// templated C++ methods causing class definitions to mismatch + /// between CUs. /// /// \returns A pair of <bool, TypeSP>. The first element is 'true' on success. /// The second element is non-null if we have previously parsed this >From d39953f663971cf9377767da1922d2354d347b39 Mon Sep 17 00:00:00 2001 From: Michael Buch <michaelbuc...@gmail.com> Date: Tue, 11 Jun 2024 11:22:59 +0100 Subject: [PATCH 4/5] fixup! fix comment formatting --- .../SymbolFile/DWARF/DWARFASTParserClang.h | 19 +++++++++++++------ 1 file changed, 13 insertions(+), 6 deletions(-) diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h index 43e6aa8626160..136a49e462fbb 100644 --- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h +++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h @@ -375,10 +375,14 @@ class DWARFASTParserClang : public lldb_private::plugin::dwarf::DWARFASTParser { /// Helper function called by \ref ParseSubroutine when parsing ObjC-methods. /// /// \param[in] objc_method Name of the ObjC method being parsed. + /// /// \param[in] die The DIE that represents the ObjC method being parsed. + /// /// \param[in] clang_type The CompilerType representing the function prototype /// of the ObjC method being parsed. + /// /// \param[in] attrs DWARF attributes for \ref die. + /// /// \param[in] is_variadic Is true iff we're parsing a variadic method. /// /// \returns true on success @@ -391,18 +395,21 @@ class DWARFASTParserClang : public lldb_private::plugin::dwarf::DWARFASTParser { /// Helper function called by \ref ParseSubroutine when parsing C++ methods. /// /// \param[in] die The DIE that represents the C++ method being parsed. + /// /// \param[in] clang_type The CompilerType representing the function prototype /// of the C++ method being parsed. + /// /// \param[in] attrs DWARF attributes for \ref die. + /// /// \param[in] decl_ctx_die The DIE representing the DeclContext of the C++ /// method being parsed. + /// /// \param[in] is_static Is true iff we're parsing a static method. - /// \param[out] ignore_containing_context Will get set to true by this - /// function - /// if the caller should treat this C++ method as-if it was not - /// a C++ method. Currently used as a hack to work around - /// templated C++ methods causing class definitions to mismatch - /// between CUs. + /// + /// \param[out] ignore_containing_context Will get set to true if the caller + /// should treat this C++ method as-if it was not a C++ method. + /// Currently used as a hack to work around templated C++ methods + /// causing class definitions to mismatch between CUs. /// /// \returns A pair of <bool, TypeSP>. The first element is 'true' on success. /// The second element is non-null if we have previously parsed this >From 65a40c69f5054533c1236b65d96797ba426ded8e Mon Sep 17 00:00:00 2001 From: Michael Buch <michaelbuc...@gmail.com> Date: Tue, 11 Jun 2024 16:40:44 +0100 Subject: [PATCH 5/5] fixup! remove braces from single-line ifs; add asserts; other stylistic cleanups --- .../SymbolFile/DWARF/DWARFASTParserClang.cpp | 39 +++++++++---------- 1 file changed, 18 insertions(+), 21 deletions(-) diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp index 5511f2667f4a9..7c193c4f083d7 100644 --- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp +++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp @@ -985,9 +985,9 @@ bool DWARFASTParserClang::ParseObjCMethod( if (!class_name) return false; - TypeSP complete_objc_class_type_sp( + TypeSP complete_objc_class_type_sp = dwarf->FindCompleteObjCDefinitionTypeForDIE(DWARFDIE(), class_name, - false)); + false); if (!complete_objc_class_type_sp) return false; @@ -1026,6 +1026,7 @@ std::pair<bool, TypeSP> DWARFASTParserClang::ParseCXXMethod( bool is_static, bool &ignore_containing_context) { Log *log = GetLog(DWARFLog::TypeCompletion | DWARFLog::Lookups); SymbolFileDWARF *dwarf = die.GetDWARF(); + assert(dwarf); Type *class_type = dwarf->ResolveType(decl_ctx_die); if (!class_type) @@ -1037,9 +1038,7 @@ std::pair<bool, TypeSP> DWARFASTParserClang::ParseCXXMethod( // We uniqued the parent class of this function to another // class so we now need to associate all dies under // "decl_ctx_die" to DIEs in the DIE for "class_type"... - DWARFDIE class_type_die = dwarf->GetDIE(class_type->GetID()); - - if (class_type_die) { + if (DWARFDIE class_type_die = dwarf->GetDIE(class_type->GetID())) { std::vector<DWARFDIE> failures; CopyUniqueClassMethodTypes(decl_ctx_die, class_type_die, class_type, @@ -1051,9 +1050,8 @@ std::pair<bool, TypeSP> DWARFASTParserClang::ParseCXXMethod( // to them after their definitions are complete... Type *type_ptr = dwarf->GetDIEToType()[die.GetDIE()]; - if (type_ptr && type_ptr != DIE_IS_BEING_PARSED) { + if (type_ptr && type_ptr != DIE_IS_BEING_PARSED) return {true, type_ptr->shared_from_this()}; - } } } @@ -1069,14 +1067,14 @@ std::pair<bool, TypeSP> DWARFASTParserClang::ParseCXXMethod( DWARFDIE spec_die = attrs.specification.Reference(); clang::DeclContext *spec_clang_decl_ctx = GetClangDeclContextForDIE(spec_die); - if (spec_clang_decl_ctx) { + if (spec_clang_decl_ctx) LinkDeclContextToDIE(spec_clang_decl_ctx, die); - } else { + else dwarf->GetObjectFile()->GetModule()->ReportWarning( "{0:x8}: DW_AT_specification({1:x16}" ") has no decl\n", die.GetID(), spec_die.GetOffset()); - } + return {true, nullptr}; } @@ -1089,14 +1087,13 @@ std::pair<bool, TypeSP> DWARFASTParserClang::ParseCXXMethod( DWARFDIE abs_die = attrs.abstract_origin.Reference(); clang::DeclContext *abs_clang_decl_ctx = GetClangDeclContextForDIE(abs_die); - if (abs_clang_decl_ctx) { + if (abs_clang_decl_ctx) LinkDeclContextToDIE(abs_clang_decl_ctx, die); - } else { + else dwarf->GetObjectFile()->GetModule()->ReportWarning( "{0:x8}: DW_AT_abstract_origin({1:x16}" ") has no decl\n", die.GetID(), abs_die.GetOffset()); - } return {true, nullptr}; } @@ -1106,13 +1103,12 @@ std::pair<bool, TypeSP> DWARFASTParserClang::ParseCXXMethod( return {}; if (class_opaque_type.IsBeingDefined()) { - if (!is_static && !die.HasChildren()) { - // We have a C++ member function with no children (this - // pointer!) and clang will get mad if we try and make - // a function that isn't well formed in the DWARF, so - // we will just skip it... + // We have a C++ member function with no children (this + // pointer!) and clang will get mad if we try and make + // a function that isn't well formed in the DWARF, so + // we will just skip it... + if (!is_static && !die.HasChildren()) return {true, nullptr}; - } const bool is_attr_used = false; // Neither GCC 4.2 nor clang++ currently set a valid @@ -1176,9 +1172,8 @@ std::pair<bool, TypeSP> DWARFASTParserClang::ParseCXXMethod( // The type for this DIE should have been filled in the // function call above. Type *type_ptr = dwarf->GetDIEToType()[die.GetDIE()]; - if (type_ptr && type_ptr != DIE_IS_BEING_PARSED) { + if (type_ptr && type_ptr != DIE_IS_BEING_PARSED) return {true, type_ptr->shared_from_this()}; - } // The previous comment isn't actually true if the class wasn't // resolved using the current method's parent DIE as source @@ -1194,6 +1189,8 @@ DWARFASTParserClang::ParseSubroutine(const DWARFDIE &die, Log *log = GetLog(DWARFLog::TypeCompletion | DWARFLog::Lookups); SymbolFileDWARF *dwarf = die.GetDWARF(); + assert(dwarf); + const dw_tag_t tag = die.Tag(); bool is_variadic = false; _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits