On 2020-04-06, Martin Liška wrote:
On 4/6/20 12:32 AM, Fangrui Song wrote:
On 2020-03-11, Martin Liška wrote:
On 2/10/20 1:02 AM, Fangrui Song via gcc-patches wrote:

Hello.

Thank you for the patch. You haven't received a review because we are right now
in stage4 of the development cycle:
https://gcc.gnu.org/develop.html#stage4

Thanks for the review!
According to https://gcc.gnu.org/pipermail/gcc-patches/2020-April/543028.html "GCC 
10.0 Status Report (2020-04-01)",
I guess GCC is not open for a new development cycle yet.

Yes, it's not opened, but I expect to be opened in 3 weeks from now.

I will just answer a few questions instead of uploading a new patch.

Sure, but don't hesitate to send a patch. It can sit here and wait for next 
stage1 ;)

Thank you! I got around and updated the patch.

Tested locally with -fuse-ld=~/Stable/bin/ld.lld and 
-fuse-ld=~/Dev/binutils-gdb/Debug/ld/ld-new


Anyway, I'm going to provide a review (even though I'm not maintainer of that).

Can you please describe your test-case why you need such option? When using
a different ld, I typically export PATH=/path/to/binutils and then run configure
and make.

I would hope -fuse-ld=ld.bfd and -fuse-ld=ld.gold were used instead of
-fuse-ld=bfd and -fuse-ld=gold, then it would be more natural to have
-fuse-ld=/abs/path/to/ld . https://gcc.gnu.org/bugzilla/show_bug.cgi?id=55470

Well, problem with that is that the option values are used and we want to 
preserve
backward compatibility of options (if possible). I mean we can't just rename
-fuse-ld=bfd to -fuse-ld=ld.bfd.


-fuse-ld=bfd, -fuse-ld=gold and -fuse-ld=lld are hard-coded in numerous
places. It is too late to change that.

One idea is to make

-fuse-ld=bfd
-fuse-ld=gold
-fuse-ld=lld

special. For any other value, e.g. -fuse-ld=foo or -fuse-ld=/abs/path, just 
searches the
executable named "foo" (instead of "ld.foo") or /abs/path .

Yes, that seems feasible to me.


Does the scheme sound good? If it is agreed, I can make a similar change to 
clang.

Yes, please send a patch and we can make another round of review process.

Thanks,
Martin


I noticed not ideal error message:

$ gcc -fuse-ld=pes /tmp/foo.c
collect2: fatal error: cannot find ‘ld’
compilation terminated.

while clang prints:

$clang -fuse-ld=pes /tmp/foo.c
clang-9.0: error: invalid linker name in argument '-fuse-ld=pes'

The rest of the patch is comment inline...

Thanks for all the comments.

    PR driver/93645
    * common.opt (-fuse-ld=): Delete -fuse-ld=[bfd|gold|lld]. Add -fuse-ld=.
    * opts.c (common_handle_option): Handle OPT_fuse_ld_.
    * gcc.c (driver_handle_option): Likewise.
    * collect2.c (main): Likewise.
---
 gcc/ChangeLog       |  8 ++++++
 gcc/collect2.c      | 67 ++++++++++++++++++++++++---------------------
 gcc/common.opt      | 14 ++--------
 gcc/doc/invoke.texi | 15 +++-------
 gcc/gcc.c           | 14 ++++------
 gcc/opts.c          |  4 +--
 6 files changed, 57 insertions(+), 65 deletions(-)

diff --git a/gcc/ChangeLog b/gcc/ChangeLog
index feb2d066d0b..6bcec12d841 100644
--- a/gcc/ChangeLog
+++ b/gcc/ChangeLog
@@ -1,3 +1,11 @@
+2020-02-09  Fangrui Song  <mask...@google.com>
+
+    PR driver/93645
+    * common.opt (-fuse-ld=): Delete -fuse-ld=[bfd|gold|lld]. Add -fuse-ld=.
+    * opts.c (common_handle_option): Handle OPT_fuse_ld_.
+    * gcc.c (driver_handle_option): Likewise.
+    * collect2.c (main): Likewise.
+
 2020-02-09  Uroš Bizjak  <ubiz...@gmail.com>
     * recog.c: Move pass_split_before_sched2 code in front of
diff --git a/gcc/collect2.c b/gcc/collect2.c
index 502d629141c..a3ef525a93b 100644
--- a/gcc/collect2.c
+++ b/gcc/collect2.c
@@ -859,18 +859,12 @@ main (int argc, char **argv)
     {
       USE_DEFAULT_LD,
       USE_PLUGIN_LD,
-      USE_GOLD_LD,
-      USE_BFD_LD,
-      USE_LLD_LD,
-      USE_LD_MAX
+      USE_LD
     } selected_linker = USE_DEFAULT_LD;
-  static const char *const ld_suffixes[USE_LD_MAX] =
+  static const char *const ld_suffixes[USE_LD] =
     {
       "ld",
-      PLUGIN_LD_SUFFIX,
-      "ld.gold",
-      "ld.bfd",
-      "ld.lld"
+      PLUGIN_LD_SUFFIX
     };
   static const char *const real_ld_suffix = "real-ld";
   static const char *const collect_ld_suffix = "collect-ld";
@@ -882,7 +876,7 @@ main (int argc, char **argv)
   static const char *const strip_suffix = "strip";
   static const char *const gstrip_suffix = "gstrip";
-  const char *full_ld_suffixes[USE_LD_MAX];
+  const char *full_ld_suffixes[USE_LD];
 #ifdef CROSS_DIRECTORY_STRUCTURE
   /* If we look for a program in the compiler directories, we just use
      the short name, since these directories are already system-specific.
@@ -924,6 +918,7 @@ main (int argc, char **argv)
   const char **ld1;
   bool use_plugin = false;
   bool use_collect_ld = false;
+  const char *use_ld = NULL;
   /* The kinds of symbols we will have to consider when scanning the
      outcome of a first pass link.  This is ALL to start with, then might
@@ -948,7 +943,7 @@ main (int argc, char **argv)
 #endif
   int i;
-  for (i = 0; i < USE_LD_MAX; i++)
+  for (i = 0; i < USE_LD; i++)
     full_ld_suffixes[i]
 #ifdef CROSS_DIRECTORY_STRUCTURE
       = concat (target_machine, "-", ld_suffixes[i], NULL);
@@ -1041,12 +1036,11 @@ main (int argc, char **argv)
         if (selected_linker == USE_DEFAULT_LD)
           selected_linker = USE_PLUGIN_LD;
       }
-    else if (strcmp (argv[i], "-fuse-ld=bfd") == 0)
-      selected_linker = USE_BFD_LD;
-    else if (strcmp (argv[i], "-fuse-ld=gold") == 0)
-      selected_linker = USE_GOLD_LD;
-    else if (strcmp (argv[i], "-fuse-ld=lld") == 0)
-      selected_linker = USE_LLD_LD;
+    else if (!strncmp (argv[i], "-fuse-ld=", 9))
+      {
+        use_ld = argv[i] + 9;
+        selected_linker = USE_LD;
+      }
     else if (strncmp (argv[i], "-o", 2) == 0)
       {
         /* Parse the output filename if it's given so that we can make
@@ -1152,8 +1146,7 @@ main (int argc, char **argv)
   /* Maybe we know the right file to use (if not cross).  */
   ld_file_name = 0;
 #ifdef DEFAULT_LINKER
-  if (selected_linker == USE_BFD_LD || selected_linker == USE_GOLD_LD ||
-      selected_linker == USE_LLD_LD)
+  if (!ld_file_name && selected_linker == USE_LD)
     {
       char *linker_name;
 # ifdef HOST_EXECUTABLE_SUFFIX
@@ -1168,15 +1161,13 @@ main (int argc, char **argv)
       if (! strcmp (&default_linker[len], HOST_EXECUTABLE_SUFFIX))
         {
           default_linker[len] = '\0';
-          linker_name = concat (default_linker,
-                    &ld_suffixes[selected_linker][2],
+          linker_name = concat (default_linker, ".", use_ld,
                     HOST_EXECUTABLE_SUFFIX, NULL);
         }
     }

This piece of code (added by commit b78e932d513b7d3373bc5e1aab456dda737d07d0 PR 
driver/59321) is a bit complex.
The new patch does not touch it.

       if (linker_name == NULL)
 # endif
-      linker_name = concat (DEFAULT_LINKER,
-                &ld_suffixes[selected_linker][2],
+      linker_name = concat (DEFAULT_LINKER, ".", use_ld,
                 NULL);
       if (access (linker_name, X_OK) == 0)
     ld_file_name = linker_name;
@@ -1197,14 +1188,28 @@ main (int argc, char **argv)
       ld_file_name = find_a_file (&cpath, collect_ld_suffix, X_OK);
       use_collect_ld = ld_file_name != 0;
     }
-  /* Search the compiler directories for `ld'.  We have protection against
-     recursive calls in find_a_file.  */
-  if (ld_file_name == 0)
-    ld_file_name = find_a_file (&cpath, ld_suffixes[selected_linker], X_OK);
-  /* Search the ordinary system bin directories
-     for `ld' (if native linking) or `TARGET-ld' (if cross).  */
-  if (ld_file_name == 0)
-    ld_file_name = find_a_file (&path, full_ld_suffixes[selected_linker], 
X_OK);
+  if (selected_linker != USE_LD) {
+    /* Search the compiler directories for `ld'.  We have protection against
+       recursive calls in find_a_file.  */
+    if (!ld_file_name)
+      ld_file_name = find_a_file(&cpath, ld_suffixes[selected_linker], X_OK);
+    /* Search the ordinary system bin directories
+       for `ld' (if native linking) or `TARGET-ld' (if cross).  */
+    if (!ld_file_name)
+      ld_file_name = find_a_file(&path, full_ld_suffixes[selected_linker], 
X_OK);
+  } else if (IS_ABSOLUTE_PATH(use_ld) && access(use_ld, X_OK) == 0) {
+    ld_file_name = use_ld;
+  } else {
+    const char *linker_name = concat("ld.", use_ld, NULL);

This leads to strange searches like:

$ strace -f -s512 gcc -fuse-ld=/x/y/z/pes /tmp/foo.c 2>&1 | grep pes
...
[pid 24295] stat("/home/marxin/bin/ld./x/y/z/pes", 0x7fffffffd1f0) = -1 ENOENT 
(No such file or directory)
[pid 24295] stat("/usr/local/bin/ld./x/y/z/pes", 0x7fffffffd1f0) = -1 ENOENT 
(No such file or directory)

Moreover, before the patch we only searched for suffixes 'ld.bfd', 'ld.gold' 
and 'ld.gold'. Now it
searches also for:

$ strace ... gcc -fuse-ld=xyz
...
[pid 24893] stat("/home/marxin/bin/ld.xyz", 0x7fffffffd200) = -1 ENOENT (No 
such file or directory)

+    if (!ld_file_name)
+      ld_file_name = find_a_file(&cpath, linker_name, X_OK);
+    if (!ld_file_name) {
+#ifdef CROSS_DIRECTORY_STRUCTURE
+    linker_name = concat(target_machine, "-", linker_name, NULL);
+#endif
+    ld_file_name = find_a_file(&path, linker_name, X_OK);
+    }
+  }
 #ifdef REAL_NM_FILE_NAME
   nm_file_name = find_a_file (&path, REAL_NM_FILE_NAME, X_OK);
diff --git a/gcc/common.opt b/gcc/common.opt
index 5692cd04374..a76ed6434bb 100644
--- a/gcc/common.opt
+++ b/gcc/common.opt
@@ -2859,17 +2859,9 @@ funwind-tables
 Common Report Var(flag_unwind_tables) Optimization
 Just generate unwind tables for exception handling.
-fuse-ld=bfd
-Common Driver Negative(fuse-ld=gold)
-Use the bfd linker instead of the default linker.
-
-fuse-ld=gold
-Common Driver Negative(fuse-ld=bfd)
-Use the gold linker instead of the default linker.
-
-fuse-ld=lld
-Common Driver Negative(fuse-ld=lld)
-Use the lld LLVM linker instead of the default linker.
+fuse-ld=
+Common Driver Joined
+-fuse-ld=[bfd|gold|lld|<absolute path>]    Use the specified linker.
 fuse-linker-plugin
 Common Undocumented Var(flag_use_linker_plugin)
diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
index 35b341e759f..c2dd410c88f 100644
--- a/gcc/doc/invoke.texi
+++ b/gcc/doc/invoke.texi
@@ -14128,17 +14128,10 @@ uses @samp{nolto-rel}. To maintain whole program 
optimization, it is
 recommended to link such objects into static library instead. Alternatively it
 is possible to use H.J. Lu's binutils with support for mixed objects.
-@item -fuse-ld=bfd
-@opindex fuse-ld=bfd
-Use the @command{bfd} linker instead of the default linker.
-
-@item -fuse-ld=gold
-@opindex fuse-ld=gold
-Use the @command{gold} linker instead of the default linker.
-
-@item -fuse-ld=lld
-@opindex fuse-ld=lld
-Use the LLVM @command{lld} linker instead of the default linker.
+@item -fuse-ld=@var{linker}
+@opindex fuse-ld=linker
+If @var{linker} is an absolute path, use it instead of the default linker;
+otherwise use @command{ld.@var{linker}}.

You should somehow mention the special values bfd,gold and ldd.
 @cindex Libraries
 @item -l@var{library}
diff --git a/gcc/gcc.c b/gcc/gcc.c
index effc384f3ef..f9a6f10502d 100644
--- a/gcc/gcc.c
+++ b/gcc/gcc.c
@@ -3989,12 +3989,8 @@ driver_handle_option (struct gcc_options *opts,
       do_save = false;
       break;
-    case OPT_fuse_ld_bfd:
-       use_ld = ".bfd";
-       break;
-
-    case OPT_fuse_ld_gold:
-       use_ld = ".gold";
+    case OPT_fuse_ld_:
+       use_ld = arg;
        break;
     case OPT_fcompare_debug_second:
@@ -7903,20 +7899,20 @@ driver::maybe_print_and_exit () const
           if (! strcmp (&default_linker[len], HOST_EXECUTABLE_SUFFIX))
         {
           default_linker[len] = '\0';
-          ld = concat (default_linker, use_ld,
+          ld = concat (default_linker, ".", use_ld,
                    HOST_EXECUTABLE_SUFFIX, NULL);
         }
         }
       if (ld == NULL)
 # endif
-      ld = concat (DEFAULT_LINKER, use_ld, NULL);
+      ld = concat (DEFAULT_LINKER, ".", use_ld, NULL);
       if (access (ld, X_OK) == 0)
         {
           printf ("%s\n", ld);
           return (0);
         }
 #endif
-      print_prog_name = concat (print_prog_name, use_ld, NULL);
+      print_prog_name = concat (print_prog_name, ".", use_ld, NULL);

In gcc.c you will probably have similar issues.

     }
       char *newname = find_a_file (&exec_prefixes, print_prog_name, X_OK, 0);
       printf ("%s\n", (newname ? newname : print_prog_name));
diff --git a/gcc/opts.c b/gcc/opts.c
index 7affeb41a96..f50d365d517 100644
--- a/gcc/opts.c
+++ b/gcc/opts.c
@@ -2763,9 +2763,7 @@ common_handle_option (struct gcc_options *opts,
       dc->max_errors = value;
       break;
-    case OPT_fuse_ld_bfd:
-    case OPT_fuse_ld_gold:
-    case OPT_fuse_ld_lld:
+    case OPT_fuse_ld_:
     case OPT_fuse_linker_plugin:
       /* No-op. Used by the driver and passed to us because it starts with f.*/
       break;



>From 72049b927b6cee8c3acb2d56b2a403d9f0946807 Mon Sep 17 00:00:00 2001
From: Fangrui Song <mask...@google.com>
Date: Tue, 19 May 2020 12:21:26 -0700
Subject: [PATCH] Add -fuse-ld= to specify an arbitrary executable as the
 linker

The value can be either an absolute path or a relative path.
Unlike -fuse-ld={bfd,gold,lld}, -fuse-ld= does not add a prefix `ld.`

	PR driver/93645
	* common.opt (-fuse-ld=): Add -fuse-ld=.
	* opts.c (common_handle_option): Handle OPT_fuse_ld_.
	* gcc.c (driver_handle_option): Likewise.
	* collect2.c (main): Likewise.
	* doc/invoke.texi: Document -fuse-ld=.
---
 gcc/ChangeLog       |  9 +++++++++
 gcc/collect2.c      | 34 ++++++++++++++++++++++------------
 gcc/common.opt      |  4 ++++
 gcc/doc/invoke.texi |  4 ++++
 gcc/opts.c          |  1 +
 5 files changed, 40 insertions(+), 12 deletions(-)

diff --git a/gcc/ChangeLog b/gcc/ChangeLog
index 703b6349516..19506bf0884 100644
--- a/gcc/ChangeLog
+++ b/gcc/ChangeLog
@@ -1,3 +1,12 @@
+2020-05-19  Fangrui Song  <mask...@google.com>
+
+	PR driver/93645
+	* common.opt (-fuse-ld=): Add -fuse-ld=.
+	* opts.c (common_handle_option): Handle OPT_fuse_ld_.
+	* gcc.c (driver_handle_option): Likewise.
+	* collect2.c (main): Likewise.
+	* doc/invoke.texi: Document -fuse-ld=.
+
 2020-05-19  Jozef Lawrynowicz  <joze...@mittosystems.com>
 
 	* doc/sourcebuild.texi: Document new short_eq_int, ptr_eq_short,
diff --git a/gcc/collect2.c b/gcc/collect2.c
index f8a5ce45994..e04892cb91f 100644
--- a/gcc/collect2.c
+++ b/gcc/collect2.c
@@ -782,15 +782,16 @@ main (int argc, char **argv)
       USE_GOLD_LD,
       USE_BFD_LD,
       USE_LLD_LD,
+      USE_LD,
       USE_LD_MAX
     } selected_linker = USE_DEFAULT_LD;
-  static const char *const ld_suffixes[USE_LD_MAX] =
+  static const char *const ld_suffixes[USE_LD] =
     {
       "ld",
       PLUGIN_LD_SUFFIX,
       "ld.gold",
       "ld.bfd",
-      "ld.lld"
+      "ld.lld",
     };
   static const char *const real_ld_suffix = "real-ld";
   static const char *const collect_ld_suffix = "collect-ld";
@@ -802,7 +803,7 @@ main (int argc, char **argv)
   static const char *const strip_suffix = "strip";
   static const char *const gstrip_suffix = "gstrip";
 
-  const char *full_ld_suffixes[USE_LD_MAX];
+  const char *full_ld_suffixes[USE_LD];
 #ifdef CROSS_DIRECTORY_STRUCTURE
   /* If we look for a program in the compiler directories, we just use
      the short name, since these directories are already system-specific.
@@ -844,6 +845,7 @@ main (int argc, char **argv)
   const char **ld1;
   bool use_plugin = false;
   bool use_collect_ld = false;
+  const char *use_ld = NULL;
 
   /* The kinds of symbols we will have to consider when scanning the
      outcome of a first pass link.  This is ALL to start with, then might
@@ -868,7 +870,7 @@ main (int argc, char **argv)
 #endif
   int i;
 
-  for (i = 0; i < USE_LD_MAX; i++)
+  for (i = 0; i < USE_LD; i++)
     full_ld_suffixes[i]
 #ifdef CROSS_DIRECTORY_STRUCTURE
       = concat (target_machine, "-", ld_suffixes[i], NULL);
@@ -967,6 +969,11 @@ main (int argc, char **argv)
 	  selected_linker = USE_GOLD_LD;
 	else if (strcmp (argv[i], "-fuse-ld=lld") == 0)
 	  selected_linker = USE_LLD_LD;
+	else if (!strncmp (argv[i], "-fuse-ld=", 9))
+	  {
+	    use_ld = argv[i] + 9;
+	    selected_linker = USE_LD;
+	  }
 	else if (strncmp (argv[i], "-o", 2) == 0)
 	  {
 	    /* Parse the output filename if it's given so that we can make
@@ -1119,12 +1126,16 @@ main (int argc, char **argv)
     }
   /* Search the compiler directories for `ld'.  We have protection against
      recursive calls in find_a_file.  */
-  if (ld_file_name == 0)
-    ld_file_name = find_a_file (&cpath, ld_suffixes[selected_linker], X_OK);
-  /* Search the ordinary system bin directories
-     for `ld' (if native linking) or `TARGET-ld' (if cross).  */
-  if (ld_file_name == 0)
-    ld_file_name = find_a_file (&path, full_ld_suffixes[selected_linker], X_OK);
+  if (selected_linker == USE_LD) {
+    ld_file_name = find_a_file (&cpath, use_ld, X_OK);
+  } else {
+    if (ld_file_name == 0)
+      ld_file_name = find_a_file (&cpath, ld_suffixes[selected_linker], X_OK);
+    /* Search the ordinary system bin directories
+       for `ld' (if native linking) or `TARGET-ld' (if cross).  */
+    if (ld_file_name == 0)
+      ld_file_name = find_a_file (&path, full_ld_suffixes[selected_linker], X_OK);
+  }
 
 #ifdef REAL_NM_FILE_NAME
   nm_file_name = find_a_file (&path, REAL_NM_FILE_NAME, X_OK);
@@ -1296,8 +1307,7 @@ main (int argc, char **argv)
 			 "configuration");
 #endif
 		}
-	      else if (!use_collect_ld
-		       && strncmp (arg, "-fuse-ld=", 9) == 0)
+	      else if (strncmp (arg, "-fuse-ld=", 9) == 0)
 		{
 		  /* Do not pass -fuse-ld={bfd|gold|lld} to the linker. */
 		  ld1--;
diff --git a/gcc/common.opt b/gcc/common.opt
index 4464049fc1f..6408d042d8c 100644
--- a/gcc/common.opt
+++ b/gcc/common.opt
@@ -2908,6 +2908,10 @@ fuse-ld=lld
 Common Driver Negative(fuse-ld=lld)
 Use the lld LLVM linker instead of the default linker.
 
+fuse-ld=
+Common Driver Joined
+Use the specified executable instead of the default linker.
+
 fuse-linker-plugin
 Common Undocumented Var(flag_use_linker_plugin)
 
diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
index 7217e27151d..30a2fc4fa53 100644
--- a/gcc/doc/invoke.texi
+++ b/gcc/doc/invoke.texi
@@ -14304,6 +14304,10 @@ Use the @command{gold} linker instead of the default linker.
 @opindex fuse-ld=lld
 Use the LLVM @command{lld} linker instead of the default linker.
 
+@item -fuse-ld=@var{linker}
+@opindex fuse-ld=linker
+Use the specified executable named @var{linker} instead of the default linker.
+
 @cindex Libraries
 @item -l@var{library}
 @itemx -l @var{library}
diff --git a/gcc/opts.c b/gcc/opts.c
index ec3ca0720f9..522a196ab0f 100644
--- a/gcc/opts.c
+++ b/gcc/opts.c
@@ -2785,6 +2785,7 @@ common_handle_option (struct gcc_options *opts,
     case OPT_fuse_ld_bfd:
     case OPT_fuse_ld_gold:
     case OPT_fuse_ld_lld:
+    case OPT_fuse_ld_:
     case OPT_fuse_linker_plugin:
       /* No-op. Used by the driver and passed to us because it starts with f.*/
       break;
-- 
2.26.2.761.g0e0b3e54be-goog

Reply via email to