[Lldb-commits] [lldb] a03dc8c - [lldb] Add basic -flimit-debug-info support to expression evaluator

Pavel Labath via lldb-commits lldb-commits at lists.llvm.org
Wed Jul 1 05:50:23 PDT 2020


Author: Pavel Labath
Date: 2020-07-01T14:50:14+02:00
New Revision: a03dc8c9fa8e9c5cf44448fac1a9ad0fdad7df41

URL: https://github.com/llvm/llvm-project/commit/a03dc8c9fa8e9c5cf44448fac1a9ad0fdad7df41
DIFF: https://github.com/llvm/llvm-project/commit/a03dc8c9fa8e9c5cf44448fac1a9ad0fdad7df41.diff

LOG: [lldb] Add basic -flimit-debug-info support to expression evaluator

Summary:
This patch adds support for evaluation of expressions referring to types
which were compiled in -flimit-debug-info (a.k.a -fno-standalone-debug)
in clang. In this mode it's possible that the debug information needed
to fully describe a c++ type is not present in a single shared library
-- for example debug info for a base class or a member of a type can
only be found in another shared library.  This situation is not
currently handled well within lldb as we are limited to searching within
a single shared library (lldb_private::Module) when searching for the
definition of these types.

The way that this patch gets around this limitation is by doing the
search at a later stage -- during the construction of the expression ast
context. This works by having the parser (currently SymbolFileDWARF, but
a similar approach is probably needed for PDBs too) mark a type as
"forcefully completed". What this means is that the parser has marked
the type as "complete" in the module ast context (as this is necessary
to e.g. derive classes from it), but its definition is not really there.
This is done via a new field on the ClangASTMetadata struct.

Later, when we are importing such a type into the expression ast, we
check this flag. If the flag is set, we try to find a better definition
for the type in other shared libraries. We do this by initiating a
new lookup for the "forcefully completed" classes, which then imports the
type from a module with a full definition.

This patch only implements this handling for base classes, but other
cases (members, array element types, etc.). The changes for that should
be fairly simple and mostly revolve around marking these types as
"forcefully completed" at an approriate time -- the importing logic is
generic already.

Another aspect, which is also not handled by this patch is viewing these
types via the "frame variable" command. This does not use the AST
importer and so it will need to handle these types on its own -- that
will be the subject of another patch.

Differential Revision: https://reviews.llvm.org/D81561

Added: 
    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

Modified: 
    lldb/source/Plugins/ExpressionParser/Clang/ClangASTImporter.cpp
    lldb/source/Plugins/ExpressionParser/Clang/ClangASTMetadata.h
    lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp

Removed: 
    


################################################################################
diff  --git a/lldb/source/Plugins/ExpressionParser/Clang/ClangASTImporter.cpp b/lldb/source/Plugins/ExpressionParser/Clang/ClangASTImporter.cpp
index 27efad214476..ac16738933ac 100644
--- a/lldb/source/Plugins/ExpressionParser/Clang/ClangASTImporter.cpp
+++ b/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 @@ ClangASTImporter::ASTImporterDelegate::ImportImpl(Decl *From) {
     }
   }
 
+  // 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);
 }
 

diff  --git a/lldb/source/Plugins/ExpressionParser/Clang/ClangASTMetadata.h b/lldb/source/Plugins/ExpressionParser/Clang/ClangASTMetadata.h
index 6cef11b52110..d3bcde2ced79 100644
--- a/lldb/source/Plugins/ExpressionParser/Clang/ClangASTMetadata.h
+++ b/lldb/source/Plugins/ExpressionParser/Clang/ClangASTMetadata.h
@@ -19,7 +19,8 @@ class ClangASTMetadata {
 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 @@ class ClangASTMetadata {
 
   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 @@ class ClangASTMetadata {
   };
 
   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

diff  --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
index 7f5ea806b20e..4bf5796ed59d 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
@@ -2061,26 +2061,19 @@ bool DWARFASTParserClang::CompleteRecordType(const DWARFDIE &die,
           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 
diff erent 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();
             }
           }
         }

diff  --git a/lldb/test/API/functionalities/limit-debug-info/Makefile b/lldb/test/API/functionalities/limit-debug-info/Makefile
new file mode 100644
index 000000000000..874b3a15e0fe
--- /dev/null
+++ b/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"

diff  --git a/lldb/test/API/functionalities/limit-debug-info/TestLimitDebugInfo.py b/lldb/test/API/functionalities/limit-debug-info/TestLimitDebugInfo.py
new file mode 100644
index 000000000000..48d32958388c
--- /dev/null
+++ b/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'"])

diff  --git a/lldb/test/API/functionalities/limit-debug-info/foo.cpp b/lldb/test/API/functionalities/limit-debug-info/foo.cpp
new file mode 100644
index 000000000000..69f2f7996f32
--- /dev/null
+++ b/lldb/test/API/functionalities/limit-debug-info/foo.cpp
@@ -0,0 +1,6 @@
+struct A {
+  int a = 47;
+  virtual ~A();
+};
+
+A::~A() = default;

diff  --git a/lldb/test/API/functionalities/limit-debug-info/main.cpp b/lldb/test/API/functionalities/limit-debug-info/main.cpp
new file mode 100644
index 000000000000..e3049ed74489
--- /dev/null
+++ b/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; }

diff  --git a/lldb/test/API/functionalities/limit-debug-info/one.cpp b/lldb/test/API/functionalities/limit-debug-info/one.cpp
new file mode 100644
index 000000000000..728875dd9e55
--- /dev/null
+++ b/lldb/test/API/functionalities/limit-debug-info/one.cpp
@@ -0,0 +1,3 @@
+#include "onetwo.h"
+
+One::~One() = default;

diff  --git a/lldb/test/API/functionalities/limit-debug-info/onetwo.h b/lldb/test/API/functionalities/limit-debug-info/onetwo.h
new file mode 100644
index 000000000000..82df76c64b58
--- /dev/null
+++ b/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;
+};

diff  --git a/lldb/test/API/functionalities/limit-debug-info/two.cpp b/lldb/test/API/functionalities/limit-debug-info/two.cpp
new file mode 100644
index 000000000000..928b091728c3
--- /dev/null
+++ b/lldb/test/API/functionalities/limit-debug-info/two.cpp
@@ -0,0 +1,3 @@
+#include "onetwo.h"
+
+Two::~Two() = default;


        


More information about the lldb-commits mailing list