[Driver] Move LIBRARY_PATH before user inputs

[MaskRay]

Summary

Fixes PR16786

Currently, library paths specified by LIBRARY_PATH are placed after inputs: inputs LIBRARY_PATH stdlib
In gcc, the order is: LIBRARY_PATH inputs stdlib if not cross compiling.
(On Darwin targets, isCrossCompiling() always returns false.)

This patch changes the behavior to match gcc.

[MaskRay]

Fix a comment

[hfinkel]

LGTM.

Local flags should certainly override LIBRARY_PATH.

[MaskRay]

@hfinkel Thanks!

[protze.joachim]

I still see some clang-specific and system link directories listed in the linker line before the directories from LIBRARY_PATH:

$ LIBRARY_PATH=test1 /usr/local/clang/bin/clang -Ltest2 -v hello.c
 "/usr/bin/ld" .../crtbegin.o -Ltest2 -L/usr/lib/gcc/x86_64-redhat-linux/4.8.5 -L/usr/lib/gcc/x86_64-redhat-linux/4.8.5/../../../../lib64 -L/lib/../lib64 -L/usr/lib/../lib64 -L/usr/lib/gcc/x86_64-redhat-linux/4.8.5/../../.. -L/usr/local/clang/bin/../lib -L/lib -L/usr/lib -Ltest1

I think they are inserted by ToolChain.AddFilePathLibArgs in Gnu.cpp. Is this the intended ordering? My expectation would be

$ LIBRARY_PATH=test1 /usr/local/clang/bin/clang -Ltest2 -v hello.c
 "/usr/bin/ld" .../crtbegin.o -Ltest2 -Ltest1 -L/usr/lib/gcc/x86_64-redhat-linux/4.8.5 -L/usr/lib/gcc/x86_64-redhat-linux/4.8.5/../../../../lib64 -L/lib/../lib64 -L/usr/lib/../lib64 -L/usr/lib/gcc/x86_64-redhat-linux/4.8.5/../../.. -L/usr/local/clang/bin/../lib -L/lib -L/usr/lib

@hfinkel any opinion?

[MaskRay]

In D65880#2294871, @protze.joachim wrote:

I still see some clang-specific and system link directories listed in the linker line before the directories from LIBRARY_PATH:

$ LIBRARY_PATH=test1 /usr/local/clang/bin/clang -Ltest2 -v hello.c
 "/usr/bin/ld" .../crtbegin.o -Ltest2 -L/usr/lib/gcc/x86_64-redhat-linux/4.8.5 -L/usr/lib/gcc/x86_64-redhat-linux/4.8.5/../../../../lib64 -L/lib/../lib64 -L/usr/lib/../lib64 -L/usr/lib/gcc/x86_64-redhat-linux/4.8.5/../../.. -L/usr/local/clang/bin/../lib -L/lib -L/usr/lib -Ltest1

I think they are inserted by ToolChain.AddFilePathLibArgs in Gnu.cpp. Is this the intended ordering? My expectation would be

$ LIBRARY_PATH=test1 /usr/local/clang/bin/clang -Ltest2 -v hello.c
 "/usr/bin/ld" .../crtbegin.o -Ltest2 -Ltest1 -L/usr/lib/gcc/x86_64-redhat-linux/4.8.5 -L/usr/lib/gcc/x86_64-redhat-linux/4.8.5/../../../../lib64 -L/lib/../lib64 -L/usr/lib/../lib64 -L/usr/lib/gcc/x86_64-redhat-linux/4.8.5/../../.. -L/usr/local/clang/bin/../lib -L/lib -L/usr/lib

@hfinkel any opinion?

In GCC, some system directories precede LIBRARY_PATH. I don't know the exact rule yet (but this patch made the behavior more similar)

% mkdir test1
% LIBRARY_PATH=test1 gcc-10 -Ltest2 -v a.c
...
LIBRARY_PATH=/usr/lib/gcc/x86_64-linux-gnu/10/:/usr/lib/gcc/x86_64-linux-gnu/10/../../../x86_64-linux-gnu/:/usr/lib/gcc/x86_64-linux-gnu/10/../../../../lib/:/l
ib/x86_64-linux-gnu/:/lib/../lib/:/usr/lib/x86_64-linux-gnu/:/usr/lib/../lib/:test1/:/usr/lib/gcc/x86_64-linux-gnu/10/../../../:/lib/:/usr/lib/

lib/Driver/ToolChains/CommonArgs.cpp

void tools::AddLinkerInputs(const ToolChain &TC, const InputInfoList &Inputs,
                             const ArgList &Args, ArgStringList &CmdArgs,
                             const JobAction &JA) {
   const Driver &D = TC.getDriver();
 
   // Add extra linker input arguments which are not treated as inputs
   // (constructed via -Xarch_).
   Args.AddAllArgValues(CmdArgs, options::OPT_Zlinker_input);
 
+  // LIBRARY_PATH are included before user inputs and only supported on native
+  // toolchains.
+  if (!TC.isCrossCompiling())
+    addDirectoryList(Args, CmdArgs, "-L", "LIBRARY_PATH");
+
   for (const auto &II : Inputs) {
     // If the current tool chain refers to an OpenMP or HIP offloading host, we
     // should ignore inputs that refer to OpenMP or HIP offloading devices -
     // they will be embedded according to a proper linker script.
     if (auto *IA = II.getAction())
       if ((JA.isHostOffloading(Action::OFK_OpenMP) &&
            IA->isDeviceOffloading(Action::OFK_OpenMP)) ||
           (JA.isHostOffloading(Action::OFK_HIP) &&
for (const auto &II : Inputs) {
     else if (A.getOption().matches(options::OPT_z)) {
       // Pass -z prefix for gcc linker compatibility.
       A.claim();
       A.render(Args, CmdArgs);
     } else {
       A.renderAsInput(Args, CmdArgs);
     }
   }
-
-  // LIBRARY_PATH - included following the user specified library paths.
-  //                and only supported on native toolchains.
-  if (!TC.isCrossCompiling()) {
-    addDirectoryList(Args, CmdArgs, "-L", "LIBRARY_PATH");
-  }
 }
 
 void tools::AddTargetFeature(const ArgList &Args,
                              std::vector<StringRef> &Features,
                              OptSpecifier OnOpt, OptSpecifier OffOpt,
                              StringRef FeatureName) {
   if (Arg *A = Args.getLastArg(OnOpt, OffOpt)) {
     if (A->getOption().matches(OnOpt))

test/Driver/linker-opts.c

 // RUN: rm -rf %t
 // RUN: mkdir %t
 //
-// RUN: env LIBRARY_PATH=%t/test1 %clang -x c %s -### 2>&1 | FileCheck %s
+// RUN: env LIBRARY_PATH=%t/test1 %clang -target %itanium_abi_triple %s -la -### 2>&1 | FileCheck %s
 // CHECK: "-L{{.*}}/test1"
+// CHECK: "{{[^"]+}}.o"
+// CHECK: "-la"
 
 // GCC driver is used as linker on cygming. It should be aware of LIBRARY_PATH.
 // XFAIL: windows-msvc
 // REQUIRES: clang-driver
 // REQUIRES: native
 
 // Make sure that LIBRARY_PATH works for both i386 and x86_64 on Darwin.
-// RUN: env LIBRARY_PATH=%t/test1 %clang -target x86_64-apple-darwin %s -### 2>&1 | FileCheck %s
-// RUN: env LIBRARY_PATH=%t/test1 %clang -target i386-apple-darwin  %s -### 2>&1 | FileCheck %s
+// RUN: env LIBRARY_PATH=%t/test1 %clang -target x86_64-apple-darwin %s -la -### 2>&1 | FileCheck %s
+// RUN: env LIBRARY_PATH=%t/test1 %clang -target i386-apple-darwin %s -la -### 2>&1 | FileCheck %s
 //
 // Make sure that we don't warn on unused compiler arguments.
 // RUN: %clang -Xclang -I. -x c %s -c -o %t/tmp.o
 // RUN: %clang -Xclang -I. %t/tmp.o -o %t/tmp -### 2>&1 | FileCheck %s --check-prefix=NO-UNUSED
 // NO-UNUSED-NOT: warning:{{.*}}unused
 //
 // Make sure that we do warn in other cases.
 // RUN: %clang %s -lfoo -c -o %t/tmp2.o -### 2>&1 | FileCheck %s --check-prefix=UNUSED
 // UNUSED: warning:{{.*}}unused