labath created this revision.
labath added reviewers: shafik, clayborg.
Herald added a project: All.
labath requested review of this revision.
Herald added a project: LLDB.

The issue is somewhat hard to describe (I recommend reading the bug),
but what roughly happens is that we have a type with no full definition
(-flimit-debug-info is active), but various compile units still contain
definitions of some methods of that type.

In this situation, we could run into a problem if we started to parse a
method of that type (this is for example needed to establish a
declaration context for any types defined inside that method), and the
class itself was already parsed in another compile unit. In this case,
we would hit a piece of code which would try to merge/copy the two sets
of methods.

The precise purpose of this code is not clear to me, but if it
definitely not doing the right thing in this case (and I'm having
trouble imagining a case where it would be doing the right thing). There
are several ways to to fix this problem, but the simplest one seems to
be to change the condition guarding the problematic code from:

  if (class_opaque_type.IsBeingDefined() || alternate_defn)

to plain:

  if (class_opaque_type.IsBeingDefined())

Since adding a method to a class that is not currently being defined
(i.e., we either having started defining it, or we've already completed
it) is not a good idea, any code path that has this condition false (as
ours did) is most likely erroneous.

This also ensures that the first method of such class is treated in the
same was as the second one -- the first method is not being added
either, although it ends up going through a completely different code
path.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D124370

Files:
  lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
  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

Index: lldb/test/API/lang/cpp/incomplete-types/members/main.cpp
===================================================================
--- /dev/null
+++ 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();
+}
Index: lldb/test/API/lang/cpp/incomplete-types/members/g.cpp
===================================================================
--- /dev/null
+++ 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
+}
Index: lldb/test/API/lang/cpp/incomplete-types/members/f.cpp
===================================================================
--- /dev/null
+++ 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
+}
Index: lldb/test/API/lang/cpp/incomplete-types/members/a.h
===================================================================
--- /dev/null
+++ 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
Index: lldb/test/API/lang/cpp/incomplete-types/members/TestCppIncompleteTypeMembers.py
===================================================================
--- /dev/null
+++ 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')
Index: lldb/test/API/lang/cpp/incomplete-types/members/Makefile
===================================================================
--- /dev/null
+++ 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)
Index: lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
===================================================================
--- lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
@@ -1112,7 +1112,7 @@
             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
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to