r209138 - Fix use-after-free and spurious error during module load

[Ben Langmuir]

Author: benlangmuir
Date: Mon May 19 11:13:45 2014
New Revision: 209138

URL: http://llvm.org/viewvc/llvm-project?rev=209138&view=rev
Log:
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.

Added:
    cfe/trunk/test/Modules/load-after-failure.m
Modified:
    cfe/trunk/lib/Serialization/ModuleManager.cpp

Modified: cfe/trunk/lib/Serialization/ModuleManager.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Serialization/ModuleManager.cpp?rev=209138&r1=209137&r2=209138&view=diff
==============================================================================
--- cfe/trunk/lib/Serialization/ModuleManager.cpp (original)
+++ cfe/trunk/lib/Serialization/ModuleManager.cpp Mon May 19 11:13:45 2014
@@ -152,9 +152,20 @@ void ModuleManager::removeModules(Module
 
   // 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<FileEntry *>(F), UpdatedStat.getSize(),
+          UpdatedStat.getLastModificationTime().toEpochTime());
+    }
 
-    FileMgr.invalidateCache((*victim)->File);
     if (modMap) {
       StringRef ModuleName = (*victim)->ModuleName;
       if (Module *mod = modMap->findModule(ModuleName)) {

Added: cfe/trunk/test/Modules/load-after-failure.m
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/load-after-failure.m?rev=209138&view=auto
==============================================================================
--- cfe/trunk/test/Modules/load-after-failure.m (added)
+++ cfe/trunk/test/Modules/load-after-failure.m Mon May 19 11:13:45 2014
@@ -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
+
+
+ at import C;
+ at import A;
+ at 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.