[Lldb-commits] [lldb] 5391176 - Complete complete types early when importing types from Clang module DWARF.

Adrian Prantl via lldb-commits lldb-commits at lists.llvm.org
Fri Nov 22 10:00:44 PST 2019


Author: Adrian Prantl
Date: 2019-11-22T09:58:16-08:00
New Revision: 539117616d7a4c5690f1b9284faf0d282cd79621

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

LOG: Complete complete types early when importing types from Clang module DWARF.

This affects -gmodules only.

Under normal operation pcm_type is a shallow forward declaration
that gets completed later. This is necessary to support cyclic
data structures. If, however, pcm_type is already complete (for
example, because it was loaded for a different target before),
the definition needs to be imported right away, too.
Type::ResolveClangType() effectively ignores the ResolveState
inside type_sp and only looks at IsDefined(), so it never calls
ClangASTImporter::ASTImporterDelegate::ImportDefinitionTo(),
which does extra work for Objective-C classes. This would result
in only the forward declaration to be visible.

An alternative implementation would be to sink this into Type::ResolveClangType ( https://github.com/llvm/llvm-project/blob/88235812a71d99c082e7aa2ef9356d43d1f83a80/lldb/source/Symbol/Type.cpp#L5809) though it isn't clear to me how to best do this from a layering perspective.

rdar://problem/52134074

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

Added: 
    lldb/packages/Python/lldbsuite/test/lang/objc/modules-app-update/Makefile
    lldb/packages/Python/lldbsuite/test/lang/objc/modules-app-update/TestClangModulesAppUpdate.py
    lldb/packages/Python/lldbsuite/test/lang/objc/modules-app-update/foo.m
    lldb/packages/Python/lldbsuite/test/lang/objc/modules-app-update/main.m
    lldb/packages/Python/lldbsuite/test/lang/objc/modules-app-update/module.modulemap
    lldb/packages/Python/lldbsuite/test/lang/objc/modules-app-update/umbrella.h

Modified: 
    lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp

Removed: 
    


################################################################################
diff  --git a/lldb/packages/Python/lldbsuite/test/lang/objc/modules-app-update/Makefile b/lldb/packages/Python/lldbsuite/test/lang/objc/modules-app-update/Makefile
new file mode 100644
index 000000000000..0d8068b30854
--- /dev/null
+++ b/lldb/packages/Python/lldbsuite/test/lang/objc/modules-app-update/Makefile
@@ -0,0 +1,5 @@
+CFLAGS_EXTRAS = -I$(BUILDDIR)
+USE_PRIVATE_MODULE_CACHE := YES
+OBJC_SOURCES := main.m foo.m
+
+include Makefile.rules

diff  --git a/lldb/packages/Python/lldbsuite/test/lang/objc/modules-app-update/TestClangModulesAppUpdate.py b/lldb/packages/Python/lldbsuite/test/lang/objc/modules-app-update/TestClangModulesAppUpdate.py
new file mode 100644
index 000000000000..9aefe1ce24d4
--- /dev/null
+++ b/lldb/packages/Python/lldbsuite/test/lang/objc/modules-app-update/TestClangModulesAppUpdate.py
@@ -0,0 +1,61 @@
+from __future__ import print_function
+
+import unittest2
+import os
+import shutil
+
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+
+class TestClangModuleAppUpdate(TestBase):
+    mydir = TestBase.compute_mydir(__file__)
+
+    def setUp(self):
+        TestBase.setUp(self)
+
+    @skipUnlessDarwin
+    @skipIf(debug_info=no_match(["gmodules"]))
+    def test_rebuild_app_modules_untouched(self):
+        with open(self.getBuildArtifact("module.modulemap"), "w") as f:
+            f.write("""
+                    module Foo { header "f.h" }
+                    """)
+        with open(self.getBuildArtifact("f.h"), "w") as f:
+            f.write("""
+                    @import Foundation;
+                    @interface Foo : NSObject {
+                       int i;
+                    }
+                    +(instancetype)init;
+                    @end
+                    """)
+
+        mod_cache = self.getBuildArtifact("private-module-cache")
+        import os
+        if os.path.isdir(mod_cache):
+          shutil.rmtree(mod_cache)
+        self.build()
+        self.assertTrue(os.path.isdir(mod_cache), "module cache exists")
+
+        target, process, _, bkpt = lldbutil.run_to_source_breakpoint(
+            self, "break here", lldb.SBFileSpec("main.m"))
+        bar = target.FindTypes('Bar').GetTypeAtIndex(0)
+        foo = bar.GetDirectBaseClassAtIndex(0).GetType()
+        self.assertEqual(foo.GetNumberOfFields(), 1)
+        self.assertEqual(foo.GetFieldAtIndex(0).GetName(), "i")
+
+        # Rebuild.
+        process.Kill()
+        os.remove(self.getBuildArtifact('main.o'))
+        os.remove(self.getBuildArtifact('a.out'))
+        self.build()
+
+        # Reattach.
+        target, process, _, _ = lldbutil.run_to_breakpoint_do_run(self, target, bkpt)
+        bar = target.FindTypes('Bar').GetTypeAtIndex(0)
+        foo = bar.GetDirectBaseClassAtIndex(0).GetType()
+        self.assertEqual(foo.GetNumberOfFields(), 1)
+        self.assertEqual(foo.GetFieldAtIndex(0).GetName(), "i")

diff  --git a/lldb/packages/Python/lldbsuite/test/lang/objc/modules-app-update/foo.m b/lldb/packages/Python/lldbsuite/test/lang/objc/modules-app-update/foo.m
new file mode 100644
index 000000000000..83a5abc04e29
--- /dev/null
+++ b/lldb/packages/Python/lldbsuite/test/lang/objc/modules-app-update/foo.m
@@ -0,0 +1,7 @@
+ at import Foundation;
+ at import Foo;
+ at implementation Foo
++(instancetype)init {
+  return [super init];
+}
+ at end

diff  --git a/lldb/packages/Python/lldbsuite/test/lang/objc/modules-app-update/main.m b/lldb/packages/Python/lldbsuite/test/lang/objc/modules-app-update/main.m
new file mode 100644
index 000000000000..37ec1f37b573
--- /dev/null
+++ b/lldb/packages/Python/lldbsuite/test/lang/objc/modules-app-update/main.m
@@ -0,0 +1,17 @@
+ at import Umbrella;
+
+ at interface Bar : Foo
++(instancetype)init;
+ at end
+
+ at implementation Bar
++(instancetype)init {
+  return [super init];
+}
+ at end
+
+int main(int argc, char **argv) {
+  id bar = [Bar new];
+  [bar i]; // break here
+  return 0;
+}

diff  --git a/lldb/packages/Python/lldbsuite/test/lang/objc/modules-app-update/module.modulemap b/lldb/packages/Python/lldbsuite/test/lang/objc/modules-app-update/module.modulemap
new file mode 100644
index 000000000000..c142410cd07f
--- /dev/null
+++ b/lldb/packages/Python/lldbsuite/test/lang/objc/modules-app-update/module.modulemap
@@ -0,0 +1,4 @@
+module Umbrella {
+  header "umbrella.h"
+  export *
+}

diff  --git a/lldb/packages/Python/lldbsuite/test/lang/objc/modules-app-update/umbrella.h b/lldb/packages/Python/lldbsuite/test/lang/objc/modules-app-update/umbrella.h
new file mode 100644
index 000000000000..375d3ea2a044
--- /dev/null
+++ b/lldb/packages/Python/lldbsuite/test/lang/objc/modules-app-update/umbrella.h
@@ -0,0 +1 @@
+ at import Foo;

diff  --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
index cacb62aa2f15..01655f04c422 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
@@ -187,7 +187,7 @@ TypeSP DWARFASTParserClang::ParseTypeFromClangModule(const SymbolContext &sc,
   languages.Insert(die.GetCU()->GetLanguageType());
   llvm::DenseSet<SymbolFile *> searched_symbol_files;
   clang_module_sp->GetSymbolFile()->FindTypes(decl_context, languages,
-                                            searched_symbol_files, pcm_types);
+                                              searched_symbol_files, pcm_types);
   if (pcm_types.Empty()) {
     // Since this type is defined in one of the Clang modules imported
     // by this symbol file, search all of them. Instead of calling
@@ -222,6 +222,19 @@ TypeSP DWARFASTParserClang::ParseTypeFromClangModule(const SymbolContext &sc,
   if (!type)
     return TypeSP();
 
+  // Under normal operation pcm_type is a shallow forward declaration
+  // that gets completed later. This is necessary to support cyclic
+  // data structures. If, however, pcm_type is already complete (for
+  // example, because it was loaded for a 
diff erent target before),
+  // the definition needs to be imported right away, too.
+  // Type::ResolveClangType() effectively ignores the ResolveState
+  // inside type_sp and only looks at IsDefined(), so it never calls
+  // ClangASTImporter::ASTImporterDelegate::ImportDefinitionTo(),
+  // which does extra work for Objective-C classes. This would result
+  // in only the forward declaration to be visible.
+  if (pcm_type.IsDefined())
+    GetClangASTImporter().RequireCompleteType(ClangUtil::GetQualType(type));
+
   SymbolFileDWARF *dwarf = die.GetDWARF();
   TypeSP type_sp(new Type(
       die.GetID(), dwarf, pcm_type_sp->GetName(), pcm_type_sp->GetByteSize(),


        


More information about the lldb-commits mailing list