[flang-commits] [clang] [flang] [clang][driver] Special care for linker flags in config files (PR #117573)
Paul Osmialowski via flang-commits
flang-commits at lists.llvm.org
Thu Dec 5 05:09:12 PST 2024
https://github.com/pawosm-arm updated https://github.com/llvm/llvm-project/pull/117573
>From 7c7649d9e18bdf18671531065e48bb6ec49e0eaf Mon Sep 17 00:00:00 2001
From: Pawel Osmialowski <pawel.osmialowski at arm.com>
Date: Mon, 25 Nov 2024 14:46:55 +0000
Subject: [PATCH] [clang][driver] Special care for linker flags in config files
Currently, if a -l (or -Wl,) flag is added into a config file
(e.g. clang.cfg), it is situated before any object file in the
effective command line. If the library requested by given -l flag is
static, its symbols will not be made visible to any of the object
files provided by the user. Also, the presence of any of the linker
flags in a config file confuses the driver whenever the user invokes
clang without any parameters (see issue #67209).
This patch attempts to solve both of the problems, by allowing a split
of the arguments list into two parts. The head part of the list will
be used as before, but the tail part will be appended after the
command line flags provided by the user and only when it is known
that the linking should occur. The $-prefixed arguments will be added
to the tail part.
---
clang/docs/UsersManual.rst | 10 +++++
clang/include/clang/Driver/Driver.h | 7 ++-
clang/lib/Driver/Driver.cpp | 63 +++++++++++++++++++++------
clang/test/Driver/Inputs/config-l.cfg | 3 ++
clang/test/Driver/config-file.c | 26 +++++++++++
flang/test/Driver/Inputs/config-l.cfg | 3 ++
flang/test/Driver/config-file.f90 | 26 +++++++++++
7 files changed, 122 insertions(+), 16 deletions(-)
create mode 100644 clang/test/Driver/Inputs/config-l.cfg
create mode 100644 flang/test/Driver/Inputs/config-l.cfg
diff --git a/clang/docs/UsersManual.rst b/clang/docs/UsersManual.rst
index 43b41a2a826890..5b7a293a4ecc27 100644
--- a/clang/docs/UsersManual.rst
+++ b/clang/docs/UsersManual.rst
@@ -1059,6 +1059,16 @@ In this way, the user may only need to specify a root configuration file with
-L <CFGDIR>/lib
-T <CFGDIR>/ldscripts/link.ld
+Usually, config file options are placed before command-line options, regardless
+of the actual operation to be performed. The exception is being made for the
+options prefixed with the ``$`` character. These will be used only when linker
+is being invoked, and added after all of the command-line specified linker
+inputs. Here is some example of ``$``-prefixed options:
+
+::
+ $-Wl,-Bstatic $-lm
+ $-Wl,-Bshared
+
Language and Target-Independent Features
========================================
diff --git a/clang/include/clang/Driver/Driver.h b/clang/include/clang/Driver/Driver.h
index 9177d56718ee77..c23d037e725bb9 100644
--- a/clang/include/clang/Driver/Driver.h
+++ b/clang/include/clang/Driver/Driver.h
@@ -297,8 +297,11 @@ class Driver {
/// Object that stores strings read from configuration file.
llvm::StringSaver Saver;
- /// Arguments originated from configuration file.
- std::unique_ptr<llvm::opt::InputArgList> CfgOptions;
+ /// Arguments originated from configuration file (head part).
+ std::unique_ptr<llvm::opt::InputArgList> CfgOptionsHead;
+
+ /// Arguments originated from configuration file (tail part).
+ std::unique_ptr<llvm::opt::InputArgList> CfgOptionsTail;
/// Arguments originated from command line.
std::unique_ptr<llvm::opt::InputArgList> CLOptions;
diff --git a/clang/lib/Driver/Driver.cpp b/clang/lib/Driver/Driver.cpp
index 7de8341b8d2d61..0c25f16ce3076e 100644
--- a/clang/lib/Driver/Driver.cpp
+++ b/clang/lib/Driver/Driver.cpp
@@ -1039,34 +1039,59 @@ bool Driver::readConfigFile(StringRef FileName,
}
// Try reading the given file.
- SmallVector<const char *, 32> NewCfgArgs;
- if (llvm::Error Err = ExpCtx.readConfigFile(FileName, NewCfgArgs)) {
+ SmallVector<const char *, 32> NewCfgFileArgs;
+ if (llvm::Error Err = ExpCtx.readConfigFile(FileName, NewCfgFileArgs)) {
Diag(diag::err_drv_cannot_read_config_file)
<< FileName << toString(std::move(Err));
return true;
}
+ // Populate head and tail lists. The tail list is used only when linking.
+ SmallVector<const char *, 32> NewCfgHeadArgs, NewCfgTailArgs;
+ for (const char *Opt : NewCfgFileArgs) {
+ // An $-prefixed option should go to the tail list.
+ if (Opt[0] == '$' && Opt[1])
+ NewCfgTailArgs.push_back(Opt + 1);
+ else
+ NewCfgHeadArgs.push_back(Opt);
+ }
+
// Read options from config file.
llvm::SmallString<128> CfgFileName(FileName);
llvm::sys::path::native(CfgFileName);
- bool ContainErrors;
- auto NewOptions = std::make_unique<InputArgList>(
- ParseArgStrings(NewCfgArgs, /*UseDriverMode=*/true, ContainErrors));
+ bool ContainErrors = false;
+ auto NewHeadOptions = std::make_unique<InputArgList>(
+ ParseArgStrings(NewCfgHeadArgs, /*UseDriverMode=*/true, ContainErrors));
+ if (ContainErrors)
+ return true;
+ auto NewTailOptions = std::make_unique<InputArgList>(
+ ParseArgStrings(NewCfgTailArgs, /*UseDriverMode=*/true, ContainErrors));
if (ContainErrors)
return true;
// Claim all arguments that come from a configuration file so that the driver
// does not warn on any that is unused.
- for (Arg *A : *NewOptions)
+ for (Arg *A : *NewHeadOptions)
+ A->claim();
+ for (Arg *A : *NewTailOptions)
A->claim();
- if (!CfgOptions)
- CfgOptions = std::move(NewOptions);
+ if (!CfgOptionsHead)
+ CfgOptionsHead = std::move(NewHeadOptions);
+ else {
+ // If this is a subsequent config file, append options to the previous one.
+ for (auto *Opt : *NewHeadOptions)
+ appendOneArg(*CfgOptionsHead, Opt);
+ }
+
+ if (!CfgOptionsTail)
+ CfgOptionsTail = std::move(NewTailOptions);
else {
// If this is a subsequent config file, append options to the previous one.
- for (auto *Opt : *NewOptions)
- appendOneArg(*CfgOptions, Opt);
+ for (auto *Opt : *NewTailOptions)
+ appendOneArg(*CfgOptionsTail, Opt);
}
+
ConfigFiles.push_back(std::string(CfgFileName));
return false;
}
@@ -1243,13 +1268,14 @@ Compilation *Driver::BuildCompilation(ArrayRef<const char *> ArgList) {
// Try parsing configuration file.
if (!ContainsError)
ContainsError = loadConfigFiles();
- bool HasConfigFile = !ContainsError && (CfgOptions.get() != nullptr);
+ bool HasConfigFileHead = !ContainsError && CfgOptionsHead;
+ bool HasConfigFileTail = !ContainsError && CfgOptionsTail;
// All arguments, from both config file and command line.
- InputArgList Args = std::move(HasConfigFile ? std::move(*CfgOptions)
- : std::move(*CLOptions));
+ InputArgList Args = std::move(HasConfigFileHead ? std::move(*CfgOptionsHead)
+ : std::move(*CLOptions));
- if (HasConfigFile)
+ if (HasConfigFileHead)
for (auto *Opt : *CLOptions)
if (!Opt->getOption().matches(options::OPT_config))
appendOneArg(Args, Opt);
@@ -1540,6 +1566,15 @@ Compilation *Driver::BuildCompilation(ArrayRef<const char *> ArgList) {
// Construct the list of inputs.
InputList Inputs;
BuildInputs(C->getDefaultToolChain(), *TranslatedArgs, Inputs);
+ if (HasConfigFileTail && Inputs.size()) {
+ Arg *FinalPhaseArg;
+ if (getFinalPhase(*TranslatedArgs, &FinalPhaseArg) == phases::Link) {
+ DerivedArgList TranslatedLinkerIns(*CfgOptionsTail);
+ for (Arg *A : *CfgOptionsTail)
+ TranslatedLinkerIns.append(A);
+ BuildInputs(C->getDefaultToolChain(), TranslatedLinkerIns, Inputs);
+ }
+ }
// Populate the tool chains for the offloading devices, if any.
CreateOffloadingDeviceToolChains(*C, Inputs);
diff --git a/clang/test/Driver/Inputs/config-l.cfg b/clang/test/Driver/Inputs/config-l.cfg
new file mode 100644
index 00000000000000..e76efafd3039aa
--- /dev/null
+++ b/clang/test/Driver/Inputs/config-l.cfg
@@ -0,0 +1,3 @@
+-Wall $-lm
+-Wl,--as-needed $-Wl,-Bstatic
+$-lhappy $-Wl,-Bdynamic
diff --git a/clang/test/Driver/config-file.c b/clang/test/Driver/config-file.c
index 9df830ca4c7538..f1ce4c78dbccfc 100644
--- a/clang/test/Driver/config-file.c
+++ b/clang/test/Driver/config-file.c
@@ -82,3 +82,29 @@
// CHECK-TWO-CONFIGS: -isysroot
// CHECK-TWO-CONFIGS-SAME: /opt/data
// CHECK-TWO-CONFIGS-SAME: -Wall
+
+//--- The linker input flags should be moved to the end of input list and appear only when linking.
+// RUN: %clang --target=aarch64-unknown-linux-gnu --config %S/Inputs/config-l.cfg %s -lmylib -Wl,foo.a -### 2>&1 | FileCheck %s -check-prefix CHECK-LINKING
+// RUN: %clang --target=aarch64-unknown-linux-gnu --config %S/Inputs/config-l.cfg -fopenmp %s -lmylib -Wl,foo.a -### 2>&1 | FileCheck %s -check-prefix CHECK-LINKING-LIBOMP-GOES-LAST
+// RUN: %clang --target=aarch64-unknown-linux-gnu --config %S/Inputs/config-l.cfg -S %s -### 2>&1 | FileCheck %s -check-prefix CHECK-NOLINKING
+// RUN: %clang --target=aarch64-unknown-linux-gnu --config %S/Inputs/config-l.cfg -fopenmp -S %s -### 2>&1 | FileCheck %s -check-prefix CHECK-NOLINKING-OPENMP
+// RUN: %clang --target=x86_64-pc-windows-msvc --config %S/Inputs/config-l.cfg %s -lmylib -Wl,foo.lib -### 2>&1 | FileCheck %s -check-prefix CHECK-LINKING-MSVC
+// RUN: %clang --target=x86_64-pc-windows-msvc --config %S/Inputs/config-l.cfg -S %s -### 2>&1 | FileCheck %s -check-prefix CHECK-NOLINKING-MSVC
+// CHECK-LINKING: Configuration file: {{.*}}Inputs{{.}}config-l.cfg
+// CHECK-LINKING: "-Wall"
+// CHECK-LINKING: "--as-needed" "{{.*}}-{{.*}}.o" "-lmylib" "foo.a" "-lm" "-Bstatic" "-lhappy" "-Bdynamic"
+// CHECK-LINKING-LIBOMP-GOES-LAST: Configuration file: {{.*}}Inputs{{.}}config-l.cfg
+// CHECK-LINKING-LIBOMP-GOES-LAST: "-Wall" {{.*}}"-fopenmp"
+// CHECK-LINKING-LIBOMP-GOES-LAST: "--as-needed" "{{.*}}-{{.*}}.o" "-lmylib" "foo.a" "-lm" "-Bstatic" "-lhappy" "-Bdynamic" {{.*}}"-lomp"
+// CHECK-NOLINKING: Configuration file: {{.*}}Inputs{{.}}config-l.cfg
+// CHECK-NOLINKING: "-Wall"
+// CHECK-NOLINKING-NO: "-lm" "-Bstatic" "-lhappy" "-Bdynamic"
+// CHECK-NOLINKING-OPENMP: Configuration file: {{.*}}Inputs{{.}}config-l.cfg
+// CHECK-NOLINKING-OPENMP: "-Wall" {{.*}}"-fopenmp"
+// CHECK-NOLINKING-OPENMP-NO: "-lm" "-Bstatic" "-lhappy" "-Bdynamic" {{.*}}"-lomp"
+// CHECK-LINKING-MSVC: Configuration file: {{.*}}Inputs{{.}}config-l.cfg
+// CHECK-LINKING-MSVC: "-Wall"
+// CHECK-LINKING-MSVC: "--as-needed" "{{.*}}-{{.*}}.o" "mylib.lib" "foo.lib" "m.lib" "-Bstatic" "happy.lib" "-Bdynamic"
+// CHECK-NOLINKING-MSVC: Configuration file: {{.*}}Inputs{{.}}config-l.cfg
+// CHECK-NOLINKING-MSVC: "-Wall"
+// CHECK-NOLINKING-MSVC-NO: "m.lib" "-Bstatic" "happy.lib" "-Bdynamic"
diff --git a/flang/test/Driver/Inputs/config-l.cfg b/flang/test/Driver/Inputs/config-l.cfg
new file mode 100644
index 00000000000000..8cc3abf166adf8
--- /dev/null
+++ b/flang/test/Driver/Inputs/config-l.cfg
@@ -0,0 +1,3 @@
+-ffast-math $-lm
+-Wl,--as-needed $-Wl,-Bstatic
+$-lhappy $-Wl,-Bdynamic
diff --git a/flang/test/Driver/config-file.f90 b/flang/test/Driver/config-file.f90
index 70316dd971f36b..cfabe2c1587e79 100644
--- a/flang/test/Driver/config-file.f90
+++ b/flang/test/Driver/config-file.f90
@@ -61,3 +61,29 @@
! CHECK-TWO-CONFIGS-NEXT: Configuration file: {{.*}}Inputs{{.}}config2{{.}}config-4.cfg
! CHECK-TWO-CONFIGS: -ffp-contract=fast
! CHECK-TWO-CONFIGS: -O3
+
+!--- The linker input flags should be moved to the end of input list and appear only when linking.
+! RUN: %flang --target=aarch64-unknown-linux-gnu --config %S/Inputs/config-l.cfg %s -lmylib -Wl,foo.a -### 2>&1 | FileCheck %s -check-prefix CHECK-LINKING
+! RUN: %flang --target=aarch64-unknown-linux-gnu --config %S/Inputs/config-l.cfg -fopenmp %s -lmylib -Wl,foo.a -### 2>&1 | FileCheck %s -check-prefix CHECK-LINKING-LIBOMP-GOES-LAST
+! RUN: %flang --target=aarch64-unknown-linux-gnu --config %S/Inputs/config-l.cfg -S %s -### 2>&1 | FileCheck %s -check-prefix CHECK-NOLINKING
+! RUN: %flang --target=aarch64-unknown-linux-gnu --config %S/Inputs/config-l.cfg -fopenmp -S %s -### 2>&1 | FileCheck %s -check-prefix CHECK-NOLINKING-OPENMP
+! RUN: %flang --target=x86_64-pc-windows-msvc --config %S/Inputs/config-l.cfg %s -lmylib -Wl,foo.lib -### 2>&1 | FileCheck %s -check-prefix CHECK-LINKING-MSVC
+! RUN: %flang --target=x86_64-pc-windows-msvc --config %S/Inputs/config-l.cfg -S %s -### 2>&1 | FileCheck %s -check-prefix CHECK-NOLINKING-MSVC
+! CHECK-LINKING: Configuration file: {{.*}}Inputs{{.}}config-l.cfg
+! CHECK-LINKING: "-ffast-math"
+! CHECK-LINKING: "--as-needed" "{{.*}}-{{.*}}.o" "-lmylib" "foo.a" "-lm" "-Bstatic" "-lhappy" "-Bdynamic"
+! CHECK-LINKING-LIBOMP-GOES-LAST: Configuration file: {{.*}}Inputs{{.}}config-l.cfg
+! CHECK-LINKING-LIBOMP-GOES-LAST: "-ffast-math" {{.*}}"-fopenmp"
+! CHECK-LINKING-LIBOMP-GOES-LAST: "--as-needed" "{{.*}}-{{.*}}.o" "-lmylib" "foo.a" "-lm" "-Bstatic" "-lhappy" "-Bdynamic" {{.*}}"-lomp"
+! CHECK-NOLINKING: Configuration file: {{.*}}Inputs{{.}}config-l.cfg
+! CHECK-NOLINKING: "-ffast-math"
+! CHECK-NOLINKING-NO: "-lm" "-Bstatic" "-lhappy" "-Bdynamic"
+! CHECK-NOLINKING-OPENMP: Configuration file: {{.*}}Inputs{{.}}config-l.cfg
+! CHECK-NOLINKING-OPENMP: "-ffast-math" {{.*}}"-fopenmp"
+! CHECK-NOLINKING-OPENMP-NO: "-lm" "-Bstatic" "-lhappy" "-Bdynamic" {{.}}"-lomp"
+! CHECK-LINKING-MSVC: Configuration file: {{.*}}Inputs{{.}}config-l.cfg
+! CHECK-LINKING-MSVC: "-ffast-math"
+! CHECK-LINKING-MSVC: "--as-needed" "{{.*}}-{{.*}}.o" "mylib.lib" "foo.lib" "m.lib" "-Bstatic" "happy.lib" "-Bdynamic"
+! CHECK-NOLINKING-MSVC: Configuration file: {{.*}}Inputs{{.}}config-l.cfg
+! CHECK-NOLINKING-MSVC: "-ffast-math"
+! CHECK-NOLINKING-MSVC-NO: "m.lib" "-Bstatic" "happy.lib" "-Bdynamic"
More information about the flang-commits
mailing list