Support --unresolved-symbols=@<file> option in LLD for ELF #142917
Conversation
|
@llvm/pr-subscribers-lld @llvm/pr-subscribers-lld-elf Author: Dmitry Nechitaev (Nechda) ChangesThis pull request implements one more feature of the Original issue: #142798 Full diff: https://github.com/llvm/llvm-project/pull/142917.diff 7 Files Affected:
diff --git a/lld/ELF/Config.h b/lld/ELF/Config.h
index f0e9592d85dd6..65624685398e8 100644
--- a/lld/ELF/Config.h
+++ b/lld/ELF/Config.h
@@ -397,6 +397,7 @@ struct Config {
SortSectionPolicy sortSection;
StripPolicy strip;
UnresolvedPolicy unresolvedSymbols;
+ llvm::SmallVector<std::string> unresolvedSymbolsList;
UnresolvedPolicy unresolvedSymbolsInShlib;
Target2Policy target2;
GcsPolicy zGcs;
diff --git a/lld/ELF/Driver.cpp b/lld/ELF/Driver.cpp
index 6150fe072156f..64cc035c6e8e4 100644
--- a/lld/ELF/Driver.cpp
+++ b/lld/ELF/Driver.cpp
@@ -714,6 +714,14 @@ static void setUnresolvedSymbolPolicy(Ctx &ctx, opt::InputArgList &args) {
} else if (s == "report-all") {
diagRegular = true;
diagShlib = true;
+ } else if (s.starts_with("@")) {
+ // Read file with set of unresolved symbols
+ StringRef filename(s.substr(1ULL));
+ std::optional<MemoryBufferRef> buffer = readFile(ctx, filename);
+ if (!buffer)
+ continue;
+ for (auto [_, line] : llvm::enumerate(args::getLines(*buffer)))
+ ctx.arg.unresolvedSymbolsList.emplace_back(line);
} else {
ErrAlways(ctx) << "unknown --unresolved-symbols value: " << s;
}
diff --git a/lld/ELF/Relocations.cpp b/lld/ELF/Relocations.cpp
index 8413d8bb2437c..6e87f78caf426 100644
--- a/lld/ELF/Relocations.cpp
+++ b/lld/ELF/Relocations.cpp
@@ -804,6 +804,13 @@ static bool maybeReportUndefined(Ctx &ctx, Undefined &sym,
if (ctx.arg.unresolvedSymbols == UnresolvedPolicy::Ignore && canBeExternal)
return false;
+ // Skip undefined symbols from list
+ for (const auto& ignoredUndef : ctx.arg.unresolvedSymbolsList) {
+ if (ignoredUndef == sym.getName()) {
+ return false;
+ }
+ }
+
// clang (as of 2019-06-12) / gcc (as of 8.2.1) PPC64 may emit a .rela.toc
// which references a switch table in a discarded .rodata/.text section. The
// .toc and the .rela.toc are incorrectly not placed in the comdat. The ELF
diff --git a/lld/docs/ReleaseNotes.rst b/lld/docs/ReleaseNotes.rst
index 5c180fd8fbeeb..26ce07156d181 100644
--- a/lld/docs/ReleaseNotes.rst
+++ b/lld/docs/ReleaseNotes.rst
@@ -58,6 +58,9 @@ ELF Improvements
on executable sections.
(`#128883 <https://github.com/llvm/llvm-project/pull/128883>`_)
+* Added ``--unresolved-symbols=@<file>`` flag to specify path to a file with the
+ list of unresolved symbols that will not trigger an error during lininking.
+
Breaking changes
----------------
* Executable-only and readable-executable sections are now allowed to be placed
diff --git a/lld/test/ELF/Inputs/unresolved-symbols.s b/lld/test/ELF/Inputs/unresolved-symbols.s
index b504708e43dab..86fb468d0b65f 100644
--- a/lld/test/ELF/Inputs/unresolved-symbols.s
+++ b/lld/test/ELF/Inputs/unresolved-symbols.s
@@ -1,3 +1,4 @@
.globl _shared
_shared:
callq undef@PLT
+ callq undef2@PLT
diff --git a/lld/test/ELF/Inputs/unresolved.ignore b/lld/test/ELF/Inputs/unresolved.ignore
new file mode 100644
index 0000000000000..141921671b8f8
--- /dev/null
+++ b/lld/test/ELF/Inputs/unresolved.ignore
@@ -0,0 +1 @@
+undef
diff --git a/lld/test/ELF/unresolved-symbols.s b/lld/test/ELF/unresolved-symbols.s
index 91194d376ca88..fbc7bea1bdcb6 100644
--- a/lld/test/ELF/unresolved-symbols.s
+++ b/lld/test/ELF/unresolved-symbols.s
@@ -63,5 +63,11 @@
# RUN: FileCheck -check-prefix=ERRUND %s
# RUN: not ld.lld %t2.o -o /dev/null 2>&1 | FileCheck -check-prefix=ERRUND %s
+## Ignoring undefines in objects should not produce error for symbol from object.
+# RUN: not ld.lld %t2.o -o /dev/null --unresolved-symbols=@%p/Inputs/unresolved.ignore 2>&1 | \
+# RUN: FileCheck %s --check-prefix=IGNLST
+
+# IGNLST: error: undefined symbol: undef2
+
.globl _start
_start:
|
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
There was a problem hiding this comment.
Please can you add some of the context from the issue into the description, at least prior to any commit so that git log will show the details without needing to follow a URL.
My main concern here is whether adding option is necessary. Or at least desirable enough to justify adding an LLD only extension for an option LLD shares with GNU ld. That will ultimately be MaskRay's call though.
In the issue you mention that --unresolved-symbols=ignore-all is not acceptable as it would mask genuine undefined symbols. I think it would be possible to find these with a separate link step that has a text file containing something like:
--defsym=foo=0
--defsym=bar=0
That second link step could be passed the file @ and use --unresolved-symbols=report-all
That would find any symbols that will be genuinely undefined. It does require a second linker invocation, but it doesn't need any linker changes.
| @@ -58,6 +58,9 @@ ELF Improvements | |||
| on executable sections. | |||
| (`#128883 <https://github.com/llvm/llvm-project/pull/128883>`_) | |||
|
|
|||
| * Added ``--unresolved-symbols=@<file>`` flag to specify path to a file with the | |||
| list of unresolved symbols that will not trigger an error during lininking. | |||
There was a problem hiding this comment.
typo lininking -> linking
You may also want to find a more permanent place to document this. Right now it is not in the help or man-page. LLD is piggybacking on top of GNU ld's docs at https://sourceware.org/binutils/docs/ld/Options.html and that won't mention it.
| @@ -0,0 +1 @@ | |||
| undef | |||
There was a problem hiding this comment.
New lld tests tend to favour using split-file to put all the inputs for a test in the same file.
I'd also recommend having more test cases. For example an empty file, multiple symbols on separate lines.
| @@ -804,6 +804,13 @@ static bool maybeReportUndefined(Ctx &ctx, Undefined &sym, | |||
| if (ctx.arg.unresolvedSymbols == UnresolvedPolicy::Ignore && canBeExternal) | |||
| return false; | |||
|
|
|||
| // Skip undefined symbols from list | |||
| for (const auto &ignoredUndef : ctx.arg.unresolvedSymbolsList) { | |||
| if (ignoredUndef == sym.getName()) { | |||
There was a problem hiding this comment.
omit braces in this case.
ctx.arg.unresolvedSymbolsList can be a DenseSet to improve performance when the list is long.
This should only apply to canBeExternal symbols.
Peter has a comment that you might be able to use --defsym=bar=0 instead.
I'd file a binutils feature request before rushing to add an option
This pull request implements one more feature of the
--unresolved-symbols=@<file>flag in LLD.Original issue: #142798