replica
/
hanchenye-llvm-project
2
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

[lld-macho] Fix dangling string reference when adding frameworks 

In Driver.cpp, addFramework used std::string instance to represent the path of a framework, which will be freed after the function returns. However, this string is stored in loadedArchive, which will be used later to compare with path of newly added frameworks. This caused https://bugs.llvm.org/show_bug.cgi?id=52133. A test is included in this commit to reproduce this bug.

Now resolveDylibPath returns a StringRef instance, and it uses StringSaver to save its data, then returns it to functions on the top. This ensures the resolved framework path is still valid after LC_LINKER_OPTION is parsed.

Reviewed By: int3, #lld-macho, oontvoo

Differential Revision: https://reviews.llvm.org/D111706
This commit is contained in:
Kaining Zhong 2021-10-20 11:20:21 -04:00 committed by Vy Nguyen
parent 3d152bc49d
commit aab0f2264a
5 changed files with 60 additions and 13 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

8
lld/MachO/Driver.cpp
View File

@ -92,7 +92,7 @@ static Optional<StringRef> findLibrary(StringRef name) {
{".tbd", ".dylib", ".a"});
}
static Optional<std::string> findFramework(StringRef name) {
static Optional<StringRef> findFramework(StringRef name) {
SmallString<260> symlink;
StringRef suffix;
std::tie(name, suffix) = name.split(",");
@ -108,12 +108,12 @@ static Optional<std::string> findFramework(StringRef name) {
// 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<std::string> path = resolveDylibPath(symlink))
if (Optional<StringRef> path = resolveDylibPath(symlink.str()))
return path;
}
return {};
@ -351,7 +351,7 @@ static void addLibrary(StringRef name, bool isNeeded, bool isWeak,
static void addFramework(StringRef name, bool isNeeded, bool isWeak,
bool isReexport, bool isExplicit) {
if (Optional<std::string> path = findFramework(name)) {
if (Optional<StringRef> path = findFramework(name)) {
if (auto *dylibFile = dyn_cast_or_null<DylibFile>(
addFile(*path, /*forceLoadArchive=*/false, isExplicit))) {
if (isNeeded)

2
lld/MachO/Driver.h
View File

@ -52,7 +52,7 @@ void parseLCLinkerOption(InputFile *, unsigned argc, StringRef data);
std::string createResponseFile(const llvm::opt::InputArgList &args);
// Check for both libfoo.dylib and libfoo.tbd (in that order).
llvm::Optional<std::string> resolveDylibPath(llvm::StringRef path);
llvm::Optional<StringRef> resolveDylibPath(llvm::StringRef path);
DylibFile *loadDylib(llvm::MemoryBufferRef mbref, DylibFile *umbrella = nullptr,
bool isBundleLoader = false);

6
lld/MachO/DriverUtils.cpp
View File

@ -183,7 +183,7 @@ static void searchedDylib(const Twine &path, bool found) {
depTracker->logFileNotFound(path);
}
Optional<std::string> macho::resolveDylibPath(StringRef dylibPath) {
Optional<StringRef> 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 @@ Optional<std::string> macho::resolveDylibPath(StringRef dylibPath) {
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 {};
}

9
lld/MachO/InputFiles.cpp
View File

@ -902,7 +902,7 @@ static DylibFile *findDylib(StringRef path, DylibFile *umbrella,
for (StringRef dir : config->frameworkSearchPaths) {
SmallString<128> candidate = dir;
path::append(candidate, frameworkName);
if (Optional<std::string> dylibPath = resolveDylibPath(candidate))
if (Optional<StringRef> dylibPath = resolveDylibPath(candidate.str()))
return loadDylib(*dylibPath, umbrella);
}
} else if (Optional<StringRef> dylibPath = findPathCombination(
@ -913,8 +913,7 @@ static DylibFile *findDylib(StringRef path, DylibFile *umbrella,
// 2. As absolute path.
if (path::is_absolute(path, path::Style::posix))
for (StringRef root : config->systemLibraryRoots)
if (Optional<std::string> dylibPath =
resolveDylibPath((root + path).str()))
if (Optional<StringRef> dylibPath = resolveDylibPath((root + path).str()))
return loadDylib(*dylibPath, umbrella);
// 3. As relative path.
@ -943,7 +942,7 @@ static DylibFile *findDylib(StringRef path, DylibFile *umbrella,
path::remove_filename(newPath);
}
path::append(newPath, rpath, path.drop_front(strlen("@rpath/")));
if (Optional<std::string> dylibPath = resolveDylibPath(newPath))
if (Optional<StringRef> dylibPath = resolveDylibPath(newPath.str()))
return loadDylib(*dylibPath, umbrella);
}
}
@ -961,7 +960,7 @@ static DylibFile *findDylib(StringRef path, DylibFile *umbrella,
}
}
if (Optional<std::string> dylibPath = resolveDylibPath(path))
if (Optional<StringRef> dylibPath = resolveDylibPath(path))
return loadDylib(*dylibPath, umbrella);
return nullptr;

48
lld/test/MachO/lc-linker-option.ll
View File

@ -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 @@ target datalayout = "e-m:o-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16
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