[ELF] - Implemented rpath-link command line option.

[grimar]

Summary

rpath-link option was ignored before.
This is PR30360, patch fixes that.
Am I missing something ? That looks too much trivial, have no idea why we ignored it before ?

[vangyzen]

This would probably work in practice, but the ld.bfd implementation is a bit more nuanced. Specifically, ld.bfd searches the -L path for libraries named on the command line, and it searches the rpath-link path for libraries required by those libraries. That is, it does not search the -L path for libraries needed by libraries. In my humble opinion, this doesn't make much sense and is slightly annoying for the end user. However, if lld wants to be command-line-compatible with ld.bfd, which I think would be wise, it should probably follow suit.

The ld.bfd documentation has a fairly detailed description:

https://sourceware.org/binutils/docs-2.27/ld/Options.html#index-link_002dtime-runtime-library-search-path-232

[vangyzen]

For convenience: https://llvm.org/bugs/show_bug.cgi?id=30360

[grimar]

In D24496#541325, @vangyzen wrote:

This would probably work in practice, but the ld.bfd implementation is a bit more nuanced. Specifically, ld.bfd searches the -L path for libraries named on the command line, and it searches the rpath-link path for libraries required by those libraries. That is, it does not search the -L path for libraries needed by libraries. In my humble opinion, this doesn't make much sense and is slightly annoying for the end user. However, if lld wants to be command-line-compatible with ld.bfd, which I think would be wise, it should probably follow suit.

The ld.bfd documentation has a fairly detailed description:

https://sourceware.org/binutils/docs-2.27/ld/Options.html#index-link_002dtime-runtime-library-search-path-232

Yes, that is what I mean when wrote description I think. There is no much sence to split searching I believe.
In lld we always follow the easiest path at first and I do not want to overcomplicate feature until there are reasons to do that.

[vangyzen]

That makes sense to me. And given that, the patch looks fine (not that my review matters much).

Thanks for your work on this.

[joerg]

Actually, keeping them separate can be quite important when considering the presence of archive libraries in the build library path (-L) vs the link-time view of the run-time path. This matters for linker script look-up.

[grimar]

In D24496#541488, @joerg wrote:

Actually, keeping them separate can be quite important when considering the presence of archive libraries in the build library path (-L) vs the link-time view of the run-time path. This matters for linker script look-up.

Sorry, I am missing the idea. Could you provide example to clarify real life usecase you are talking about ?

[ruiu]

I was bitten by a subtle difference (or a bug) in the file search path code before, which is fixed in r273846. I can tell from that experience that it was not easy to find a bug. If the linker finds another file in a different directory in search paths, it tries to use that file instead of an intended one and eventually fail in a unresolved symbol error or something like that which seems totally unrelated to a search path problem. So my opinion is that we probably should implement what the GNU linker does to avoid such mess in the future. At least we need to understand the exact semantics of the GNU linker.

ELF/Options.td

150

Define rpath_link_eq as an alias for rpath_link.

def alias_rpath_link_eq: J<"rpath-link=">, Alias<rpath_link>;

[grimar]

In D24496#541673, @ruiu wrote:

I was bitten by a subtle difference (or a bug) in the file search path code before, which is fixed in r273846. I can tell from that experience that it was not easy to find a bug. If the linker finds another file in a different directory in search paths, it tries to use that file instead of an intended one and eventually fail in a unresolved symbol error or something like that which seems totally unrelated to a search path problem. So my opinion is that we probably should implement what the GNU linker does to avoid such mess in the future. At least we need to understand the exact semantics of the GNU linker.

When prepared this patch, I just already forgot why we ignored it before, but now I remember: it was D18269,
we desided to ignore it like gold do that time.

ELF/Driver.cpp

if (Arg->getOption().getID() == OPT_strip_all)
       return StripPolicy::All;
     return StripPolicy::Debug;
   }
   return StripPolicy::None;
 }
 
 // Initializes Config members by the command line options.
 void LinkerDriver::readConfigs(opt::InputArgList &Args) {
+  for (auto *Arg : Args.filtered(OPT_rpath_link_eq))
+    Config->SearchPaths.push_back(Arg->getValue());
+  for (auto *Arg : Args.filtered(OPT_rpath_link))
+    Config->SearchPaths.push_back(Arg->getValue());
   for (auto *Arg : Args.filtered(OPT_L))
     Config->SearchPaths.push_back(Arg->getValue());
 
   std::vector<StringRef> RPaths;
   for (auto *Arg : Args.filtered(OPT_rpath))
     RPaths.push_back(Arg->getValue());
   if (!RPaths.empty())
     Config->RPath = llvm::join(RPaths.begin(), RPaths.end(), ":");

ELF/Options.td

 
 def reproduce: S<"reproduce">,
   HelpText<"Dump linker invocation and input files for debugging">;
 
 def rpath: S<"rpath">, HelpText<"Add a DT_RUNPATH to the output">;
 
 def relocatable: F<"relocatable">, HelpText<"Create relocatable object file">;
 
+def rpath_link: S<"rpath-link">, HelpText<"Add DIR to link time shared library search path">;
+
+def rpath_link_eq: J<"rpath-link=">, HelpText<"Add DIR to link time shared library search path">;
+
 def script: S<"script">, HelpText<"Read linker script">;
 
 def shared: F<"shared">, HelpText<"Build a shared object">;
 
 def soname: J<"soname=">, HelpText<"Set DT_SONAME">;
 
 def start_lib: F<"start-lib">,
   HelpText<"Start a grouping of objects that should be treated as if they were together in an archive">;

 def no_allow_shlib_undefined: F<"no-allow-shlib-undefined">;
 def no_copy_dt_needed_entries: F<"no-copy-dt-needed-entries">,
   Alias<no_add_needed>;
 def no_dynamic_linker: F<"no-dynamic-linker">;
 def no_fatal_warnings: F<"no-fatal-warnings">;
 def no_mmap_output_file: F<"no-mmap-output-file">;
 def no_warn_common: F<"no-warn-common">;
 def no_warn_mismatch: F<"no-warn-mismatch">;
-def rpath_link: S<"rpath-link">;
-def rpath_link_eq: J<"rpath-link=">;
 def sort_common: F<"sort-common">;
 def warn_execstack: F<"warn-execstack">;
 def warn_shared_textrel: F<"warn-shared-textrel">;
 def EB : F<"EB">;
 def EL : F<"EL">;
 def G: JoinedOrSeparate<["-"], "G">;
 def Qy : F<"Qy">;
 

test/ELF/rpath-link.s

+# REQUIRES: x86
+# RUN: llvm-mc -filetype=obj -triple=x86_64-pc-linux %p/Inputs/shared.s -o %t1.o
+# RUN: mkdir -p %T/libs
+# RUN: ld.lld -shared %t1.o -o %T/libs/libz.so
+# RUN: llvm-mc -filetype=obj -triple=x86_64-pc-linux %s -o %t2.o
+# RUN: ld.lld -o %t --rpath-link=%T/libs -lz %t2.o 2>&1
+# RUN: ld.lld -o %t --rpath-link %T/libs -lz %t2.o 2>&1