[v3] Add --ld-path= to specify an arbitrary executable as the linker

Message ID 20200725055937.4x6cblyb5efuhrkx@google.com
State New
Headers show
Series
  • [v3] Add --ld-path= to specify an arbitrary executable as the linker
Related show

Commit Message

Mike Crowe via Gcc-patches July 25, 2020, 5:59 a.m.
Attached v3 to address nits.

On 2020-07-23, Martin Liška wrote:
>On 7/21/20 6:07 AM, Fangrui Song wrote:

>>If the value does not contain any path component separator (e.g. a

>>slash), the linker will be searched for using COMPILER_PATH followed by

>>PATH. Otherwise, it is either an absolute path or a path relative to the

>>current working directory.

>>

>>--ld-path= complements and overrides -fuse-ld={bfd,gold,lld}. If in the

>>future, we want to make dfferent linker option decisions we can let

>>-fuse-ld= represent the linker flavor and --ld-path= the linker path.

>

>Hello.

>

>I have just few nits:

>

>=== ERROR type #3: trailing operator (1 error(s)) ===

>gcc/collect2.c:1155:14:	ld_file_name =

>

>>

>>	PR driver/93645

>>	* common.opt (--ld-path=): Add --ld-path=

>>	* opts.c (common_handle_option): Handle OPT__ld_path_.

>>	* gcc.c (driver_handle_option): Likewise.

>>	* collect2.c (main): Likewise.

>>	* doc/invoke.texi: Document --ld-path=.

>>

>>---

>>Changes in v2:

>>* Renamed -fld-path= to --ld-path= (clang 12.0.0 new option).

>>   The option does not affect code generation and is not a language feature,

>>   -f* is not suitable. Additionally, clang has other similar --*-path=

>>   options, e.g. --cuda-path=.

>>---

>>  gcc/collect2.c      | 63 +++++++++++++++++++++++++++++++++++----------

>>  gcc/common.opt      |  4 +++

>>  gcc/doc/invoke.texi |  9 +++++++

>>  gcc/gcc.c           |  2 +-

>>  gcc/opts.c          |  1 +

>>  5 files changed, 64 insertions(+), 15 deletions(-)

>>

>>diff --git a/gcc/collect2.c b/gcc/collect2.c

>>index f8a5ce45994..caa1b96ab52 100644

>>--- a/gcc/collect2.c

>>+++ b/gcc/collect2.c

>>@@ -844,6 +844,7 @@ main (int argc, char **argv)

>>    const char **ld1;

>>    bool use_plugin = false;

>>    bool use_collect_ld = false;

>>+  const char *ld_path = 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

>>@@ -961,12 +962,21 @@ 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) == 0

>>+		 && selected_linker != USE_LD_MAX)

>>+	  {

>>+	    if (strcmp (argv[i] + 9, "bfd") == 0)

>>+	      selected_linker = USE_BFD_LD;

>>+	    else if (strcmp (argv[i] + 9, "gold") == 0)

>>+	      selected_linker = USE_GOLD_LD;

>>+	    else if (strcmp (argv[i] + 9, "lld") == 0)

>>+	      selected_linker = USE_LLD_LD;

>>+	  }

>>+	else if (strncmp (argv[i], "--ld-path=", 10) == 0)

>>+	  {

>>+	    ld_path = argv[i] + 10;

>>+	    selected_linker = USE_LD_MAX;

>>+	  }

>>  	else if (strncmp (argv[i], "-o", 2) == 0)

>>  	  {

>>  	    /* Parse the output filename if it's given so that we can make

>>@@ -1117,14 +1127,34 @@ 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_MAX)

>>+    {

>>+      /* If --ld-path= does not contain a path component separator, search for

>>+	 the command using cpath, then using path.  Otherwise find the linker

>>+	 relative to the current working directory.  */

>>+      if (lbasename (ld_path) == ld_path)

>>+	{

>>+	  ld_file_name = find_a_file (&cpath, ld_path, X_OK);

>>+	  if (ld_file_name == 0)

>>+	    ld_file_name = find_a_file (&path, ld_path, X_OK);

>>+	}

>>+      else if (file_exists (ld_path))

>>+	{

>

>^^^ these braces are not needed.


Usually, if the 'if' branch has braces, I'd add braces to 'else' as
well.  Updated to follow your advice anyway.

>>+	  ld_file_name = ld_path;

>>+	}

>>+    }

>>+  else

>>+    {

>>+      /* Search the compiler directories for `ld'.  We have protection against

>>+	 recursive calls in find_a_file.  */

>>+      if (ld_file_name == 0)

>

>I would prefer '== NULL'.


Done.

>>+	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);

>>@@ -1461,6 +1491,11 @@ main (int argc, char **argv)

>>  		  ld2--;

>>  #endif

>>  		}

>>+	      else if (strncmp (arg, "--ld-path=", 10) == 0)

>>+		{

>>+		  ld1--;

>>+		  ld2--;

>>+		}

>

>Can you please explain more this change?


This is to mimick nearly lines. collect2 should filter out options like -fno-lto, -flto, -fuse-ld= before passing to ld. -f* are handled by case 'f'. --ld-path needs its own `else if`.

Honestly I don't know why both both ld1 and ld2 exist and I suspect this
can be cleaned up/at least documented, but it is not the scope of this patch.

>Thank you,

>Martin

>

>>  	      else if (strncmp (arg, "--sysroot=", 10) == 0)

>>  		target_system_root = arg + 10;

>>  	      else if (strcmp (arg, "--version") == 0)

>>diff --git a/gcc/common.opt b/gcc/common.opt

>>index a3893a4725e..5adbd8c18a3 100644

>>--- a/gcc/common.opt

>>+++ b/gcc/common.opt

>>@@ -2940,6 +2940,10 @@ fuse-ld=lld

>>  Common Driver Negative(fuse-ld=lld)

>>  Use the lld LLVM linker instead of the default linker.

>>+-ld-path=

>>+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 ba18e05fb1a..e185e40ffac 100644

>>--- a/gcc/doc/invoke.texi

>>+++ b/gcc/doc/invoke.texi

>>@@ -14718,6 +14718,15 @@ 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 --ld-path=@var{linker}

>>+@opindex -ld-path=linker

>>+Use the specified executable named @var{linker} instead of the default linker.

>>+If @var{linker} does not contain any path component separator (e.g. a slash),

>>+the linker will be searched for using @env{COMPILER_PATH} followed by

>>+@env{PATH}. Otherwise, it is either an absolute path or a path relative to the

>>+current working directory.  If @option{-fuse-ld=} is also specified, the linker

>>+path is specified by @option{--ld-path=}.

>>+

>>  @cindex Libraries

>>  @item -l@var{library}

>>  @itemx -l @var{library}

>>diff --git a/gcc/gcc.c b/gcc/gcc.c

>>index c0eb3c10cfd..05fa5375f06 100644

>>--- a/gcc/gcc.c

>>+++ b/gcc/gcc.c

>>@@ -1077,7 +1077,7 @@ proper position among the other output files.  */

>>      LINK_PLUGIN_SPEC \

>>     "%{flto|flto=*:%<fcompare-debug*} \

>>      %{flto} %{fno-lto} %{flto=*} %l " LINK_PIE_SPEC \

>>-   "%{fuse-ld=*:-fuse-ld=%*} " LINK_COMPRESS_DEBUG_SPEC \

>>+   "%{fuse-ld=*:-fuse-ld=%*} %{-ld-path=*:--ld-path=%*} " LINK_COMPRESS_DEBUG_SPEC \

>>     "%X %{o*} %{e*} %{N} %{n} %{r}\

>>      %{s} %{t} %{u*} %{z} %{Z} %{!nostdlib:%{!r:%{!nostartfiles:%S}}} \

>>      %{static|no-pie|static-pie:} %@{L*} %(mfwrap) %(link_libgcc) " \

>>diff --git a/gcc/opts.c b/gcc/opts.c

>>index 499eb900643..5cbf9b85549 100644

>>--- a/gcc/opts.c

>>+++ b/gcc/opts.c

>>@@ -2755,6 +2755,7 @@ common_handle_option (struct gcc_options *opts,

>>        dc->max_errors = value;

>>        break;

>>+    case OPT__ld_path_:

>>      case OPT_fuse_ld_bfd:

>>      case OPT_fuse_ld_gold:

>>      case OPT_fuse_ld_lld:

>>

>

Comments

Andreas Schwab July 25, 2020, 7:09 a.m. | #1
On Jul 24 2020, Fangrui Song via Gcc-patches wrote:

> This is to mimick nearly lines. collect2 should filter out options like -fno-lto, -flto, -fuse-ld= before passing to ld. -f* are handled by case 'f'. --ld-path needs its own `else if`.


If you handle --ld-path as -fld-path then you don't need that.

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510  2552 DF73 E780 A9DA AEC1
"And now for something completely different."
Mike Crowe via Gcc-patches July 26, 2020, 6:31 a.m. | #2
On Sat, Jul 25, 2020 at 12:09 AM Andreas Schwab <schwab@linux-m68k.org> wrote:
>

> On Jul 24 2020, Fangrui Song via Gcc-patches wrote:

>

> > This is to mimick nearly lines. collect2 should filter out options like -fno-lto, -flto, -fuse-ld= before passing to ld. -f* are handled by case 'f'. --ld-path needs its own `else if`.

>

> If you handle --ld-path as -fld-path then you don't need that.

>


--ld-path is a deliberate design choice. -f* is not suitable
(https://sourceware.org/pipermail/gcc/2020-July/233089.html )
clang has several other --*-path= (e.g. --cuda-path)
Andreas Schwab July 26, 2020, 11:02 a.m. | #3
On Jul 25 2020, Fāng-ruì Sòng wrote:

> On Sat, Jul 25, 2020 at 12:09 AM Andreas Schwab <schwab@linux-m68k.org> wrote:

>>

>> On Jul 24 2020, Fangrui Song via Gcc-patches wrote:

>>

>> > This is to mimick nearly lines. collect2 should filter out options like -fno-lto, -flto, -fuse-ld= before passing to ld. -f* are handled by case 'f'. --ld-path needs its own `else if`.

>>

>> If you handle --ld-path as -fld-path then you don't need that.

>>

>

> --ld-path is a deliberate design choice. -f* is not suitable

> (https://sourceware.org/pipermail/gcc/2020-July/233089.html )


I don't agree with that opinion.  --foo and -ffoo are equivalent and
interchangeable.

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510  2552 DF73 E780 A9DA AEC1
"And now for something completely different."
Mike Crowe via Gcc-patches July 26, 2020, 3:48 p.m. | #4
On Sun, Jul 26, 2020 at 4:02 AM Andreas Schwab <schwab@linux-m68k.org> wrote:
>

> On Jul 25 2020, Fāng-ruì Sòng wrote:

>

> > On Sat, Jul 25, 2020 at 12:09 AM Andreas Schwab <schwab@linux-m68k.org> wrote:

> >>

> >> On Jul 24 2020, Fangrui Song via Gcc-patches wrote:

> >>

> >> > This is to mimick nearly lines. collect2 should filter out options like -fno-lto, -flto, -fuse-ld= before passing to ld. -f* are handled by case 'f'. --ld-path needs its own `else if`.

> >>

> >> If you handle --ld-path as -fld-path then you don't need that.

> >>

> >

> > --ld-path is a deliberate design choice. -f* is not suitable

> > (https://sourceware.org/pipermail/gcc/2020-July/233089.html )

> > clang has several other --*-path= (e.g. --cuda-path)

>

> I don't agree with that opinion.  --foo and -ffoo are equivalent and

> interchangeable.

>

> Andreas.


For options only affecting linking, -f* are minority.

-fgnu-tm enables language-level constructs.
-flto/-fopenmp/-fprofile-arcs/-fsanitize=*/etc affect code generation.
-fuse-ld= is an exception. The other options (the majority) don't
start with -f.

Patch

From 042394387ecf67d0d4e57004f8c499d037dacf52 Mon Sep 17 00:00:00 2001
From: Fangrui Song <maskray@google.com>
Date: Fri, 24 Jul 2020 22:50:26 -0700
Subject: [PATCH v3] Add --ld-path= to specify an arbitrary executable as the
 linker

If the value does not contain any path component separator (e.g. a
slash), the linker will be searched for using COMPILER_PATH followed by
PATH. Otherwise, it is either an absolute path or a path relative to the
current working directory.

--ld-path= complements and overrides -fuse-ld={bfd,gold,lld}. If in the
future, we want to make dfferent linker option decisions we can let
-fuse-ld= represent the linker flavor and --ld-path= the linker path.

	PR driver/93645
	* common.opt (--ld-path=): Add --ld-path=
	* opts.c (common_handle_option): Handle OPT__ld_path_.
	* gcc.c (driver_handle_option): Likewise.
	* collect2.c (main): Likewise.
	* doc/invoke.texi: Document --ld-path=.

---
Changes in v2:
* Renamed -fld-path= to --ld-path= (clang 12.0.0 new option).
  The option does not affect code generation and is not a language feature,
  -f* is not suitable.
---
 gcc/collect2.c      | 62 +++++++++++++++++++++++++++++++++++----------
 gcc/common.opt      |  4 +++
 gcc/doc/invoke.texi |  9 +++++++
 gcc/gcc.c           |  2 +-
 gcc/opts.c          |  1 +
 5 files changed, 63 insertions(+), 15 deletions(-)

diff --git a/gcc/collect2.c b/gcc/collect2.c
index f8a5ce45994..65d6b1414f6 100644
--- a/gcc/collect2.c
+++ b/gcc/collect2.c
@@ -844,6 +844,7 @@  main (int argc, char **argv)
   const char **ld1;
   bool use_plugin = false;
   bool use_collect_ld = false;
+  const char *ld_path = 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
@@ -961,12 +962,21 @@  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) == 0
+		 && selected_linker != USE_LD_MAX)
+	  {
+	    if (strcmp (argv[i] + 9, "bfd") == 0)
+	      selected_linker = USE_BFD_LD;
+	    else if (strcmp (argv[i] + 9, "gold") == 0)
+	      selected_linker = USE_GOLD_LD;
+	    else if (strcmp (argv[i] + 9, "lld") == 0)
+	      selected_linker = USE_LLD_LD;
+	  }
+	else if (strncmp (argv[i], "--ld-path=", 10) == 0)
+	  {
+	    ld_path = argv[i] + 10;
+	    selected_linker = USE_LD_MAX;
+	  }
 	else if (strncmp (argv[i], "-o", 2) == 0)
 	  {
 	    /* Parse the output filename if it's given so that we can make
@@ -1117,14 +1127,33 @@  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_MAX)
+    {
+      /* If --ld-path= does not contain a path component separator, search for
+	 the command using cpath, then using path. Otherwise the value is
+	 either an absolute path or a path relative to the current working
+	 directory. */
+      if (lbasename (ld_path) == ld_path)
+	{
+	  ld_file_name = find_a_file (&cpath, ld_path, X_OK);
+	  if (ld_file_name == NULL)
+	    ld_file_name = find_a_file (&path, ld_path, X_OK);
+	}
+      else if (file_exists (ld_path))
+	ld_file_name = ld_path;
+    }
+  else
+    {
+      /* Search the compiler directories for `ld'.  We have protection against
+	 recursive calls in find_a_file.  */
+      if (ld_file_name == NULL)
+	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 == NULL)
+	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);
@@ -1461,6 +1490,11 @@  main (int argc, char **argv)
 		  ld2--;
 #endif
 		}
+	      else if (strncmp (arg, "--ld-path=", 10) == 0)
+		{
+		  ld1--;
+		  ld2--;
+		}
 	      else if (strncmp (arg, "--sysroot=", 10) == 0)
 		target_system_root = arg + 10;
 	      else if (strcmp (arg, "--version") == 0)
diff --git a/gcc/common.opt b/gcc/common.opt
index a3893a4725e..5adbd8c18a3 100644
--- a/gcc/common.opt
+++ b/gcc/common.opt
@@ -2940,6 +2940,10 @@  fuse-ld=lld
 Common Driver Negative(fuse-ld=lld)
 Use the lld LLVM linker instead of the default linker.
 
+-ld-path=
+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 ba18e05fb1a..e185e40ffac 100644
--- a/gcc/doc/invoke.texi
+++ b/gcc/doc/invoke.texi
@@ -14718,6 +14718,15 @@  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 --ld-path=@var{linker}
+@opindex -ld-path=linker
+Use the specified executable named @var{linker} instead of the default linker.
+If @var{linker} does not contain any path component separator (e.g. a slash),
+the linker will be searched for using @env{COMPILER_PATH} followed by
+@env{PATH}. Otherwise, it is either an absolute path or a path relative to the
+current working directory.  If @option{-fuse-ld=} is also specified, the linker
+path is specified by @option{--ld-path=}.
+
 @cindex Libraries
 @item -l@var{library}
 @itemx -l @var{library}
diff --git a/gcc/gcc.c b/gcc/gcc.c
index 10bc9881aed..a88da48157b 100644
--- a/gcc/gcc.c
+++ b/gcc/gcc.c
@@ -1077,7 +1077,7 @@  proper position among the other output files.  */
     LINK_PLUGIN_SPEC \
    "%{flto|flto=*:%<fcompare-debug*} \
     %{flto} %{fno-lto} %{flto=*} %l " LINK_PIE_SPEC \
-   "%{fuse-ld=*:-fuse-ld=%*} " LINK_COMPRESS_DEBUG_SPEC \
+   "%{fuse-ld=*:-fuse-ld=%*} %{-ld-path=*:--ld-path=%*} " LINK_COMPRESS_DEBUG_SPEC \
    "%X %{o*} %{e*} %{N} %{n} %{r}\
     %{s} %{t} %{u*} %{z} %{Z} %{!nostdlib:%{!r:%{!nostartfiles:%S}}} \
     %{static|no-pie|static-pie:} %@{L*} %(mfwrap) %(link_libgcc) " \
diff --git a/gcc/opts.c b/gcc/opts.c
index 499eb900643..5cbf9b85549 100644
--- a/gcc/opts.c
+++ b/gcc/opts.c
@@ -2755,6 +2755,7 @@  common_handle_option (struct gcc_options *opts,
       dc->max_errors = value;
       break;
 
+    case OPT__ld_path_:
     case OPT_fuse_ld_bfd:
     case OPT_fuse_ld_gold:
     case OPT_fuse_ld_lld:
-- 
2.28.0.rc0.142.g3c755180ce-goog