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

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.

This fixes a crash in IRGenDebugInfo when generation DIModule context
chains.

rdar://problem/47600180

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",