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

Paul Osmialowski via flang-commits flang-commits at lists.llvm.org
Sun Dec 1 07:59:26 PST 2024


https://github.com/pawosm-arm updated https://github.com/llvm/llvm-project/pull/117573

>From b9dd5f9356d457ef1ad050083d650a1da21c4377 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 -l and -Wl, 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.

This patch attempts to solve both of the problems, by allowing a split
of the arguments list around the pipe character. 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.
---
 clang/include/clang/Driver/Driver.h   |  3 ++
 clang/lib/Driver/Driver.cpp           | 55 +++++++++++++++++++++++----
 clang/test/Driver/Inputs/config-l.cfg |  1 +
 clang/test/Driver/config-file.c       | 26 +++++++++++++
 flang/test/Driver/Inputs/config-l.cfg |  1 +
 flang/test/Driver/config-file.f90     | 26 +++++++++++++
 6 files changed, 104 insertions(+), 8 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/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 7de8341b8d2d61..9ea22419d392b0 100644
--- a/clang/lib/Driver/Driver.cpp
+++ b/clang/lib/Driver/Driver.cpp
@@ -1039,34 +1039,63 @@ 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;
+  bool SeenPipe = false;
+  for (const char *Opt : NewCfgFileArgs) {
+    if (!strcmp(Opt, "|")) {
+      SeenPipe = true;
+    } else {
+      if (SeenPipe)
+        NewCfgTailArgs.push_back(Opt);
+      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);
+    CfgOptions = std::move(NewHeadOptions);
   else {
     // If this is a subsequent config file, append options to the previous one.
-    for (auto *Opt : *NewOptions)
+    for (auto *Opt : *NewHeadOptions)
       appendOneArg(*CfgOptions, Opt);
   }
+
+  if (!CfgLinkerInputs)
+    CfgLinkerInputs = std::move(NewTailOptions);
+  else {
+    // If this is a subsequent config file, append options to the previous one.
+    for (auto *Opt : *NewTailOptions)
+      appendOneArg(*CfgLinkerInputs, Opt);
+  }
+
   ConfigFiles.push_back(std::string(CfgFileName));
   return false;
 }
@@ -1244,6 +1273,7 @@ Compilation *Driver::BuildCompilation(ArrayRef<const char *> ArgList) {
   if (!ContainsError)
     ContainsError = loadConfigFiles();
   bool HasConfigFile = !ContainsError && (CfgOptions.get() != nullptr);
+  bool HasConfigLinkerIn = !ContainsError && CfgLinkerInputs;
 
   // All arguments, from both config file and command line.
   InputArgList Args = std::move(HasConfigFile ? std::move(*CfgOptions)
@@ -1540,6 +1570,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.cfg b/clang/test/Driver/Inputs/config-l.cfg
new file mode 100644
index 00000000000000..af94ca30c187a0
--- /dev/null
+++ b/clang/test/Driver/Inputs/config-l.cfg
@@ -0,0 +1 @@
+-Wall -Wl,--as-needed | -lm -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..f0ac049f97229d
--- /dev/null
+++ b/flang/test/Driver/Inputs/config-l.cfg
@@ -0,0 +1 @@
+-ffast-math -Wl,--as-needed | -lm -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