[lld] [lld][ELF] Error when deplibs adds new input file after LTO (PR #98565)
Daniel Thornburgh via llvm-commits
llvm-commits at lists.llvm.org
Fri Jul 12 12:02:50 PDT 2024
https://github.com/mysterymath updated https://github.com/llvm/llvm-project/pull/98565
>From e4b8c1029b0893763e1279d217ee41aafd9b50bc Mon Sep 17 00:00:00 2001
From: Daniel Thornburgh <dthorn at google.com>
Date: Tue, 25 Jun 2024 16:41:42 -0700
Subject: [PATCH 1/3] [lld][ELF] Error when deplibs adds new input file after
LTO
Parsing the new input file's symbols might invalidate LTO codegen, but
the semantics of deplibs require them to be parsed. Accordingly, report
an error unless the file had already been added to the link.
Fixes #56070
---
lld/ELF/Driver.cpp | 15 +++++++++++++++
lld/test/ELF/deplibs-lto.s | 39 ++++++++++++++++++++++++++++++++++++++
2 files changed, 54 insertions(+)
create mode 100644 lld/test/ELF/deplibs-lto.s
diff --git a/lld/ELF/Driver.cpp b/lld/ELF/Driver.cpp
index ed773f5e69f77..18029cb71f074 100644
--- a/lld/ELF/Driver.cpp
+++ b/lld/ELF/Driver.cpp
@@ -2956,6 +2956,7 @@ template <class ELFT> void LinkerDriver::link(opt::InputArgList &args) {
// With this the symbol table should be complete. After this, no new names
// except a few linker-synthesized ones will be added to the symbol table.
const size_t numObjsBeforeLTO = ctx.objectFiles.size();
+ const size_t numInputFilesBeforeLTO = ctx.driver.files.size();
compileBitcodeFiles<ELFT>(skipLinkedOutput);
// Symbol resolution finished. Report backward reference problems,
@@ -2980,6 +2981,20 @@ template <class ELFT> void LinkerDriver::link(opt::InputArgList &args) {
for (const DuplicateSymbol &d : ctx.duplicates)
reportDuplicate(*d.sym, d.file, d.section, d.value);
+ // ELF dependent libraries may have introduced new input files after LTO has
+ // completed. This is an error if the file hadn't already been parsed, since
+ // it's no longer legal to change the symbol table by parsing it.
+ auto newInputFiles = ArrayRef(ctx.driver.files).slice(numInputFilesBeforeLTO);
+ if (!newInputFiles.empty()) {
+ DenseSet<StringRef> oldFilenames;
+ for (InputFile *f :
+ ArrayRef(ctx.driver.files).slice(0, numInputFilesBeforeLTO))
+ oldFilenames.insert(f->getName());
+ for (InputFile *newFile : newInputFiles)
+ if (!oldFilenames.contains(newFile->getName()))
+ error("input file '" + newFile->getName() + "' added after LTO");
+ }
+
// Handle --exclude-libs again because lto.tmp may reference additional
// libcalls symbols defined in an excluded archive. This may override
// versionId set by scanVersionScript().
diff --git a/lld/test/ELF/deplibs-lto.s b/lld/test/ELF/deplibs-lto.s
new file mode 100644
index 0000000000000..5c909197b445f
--- /dev/null
+++ b/lld/test/ELF/deplibs-lto.s
@@ -0,0 +1,39 @@
+# REQUIRES: x86
+
+# RUN: rm -rf %t && split-file %s %t && cd %t
+# RUN: llvm-mc -filetype=obj -triple=aarch64 -o deplibs.o deplibs.s
+# RUN: llvm-mc -filetype=obj -triple=aarch64 -o foo.o foo.s
+# RUN: llvm-as -o lto.o lto.ll
+# RUN: llvm-ar rc libdeplibs.a deplibs.o
+# RUN: llvm-ar rc libfoo.a foo.o
+
+## LTO emits a libcall (`__aarch64_ldadd4_relax`) that is resolved using a
+## library (libdeplibc.a) that contains a `.deplibs` section pointing to a file
+## not yet added to the link.
+
+# RUN: not ld.lld lto.o -u a -L. -ldeplibs 2>&1 | FileCheck %s
+# CHECK: error: input file 'foo.o' added after LTO
+
+## Including the file before LTO prevents the issue.
+
+# RUN: ld.lld lto.o -u a -L. -ldeplibs -lfoo
+
+#--- foo.s
+.global foo
+foo:
+#--- deplibs.s
+.global __aarch64_ldadd4_relax
+__aarch64_ldadd4_relax:
+ b foo
+.section ".deplibs","MS", at llvm_dependent_libraries,1
+ .asciz "foo"
+#--- lto.ll
+target datalayout = "e-m:e-i8:8:32-i16:16:32-i64:64-i128:128-n32:64-S128"
+target triple = "aarch64"
+
+define void @a(i32* nocapture %0) #0 {
+ %2 = atomicrmw add i32* %0, i32 1 monotonic, align 4
+ ret void
+}
+
+attributes #0 = { "target-features"="+outline-atomics" }
>From c18ccd8948ee678c1c1f017f66a755cb370b975e Mon Sep 17 00:00:00 2001
From: Daniel Thornburgh <dthorn at google.com>
Date: Thu, 11 Jul 2024 16:13:54 -0700
Subject: [PATCH 2/3] Quick update: - Consistent plurality in comment -
errorOrWarn, since an output file may still producible
---
lld/ELF/Driver.cpp | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/lld/ELF/Driver.cpp b/lld/ELF/Driver.cpp
index 18029cb71f074..89e0434b21e37 100644
--- a/lld/ELF/Driver.cpp
+++ b/lld/ELF/Driver.cpp
@@ -2982,8 +2982,8 @@ template <class ELFT> void LinkerDriver::link(opt::InputArgList &args) {
reportDuplicate(*d.sym, d.file, d.section, d.value);
// ELF dependent libraries may have introduced new input files after LTO has
- // completed. This is an error if the file hadn't already been parsed, since
- // it's no longer legal to change the symbol table by parsing it.
+ // completed. This is an error if the files haven't already been parsed, since
+ // it's no longer legal to change the symbol table by parsing them.
auto newInputFiles = ArrayRef(ctx.driver.files).slice(numInputFilesBeforeLTO);
if (!newInputFiles.empty()) {
DenseSet<StringRef> oldFilenames;
@@ -2992,7 +2992,7 @@ template <class ELFT> void LinkerDriver::link(opt::InputArgList &args) {
oldFilenames.insert(f->getName());
for (InputFile *newFile : newInputFiles)
if (!oldFilenames.contains(newFile->getName()))
- error("input file '" + newFile->getName() + "' added after LTO");
+ errorOrWarn("input file '" + newFile->getName() + "' added after LTO");
}
// Handle --exclude-libs again because lto.tmp may reference additional
>From 86b1c20a3dddd49365a91f64efe7f7e1bfee18c1 Mon Sep 17 00:00:00 2001
From: Daniel Thornburgh <dthorn at google.com>
Date: Fri, 12 Jul 2024 12:00:40 -0700
Subject: [PATCH 3/3] Address review feedback:
- Require aarch64
- Fix typo
- Remove blank lines in test
- Include reason for illegality in comment
Misc:
- Remove backticks around file and section names
---
lld/ELF/Driver.cpp | 2 +-
lld/test/ELF/deplibs-lto.s | 10 ++++------
2 files changed, 5 insertions(+), 7 deletions(-)
diff --git a/lld/ELF/Driver.cpp b/lld/ELF/Driver.cpp
index 89e0434b21e37..8d47fde651f80 100644
--- a/lld/ELF/Driver.cpp
+++ b/lld/ELF/Driver.cpp
@@ -2983,7 +2983,7 @@ template <class ELFT> void LinkerDriver::link(opt::InputArgList &args) {
// ELF dependent libraries may have introduced new input files after LTO has
// completed. This is an error if the files haven't already been parsed, since
- // it's no longer legal to change the symbol table by parsing them.
+ // changing the symbol table could break the semantic assumptions of LTO.
auto newInputFiles = ArrayRef(ctx.driver.files).slice(numInputFilesBeforeLTO);
if (!newInputFiles.empty()) {
DenseSet<StringRef> oldFilenames;
diff --git a/lld/test/ELF/deplibs-lto.s b/lld/test/ELF/deplibs-lto.s
index 5c909197b445f..ea8be56b1515a 100644
--- a/lld/test/ELF/deplibs-lto.s
+++ b/lld/test/ELF/deplibs-lto.s
@@ -1,4 +1,4 @@
-# REQUIRES: x86
+# REQUIRES: aarch64
# RUN: rm -rf %t && split-file %s %t && cd %t
# RUN: llvm-mc -filetype=obj -triple=aarch64 -o deplibs.o deplibs.s
@@ -7,15 +7,13 @@
# RUN: llvm-ar rc libdeplibs.a deplibs.o
# RUN: llvm-ar rc libfoo.a foo.o
-## LTO emits a libcall (`__aarch64_ldadd4_relax`) that is resolved using a
-## library (libdeplibc.a) that contains a `.deplibs` section pointing to a file
-## not yet added to the link.
-
+## LTO emits a libcall (__aarch64_ldadd4_relax) that is resolved using a
+## library (libdeplibs.a) that contains a .deplibs section pointing to a file
+## (libfoo.a) not yet added to the link.
# RUN: not ld.lld lto.o -u a -L. -ldeplibs 2>&1 | FileCheck %s
# CHECK: error: input file 'foo.o' added after LTO
## Including the file before LTO prevents the issue.
-
# RUN: ld.lld lto.o -u a -L. -ldeplibs -lfoo
#--- foo.s
More information about the llvm-commits
mailing list