labath updated this revision to Diff 274768.
labath added a comment.

- redo the sanity check in the test (get_field_array does not include inherited 
types)
- skip gmodules test flavours due to pr46284


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D81561/new/

https://reviews.llvm.org/D81561

Files:
  lldb/source/Plugins/ExpressionParser/Clang/ClangASTImporter.cpp
  lldb/source/Plugins/ExpressionParser/Clang/ClangASTMetadata.h
  lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
  lldb/test/API/functionalities/limit-debug-info/Makefile
  lldb/test/API/functionalities/limit-debug-info/TestLimitDebugInfo.py
  lldb/test/API/functionalities/limit-debug-info/foo.cpp
  lldb/test/API/functionalities/limit-debug-info/main.cpp
  lldb/test/API/functionalities/limit-debug-info/one.cpp
  lldb/test/API/functionalities/limit-debug-info/onetwo.h
  lldb/test/API/functionalities/limit-debug-info/two.cpp

Index: lldb/test/API/functionalities/limit-debug-info/two.cpp
===================================================================
--- /dev/null
+++ lldb/test/API/functionalities/limit-debug-info/two.cpp
@@ -0,0 +1,3 @@
+#include "onetwo.h"
+
+Two::~Two() = default;
Index: lldb/test/API/functionalities/limit-debug-info/onetwo.h
===================================================================
--- /dev/null
+++ lldb/test/API/functionalities/limit-debug-info/onetwo.h
@@ -0,0 +1,11 @@
+struct One {
+  int one = 142;
+  constexpr One() = default;
+  virtual ~One();
+};
+
+struct Two : One {
+  int two = 242;
+  constexpr Two() = default;
+  ~Two() override;
+};
Index: lldb/test/API/functionalities/limit-debug-info/one.cpp
===================================================================
--- /dev/null
+++ lldb/test/API/functionalities/limit-debug-info/one.cpp
@@ -0,0 +1,3 @@
+#include "onetwo.h"
+
+One::~One() = default;
Index: lldb/test/API/functionalities/limit-debug-info/main.cpp
===================================================================
--- /dev/null
+++ lldb/test/API/functionalities/limit-debug-info/main.cpp
@@ -0,0 +1,13 @@
+#include "onetwo.h"
+
+struct InheritsFromOne : One {
+  constexpr InheritsFromOne() = default;
+  int member = 47;
+} inherits_from_one;
+
+struct InheritsFromTwo : Two {
+  constexpr InheritsFromTwo() = default;
+  int member = 47;
+} inherits_from_two;
+
+int main() { return 0; }
Index: lldb/test/API/functionalities/limit-debug-info/foo.cpp
===================================================================
--- /dev/null
+++ lldb/test/API/functionalities/limit-debug-info/foo.cpp
@@ -0,0 +1,6 @@
+struct A {
+  int a = 47;
+  virtual ~A();
+};
+
+A::~A() = default;
Index: lldb/test/API/functionalities/limit-debug-info/TestLimitDebugInfo.py
===================================================================
--- /dev/null
+++ lldb/test/API/functionalities/limit-debug-info/TestLimitDebugInfo.py
@@ -0,0 +1,90 @@
+"""
+Test completing types using information from other shared libraries.
+"""
+
+import os
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+
+class LimitDebugInfoTestCase(TestBase):
+
+    mydir = TestBase.compute_mydir(__file__)
+
+    def _check_type(self, target, name):
+        exe = target.FindModule(lldb.SBFileSpec("a.out"))
+        type_ = exe.FindFirstType(name)
+        self.trace("type_: %s"%type_)
+        self.assertTrue(type_)
+        base = type_.GetDirectBaseClassAtIndex(0).GetType()
+        self.trace("base:%s"%base)
+        self.assertTrue(base)
+        self.assertEquals(base.GetNumberOfFields(), 0)
+
+    def _check_debug_info_is_limited(self, target):
+        # Without other shared libraries we should only see the member declared
+        # in the derived class. This serves as a sanity check that we are truly
+        # building with limited debug info.
+        self._check_type(target, "InheritsFromOne")
+        self._check_type(target, "InheritsFromTwo")
+
+    @skipIf(bugnumber="pr46284", debug_info="gmodules")
+    def test_one_and_two_debug(self):
+        self.build()
+        target = self.dbg.CreateTarget(self.getBuildArtifact("a.out"))
+
+        self._check_debug_info_is_limited(target)
+
+        self.registerSharedLibrariesWithTarget(target, ["one", "two"])
+
+        # But when other shared libraries are loaded, we should be able to see
+        # all members.
+        self.expect_expr("inherits_from_one.member", result_value="47")
+        self.expect_expr("inherits_from_one.one", result_value="142")
+
+        self.expect_expr("inherits_from_two.member", result_value="47")
+        self.expect_expr("inherits_from_two.one", result_value="142")
+        self.expect_expr("inherits_from_two.two", result_value="242")
+
+    @skipIf(bugnumber="pr46284", debug_info="gmodules")
+    def test_two_debug(self):
+        self.build(dictionary=dict(STRIP_ONE="1"))
+        target = self.dbg.CreateTarget(self.getBuildArtifact("a.out"))
+
+        self._check_debug_info_is_limited(target)
+
+        self.registerSharedLibrariesWithTarget(target, ["one", "two"])
+
+        # This time, we should only see the members from the second library.
+        self.expect_expr("inherits_from_one.member", result_value="47")
+        self.expect("expr inherits_from_one.one", error=True,
+            substrs=["no member named 'one' in 'InheritsFromOne'"])
+
+        self.expect_expr("inherits_from_two.member", result_value="47")
+        self.expect("expr inherits_from_two.one", error=True,
+            substrs=["no member named 'one' in 'InheritsFromTwo'"])
+        self.expect_expr("inherits_from_two.two", result_value="242")
+
+    @skipIf(bugnumber="pr46284", debug_info="gmodules")
+    def test_one_debug(self):
+        self.build(dictionary=dict(STRIP_TWO="1"))
+        target = self.dbg.CreateTarget(self.getBuildArtifact("a.out"))
+
+        self._check_debug_info_is_limited(target)
+
+        self.registerSharedLibrariesWithTarget(target, ["one", "two"])
+
+        # In this case we should only see the members from the second library.
+        # Note that we cannot see inherits_from_two.one because without debug
+        # info for "Two", we cannot determine that it in fact inherits from
+        # "One".
+        self.expect_expr("inherits_from_one.member", result_value="47")
+        self.expect_expr("inherits_from_one.one", result_value="142")
+
+        self.expect_expr("inherits_from_two.member", result_value="47")
+        self.expect("expr inherits_from_two.one", error=True,
+            substrs=["no member named 'one' in 'InheritsFromTwo'"])
+        self.expect("expr inherits_from_two.two", error=True,
+            substrs=["no member named 'two' in 'InheritsFromTwo'"])
Index: lldb/test/API/functionalities/limit-debug-info/Makefile
===================================================================
--- /dev/null
+++ lldb/test/API/functionalities/limit-debug-info/Makefile
@@ -0,0 +1,27 @@
+CFLAGS_EXTRAS = $(LIMIT_DEBUG_INFO_FLAGS)
+LD_EXTRAS = -L. -lone -ltwo
+CXX_SOURCES = main.cpp
+
+ONE_CXXFLAGS = $(LIMIT_DEBUG_INFO_FLAGS)
+ifdef STRIP_ONE
+  ONE_CXXFLAGS += -g0
+endif
+
+TWO_CXXFLAGS = $(LIMIT_DEBUG_INFO_FLAGS)
+ifdef STRIP_TWO
+  TWO_CXXFLAGS += -g0
+endif
+
+include Makefile.rules
+
+a.out: libone libtwo
+
+libone:
+	$(MAKE) -f $(MAKEFILE_RULES) \
+	  DYLIB_ONLY=YES DYLIB_CXX_SOURCES=one.cpp DYLIB_NAME=one \
+	  CFLAGS_EXTRAS="$(ONE_CXXFLAGS)"
+
+libtwo: libone
+	$(MAKE) -f $(MAKEFILE_RULES) \
+	  DYLIB_ONLY=YES DYLIB_CXX_SOURCES=two.cpp DYLIB_NAME=two \
+	  CFLAGS_EXTRAS="$(TWO_CXXFLAGS)" LD_EXTRAS="-L. -lone"
Index: lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
===================================================================
--- lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
@@ -2061,26 +2061,19 @@
           CompilerType base_class_type =
               m_ast.GetType(type_source_info->getType());
           if (!base_class_type.GetCompleteType()) {
-            auto module = dwarf->GetObjectFile()->GetModule();
-            module->ReportError(":: Class '%s' has a base class '%s' which "
-                                "does not have a complete definition.",
-                                die.GetName(),
-                                base_class_type.GetTypeName().GetCString());
-            if (die.GetCU()->GetProducer() == eProducerClang)
-              module->ReportError(":: Try compiling the source file with "
-                                  "-fstandalone-debug.");
-
-            // We have no choice other than to pretend that the base class
-            // is complete. If we don't do this, clang will crash when we
-            // call setBases() inside of
-            // "clang_type.TransferBaseClasses()" below. Since we
-            // provide layout assistance, all ivars in this class and other
-            // classes will be fine, this is the best we can do short of
-            // crashing.
+            // We mark the class as complete to allow the TransferBaseClasses
+            // call to succeed. But we make a note of the fact that this class
+            // is not _really_ complete so we can later search for a definition
+            // in a different module.
             if (TypeSystemClang::StartTagDeclarationDefinition(
                     base_class_type)) {
               TypeSystemClang::CompleteTagDeclarationDefinition(
                   base_class_type);
+              const auto *td = TypeSystemClang::GetQualType(
+                                   base_class_type.GetOpaqueQualType())
+                                   .getTypePtr()
+                                   ->getAsTagDecl();
+              m_ast.GetMetadata(td)->SetIsForcefullyCompleted();
             }
           }
         }
Index: lldb/source/Plugins/ExpressionParser/Clang/ClangASTMetadata.h
===================================================================
--- lldb/source/Plugins/ExpressionParser/Clang/ClangASTMetadata.h
+++ lldb/source/Plugins/ExpressionParser/Clang/ClangASTMetadata.h
@@ -19,7 +19,8 @@
 public:
   ClangASTMetadata()
       : m_user_id(0), m_union_is_user_id(false), m_union_is_isa_ptr(false),
-        m_has_object_ptr(false), m_is_self(false), m_is_dynamic_cxx(true) {}
+        m_has_object_ptr(false), m_is_self(false), m_is_dynamic_cxx(true),
+        m_is_forcefully_completed(false) {}
 
   bool GetIsDynamicCXXType() const { return m_is_dynamic_cxx; }
 
@@ -83,6 +84,15 @@
 
   bool HasObjectPtr() const { return m_has_object_ptr; }
 
+  /// A type is "forcefully completed" if it was declared complete to satisfy an
+  /// AST invariant (e.g. base classes must be complete types), but in fact we
+  /// were not able to find a actual definition for it.
+  bool IsForcefullyCompleted() const { return m_is_forcefully_completed; }
+
+  void SetIsForcefullyCompleted(bool value = true) {
+    m_is_forcefully_completed = true;
+  }
+
   void Dump(Stream *s);
 
 private:
@@ -92,7 +102,7 @@
   };
 
   bool m_union_is_user_id : 1, m_union_is_isa_ptr : 1, m_has_object_ptr : 1,
-      m_is_self : 1, m_is_dynamic_cxx : 1;
+      m_is_self : 1, m_is_dynamic_cxx : 1, m_is_forcefully_completed : 1;
 };
 
 } // namespace lldb_private
Index: lldb/source/Plugins/ExpressionParser/Clang/ClangASTImporter.cpp
===================================================================
--- lldb/source/Plugins/ExpressionParser/Clang/ClangASTImporter.cpp
+++ lldb/source/Plugins/ExpressionParser/Clang/ClangASTImporter.cpp
@@ -18,6 +18,7 @@
 
 #include "Plugins/ExpressionParser/Clang/ClangASTImporter.h"
 #include "Plugins/ExpressionParser/Clang/ClangASTMetadata.h"
+#include "Plugins/ExpressionParser/Clang/ClangASTSource.h"
 #include "Plugins/ExpressionParser/Clang/ClangExternalASTSourceCallbacks.h"
 #include "Plugins/ExpressionParser/Clang/ClangUtil.h"
 #include "Plugins/TypeSystem/Clang/TypeSystemClang.h"
@@ -876,6 +877,33 @@
     }
   }
 
+  // If we have a forcefully completed type, try to find an actual definition
+  // for it in other modules.
+  const ClangASTMetadata *md = m_master.GetDeclMetadata(From);
+  auto *td = dyn_cast<TagDecl>(From);
+  if (td && md && md->IsForcefullyCompleted()) {
+    Log *log = GetLogIfAllCategoriesSet(LIBLLDB_LOG_EXPRESSIONS);
+    LLDB_LOG(log,
+             "[ClangASTImporter] Searching for a complete definition of {0} in "
+             "other modules",
+             td->getName());
+    Expected<DeclContext *> dc_or_err = ImportContext(td->getDeclContext());
+    if (!dc_or_err)
+      return dc_or_err.takeError();
+    Expected<DeclarationName> dn_or_err = Import(td->getDeclName());
+    if (!dn_or_err)
+      return dn_or_err.takeError();
+    DeclContext *dc = *dc_or_err;
+    DeclContext::lookup_result lr = dc->lookup(*dn_or_err);
+    if (lr.size()) {
+      clang::Decl *lookup_found = lr.front();
+      RegisterImportedDecl(From, lookup_found);
+      m_decls_to_ignore.insert(lookup_found);
+      return lookup_found;
+    } else
+      LLDB_LOG(log, "[ClangASTImporter] Complete definition not found");
+  }
+
   return ASTImporter::ImportImpl(From);
 }
 
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to