-
Notifications
You must be signed in to change notification settings - Fork 14.9k
[lld][ELF]Emit warning when both scripts are specified #163497
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
@llvm/pr-subscribers-lld-elf Author: Mingming Liu (mingmingl-llvm) ChangesFor lld ELF, there are two ways of specifying linker script, This patch proposes to emit warning for compiler user information when both are specified. Full diff: https://github.com/llvm/llvm-project/pull/163497.diff 2 Files Affected:
diff --git a/lld/ELF/Driver.cpp b/lld/ELF/Driver.cpp
index 62f7fffce7dbe..c8cce0810ef9a 100644
--- a/lld/ELF/Driver.cpp
+++ b/lld/ELF/Driver.cpp
@@ -2091,6 +2091,7 @@ void LinkerDriver::createFiles(opt::InputArgList &args) {
nextGroupId = 0;
isInGroup = false;
bool hasInput = false, hasScript = false;
+ StringRef scriptFilePath, defaultScriptFilePath;
for (auto *arg : args) {
switch (arg->getOption().getID()) {
case OPT_library:
@@ -2106,7 +2107,11 @@ void LinkerDriver::createFiles(opt::InputArgList &args) {
break;
}
case OPT_script:
+ scriptFilePath = arg->getValue();
+ [[fallthrough]];
case OPT_default_script:
+ if (arg->getOption().matches(OPT_default_script))
+ defaultScriptFilePath = arg->getValue();
if (std::optional<std::string> path =
searchScript(ctx, arg->getValue())) {
if (std::optional<MemoryBufferRef> mb = readFile(ctx, *path)) {
@@ -2201,6 +2206,10 @@ void LinkerDriver::createFiles(opt::InputArgList &args) {
if (defaultScript && !hasScript)
readLinkerScript(ctx, *defaultScript);
+ if (defaultScript && hasScript)
+ Warn(ctx) << "--script at path " << scriptFilePath
+ << " will override --default-script at path "
+ << defaultScriptFilePath;
if (files.empty() && !hasInput && errCount(ctx) == 0)
ErrAlways(ctx) << "no input files";
}
diff --git a/lld/test/ELF/linkerscript/default-script.s b/lld/test/ELF/linkerscript/default-script.s
index bb716a5fe0cdd..a06f96b7165d4 100644
--- a/lld/test/ELF/linkerscript/default-script.s
+++ b/lld/test/ELF/linkerscript/default-script.s
@@ -32,6 +32,17 @@
# CHECK1-NEXT: 3: 000000000000002a 0 NOTYPE GLOBAL DEFAULT ABS def
# CHECK1-EMPTY:
+## When both --script and --default-script are specified, --script overrides
+## --default-script.
+# RUN: ld.lld --default-script def.t --script b.t a.o -o out2 2>warn.txt
+# RUN: llvm-readelf -Ss out2 | FileCheck %s --check-prefix=OVERRIDE
+# OVERRIDE: .foo0
+# OVERRIDE-NEXT: foo1
+# OVERRIDE-NEXT: foo2
+# RUN: cat warn.txt | FileCheck %s --check-prefix=WARNING
+# WARNING: --script at path b.t will override --default-script at path def.t
+
+
# RUN: not ld.lld --default-script not-exist.t b.t -T a.t a.o 2>&1 | FileCheck %s --check-prefix=ERR
# ERR: error: cannot find linker script not-exist.t
@@ -61,3 +72,4 @@ SECTIONS {
.foo1 : {}
.foo0 : {}
}
+
|
@llvm/pr-subscribers-lld Author: Mingming Liu (mingmingl-llvm) ChangesFor lld ELF, there are two ways of specifying linker script, This patch proposes to emit warning for compiler user information when both are specified. Full diff: https://github.com/llvm/llvm-project/pull/163497.diff 2 Files Affected:
diff --git a/lld/ELF/Driver.cpp b/lld/ELF/Driver.cpp
index 62f7fffce7dbe..c8cce0810ef9a 100644
--- a/lld/ELF/Driver.cpp
+++ b/lld/ELF/Driver.cpp
@@ -2091,6 +2091,7 @@ void LinkerDriver::createFiles(opt::InputArgList &args) {
nextGroupId = 0;
isInGroup = false;
bool hasInput = false, hasScript = false;
+ StringRef scriptFilePath, defaultScriptFilePath;
for (auto *arg : args) {
switch (arg->getOption().getID()) {
case OPT_library:
@@ -2106,7 +2107,11 @@ void LinkerDriver::createFiles(opt::InputArgList &args) {
break;
}
case OPT_script:
+ scriptFilePath = arg->getValue();
+ [[fallthrough]];
case OPT_default_script:
+ if (arg->getOption().matches(OPT_default_script))
+ defaultScriptFilePath = arg->getValue();
if (std::optional<std::string> path =
searchScript(ctx, arg->getValue())) {
if (std::optional<MemoryBufferRef> mb = readFile(ctx, *path)) {
@@ -2201,6 +2206,10 @@ void LinkerDriver::createFiles(opt::InputArgList &args) {
if (defaultScript && !hasScript)
readLinkerScript(ctx, *defaultScript);
+ if (defaultScript && hasScript)
+ Warn(ctx) << "--script at path " << scriptFilePath
+ << " will override --default-script at path "
+ << defaultScriptFilePath;
if (files.empty() && !hasInput && errCount(ctx) == 0)
ErrAlways(ctx) << "no input files";
}
diff --git a/lld/test/ELF/linkerscript/default-script.s b/lld/test/ELF/linkerscript/default-script.s
index bb716a5fe0cdd..a06f96b7165d4 100644
--- a/lld/test/ELF/linkerscript/default-script.s
+++ b/lld/test/ELF/linkerscript/default-script.s
@@ -32,6 +32,17 @@
# CHECK1-NEXT: 3: 000000000000002a 0 NOTYPE GLOBAL DEFAULT ABS def
# CHECK1-EMPTY:
+## When both --script and --default-script are specified, --script overrides
+## --default-script.
+# RUN: ld.lld --default-script def.t --script b.t a.o -o out2 2>warn.txt
+# RUN: llvm-readelf -Ss out2 | FileCheck %s --check-prefix=OVERRIDE
+# OVERRIDE: .foo0
+# OVERRIDE-NEXT: foo1
+# OVERRIDE-NEXT: foo2
+# RUN: cat warn.txt | FileCheck %s --check-prefix=WARNING
+# WARNING: --script at path b.t will override --default-script at path def.t
+
+
# RUN: not ld.lld --default-script not-exist.t b.t -T a.t a.o 2>&1 | FileCheck %s --check-prefix=ERR
# ERR: error: cannot find linker script not-exist.t
@@ -61,3 +72,4 @@ SECTIONS {
.foo1 : {}
.foo0 : {}
}
+
|
I believe the intention of My understanding could be wrong, I'll defer to MaskRay if he has other opinions. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree with lenary's analysis. The design intent of --default-script is to allow it to be overridden by linker scripts without generating a warning (or an error if --fatal-warnings is used). A lint could be added to a linker wrapper to report the warning, but this functionality should not be part of LLD itself.
Thanks for sharing the thoughts folks!
Exactly. This matches how
This is not unreasonable. I'll close this patch then. |
One thought is whether we should be documenting |
I'm in favor of the idea to make |
This option is relatively obscure but offers value when necessary. |
For lld ELF, there are two ways of specifying linker script,
-Wl,--script
and-Wl,--default-script
. The latter one is used if the former one is absent, and the former one takes precedence.This patch proposes to emit warning for compiler user awareness when both are specified.
The motivating use case is elaborated below:
--script
and--default-script
specification might be scattered in more than one file, and it's obscure for compiler user to tell which one is used.