[Serialization] Store the path inside ModuleSearchPath as a string, not a StringRef

The idea behind storing a StringRef was to reduce the memory footprint because we made the assumption that a `ModuleSearchPath` always outlives the `SearchPathOptions` it was created from. What I did not consider was that the path was referencing into a `std::vector`, which could get resized, thus invaliding the memory the `ModuleSearchPath`’s `StringRef` was pointing to, causing memory corruption.

To fix this, store the path string inisde the `ModuleSearchPath` itself. Since we store a `ModuleSearchPath` for every file inside that module search path in the `LookupTable`, by itself this would cause a new copy of the path to be stored for every file inside a module search path. To avoid this, make `ModuleSearchPath` ref counted and only store a reference to one shared `ModuleSearchPath` entry in the lookup table.

rdar://88888679

Author: ahoppen

include/swift/AST/SearchPathOptions.h

@@ -36,10 +36,9 @@ enum class ModuleSearchPathKind {
 
 /// A single module search path that can come from different sources, e.g.
 /// framework search paths, import search path etc.
-class ModuleSearchPath {
-  /// The actual path of the module search path. References a search path string
-  /// stored inside \c SearchPathOptions, which must outlive this reference.
-  StringRef Path;
+class ModuleSearchPath : public llvm::RefCountedBase<ModuleSearchPath> {
+  /// The actual path of the module search path.
+  std::string Path;
 
   /// The kind of the search path.
   ModuleSearchPathKind Kind;
@@ -73,6 +72,8 @@ class ModuleSearchPath {
   }
 };
 
+using ModuleSearchPathPtr = llvm::IntrusiveRefCntPtr<ModuleSearchPath>;
+
 class SearchPathOptions;
 
 /// Maintains a mapping of filenames to search paths that contain a file with
@@ -109,7 +110,7 @@ class ModuleSearchPathLookup {
     const SearchPathOptions *Opts;
   } State;
 
-  llvm::StringMap<SmallVector<ModuleSearchPath, 4>> LookupTable;
+  llvm::StringMap<SmallVector<ModuleSearchPathPtr, 4>> LookupTable;
 
   /// Scan the directory at \p SearchPath for files and add those files to the
   /// lookup table. \p Kind specifies the search path kind and \p Index the

lib/AST/SearchPathOptions.cpp

@@ -19,22 +19,24 @@ void ModuleSearchPathLookup::addFilesInPathToLookupTable(
     llvm::vfs::FileSystem *FS, StringRef SearchPath, ModuleSearchPathKind Kind,
     bool IsSystem, unsigned SearchPathIndex) {
   std::error_code Error;
-  assert(llvm::all_of(
-             LookupTable,
-             [&](const auto &LookupTableEntry) {
-               return llvm::none_of(
-                   LookupTableEntry.second,
-                   [&](const ModuleSearchPath &ExistingSearchPath) -> bool {
-                     return ExistingSearchPath.getKind() == Kind &&
-                            ExistingSearchPath.getIndex() == SearchPathIndex;
-                   });
-             }) &&
+  auto entryAlreadyExists = [this](ModuleSearchPathKind Kind,
+                                   unsigned SearchPathIndex) -> bool {
+    return llvm::any_of(LookupTable, [&](const auto &LookupTableEntry) {
+      return llvm::any_of(
+          LookupTableEntry.second, [&](ModuleSearchPathPtr ExistingSearchPath) {
+            return ExistingSearchPath->getKind() == Kind &&
+                   ExistingSearchPath->getIndex() == SearchPathIndex;
+          });
+    });
+  };
+  assert(!entryAlreadyExists(Kind, SearchPathIndex) &&
          "Search path with this kind and index already exists");
+  ModuleSearchPathPtr TableEntry =
+      new ModuleSearchPath(SearchPath, Kind, IsSystem, SearchPathIndex);
   for (auto Dir = FS->dir_begin(SearchPath, Error);
        !Error && Dir != llvm::vfs::directory_iterator(); Dir.increment(Error)) {
     StringRef Filename = llvm::sys::path::filename(Dir->path());
-    LookupTable[Filename].emplace_back(SearchPath, Kind, IsSystem,
-                                       SearchPathIndex);
+    LookupTable[Filename].push_back(TableEntry);
   }
 }
 
@@ -98,9 +100,9 @@ ModuleSearchPathLookup::searchPathsContainingFile(
 
   for (auto &Filename : Filenames) {
     for (auto &Entry : LookupTable[Filename]) {
-      if (ResultIds.insert(std::make_pair(Entry.getKind(), Entry.getIndex()))
+      if (ResultIds.insert(std::make_pair(Entry->getKind(), Entry->getIndex()))
               .second) {
-        Result.push_back(&Entry);
+        Result.push_back(Entry.get());
       }
     }
   }