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

[George Rimar via Phabricator via llvm-commits]

grimar created this revision.
Herald added a subscriber: emaste.

-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.


https://reviews.llvm.org/D40558

Files:
  ELF/Driver.cpp
  ELF/Options.td
  test/ELF/rpath-vs-just-symbols.s


Index: test/ELF/rpath-vs-just-symbols.s
===================================================================
--- test/ELF/rpath-vs-just-symbols.s
+++ test/ELF/rpath-vs-just-symbols.s
@@ -0,0 +1,16 @@
+# 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.
Index: ELF/Options.td
===================================================================
--- ELF/Options.td
+++ ELF/Options.td
@@ -242,6 +242,9 @@
 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">;
@@ -345,7 +348,6 @@
 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>;
Index: ELF/Driver.cpp
===================================================================
--- ELF/Driver.cpp
+++ ELF/Driver.cpp
@@ -421,8 +421,22 @@
 }
 
 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


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D40558.124568.patch
Type: text/x-patch
Size: 3011 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20171128/efd96ca8/attachment.bin>