CodaFi updated this revision to Diff 286679.
CodaFi retitled this revision from "[clang][Modules] Use File Names Instead of
inodes As Loaded Module Keys" to "[clang][Modules] Increase the Entropy of
ModuleManager Map Keys".
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D85981/new/
https://reviews.llvm.org/D85981
Files:
clang/include/clang/Serialization/ModuleManager.h
clang/lib/Serialization/ModuleManager.cpp
Index: clang/lib/Serialization/ModuleManager.cpp
===================================================================
--- clang/lib/Serialization/ModuleManager.cpp
+++ clang/lib/Serialization/ModuleManager.cpp
@@ -59,7 +59,7 @@
}
ModuleFile *ModuleManager::lookup(const FileEntry *File) const {
- auto Known = Modules.find(File);
+ auto Known = Modules.find(EntryKey{File});
if (Known == Modules.end())
return nullptr;
@@ -72,7 +72,7 @@
/*CacheFailure=*/false);
if (!Entry)
return nullptr;
- return std::move(InMemoryBuffers[*Entry]);
+ return std::move(InMemoryBuffers[EntryKey{*Entry}]);
}
static bool checkSignature(ASTFileSignature Signature,
@@ -133,7 +133,7 @@
}
// Check whether we already loaded this module, before
- if (ModuleFile *ModuleEntry = Modules.lookup(Entry)) {
+ if (ModuleFile *ModuleEntry = Modules.lookup(EntryKey{Entry})) {
// Check the stored signature.
if (checkSignature(ModuleEntry->Signature, ExpectedSignature, ErrorStr))
return OutOfDate;
@@ -208,7 +208,7 @@
return OutOfDate;
// We're keeping this module. Store it everywhere.
- Module = Modules[Entry] = NewModule.get();
+ Module = Modules[EntryKey{Entry}] = NewModule.get();
updateModuleImports(*NewModule, ImportedBy, ImportLoc);
@@ -255,7 +255,7 @@
// Delete the modules and erase them from the various structures.
for (ModuleIterator victim = First; victim != Last; ++victim) {
- Modules.erase(victim->File);
+ Modules.erase(EntryKey{victim->File});
if (modMap) {
StringRef ModuleName = victim->ModuleName;
@@ -274,7 +274,7 @@
std::unique_ptr<llvm::MemoryBuffer> Buffer) {
const FileEntry *Entry =
FileMgr.getVirtualFile(FileName, Buffer->getBufferSize(), 0);
- InMemoryBuffers[Entry] = std::move(Buffer);
+ InMemoryBuffers[EntryKey{Entry}] = std::move(Buffer);
}
ModuleManager::VisitState *ModuleManager::allocateVisitState() {
Index: clang/include/clang/Serialization/ModuleManager.h
===================================================================
--- clang/include/clang/Serialization/ModuleManager.h
+++ clang/include/clang/Serialization/ModuleManager.h
@@ -59,8 +59,67 @@
// to implement short-circuiting logic when running DFS over the dependencies.
SmallVector<ModuleFile *, 2> Roots;
+ /// An \c EntryKey is a thin wrapper around a \c FileEntry that implements
+ /// a richer notion of identity.
+ ///
+ /// A plain \c FileEntry has its identity tied to inode numbers. When the
+ /// module cache regenerates a PCM, some filesystem allocators may reuse
+ /// inode numbers for distinct modules, which can cause the cache to return
+ /// mismatched entries. An \c EntryKey ensures that the size and modification
+ /// time are taken into account when determining the identity of a key, which
+ /// significantly decreases - but does not eliminate - the chance of
+ /// a collision.
+ struct EntryKey {
+ const FileEntry *Entry;
+ off_t Size;
+ time_t ModTime;
+
+ EntryKey(const FileEntry *Entry) : Entry(Entry), Size(0), ModTime(0) {
+ if (Entry) {
+ Size = Entry->getSize();
+ ModTime = Entry->getModificationTime();
+ }
+ }
+
+ EntryKey(const FileEntry *Entry, off_t Size, time_t ModTime)
+ : Entry(Entry), Size(Size), ModTime(ModTime) {}
+
+ struct Info {
+ static inline EntryKey getEmptyKey() {
+ return EntryKey{
+ llvm::DenseMapInfo<const FileEntry *>::getEmptyKey(),
+ llvm::DenseMapInfo<off_t>::getEmptyKey(),
+ llvm::DenseMapInfo<time_t>::getEmptyKey(),
+ };
+ }
+ static inline EntryKey getTombstoneKey() {
+ return EntryKey{
+ llvm::DenseMapInfo<const FileEntry *>::getTombstoneKey(),
+ llvm::DenseMapInfo<off_t>::getTombstoneKey(),
+ llvm::DenseMapInfo<time_t>::getTombstoneKey(),
+ };
+ }
+ static unsigned getHashValue(const EntryKey &Val) {
+ return llvm::DenseMapInfo<const FileEntry *>::getHashValue(Val.Entry);
+ }
+ static bool isEqual(const EntryKey &LHS, const EntryKey &RHS) {
+ if (LHS.Entry == getEmptyKey().Entry ||
+ LHS.Entry == getTombstoneKey().Entry ||
+ RHS.Entry == getEmptyKey().Entry ||
+ RHS.Entry == getTombstoneKey().Entry) {
+ return LHS.Entry == RHS.Entry;
+ }
+ if (LHS.Entry == nullptr || RHS.Entry == nullptr) {
+ return LHS.Entry == RHS.Entry;
+ }
+ return LHS.Entry == RHS.Entry && LHS.Size == RHS.Size &&
+ LHS.ModTime == RHS.ModTime;
+ }
+ };
+ };
+
/// All loaded modules, indexed by name.
- llvm::DenseMap<const FileEntry *, ModuleFile *> Modules;
+ llvm::DenseMap<EntryKey, ModuleFile *, EntryKey::Info> Modules;
/// FileManager that handles translating between filenames and
/// FileEntry *.
@@ -76,7 +135,7 @@
const HeaderSearch &HeaderSearchInfo;
/// A lookup of in-memory (virtual file) buffers
- llvm::DenseMap<const FileEntry *, std::unique_ptr<llvm::MemoryBuffer>>
+ llvm::DenseMap<EntryKey, std::unique_ptr<llvm::MemoryBuffer>, EntryKey::Info>
InMemoryBuffers;
/// The visitation order.
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
