r217385 - Make FileEntry::getName() valid across calls to FileManager::getFile()

Ben Langmuir blangmuir at apple.com
Mon Sep 8 09:15:54 PDT 2014


Author: benlangmuir
Date: Mon Sep  8 11:15:54 2014
New Revision: 217385

URL: http://llvm.org/viewvc/llvm-project?rev=217385&view=rev
Log:
Make FileEntry::getName() valid across calls to FileManager::getFile()

Because we may change the name of a FileEntry inside getFile, the name
returned by FileEntry::getName() could be destroyed.  This was causing a
use-after-free when searching the HeaderFileInfo on-disk hashtable for a
module or pch.

Modified:
    cfe/trunk/include/clang/Basic/FileManager.h
    cfe/trunk/lib/Basic/FileManager.cpp

Modified: cfe/trunk/include/clang/Basic/FileManager.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/FileManager.h?rev=217385&r1=217384&r2=217385&view=diff
==============================================================================
--- cfe/trunk/include/clang/Basic/FileManager.h (original)
+++ cfe/trunk/include/clang/Basic/FileManager.h Mon Sep  8 11:15:54 2014
@@ -59,7 +59,7 @@ public:
 /// If the 'File' member is valid, then this FileEntry has an open file
 /// descriptor for the file.
 class FileEntry {
-  std::string Name;           // Name of the file.
+  const char *Name;           // Name of the file.
   off_t Size;                 // File size in bytes.
   time_t ModTime;             // Modification time of file.
   const DirectoryEntry *Dir;  // Directory file lives in.
@@ -93,7 +93,7 @@ public:
     assert(!isValid() && "Cannot copy an initialized FileEntry");
   }
 
-  const char *getName() const { return Name.c_str(); }
+  const char *getName() const { return Name; }
   bool isValid() const { return IsValid; }
   off_t getSize() const { return Size; }
   unsigned getUID() const { return UID; }

Modified: cfe/trunk/lib/Basic/FileManager.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Basic/FileManager.cpp?rev=217385&r1=217384&r2=217385&view=diff
==============================================================================
--- cfe/trunk/lib/Basic/FileManager.cpp (original)
+++ cfe/trunk/lib/Basic/FileManager.cpp Mon Sep  8 11:15:54 2014
@@ -270,6 +270,19 @@ const FileEntry *FileManager::getFile(St
   FileEntry &UFE = UniqueRealFiles[Data.UniqueID];
 
   NamedFileEnt.setValue(&UFE);
+
+  // If the name returned by getStatValue is different than Filename, re-intern
+  // the name.
+  if (Data.Name != Filename) {
+    auto &NamedFileEnt = SeenFileEntries.GetOrCreateValue(Data.Name);
+    if (!NamedFileEnt.getValue())
+      NamedFileEnt.setValue(&UFE);
+    else
+      assert(NamedFileEnt.getValue() == &UFE &&
+             "filename from getStatValue() refers to wrong file");
+    InterndFileName = NamedFileEnt.getKeyData();
+  }
+
   if (UFE.isValid()) { // Already have an entry with this inode, return it.
 
     // FIXME: this hack ensures that if we look up a file by a virtual path in
@@ -286,13 +299,13 @@ const FileEntry *FileManager::getFile(St
     // to switch towards a design where we return a FileName object that
     // encapsulates both the name by which the file was accessed and the
     // corresponding FileEntry.
-    UFE.Name = Data.Name;
+    UFE.Name = InterndFileName;
 
     return &UFE;
   }
 
   // Otherwise, we don't have this file yet, add it.
-  UFE.Name    = Data.Name;
+  UFE.Name    = InterndFileName;
   UFE.Size = Data.Size;
   UFE.ModTime = Data.ModTime;
   UFE.Dir     = DirInfo;





More information about the cfe-commits mailing list