[clang] [flang] [clang][driver] Special care for -l flags in config files (PR #117573)

via cfe-commits cfe-commits at lists.llvm.org
Mon Nov 25 08:21:02 PST 2024


llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-clang-driver

Author: Paul Osmialowski (pawosm-arm)

<details>
<summary>Changes</summary>

Currently, if a -l 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 -l flags in a config file confuses the driver whenever the user invokes clang without any parameters.

This patch solves both of those problems, by moving the -lm flags specified in a config file to the end of the input list, and by discarding them when there are no other inputs provided by the user, or the last phase is not the linking phase.

---
Full diff: https://github.com/llvm/llvm-project/pull/117573.diff


8 Files Affected:

- (modified) clang/include/clang/Driver/Driver.h (+3) 
- (modified) clang/lib/Driver/Driver.cpp (+32) 
- (added) clang/test/Driver/Inputs/config-l-openmp.cfg (+1) 
- (added) clang/test/Driver/Inputs/config-l.cfg (+1) 
- (modified) clang/test/Driver/config-file.c (+19) 
- (added) flang/test/Driver/Inputs/config-l-openmp.cfg (+1) 
- (added) flang/test/Driver/Inputs/config-l.cfg (+1) 
- (modified) flang/test/Driver/config-file.f90 (+19) 


``````````diff
diff --git a/clang/include/clang/Driver/Driver.h b/clang/include/clang/Driver/Driver.h
index 9177d56718ee77..b21606b6f54b77 100644
--- a/clang/include/clang/Driver/Driver.h
+++ b/clang/include/clang/Driver/Driver.h
@@ -297,6 +297,9 @@ class Driver {
   /// Object that stores strings read from configuration file.
   llvm::StringSaver Saver;
 
+  /// Linker inputs originated from configuration file.
+  std::unique_ptr<llvm::opt::InputArgList> CfgLinkerInputs;
+
   /// Arguments originated from configuration file.
   std::unique_ptr<llvm::opt::InputArgList> CfgOptions;
 
diff --git a/clang/lib/Driver/Driver.cpp b/clang/lib/Driver/Driver.cpp
index ad14b5c9b6dc80..bfd440461ea601 100644
--- a/clang/lib/Driver/Driver.cpp
+++ b/clang/lib/Driver/Driver.cpp
@@ -1062,6 +1062,15 @@ bool Driver::readConfigFile(StringRef FileName,
   for (Arg *A : *NewOptions)
     A->claim();
 
+  std::unique_ptr<InputArgList> NewLinkerIns = std::make_unique<InputArgList>();
+  for (Arg *A : NewOptions->filtered(options::OPT_l)) {
+    const Arg *BaseArg = &A->getBaseArg();
+    if (BaseArg == A)
+      BaseArg = nullptr;
+    appendOneArg(*NewLinkerIns, A, BaseArg);
+  }
+  NewOptions->eraseArg(options::OPT_l);
+
   if (!CfgOptions)
     CfgOptions = std::move(NewOptions);
   else {
@@ -1073,6 +1082,19 @@ bool Driver::readConfigFile(StringRef FileName,
       appendOneArg(*CfgOptions, Opt, BaseArg);
     }
   }
+
+  if (!CfgLinkerInputs)
+    CfgLinkerInputs = std::move(NewLinkerIns);
+  else {
+    // If this is a subsequent config file, append options to the previous one.
+    for (auto *Opt : *NewLinkerIns) {
+      const Arg *BaseArg = &Opt->getBaseArg();
+      if (BaseArg == Opt)
+        BaseArg = nullptr;
+      appendOneArg(*CfgLinkerInputs, Opt, BaseArg);
+    }
+  }
+
   ConfigFiles.push_back(std::string(CfgFileName));
   return false;
 }
@@ -1250,6 +1272,7 @@ Compilation *Driver::BuildCompilation(ArrayRef<const char *> ArgList) {
   if (!ContainsError)
     ContainsError = loadConfigFiles();
   bool HasConfigFile = !ContainsError && (CfgOptions.get() != nullptr);
+  bool HasConfigLinkerIn = !ContainsError && (CfgLinkerInputs.get() != nullptr);
 
   // All arguments, from both config file and command line.
   InputArgList Args = std::move(HasConfigFile ? std::move(*CfgOptions)
@@ -1552,6 +1575,15 @@ Compilation *Driver::BuildCompilation(ArrayRef<const char *> ArgList) {
   // Construct the list of inputs.
   InputList Inputs;
   BuildInputs(C->getDefaultToolChain(), *TranslatedArgs, Inputs);
+  if (HasConfigLinkerIn && Inputs.size()) {
+    Arg *FinalPhaseArg;
+    if (getFinalPhase(*TranslatedArgs, &FinalPhaseArg) == phases::Link) {
+      DerivedArgList TranslatedLinkerIns(*CfgLinkerInputs);
+      for (Arg *A : *CfgLinkerInputs)
+        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-openmp.cfg b/clang/test/Driver/Inputs/config-l-openmp.cfg
new file mode 100644
index 00000000000000..91f8884b6e9e23
--- /dev/null
+++ b/clang/test/Driver/Inputs/config-l-openmp.cfg
@@ -0,0 +1 @@
+-Wall -fopenmp -lm -lhappy
diff --git a/clang/test/Driver/Inputs/config-l.cfg b/clang/test/Driver/Inputs/config-l.cfg
new file mode 100644
index 00000000000000..0ebc28baddbe2c
--- /dev/null
+++ b/clang/test/Driver/Inputs/config-l.cfg
@@ -0,0 +1 @@
+-Wall -lm -lhappy
diff --git a/clang/test/Driver/config-file.c b/clang/test/Driver/config-file.c
index 9df830ca4c7538..799e545e1e518b 100644
--- a/clang/test/Driver/config-file.c
+++ b/clang/test/Driver/config-file.c
@@ -82,3 +82,22 @@
 // CHECK-TWO-CONFIGS: -isysroot
 // CHECK-TWO-CONFIGS-SAME: /opt/data
 // CHECK-TWO-CONFIGS-SAME: -Wall
+
+//--- The -l flags should be moved to the end of input list and appear only when linking.
+// RUN: %clang --config %S/Inputs/config-l.cfg -o %s.out %s -### 2>&1 | FileCheck %s -check-prefix CHECK-LINKING
+// RUN: %clang --config %S/Inputs/config-l-openmp.cfg -o %s.out %s -### 2>&1 | FileCheck %s -check-prefix CHECK-LINKING-LIBOMP-GOES-LAST
+// RUN: %clang --config %S/Inputs/config-l.cfg -S %s -### 2>&1 | FileCheck %s -check-prefix CHECK-NOLINKING
+// RUN: %clang --config %S/Inputs/config-l-openmp.cfg -S %s -### 2>&1 | FileCheck %s -check-prefix CHECK-NOLINKING
+// CHECK-LINKING: Configuration file: {{.*}}Inputs{{.}}config-l.cfg
+// CHECK-LINKING: "-Wall"
+// CHECK-LINKING: "/tmp/{{.*}}-{{.*}}.o" "-lm" "-lhappy"
+// CHECK-LINKING-LIBOMP-GOES-LAST: Configuration file: {{.*}}Inputs{{.}}config-l-openmp.cfg
+// CHECK-LINKING-LIBOMP-GOES-LAST: "-Wall"
+// CHECK-LINKING-LIBOMP-GOES-LAST-SAME: "-fopenmp"
+// CHECK-LINKING-LIBOMP-GOES-LAST: "/tmp/{{.*}}-{{.*}}.o" "-lm" "-lhappy"
+// CHECK-LINKING-LIBOMP-GOES-LAST-SAME: "-lomp"
+// CHECK-NOLINKING: Configuration file: {{.*}}Inputs{{.}}config-l{{.*}}.cfg
+// CHECK-NOLINKING: "-Wall"
+// CHECK-NOLINKING-NO: "-lm"
+// CHECK-NOLINKING-NO: "-lhappy"
+// CHECK-NOLINKING-NO: "-lomp"
diff --git a/flang/test/Driver/Inputs/config-l-openmp.cfg b/flang/test/Driver/Inputs/config-l-openmp.cfg
new file mode 100644
index 00000000000000..46ab3273f84f19
--- /dev/null
+++ b/flang/test/Driver/Inputs/config-l-openmp.cfg
@@ -0,0 +1 @@
+-ffast-math -fopenmp -lm -lhappy
diff --git a/flang/test/Driver/Inputs/config-l.cfg b/flang/test/Driver/Inputs/config-l.cfg
new file mode 100644
index 00000000000000..af2d0a99475a2a
--- /dev/null
+++ b/flang/test/Driver/Inputs/config-l.cfg
@@ -0,0 +1 @@
+-ffast-math -lm -lhappy
diff --git a/flang/test/Driver/config-file.f90 b/flang/test/Driver/config-file.f90
index 70316dd971f36b..15b18f4420e6b3 100644
--- a/flang/test/Driver/config-file.f90
+++ b/flang/test/Driver/config-file.f90
@@ -61,3 +61,22 @@
 ! CHECK-TWO-CONFIGS-NEXT: Configuration file: {{.*}}Inputs{{.}}config2{{.}}config-4.cfg
 ! CHECK-TWO-CONFIGS: -ffp-contract=fast
 ! CHECK-TWO-CONFIGS: -O3
+
+!--- The -l flags should be moved to the end of input list and appear only when linking.
+! RUN: %flang --config %S/Inputs/config-l.cfg -o %s.out %s -### 2>&1 | FileCheck %s -check-prefix CHECK-LINKING
+! RUN: %flang --config %S/Inputs/config-l-openmp.cfg -o %s.out %s -### 2>&1 | FileCheck %s -check-prefix CHECK-LINKING-LIBOMP-GOES-LAST
+! RUN: %flang --config %S/Inputs/config-l.cfg -S %s -### 2>&1 | FileCheck %s -check-prefix CHECK-NOLINKING
+! RUN: %flang --config %S/Inputs/config-l-openmp.cfg -S %s -### 2>&1 | FileCheck %s -check-prefix CHECK-NOLINKING
+! CHECK-LINKING: Configuration file: {{.*}}Inputs{{.}}config-l.cfg
+! CHECK-LINKING: "-ffast-math"
+! CHECK-LINKING: "/tmp/{{.*}}-{{.*}}.o" "-lm" "-lhappy"
+! CHECK-LINKING-LIBOMP-GOES-LAST: Configuration file: {{.*}}Inputs{{.}}config-l-openmp.cfg
+! CHECK-LINKING-LIBOMP-GOES-LAST: "-ffast-math"
+! CHECK-LINKING-LIBOMP-GOES-LAST-SAME: "-fopenmp"
+! CHECK-LINKING-LIBOMP-GOES-LAST: "/tmp/{{.*}}-{{.*}}.o" "-lm" "-lhappy"
+! CHECK-LINKING-LIBOMP-GOES-LAST-SAME: "-lomp"
+! CHECK-NOLINKING: Configuration file: {{.*}}Inputs{{.}}config-l{{.*}}.cfg
+! CHECK-NOLINKING: "-ffast-math"
+! CHECK-NOLINKING-NO: "-lm"
+! CHECK-NOLINKING-NO: "-lhappy"
+! CHECK-NOLINKING-NO: "-lomp"

``````````

</details>


https://github.com/llvm/llvm-project/pull/117573


More information about the cfe-commits mailing list