From 735d818c01a5e1c96a8c30c13993c939b816f42c Mon Sep 17 00:00:00 2001 From: Ben Langmuir Date: Mon, 19 May 2014 16:13:45 +0000 Subject: [PATCH] Fix use-after-free and spurious error during module load FileManager::invalidateCache is not safe to call when there may be existing references to the file. What module load failure needs is to refresh so stale stat() info isn't stored. This may be the last user of invalidateCache; I'll take a look and remove it if possible in a future commit. This caused a use-after-free error as well as a spurious error message that a module was "found in both 'X.pcm' and 'X.pcm'" in some cases. git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@209138 91177308-0d34-0410-b5e6-96231b3b80d8 --- lib/Serialization/ModuleManager.cpp | 15 +++++++++++++-- test/Modules/load-after-failure.m | 25 +++++++++++++++++++++++++ 2 files changed, 38 insertions(+), 2 deletions(-) create mode 100644 test/Modules/load-after-failure.m diff --git a/lib/Serialization/ModuleManager.cpp b/lib/Serialization/ModuleManager.cpp index 66f18158a6..c3152c0b02 100644 --- a/lib/Serialization/ModuleManager.cpp +++ b/lib/Serialization/ModuleManager.cpp @@ -152,9 +152,20 @@ void ModuleManager::removeModules(ModuleIterator first, ModuleIterator last, // Delete the modules and erase them from the various structures. for (ModuleIterator victim = first; victim != last; ++victim) { - Modules.erase((*victim)->File); + const FileEntry *F = (*victim)->File; + Modules.erase(F); + + // Refresh the stat() information for the module file so stale information + // doesn't get stored accidentally. + vfs::Status UpdatedStat; + if (FileMgr.getNoncachedStatValue(F->getName(), UpdatedStat)) { + llvm::report_fatal_error(Twine("module file '") + F->getName() + + "' removed after it has been used"); + } else { + FileMgr.modifyFileEntry(const_cast(F), UpdatedStat.getSize(), + UpdatedStat.getLastModificationTime().toEpochTime()); + } - FileMgr.invalidateCache((*victim)->File); if (modMap) { StringRef ModuleName = (*victim)->ModuleName; if (Module *mod = modMap->findModule(ModuleName)) { diff --git a/test/Modules/load-after-failure.m b/test/Modules/load-after-failure.m new file mode 100644 index 0000000000..f471fd88d5 --- /dev/null +++ b/test/Modules/load-after-failure.m @@ -0,0 +1,25 @@ +// REQUIRES: shell +// RUN: rm -rf %t +// RUN: mkdir -p %t + +// RUN: echo '@import B;' > %t/A.h +// RUN: echo '@import C;' > %t/B.h +// RUN: echo '@import D;' >> %t/B.h +// RUN: echo '// C.h' > %t/C.h +// RUN: echo '// D.h' > %t/D.h +// RUN: echo 'module A { header "A.h" }' > %t/module.modulemap +// RUN: echo 'module B { header "B.h" }' >> %t/module.modulemap +// RUN: echo 'module C { header "C.h" }' >> %t/module.modulemap +// RUN: echo 'module D { header "D.h" }' >> %t/module.modulemap + +// RUN: %clang_cc1 -fmodules -fmodules-cache-path=%t -I %t %s -verify +// RUN: echo " " >> %t/D.h +// RUN: %clang_cc1 -fmodules -fmodules-cache-path=%t -I %t %s -verify +// expected-no-diagnostics + + +@import C; +@import A; +@import C; +// When compiling A, C will be be loaded then removed when D fails. Ensure +// this does not cause problems importing C again later. -- 2.50.1