forked from OSchip/llvm-project
Perform an extra consistency check when searching ModuleManager's
cache for implicit modules. The ModuleManager's use of FileEntry nodes as the keys for its map of loaded modules is less than ideal. Uniqueness for FileEntry nodes is maintained by FileManager, which in turn uses inode numbers on hosts that support that. When coupled with the module cache's proclivity for turning over and deleting stale PCMs, this means entries for different module files can wind up reusing the same underlying inode. When this happens, subsequent accesses to the Modules map will disagree on the ModuleFile associated with a given file. In general, it is not sufficient to resolve this conundrum with a type like FileEntryRef that stores the name of the FileEntry node on first access because of path canonicalization issues. However, the paths constructed for implicit module builds are fully under Clang's control. We *can*, therefore, rely on their structure being consistent across operating systems and across subsequent accesses to the Modules map. To mitigate the effects of inode reuse, perform an extra name check when implicit modules are returned from the cache. This has the effect of forcing reused FileEntry nodes to stomp over existing-but-stale entries in the cache, which simulates a miss - exactly the desired behavior. rdar://48443680 Patch by Robert Widmann! Differential Revision: https://reviews.llvm.org/D86823
This commit is contained in:
parent
3b12e12d4b
commit
272742a92d
|
@ -132,15 +132,38 @@ ModuleManager::addModule(StringRef FileName, ModuleKind Type,
|
|||
return Missing;
|
||||
}
|
||||
|
||||
// The ModuleManager's use of FileEntry nodes as the keys for its map of
|
||||
// loaded modules is less than ideal. Uniqueness for FileEntry nodes is
|
||||
// maintained by FileManager, which in turn uses inode numbers on hosts
|
||||
// that support that. When coupled with the module cache's proclivity for
|
||||
// turning over and deleting stale PCMs, this means entries for different
|
||||
// module files can wind up reusing the same underlying inode. When this
|
||||
// happens, subsequent accesses to the Modules map will disagree on the
|
||||
// ModuleFile associated with a given file. In general, it is not sufficient
|
||||
// to resolve this conundrum with a type like FileEntryRef that stores the
|
||||
// name of the FileEntry node on first access because of path canonicalization
|
||||
// issues. However, the paths constructed for implicit module builds are
|
||||
// fully under Clang's control. We *can*, therefore, rely on their structure
|
||||
// being consistent across operating systems and across subsequent accesses
|
||||
// to the Modules map.
|
||||
auto implicitModuleNamesMatch = [](ModuleKind Kind, const ModuleFile *MF,
|
||||
const FileEntry *Entry) -> bool {
|
||||
if (Kind != MK_ImplicitModule)
|
||||
return true;
|
||||
return Entry->getName() == MF->FileName;
|
||||
};
|
||||
|
||||
// Check whether we already loaded this module, before
|
||||
if (ModuleFile *ModuleEntry = Modules.lookup(Entry)) {
|
||||
// Check the stored signature.
|
||||
if (checkSignature(ModuleEntry->Signature, ExpectedSignature, ErrorStr))
|
||||
return OutOfDate;
|
||||
if (implicitModuleNamesMatch(Type, ModuleEntry, Entry)) {
|
||||
// Check the stored signature.
|
||||
if (checkSignature(ModuleEntry->Signature, ExpectedSignature, ErrorStr))
|
||||
return OutOfDate;
|
||||
|
||||
Module = ModuleEntry;
|
||||
updateModuleImports(*ModuleEntry, ImportedBy, ImportLoc);
|
||||
return AlreadyLoaded;
|
||||
Module = ModuleEntry;
|
||||
updateModuleImports(*ModuleEntry, ImportedBy, ImportLoc);
|
||||
return AlreadyLoaded;
|
||||
}
|
||||
}
|
||||
|
||||
// Allocate a new module.
|
||||
|
|
Loading…
Reference in New Issue