diff --git a/lld/MachO/Driver.h b/lld/MachO/Driver.h --- a/lld/MachO/Driver.h +++ b/lld/MachO/Driver.h @@ -52,7 +52,7 @@ std::string createResponseFile(const llvm::opt::InputArgList &args); // Check for both libfoo.dylib and libfoo.tbd (in that order). -llvm::Optional resolveDylibPath(llvm::StringRef path); +llvm::Optional resolveDylibPath(llvm::StringRef path); DylibFile *loadDylib(llvm::MemoryBufferRef mbref, DylibFile *umbrella = nullptr, bool isBundleLoader = false); diff --git a/lld/MachO/Driver.cpp b/lld/MachO/Driver.cpp --- a/lld/MachO/Driver.cpp +++ b/lld/MachO/Driver.cpp @@ -92,7 +92,7 @@ {".tbd", ".dylib", ".a"}); } -static Optional findFramework(StringRef name) { +static Optional findFramework(StringRef name) { SmallString<260> symlink; StringRef suffix; std::tie(name, suffix) = name.split(","); @@ -108,12 +108,12 @@ // only append suffix if realpath() succeeds Twine suffixed = location + suffix; if (fs::exists(suffixed)) - return suffixed.str(); + return saver.save(suffixed.str()); } // Suffix lookup failed, fall through to the no-suffix case. } - if (Optional path = resolveDylibPath(symlink)) + if (Optional path = resolveDylibPath(symlink.str())) return path; } return {}; @@ -351,7 +351,7 @@ static void addFramework(StringRef name, bool isNeeded, bool isWeak, bool isReexport, bool isExplicit) { - if (Optional path = findFramework(name)) { + if (Optional path = findFramework(name)) { if (auto *dylibFile = dyn_cast_or_null( addFile(*path, /*forceLoadArchive=*/false, isExplicit))) { if (isNeeded) diff --git a/lld/MachO/DriverUtils.cpp b/lld/MachO/DriverUtils.cpp --- a/lld/MachO/DriverUtils.cpp +++ b/lld/MachO/DriverUtils.cpp @@ -183,7 +183,7 @@ depTracker->logFileNotFound(path); } -Optional macho::resolveDylibPath(StringRef dylibPath) { +Optional macho::resolveDylibPath(StringRef dylibPath) { // TODO: if a tbd and dylib are both present, we should check to make sure // they are consistent. SmallString<261> tbdPath = dylibPath; @@ -191,12 +191,12 @@ bool tbdExists = fs::exists(tbdPath); searchedDylib(tbdPath, tbdExists); if (tbdExists) - return std::string(tbdPath); + return saver.save(tbdPath.str()); bool dylibExists = fs::exists(dylibPath); searchedDylib(dylibPath, dylibExists); if (dylibExists) - return std::string(dylibPath); + return saver.save(dylibPath); return {}; } diff --git a/lld/MachO/InputFiles.cpp b/lld/MachO/InputFiles.cpp --- a/lld/MachO/InputFiles.cpp +++ b/lld/MachO/InputFiles.cpp @@ -902,7 +902,7 @@ for (StringRef dir : config->frameworkSearchPaths) { SmallString<128> candidate = dir; path::append(candidate, frameworkName); - if (Optional dylibPath = resolveDylibPath(candidate)) + if (Optional dylibPath = resolveDylibPath(candidate.str())) return loadDylib(*dylibPath, umbrella); } } else if (Optional dylibPath = findPathCombination( @@ -913,8 +913,7 @@ // 2. As absolute path. if (path::is_absolute(path, path::Style::posix)) for (StringRef root : config->systemLibraryRoots) - if (Optional dylibPath = - resolveDylibPath((root + path).str())) + if (Optional dylibPath = resolveDylibPath((root + path).str())) return loadDylib(*dylibPath, umbrella); // 3. As relative path. @@ -943,7 +942,7 @@ path::remove_filename(newPath); } path::append(newPath, rpath, path.drop_front(strlen("@rpath/"))); - if (Optional dylibPath = resolveDylibPath(newPath)) + if (Optional dylibPath = resolveDylibPath(newPath.str())) return loadDylib(*dylibPath, umbrella); } } @@ -961,7 +960,7 @@ } } - if (Optional dylibPath = resolveDylibPath(path)) + if (Optional dylibPath = resolveDylibPath(path)) return loadDylib(*dylibPath, umbrella); return nullptr; diff --git a/lld/test/MachO/lc-linker-option.ll b/lld/test/MachO/lc-linker-option.ll --- a/lld/test/MachO/lc-linker-option.ll +++ b/lld/test/MachO/lc-linker-option.ll @@ -48,6 +48,23 @@ ; RUN: not %lld %t/invalid.o -o /dev/null 2>&1 | FileCheck --check-prefix=INVALID %s ; INVALID: error: -why_load is not allowed in LC_LINKER_OPTION +;; We are testing this because we want to check a dangling string reference problem (see https://reviews.llvm.org/D111706). +;; To trigger this problem, we need to create a framework that is an archive, +;; and it needs to contain a symbol starting with OBJC_CLASS_$. +;; The bug is triggered as the linker loads this framework twice via LC_LINKER_OPTION. +;; When the linker adds this framework, it will fail to map the path of framework to this archive due to dangling reference. +;; Therefore, it will load the framework twice, and if there is any symbol with OBJC_CLASS_$ prefix with forceLoadObjC enabled, +;; the linker will fetch this symbol twice, which leads to a duplicate symbol error. +; RUN: llc %t/foo.ll -o %t/foo.o -filetype=obj +; RUN: mkdir -p %t/foo.framework/Versions/A +; RUN: llvm-ar rcs %t/foo.framework/Versions/A/foo %t/foo.o +; RUN: ln -sf A %t/foo.framework/Versions/Current +; RUN: ln -sf Versions/Current/foo %t/foo.framework/foo +; RUN: llc %t/objfile1.ll -o %t/objfile1.o -filetype=obj +; RUN: llc %t/objfile2.ll -o %t/objfile2.o -filetype=obj +; RUN: llc %t/main.ll -o %t/main.o -filetype=obj +; RUN: %lld -demangle -ObjC %t/objfile1.o %t/objfile2.o %t/main.o -o %t/main.out -arch x86_64 -platform_version macos 11.0.0 0.0.0 -F%t + ;--- framework.ll target triple = "x86_64-apple-macosx10.15.0" target datalayout = "e-m:o-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128" @@ -90,3 +107,34 @@ define void @main() { ret void } + +;--- objfile1.ll +target triple = "x86_64-apple-macosx10.15.0" +target datalayout = "e-m:o-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128" + +!0 = !{!"-framework", !"foo"} +!llvm.linker.options = !{!0} + +;--- objfile2.ll +target triple = "x86_64-apple-macosx10.15.0" +target datalayout = "e-m:o-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128" + +!0 = !{!"-framework", !"foo"} +!llvm.linker.options = !{!0} + +;--- main.ll +target triple = "x86_64-apple-macosx10.15.0" +target datalayout = "e-m:o-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128" + +define void @main() { + ret void +} + +;--- foo.ll +target triple = "x86_64-apple-macosx10.15.0" +target datalayout = "e-m:o-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128" + +%0 = type opaque +%struct._class_t = type {} + +@"OBJC_CLASS_$_TestClass" = global %struct._class_t {}, section "__DATA, __objc_data", align 8 \ No newline at end of file