[ELF] - Trigger error when -R <filename> is given.

[grimar]

Summary

-R option has 2 meanings according to spec:

-R filename
--just-symbols=filename
..... For compatibility with other ELF linkers, if the -R option is followed by a directory name, rather than a file name, it is treated as the -rpath option.

-rpath dir
.... For compatibility with other ELF linkers, if the -R option is followed by a directory name, rather than a file name, it is treated as the -rpath option.

Currently we always treat -R as -rpath what confuses users (see comments for PR35067) and also produces unexpected/broken output.
I am not sure we want to implement logic sharing -R between --just-symbols and -rpath. I would at least try to avoid that, it seems bad
approach to share flags. Since --just-symbols looks not used too often I would suggest users to switch from -R <filename> to explicit --just-symbols.
But that is a bit different story though, what I think we should do right now is stop handling -R <filename> as -rpath and error out in that case instead.
That is what this patch do.

[ruiu]

I'm not a fan of this patch. Even though it is traditionally implemented in other linkers, this behavior looks too hairy.

[ruiu]

Did you mean we emit an error for -R?

[ruiu]

If you are suggesting we emit something like "cannot open file for -R: foo/bar is a directory. Did you mean -rpath?" for -R, I think I'd agree that that is probably better than just emitting an error without the hint.

[grimar]

Abandoning in according to discussions about this one in corresponding llvm-mails thread.

ELF/Driver.cpp

 static std::vector<StringRef> getArgs(opt::InputArgList &Args, int Id) {
   std::vector<StringRef> V;
   for (auto *Arg : Args.filtered(Id))
     V.push_back(Arg->getValue());
   return V;
 }
 
 static std::string getRpath(opt::InputArgList &Args) {
-  std::vector<StringRef> V = getArgs(Args, OPT_rpath);
-  return llvm::join(V.begin(), V.end(), ":");
+  // GNU linkers list -R <dir> as alias for -rpath and -R <filename>
+  // as alias for --just-symbols. For backward compatibility we support
+  // first case, but we do not want to share the same flag with other
+  // options, hence trigger an error for the second case.
+  std::vector<StringRef> V;
+  for (auto *Arg : Args.filtered(OPT_rpath, OPT_R)) {
+    V.push_back(Arg->getValue());
+    if (Arg->getOption().getID() == OPT_rpath)
+      continue;
+
+    sys::fs::file_type Type = sys::fs::get_file_type(V.back());
+    if (Type != sys::fs::file_type::status_error &&
+        Type != sys::fs::file_type::directory_file)
+      error("-R (-rpath): " + V.back() + " is not a directory.");
+  }
+  return llvm::join(V, ":");
 }
 
 // Determines what we should do if there are remaining unresolved
 // symbols after the name resolution.
 static UnresolvedPolicy getUnresolvedSymbolPolicy(opt::InputArgList &Args) {
   if (Args.hasArg(OPT_relocatable))
     return UnresolvedPolicy::IgnoreAll;
 

ELF/Options.td

def print_gc_sections: F<"print-gc-sections">,
   HelpText<"List removed unused sections">;
 
 def print_map: F<"print-map">,
   HelpText<"Print a link map to the standard output">;
 
 defm reproduce: Eq<"reproduce">,
   HelpText<"Dump linker invocation and input files for debugging">;
 
+def R: JoinedOrSeparate<["-"], "R">, MetaVarName<"<dir>">,
+  HelpText<"Add a DT_RUNPATH to the output">;
+
 defm rpath: Eq<"rpath">, HelpText<"Add a DT_RUNPATH to the output">;
 
 def relocatable: F<"relocatable">, HelpText<"Create relocatable object file">;
 
 defm retain_symbols_file: Eq<"retain-symbols-file">,
   HelpText<"Retain only the symbols listed in the file">,
   MetaVarName<"<file>">;
 

 def alias_library: JoinedOrSeparate<["-"], "l">, Alias<library>;
 def alias_library_path: JoinedOrSeparate<["-"], "L">, Alias<library_path>;
 def alias_omagic: Flag<["-"], "N">, Alias<omagic>;
 def alias_o_output: Joined<["--"], "output=">, Alias<o>;
 def alias_o_output2 : Separate<["--"], "output">, Alias<o>;
 def alias_pie_pic_executable: F<"pic-executable">, Alias<pie>;
 def alias_print_map_M: Flag<["-"], "M">, Alias<print_map>;
 def alias_relocatable_r: Flag<["-"], "r">, Alias<relocatable>;
-def alias_rpath_R: JoinedOrSeparate<["-"], "R">, Alias<rpath>;
 def alias_script_T: JoinedOrSeparate<["-"], "T">, Alias<script>;
 def alias_shared_Bshareable: F<"Bshareable">, Alias<shared>;
 def alias_soname_h: JoinedOrSeparate<["-"], "h">, Alias<soname>;
 def alias_strip_all: Flag<["-"], "s">, Alias<strip_all>;
 def alias_strip_debug_S: Flag<["-"], "S">, Alias<strip_debug>;
 def alias_trace: Flag<["-"], "t">, Alias<trace>;
 def alias_trace_symbol_y : JoinedOrSeparate<["-"], "y">, Alias<trace_symbol>;
 def alias_Ttext_segment: S<"Ttext-segment">, Alias<Ttext>;

test/ELF/rpath-vs-just-symbols.s

+# REQUIRES: x86
+# RUN: rm -rf %t.dir
+# RUN: mkdir %t.dir
+# RUN: llvm-mc -filetype=obj -triple=x86_64-unknown-linux %s -o %t.o
+
+## Check that -R <dir> works as alias for -rpath.
+# RUN: ld.lld -shared %t.o -R %t.dir -rpath=%t.dir/somepath1 \
+# RUN:   -R %t.dir/somepath2 -rpath=%t.dir/somepath3 -o %t.exe
+# RUN: llvm-readobj --dynamic-table %t.exe | FileCheck %s
+# CHECK: DynamicSection [
+# CHECK: RUNPATH {{.*}}.dir:{{.*}}.dir/somepath1:{{.*}}.dir/somepath2:{{.*}}.dir/somepath3
+
+## Check that -R <filename> errors out.
+# RUN: echo '' > %t.dir/file
+# RUN: not ld.lld -shared %t.o -R %t.dir/file -o %t.exe 2>&1 | FileCheck %s --check-prefix=ERR
+# ERR: -R (-rpath): {{.*}}.dir/file is not a directory.