Merge pull request swiftlang#22313 from adrian-prantl/47600180

 Cache clang ASTFile information in swift::Module (NFC from the outside).

Author: adrian-prantl

include/swift/ClangImporter/ClangModule.h

@@ -18,6 +18,7 @@
 
 #include "swift/AST/Module.h"
 #include "swift/ClangImporter/ClangImporter.h"
+#include "clang/AST/ExternalASTSource.h"
 
 namespace clang {
   class ASTContext;
@@ -35,6 +36,8 @@ class ClangModuleUnit final : public LoadedFile {
   const clang::Module *clangModule;
   llvm::PointerIntPair<ModuleDecl *, 1, bool> adapterModule;
   mutable ArrayRef<ModuleDecl::ImportedModule> importedModulesForLookup;
+  /// The metadata of the underlying Clang module.
+  clang::ExternalASTSource::ASTSourceDescriptor ASTSourceDescriptor;
 
   ~ClangModuleUnit() = default;
 
@@ -113,6 +116,11 @@ class ClangModuleUnit final : public LoadedFile {
 
   clang::ASTContext &getClangASTContext() const;
 
+  /// Returns the ASTSourceDescriptor of the associated Clang module if one
+  /// exists.
+  Optional<clang::ExternalASTSource::ASTSourceDescriptor>
+  getASTSourceDescriptor() const;
+
   static bool classof(const FileUnit *file) {
     return file->getKind() == FileUnitKind::ClangModule;
   }

lib/ClangImporter/ClangImporter.cpp

@@ -3075,6 +3075,18 @@ ClangModuleUnit::ClangModuleUnit(ModuleDecl &M,
                                  const clang::Module *clangModule)
   : LoadedFile(FileUnitKind::ClangModule, M), owner(owner),
     clangModule(clangModule) {
+  // Capture the file metadata before it goes away.
+  if (clangModule)
+    ASTSourceDescriptor = {*clangModule};
+}
+
+Optional<clang::ExternalASTSource::ASTSourceDescriptor>
+ClangModuleUnit::getASTSourceDescriptor() const {
+  if (clangModule) {
+    assert(ASTSourceDescriptor.getModuleOrNull() == clangModule);
+    return ASTSourceDescriptor;
+  }
+  return None;
 }
 
 bool ClangModuleUnit::hasClangModule(ModuleDecl *M) {

lib/IRGen/IRGenDebugInfo.cpp

@@ -28,7 +28,9 @@
 #include "swift/Basic/SourceManager.h"
 #include "swift/Basic/Version.h"
 #include "swift/ClangImporter/ClangImporter.h"
+#include "swift/ClangImporter/ClangModule.h"
 #include "swift/Demangling/ManglingMacros.h"
+#include "swift/Serialization/SerializedModuleLoader.h"
 #include "swift/SIL/SILArgument.h"
 #include "swift/SIL/SILBasicBlock.h"
 #include "swift/SIL/SILDebugScope.h"
@@ -582,14 +584,18 @@ class IRGenDebugInfoImpl : public IRGenDebugInfo {
     }
 
     StringRef Sysroot = IGM.Context.SearchPathOpts.SDKPath;
-    auto M =
+    llvm::DIModule *M =
         DBuilder.createModule(Parent, Name, ConfigMacros, IncludePath, Sysroot);
     DIModuleCache.insert({Key, llvm::TrackingMDNodeRef(M)});
     return M;
   }
 
-  llvm::DIModule *
-  getOrCreateModule(clang::ExternalASTSource::ASTSourceDescriptor Desc) {
+  using ASTSourceDescriptor = clang::ExternalASTSource::ASTSourceDescriptor;
+  /// Create a DIModule from a clang module or PCH.
+  /// The clang::Module pointer is passed separately because the recursive case
+  /// needs to fudge the AST descriptor.
+  llvm::DIModule *getOrCreateModule(ASTSourceDescriptor Desc,
+                                    const clang::Module *ClangModule) {
     // PCH files don't have a signature field in the control block,
     // but LLVM detects skeleton CUs by looking for a non-zero DWO id.
     // We use the lower 64 bits for debug info.
@@ -599,10 +605,24 @@ class IRGenDebugInfoImpl : public IRGenDebugInfo {
             : ~1ULL;
 
     // Handle Clang modules.
-    if (const clang::Module *ClangModule = Desc.getModuleOrNull()) {
+    if (ClangModule) {
       llvm::DIModule *Parent = nullptr;
-      if (ClangModule->Parent)
-        Parent = getOrCreateModule(*ClangModule->Parent);
+      if (ClangModule->Parent) {
+        // The loading of additional modules by Sema may trigger an out-of-date
+        // PCM rebuild in the Clang module dependencies of the additional
+        // module. A PCM rebuild causes the ModuleManager to unload previously
+        // loaded ASTFiles. For this reason we must use the cached ASTFile
+        // information here instead of the potentially dangling pointer to the
+        // ASTFile that is stored in the clang::Module object.
+        //
+        // Note: The implementation here assumes that all clang submodules
+        //       belong to the same PCM file.
+        ASTSourceDescriptor ParentDescriptor(*ClangModule->Parent);
+        Parent = getOrCreateModule({ParentDescriptor.getModuleName(),
+                                    ParentDescriptor.getPath(),
+                                    Desc.getASTFile(), Desc.getSignature()},
+                                   ClangModule->Parent);
+      }
       return getOrCreateModule(ClangModule, Parent, Desc.getModuleName(),
                                Desc.getPath(), Signature, Desc.getASTFile());
     }
@@ -612,10 +632,18 @@ class IRGenDebugInfoImpl : public IRGenDebugInfo {
                              Desc.getASTFile());
   };
 
+  static Optional<ASTSourceDescriptor> getClangModule(const ModuleDecl &M) {
+    for (auto *FU : M.getFiles())
+      if (auto *CMU = dyn_cast_or_null<ClangModuleUnit>(FU))
+        if (auto Desc = CMU->getASTSourceDescriptor())
+          return Desc;
+    return None;
+  }
+  
   llvm::DIModule *getOrCreateModule(ModuleDecl::ImportedModule IM) {
     ModuleDecl *M = IM.second;
-    if (auto *ClangModule = M->findUnderlyingClangModule())
-      return getOrCreateModule(*ClangModule);
+    if (Optional<ASTSourceDescriptor> ModuleDesc = getClangModule(*M))
+      return getOrCreateModule(*ModuleDesc, ModuleDesc->getModuleOrNull());
 
     StringRef Path = getFilenameFromDC(M);
     StringRef Name = M->getName().str();
@@ -1457,8 +1485,18 @@ class IRGenDebugInfoImpl : public IRGenDebugInfo {
         if (auto *ClangDecl = D->getClangDecl()) {
           clang::ASTReader &Reader = *CI.getClangInstance().getModuleManager();
           auto Idx = ClangDecl->getOwningModuleID();
-          if (auto Info = Reader.getSourceDescriptor(Idx))
-            Scope = getOrCreateModule(*Info);
+          auto SubModuleDesc = Reader.getSourceDescriptor(Idx);
+          auto TopLevelModuleDesc = getClangModule(*D->getModuleContext());
+          if (SubModuleDesc && TopLevelModuleDesc) {
+            // Describe the submodule, but substitute the cached ASTFile from
+            // the toplevel module. The ASTFile pointer in SubModule may be
+            // dangling and cant be trusted.
+            Scope = getOrCreateModule({SubModuleDesc->getModuleName(),
+                                       SubModuleDesc->getPath(),
+                                       TopLevelModuleDesc->getASTFile(),
+                                       TopLevelModuleDesc->getSignature()},
+                                      SubModuleDesc->getModuleOrNull());
+          }
         }
       Context = Context->getParent();
     }

test/DebugInfo/test-foundation.swift

@@ -18,10 +18,13 @@ class MyObject : NSObject {
   // LOC-CHECK: ret
   @objc var MyArr = NSArray()
   // IMPORT-CHECK: filename: "test-foundation.swift"
-  // IMPORT-CHECK-DAG: [[FOUNDATION:[0-9]+]] = !DIModule({{.*}} name: "Foundation",{{.*}} includePath:
+  // IMPORT-CHECK-DAG: [[FOUNDATION:[0-9]+]] = !DIModule({{.*}} name: "Foundation",{{.*}} includePath: {{.*}}Foundation.framework
+  // IMPORT-CHECK-DAG: [[OVERLAY:[0-9]+]] = !DIModule({{.*}} name: "Foundation",{{.*}} includePath: {{.*}}Foundation.swiftmodule
   // IMPORT-CHECK-DAG: !DICompositeType(tag: DW_TAG_structure_type, name: "NSArray", scope: ![[NSARRAY:[0-9]+]]
   //  IMPORT-CHECK-DAG: ![[NSARRAY]] = !DIModule(scope: ![[FOUNDATION:[0-9]+]], name: "NSArray"
-  // IMPORT-CHECK-DAG: !DIImportedEntity(tag: DW_TAG_imported_module, {{.*}}entity: ![[FOUNDATION]]
+  // We actually imported the Foundation SDK overlay and not the Clang module
+  // directly.
+  // IMPORT-CHECK-DAG: !DIImportedEntity(tag: DW_TAG_imported_module, {{.*}}entity: ![[OVERLAY]]
 
   // ALLOCCTOR-CHECK: ![[F:.*]] = !DIFile(filename: "<compiler-generated>",
   // ALLOCCTOR-CHECK: distinct !DISubprogram(name: "init",