[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

diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
index 8662578..4ec5013 100644 (file)
--- 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 (file)
index 0000000..e2377a4
--- /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 (file)
index 0000000..3d6df02
--- /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 (file)
index 0000000..55ce923
--- /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 (file)
index 0000000..f0086f5
--- /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 (file)
index 0000000..4c94ff6
--- /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 (file)
index 0000000..d34f924
--- /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();
+}