Manage ownership and assert on overlapping lifetimes

Author: jansvoboda11

clang/include/clang/Serialization/ASTReader.h

@@ -50,6 +50,7 @@
 #include "llvm/ADT/iterator_range.h"
 #include "llvm/Bitstream/BitstreamReader.h"
 #include "llvm/Support/MemoryBuffer.h"
+#include "llvm/Support/SaveAndRestore.h"
 #include "llvm/Support/Timer.h"
 #include "llvm/Support/VersionTuple.h"
 #include <cassert>
@@ -1342,19 +1343,47 @@ class ASTReader
                                         bool Complain = true);
 
   /// Buffer we use as temporary storage backing resolved paths.
-  SmallString<256> PathBuf;
+  std::optional<SmallString<256>> PathBuf{{}};
+
+  /// A RAII wrapper around \c StringRef that temporarily takes ownership of the
+  /// underlying buffer and gives it back on destruction.
+  class TemporarilyOwnedStringRef {
+    StringRef String;
+    llvm::SaveAndRestore<std::optional<SmallString<256>>> TemporaryLoan;
+
+  public:
+    TemporarilyOwnedStringRef(StringRef S, std::optional<SmallString<256>> &Buf)
+        : String(S), TemporaryLoan(Buf, {}) {}
+
+    /// Returns the wrapped \c StringRef that must be outlived by \c this.
+    const StringRef *operator->() const { return &String; }
+    /// Returns the wrapped \c StringRef that must be outlived by \c this.
+    const StringRef &operator*() const { return String; }
+  };
 
 public:
-  StringRef ResolveImportedPath(StringRef Path, ModuleFile &M) {
-    return ResolveImportedPath(PathBuf, Path, M);
+  /// Resolve \c Path in the context of module file \c M. The return value must
+  /// be destroyed before another call to \c ResolveImportPath.
+  TemporarilyOwnedStringRef ResolveImportedPath(StringRef Path, ModuleFile &M) {
+    return ResolveImportedPath(Path, M.BaseDirectory);
   }
-  StringRef ResolveImportedPath(StringRef Path, StringRef Prefix) {
-    return ResolveImportedPath(PathBuf, Path, Prefix);
+  /// Resolve \c Path in the context of the \c Prefix directory. The return
+  /// value must be destroyed before another call to \c ResolveImportPath.
+  TemporarilyOwnedStringRef ResolveImportedPath(StringRef Path,
+                                                StringRef Prefix) {
+    assert(PathBuf && "Multiple overlapping calls to ResolveImportedPath");
+    StringRef ResolvedPath = ResolveImportedPath(*PathBuf, Path, Prefix);
+    return {ResolvedPath, PathBuf};
   }
+
+  /// Resolve \c Path in the context of module file \c M. The \c Buffer must
+  /// outlive the returned \c StringRef.
   static StringRef ResolveImportedPath(SmallVectorImpl<char> &Buffer,
                                        StringRef Path, ModuleFile &M) {
     return ResolveImportedPath(Buffer, Path, M.BaseDirectory);
   }
+  /// Resolve \c Path in the context of the \c Prefix directory. The \c Buffer
+  /// must outlive the returned \c StringRef.
   static StringRef ResolveImportedPath(SmallVectorImpl<char> &Buffer,
                                        StringRef Path, StringRef Prefix);
 

clang/lib/Serialization/ASTReader.cpp

@@ -2048,8 +2048,8 @@ HeaderFileInfoTrait::getFile(const internal_key_type &Key) {
   if (!Key.Imported)
     return FileMgr.getOptionalFileRef(Key.Filename);
 
-  StringRef Resolved = Reader.ResolveImportedPath(Key.Filename, M);
-  return FileMgr.getOptionalFileRef(Resolved);
+  auto Resolved = Reader.ResolveImportedPath(Key.Filename, M);
+  return FileMgr.getOptionalFileRef(*Resolved);
 }
 
 unsigned HeaderFileInfoTrait::ComputeHash(internal_key_ref ikey) {
@@ -2513,9 +2513,9 @@ InputFileInfo ASTReader::getInputFileInfo(ModuleFile &F, unsigned ID) {
     uint16_t AsRequestedLength = Record[7];
 
     std::string NameAsRequested =
-        ResolveImportedPath(Blob.substr(0, AsRequestedLength), F).str();
+        ResolveImportedPath(Blob.substr(0, AsRequestedLength), F)->str();
     std::string Name =
-        ResolveImportedPath(Blob.substr(AsRequestedLength), F).str();
+        ResolveImportedPath(Blob.substr(AsRequestedLength), F)->str();
 
     if (Name.empty())
       Name = NameAsRequested;
@@ -3181,7 +3181,7 @@ ASTReader::ReadControlBlock(ModuleFile &F,
       F.OriginalSourceFileID = FileID::get(Record[0]);
       F.ActualOriginalSourceFileName = std::string(Blob);
       F.OriginalSourceFileName =
-          ResolveImportedPath(F.ActualOriginalSourceFileName, F).str();
+          ResolveImportedPath(F.ActualOriginalSourceFileName, F)->str();
       break;
 
     case ORIGINAL_FILE_ID:
@@ -5895,8 +5895,8 @@ llvm::Error ASTReader::ReadSubmoduleBlock(ModuleFile &F,
       // FIXME: This doesn't work for framework modules as `Filename` is the
       //        name as written in the module file and does not include
       //        `Headers/`, so this path will never exist.
-      StringRef Filename = ResolveImportedPath(Blob, F);
-      if (auto Umbrella = PP.getFileManager().getOptionalFileRef(Filename)) {
+      auto Filename = ResolveImportedPath(Blob, F);
+      if (auto Umbrella = PP.getFileManager().getOptionalFileRef(*Filename)) {
         if (!CurrentModule->getUmbrellaHeaderAsWritten()) {
           // FIXME: NameAsWritten
           ModMap.setUmbrellaHeaderAsWritten(CurrentModule, *Umbrella, Blob, "");
@@ -5924,16 +5924,16 @@ llvm::Error ASTReader::ReadSubmoduleBlock(ModuleFile &F,
       break;
 
     case SUBMODULE_TOPHEADER: {
-      StringRef HeaderName = ResolveImportedPath(Blob, F);
-      CurrentModule->addTopHeaderFilename(HeaderName);
+      auto HeaderName = ResolveImportedPath(Blob, F);
+      CurrentModule->addTopHeaderFilename(*HeaderName);
       break;
     }
 
     case SUBMODULE_UMBRELLA_DIR: {
       // See comments in SUBMODULE_UMBRELLA_HEADER
-      StringRef Dirname = ResolveImportedPath(Blob, F);
+      auto Dirname = ResolveImportedPath(Blob, F);
       if (auto Umbrella =
-              PP.getFileManager().getOptionalDirectoryRef(Dirname)) {
+              PP.getFileManager().getOptionalDirectoryRef(*Dirname)) {
         if (!CurrentModule->getUmbrellaDirAsWritten()) {
           // FIXME: NameAsWritten
           ModMap.setUmbrellaDirAsWritten(CurrentModule, *Umbrella, Blob, "");
@@ -9589,13 +9589,13 @@ std::string ASTReader::ReadString(const RecordDataImpl &Record, unsigned &Idx) {
 std::string ASTReader::ReadPath(ModuleFile &F, const RecordData &Record,
                                 unsigned &Idx) {
   std::string Filename = ReadString(Record, Idx);
-  return ResolveImportedPath(Filename, F).str();
+  return ResolveImportedPath(Filename, F)->str();
 }
 
 std::string ASTReader::ReadPath(StringRef BaseDirectory,
                                 const RecordData &Record, unsigned &Idx) {
   std::string Filename = ReadString(Record, Idx);
-  return ResolveImportedPath(Filename, BaseDirectory).str();
+  return ResolveImportedPath(Filename, BaseDirectory)->str();
 }
 
 VersionTuple ASTReader::ReadVersionTuple(const RecordData &Record,