Skip to content

Commit 768ab4d

Browse files
committed
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
1 parent d2f60e6 commit 768ab4d

File tree

5 files changed

+81
-16
lines changed

5 files changed

+81
-16
lines changed

include/swift/ClangImporter/ClangModule.h

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818

1919
#include "swift/AST/Module.h"
2020
#include "swift/ClangImporter/ClangImporter.h"
21+
#include "clang/AST/ExternalASTSource.h"
2122

2223
namespace clang {
2324
class ASTContext;
@@ -35,6 +36,8 @@ class ClangModuleUnit final : public LoadedFile {
3536
const clang::Module *clangModule;
3637
llvm::PointerIntPair<ModuleDecl *, 1, bool> adapterModule;
3738
mutable ArrayRef<ModuleDecl::ImportedModule> importedModulesForLookup;
39+
/// The metadata of the underlying Clang module.
40+
clang::ExternalASTSource::ASTSourceDescriptor ASTSourceDescriptor;
3841

3942
~ClangModuleUnit() = default;
4043

@@ -113,6 +116,11 @@ class ClangModuleUnit final : public LoadedFile {
113116

114117
clang::ASTContext &getClangASTContext() const;
115118

119+
/// Returns the ASTSourceDescriptor of the associated Clang module if one
120+
/// exists.
121+
Optional<clang::ExternalASTSource::ASTSourceDescriptor>
122+
getASTSourceDescriptor() const;
123+
116124
static bool classof(const FileUnit *file) {
117125
return file->getKind() == FileUnitKind::ClangModule;
118126
}

include/swift/Serialization/SerializedModuleLoader.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -224,6 +224,7 @@ class SerializedASTFile final : public LoadedFile {
224224
bool hasEntryPoint() const override;
225225

226226
virtual const clang::Module *getUnderlyingClangModule() const override;
227+
const ModuleDecl *getShadowedModule() const;
227228

228229
virtual bool getAllGenericSignatures(
229230
SmallVectorImpl<GenericSignature*> &genericSignatures)

lib/ClangImporter/ClangImporter.cpp

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3049,6 +3049,18 @@ ClangModuleUnit::ClangModuleUnit(ModuleDecl &M,
30493049
const clang::Module *clangModule)
30503050
: LoadedFile(FileUnitKind::ClangModule, M), owner(owner),
30513051
clangModule(clangModule) {
3052+
// Capture the file metadata before it goes away.
3053+
if (clangModule)
3054+
ASTSourceDescriptor = {*clangModule};
3055+
}
3056+
3057+
Optional<clang::ExternalASTSource::ASTSourceDescriptor>
3058+
ClangModuleUnit::getASTSourceDescriptor() const {
3059+
if (clangModule) {
3060+
assert(ASTSourceDescriptor.getModuleOrNull() == clangModule);
3061+
return ASTSourceDescriptor;
3062+
}
3063+
return None;
30523064
}
30533065

30543066
bool ClangModuleUnit::hasClangModule(ModuleDecl *M) {

lib/IRGen/IRGenDebugInfo.cpp

Lines changed: 56 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,9 @@
2828
#include "swift/Basic/SourceManager.h"
2929
#include "swift/Basic/Version.h"
3030
#include "swift/ClangImporter/ClangImporter.h"
31+
#include "swift/ClangImporter/ClangModule.h"
3132
#include "swift/Demangling/ManglingMacros.h"
33+
#include "swift/Serialization/SerializedModuleLoader.h"
3234
#include "swift/SIL/SILArgument.h"
3335
#include "swift/SIL/SILBasicBlock.h"
3436
#include "swift/SIL/SILDebugScope.h"
@@ -566,35 +568,63 @@ class IRGenDebugInfoImpl : public IRGenDebugInfo {
566568
return cast<llvm::DIModule>(Val->second);
567569

568570
StringRef Sysroot = IGM.Context.SearchPathOpts.SDKPath;
569-
auto M = DBuilder.createModule(
570-
Parent, Name, ConfigMacros, DebugPrefixMap.remapPath(IncludePath),
571-
Sysroot);
571+
llvm::DIModule *M =
572+
DBuilder.createModule(Parent, Name, ConfigMacros, IncludePath, Sysroot);
572573
DIModuleCache.insert({Key, llvm::TrackingMDNodeRef(M)});
573574
return M;
574575
}
575576

576-
llvm::DIModule *
577-
getOrCreateModule(clang::ExternalASTSource::ASTSourceDescriptor Desc) {
577+
using ASTSourceDescriptor = clang::ExternalASTSource::ASTSourceDescriptor;
578+
/// Create a DIModule from a clang module or PCH.
579+
/// The clang::Module pointer is passed separately because the recursive case
580+
/// needs to fudge the AST descriptor.
581+
llvm::DIModule *getOrCreateModule(ASTSourceDescriptor Desc,
582+
const clang::Module *ClangModule) {
578583
// Handle Clang modules.
579-
if (const clang::Module *ClangModule = Desc.getModuleOrNull()) {
584+
if (ClangModule) {
580585
llvm::DIModule *Parent = nullptr;
581-
if (ClangModule->Parent)
582-
Parent = getOrCreateModule(*ClangModule->Parent);
583-
584-
return getOrCreateModule(ClangModule, Parent,
585-
Desc.getModuleName(), Desc.getPath(),
586-
ConfigMacros);
586+
if (ClangModule->Parent) {
587+
// The loading of additional modules by Sema may trigger an out-of-date
588+
// PCM rebuild in the Clang module dependencies of the additional
589+
// module. A PCM rebuild causes the ModuleManager to unload previously
590+
// loaded ASTFiles. For this reason we must use the cached ASTFile
591+
// information here instead of the potentially dangling pointer to the
592+
// ASTFile that is stored in the clang::Module object.
593+
//
594+
// Note: The implementation here assumes that all clang submodules
595+
// belong to the same PCM file.
596+
ASTSourceDescriptor ParentDescriptor(*ClangModule->Parent);
597+
Parent = getOrCreateModule({ParentDescriptor.getModuleName(),
598+
ParentDescriptor.getPath(),
599+
Desc.getASTFile(), Desc.getSignature()},
600+
ClangModule->Parent);
601+
}
602+
return getOrCreateModule(ClangModule, Parent, Desc.getModuleName(),
603+
Desc.getPath(), ConfigMacros);
587604
}
588605
// Handle PCH.
589606
return getOrCreateModule(Desc.getASTFile().bytes_begin(), nullptr,
590607
Desc.getModuleName(), Desc.getPath(),
591608
ConfigMacros);
592609
};
593610

611+
static Optional<ASTSourceDescriptor> getClangModule(const ModuleDecl &M) {
612+
for (auto *FU : M.getFiles()) {
613+
if (auto *SAF = dyn_cast_or_null<SerializedASTFile>(FU)) {
614+
if (auto *ShadowedModule = SAF->getShadowedModule())
615+
if (auto Desc = getClangModule(*ShadowedModule))
616+
return Desc;
617+
} else if (auto *CMU = dyn_cast_or_null<ClangModuleUnit>(FU))
618+
if (auto Desc = CMU->getASTSourceDescriptor())
619+
return Desc;
620+
}
621+
return {};
622+
}
623+
594624
llvm::DIModule *getOrCreateModule(ModuleDecl::ImportedModule IM) {
595625
ModuleDecl *M = IM.second;
596-
if (auto *ClangModule = M->findUnderlyingClangModule())
597-
return getOrCreateModule(*ClangModule);
626+
if (Optional<ASTSourceDescriptor> ModuleDesc = getClangModule(*M))
627+
return getOrCreateModule(*ModuleDesc, ModuleDesc->getModuleOrNull());
598628

599629
StringRef Path = getFilenameFromDC(M);
600630
StringRef Name = M->getName().str();
@@ -1431,8 +1461,18 @@ class IRGenDebugInfoImpl : public IRGenDebugInfo {
14311461
if (auto *ClangDecl = D->getClangDecl()) {
14321462
clang::ASTReader &Reader = *CI.getClangInstance().getModuleManager();
14331463
auto Idx = ClangDecl->getOwningModuleID();
1434-
if (auto Info = Reader.getSourceDescriptor(Idx))
1435-
Scope = getOrCreateModule(*Info);
1464+
auto SubModuleDesc = Reader.getSourceDescriptor(Idx);
1465+
auto TopLevelModuleDesc = getClangModule(*D->getModuleContext());
1466+
if (SubModuleDesc && TopLevelModuleDesc) {
1467+
// Describe the submodule, but substitute the cached ASTFile from
1468+
// the toplevel module. The ASTFile pointer in SubModule may be
1469+
// dangling and cant be trusted.
1470+
Scope = getOrCreateModule({SubModuleDesc->getModuleName(),
1471+
SubModuleDesc->getPath(),
1472+
TopLevelModuleDesc->getASTFile(),
1473+
TopLevelModuleDesc->getSignature()},
1474+
SubModuleDesc->getModuleOrNull());
1475+
}
14361476
}
14371477
Context = Context->getParent();
14381478
}

lib/Serialization/SerializedModuleLoader.cpp

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -788,6 +788,10 @@ const clang::Module *SerializedASTFile::getUnderlyingClangModule() const {
788788
return nullptr;
789789
}
790790

791+
const ModuleDecl *SerializedASTFile::getShadowedModule() const {
792+
return File.getShadowedModule();
793+
}
794+
791795
Identifier
792796
SerializedASTFile::getDiscriminatorForPrivateValue(const ValueDecl *D) const {
793797
Identifier discriminator = File.getDiscriminatorForPrivateValue(D);

0 commit comments

Comments
 (0)