Author: Pavel Labath Date: 2022-05-09T11:47:55+02:00 New Revision: ae7fe65cf65dd4f71e117dee868965c152d27542
URL: https://github.com/llvm/llvm-project/commit/ae7fe65cf65dd4f71e117dee868965c152d27542 DIFF: https://github.com/llvm/llvm-project/commit/ae7fe65cf65dd4f71e117dee868965c152d27542.diff LOG: [lldb/DWARF] Fix linking direction in CopyUniqueClassMethodTypes IIUC, the purpose of CopyUniqueClassMethodTypes is to link together class definitions in two compile units so that we only have a single definition of a class. It does this by adding entries to the die_to_type and die_to_decl_ctx maps. However, the direction of the linking seems to be reversed. It is taking entries from the class that has not yet been parsed, and copying them to the class which has been parsed already -- i.e., it is a very complicated no-op. Changing the linking order allows us to revert the changes in D13224 (while keeping the associated test case passing), and is sufficient to fix PR54761, which was caused by an undesired interaction with that patch. Differential Revision: https://reviews.llvm.org/D124370 Added: lldb/test/API/lang/cpp/incomplete-types/members/Makefile lldb/test/API/lang/cpp/incomplete-types/members/TestCppIncompleteTypeMembers.py lldb/test/API/lang/cpp/incomplete-types/members/a.h lldb/test/API/lang/cpp/incomplete-types/members/f.cpp lldb/test/API/lang/cpp/incomplete-types/members/g.cpp lldb/test/API/lang/cpp/incomplete-types/members/main.cpp Modified: lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp Removed: ################################################################################ diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp index 8662578336bd3..4ec5013b135a0 100644 --- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp +++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp @@ -1040,10 +1040,8 @@ TypeSP DWARFASTParserClang::ParseSubroutine(const DWARFDIE &die, // struct and see if this is actually a C++ method Type *class_type = dwarf->ResolveType(decl_ctx_die); if (class_type) { - bool alternate_defn = false; if (class_type->GetID() != decl_ctx_die.GetID() || IsClangModuleFwdDecl(decl_ctx_die)) { - alternate_defn = true; // We uniqued the parent class of this function to another // class so we now need to associate all dies under @@ -1112,7 +1110,7 @@ TypeSP DWARFASTParserClang::ParseSubroutine(const DWARFDIE &die, CompilerType class_opaque_type = class_type->GetForwardCompilerType(); if (TypeSystemClang::IsCXXClassType(class_opaque_type)) { - if (class_opaque_type.IsBeingDefined() || alternate_defn) { + 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 @@ -1120,84 +1118,50 @@ TypeSP DWARFASTParserClang::ParseSubroutine(const DWARFDIE &die, // we will just skip it... type_handled = true; } else { - bool add_method = true; - if (alternate_defn) { - // If an alternate definition for the class exists, - // then add the method only if an equivalent is not - // already present. - clang::CXXRecordDecl *record_decl = - m_ast.GetAsCXXRecordDecl( - class_opaque_type.GetOpaqueQualType()); - if (record_decl) { - for (auto method_iter = record_decl->method_begin(); - method_iter != record_decl->method_end(); - method_iter++) { - clang::CXXMethodDecl *method_decl = *method_iter; - if (method_decl->getNameInfo().getAsString() == - attrs.name.GetStringRef()) { - if (method_decl->getType() == - ClangUtil::GetQualType(clang_type)) { - add_method = false; - LinkDeclContextToDIE(method_decl, die); - type_handled = true; - - break; - } - } - } - } - } - - if (add_method) { - 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... - if (attrs.accessibility == eAccessNone) - attrs.accessibility = eAccessPublic; - - clang::CXXMethodDecl *cxx_method_decl = - m_ast.AddMethodToCXXRecordType( - class_opaque_type.GetOpaqueQualType(), - attrs.name.GetCString(), attrs.mangled_name, - clang_type, attrs.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; + 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... + if (attrs.accessibility == eAccessNone) + attrs.accessibility = eAccessPublic; + + clang::CXXMethodDecl *cxx_method_decl = + m_ast.AddMethodToCXXRecordType( + class_opaque_type.GetOpaqueQualType(), + attrs.name.GetCString(), attrs.mangled_name, + clang_type, attrs.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 { @@ -3570,10 +3534,10 @@ bool DWARFASTParserClang::CopyUniqueClassMethodTypes( auto link = [&](DWARFDIE src, DWARFDIE dst) { SymbolFileDWARF::DIEToTypePtr &die_to_type = dst_class_die.GetDWARF()->GetDIEToType(); - clang::DeclContext *src_decl_ctx = - src_dwarf_ast_parser->m_die_to_decl_ctx[src.GetDIE()]; - if (src_decl_ctx) - dst_dwarf_ast_parser->LinkDeclContextToDIE(src_decl_ctx, dst); + clang::DeclContext *dst_decl_ctx = + dst_dwarf_ast_parser->m_die_to_decl_ctx[dst.GetDIE()]; + if (dst_decl_ctx) + src_dwarf_ast_parser->LinkDeclContextToDIE(dst_decl_ctx, src); if (Type *src_child_type = die_to_type[src.GetDIE()]) die_to_type[dst.GetDIE()] = src_child_type; diff --git a/lldb/test/API/lang/cpp/incomplete-types/members/Makefile b/lldb/test/API/lang/cpp/incomplete-types/members/Makefile new file mode 100644 index 0000000000000..e2377a4084c01 --- /dev/null +++ b/lldb/test/API/lang/cpp/incomplete-types/members/Makefile @@ -0,0 +1,10 @@ +CXX_SOURCES := main.cpp f.cpp g.cpp + +include Makefile.rules + +# Force main.cpp to be built with no debug information +main.o: CFLAGS = $(CFLAGS_NO_DEBUG) + +# And force -flimit-debug-info on the rest. +f.o: CFLAGS_EXTRAS += $(LIMIT_DEBUG_INFO_FLAGS) +g.o: CFLAGS_EXTRAS += $(LIMIT_DEBUG_INFO_FLAGS) diff --git a/lldb/test/API/lang/cpp/incomplete-types/members/TestCppIncompleteTypeMembers.py b/lldb/test/API/lang/cpp/incomplete-types/members/TestCppIncompleteTypeMembers.py new file mode 100644 index 0000000000000..3d6df0295e588 --- /dev/null +++ b/lldb/test/API/lang/cpp/incomplete-types/members/TestCppIncompleteTypeMembers.py @@ -0,0 +1,32 @@ +""" +Test situations where we don't have a definition for a type, but we have (some) +of its member functions. +""" + +import lldb +from lldbsuite.test.decorators import * +from lldbsuite.test.lldbtest import * +from lldbsuite.test import lldbutil + + +class TestCppIncompleteTypeMembers(TestBase): + + mydir = TestBase.compute_mydir(__file__) + + def test(self): + self.build() + lldbutil.run_to_source_breakpoint(self, "// break here", + lldb.SBFileSpec("f.cpp")) + + # Sanity check that we really have to debug info for this type. + this = self.expect_var_path("this", type="A *") + self.assertEquals(this.GetType().GetPointeeType().GetNumberOfFields(), + 0, str(this)) + + self.expect_var_path("af.x", value='42') + + lldbutil.run_break_set_by_source_regexp(self, "// break here", + extra_options="-f g.cpp") + self.runCmd("continue") + + self.expect_var_path("ag.a", value='47') diff --git a/lldb/test/API/lang/cpp/incomplete-types/members/a.h b/lldb/test/API/lang/cpp/incomplete-types/members/a.h new file mode 100644 index 0000000000000..55ce92392888c --- /dev/null +++ b/lldb/test/API/lang/cpp/incomplete-types/members/a.h @@ -0,0 +1,14 @@ +#ifndef A_H +#define A_H + +class A { +public: + A(); + virtual void anchor(); + int f(); + int g(); + + int member = 47; +}; + +#endif diff --git a/lldb/test/API/lang/cpp/incomplete-types/members/f.cpp b/lldb/test/API/lang/cpp/incomplete-types/members/f.cpp new file mode 100644 index 0000000000000..f0086f54659d7 --- /dev/null +++ b/lldb/test/API/lang/cpp/incomplete-types/members/f.cpp @@ -0,0 +1,8 @@ +#include "a.h" + +int A::f() { + struct Af { + int x, y; + } af{42, 47}; + return af.x + af.y; // break here +} diff --git a/lldb/test/API/lang/cpp/incomplete-types/members/g.cpp b/lldb/test/API/lang/cpp/incomplete-types/members/g.cpp new file mode 100644 index 0000000000000..4c94ff6ff89c6 --- /dev/null +++ b/lldb/test/API/lang/cpp/incomplete-types/members/g.cpp @@ -0,0 +1,8 @@ +#include "a.h" + +int A::g() { + struct Ag { + int a, b; + } ag{47, 42}; + return ag.a + ag.b; // break here +} diff --git a/lldb/test/API/lang/cpp/incomplete-types/members/main.cpp b/lldb/test/API/lang/cpp/incomplete-types/members/main.cpp new file mode 100644 index 0000000000000..d34f9248ec29b --- /dev/null +++ b/lldb/test/API/lang/cpp/incomplete-types/members/main.cpp @@ -0,0 +1,9 @@ +#include "a.h" + +A::A() = default; +void A::anchor() {} + +int main() { + A().f(); + A().g(); +} _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits