[lld] a79c018 - Revert "[lld-macho] Have path-related functions return std::string, not StringRef"

[Jez Ng via llvm-commits]

Author: Jez Ng
Date: 2021-06-18T22:43:49-04:00
New Revision: a79c01832553a8a192ab157098662a9e8f2c16af

URL: https://github.com/llvm/llvm-project/commit/a79c01832553a8a192ab157098662a9e8f2c16af
DIFF: https://github.com/llvm/llvm-project/commit/a79c01832553a8a192ab157098662a9e8f2c16af.diff

LOG: Revert "[lld-macho] Have path-related functions return std::string, not StringRef"

This reverts commit 1d31fb8d122b1117cf20a9edc09812db8472e930.

Making `rerootPath` return a temporary std::string caused a
use-after-free:

https://ci.chromium.org/ui/p/chromium/builders/try/win_upload_clang/1608/overview

Added: 
    

Modified: 
    lld/MachO/Driver.cpp
    lld/MachO/Driver.h
    lld/MachO/DriverUtils.cpp

Removed: 
    


################################################################################
diff  --git a/lld/MachO/Driver.cpp b/lld/MachO/Driver.cpp
index c81160c51cf7..52771c2a6fe1 100644
--- a/lld/MachO/Driver.cpp
+++ b/lld/MachO/Driver.cpp
@@ -78,9 +78,9 @@ static HeaderFileType getOutputType(const InputArgList &args) {
   }
 }
 
-static Optional<std::string> findLibrary(StringRef name) {
+static Optional<StringRef> findLibrary(StringRef name) {
   if (config->searchDylibsFirst) {
-    if (Optional<std::string> path = findPathCombination(
+    if (Optional<StringRef> path = findPathCombination(
             "lib" + name, config->librarySearchPaths, {".tbd", ".dylib"}))
       return path;
     return findPathCombination("lib" + name, config->librarySearchPaths,
@@ -328,7 +328,7 @@ static InputFile *addFile(StringRef path, bool forceLoadArchive,
 
 static void addLibrary(StringRef name, bool isNeeded, bool isWeak,
                        bool isReexport, bool isExplicit, bool forceLoad) {
-  if (Optional<std::string> path = findLibrary(name)) {
+  if (Optional<StringRef> path = findLibrary(name)) {
     if (auto *dylibFile = dyn_cast_or_null<DylibFile>(
             addFile(*path, forceLoad, isExplicit))) {
       if (isNeeded)

diff  --git a/lld/MachO/Driver.h b/lld/MachO/Driver.h
index 9ae468cd2a36..148c309b52d5 100644
--- a/lld/MachO/Driver.h
+++ b/lld/MachO/Driver.h
@@ -58,14 +58,14 @@ DylibFile *loadDylib(llvm::MemoryBufferRef mbref, DylibFile *umbrella = nullptr,
 
 // Search for all possible combinations of `{root}/{name}.{extension}`.
 // If \p extensions are not specified, then just search for `{root}/{name}`.
-llvm::Optional<std::string>
+llvm::Optional<llvm::StringRef>
 findPathCombination(const llvm::Twine &name,
                     const std::vector<llvm::StringRef> &roots,
                     ArrayRef<llvm::StringRef> extensions = {""});
 
 // If -syslibroot is specified, absolute paths to non-object files may be
 // rerooted.
-std::string rerootPath(llvm::StringRef path);
+llvm::StringRef rerootPath(llvm::StringRef path);
 
 llvm::Optional<InputFile *> loadArchiveMember(MemoryBufferRef, uint32_t modTime,
                                               StringRef archiveName,

diff  --git a/lld/MachO/DriverUtils.cpp b/lld/MachO/DriverUtils.cpp
index f32027b2bcbf..55582731aab9 100644
--- a/lld/MachO/DriverUtils.cpp
+++ b/lld/MachO/DriverUtils.cpp
@@ -246,7 +246,7 @@ DylibFile *macho::loadDylib(MemoryBufferRef mbref, DylibFile *umbrella,
   return newFile;
 }
 
-Optional<std::string>
+Optional<StringRef>
 macho::findPathCombination(const Twine &name,
                            const std::vector<StringRef> &roots,
                            ArrayRef<StringRef> extensions) {
@@ -259,21 +259,21 @@ macho::findPathCombination(const Twine &name,
       bool exists = fs::exists(location);
       searchedDylib(location, exists);
       if (exists)
-        return location.str();
+        return saver.save(location.str());
     }
   }
   return {};
 }
 
-std::string macho::rerootPath(StringRef path) {
+StringRef macho::rerootPath(StringRef path) {
   if (!path::is_absolute(path, path::Style::posix) || path.endswith(".o"))
-    return std::string(path);
+    return path;
 
-  if (Optional<std::string> rerootedPath =
+  if (Optional<StringRef> rerootedPath =
           findPathCombination(path, config->systemLibraryRoots))
     return *rerootedPath;
 
-  return std::string(path);
+  return path;
 }
 
 Optional<InputFile *> macho::loadArchiveMember(MemoryBufferRef mb,