[PATCH] Add -fld-path= to specify an arbitrary executable as the linker

Fāng-ruì Sòng maskray@google.com
Fri Jul 3 17:18:15 GMT 2020


On 2020-07-03, Martin Liška wrote:
>On 7/2/20 9:34 PM, Fāng-ruì Sòng wrote:
>>On 2020-07-01, Fāng-ruì Sòng wrote:
>>>On 2020-07-01, Martin Liška wrote:
>>>>On 6/30/20 5:32 PM, Fāng-ruì Sòng wrote:
>>>>>There is some concern about clang's -fuse-ld=path
>>>>>http://lists.llvm.org/pipermail/cfe-dev/2020-June/065710.html and use
>>>>>of COMPILER_PATH vs PATH.
>>>>>Shall we introduce another option like -fld-path=path (PATH is used,
>>>>>COMPILER_PATH is not used)?
>>>>
>>>>I would recommend first landing a patch to LLVM and then we can do
>>>>a corresponding change to GCC.
>>>>
>>>>Martin
>>>
>>>Thank a lot for you welcoming words! This is what I intend to add for clang: https://reviews.llvm.org/D83015
>>>
>>>I'll create a GCC patch superseding this one later.
>>
>>Attached the new patch.
>
>Thank you for the update patch:
>
>>From e7f86cdcaf03e4ddb98d0df9d07894d9ffb7d91a Mon Sep 17 00:00:00 2001
>>From: Fangrui Song <maskray@google.com>
>>Date: Thu, 2 Jul 2020 12:26:09 -0700
>>Subject: [PATCH] Add -fld-path= to specify an arbitrary executable as the
>> linker
>>
>>The value can be either a relative path (relative to a COMPILER_PATH
>>directory or a PATH directory) or an absolute path. -fld-path=
>>complements -fuse-ld={bfd,gold,lld} which specifies the linker flavor.
>>
>>	PR driver/93645
>>	* common.opt (-fld-path=): Add -fld-path=
>>	* opts.c (common_handle_option): Handle OPT_fld_path_.
>>	* gcc.c (driver_handle_option): Likewise.
>>	* collect2.c (main): Likewise.
>>	* doc/invoke.texi: Document -fld-path=.
>>---
>> gcc/collect2.c      | 57 ++++++++++++++++++++++++++++++++-------------
>> gcc/common.opt      |  4 ++++
>> gcc/doc/invoke.texi |  6 +++++
>> gcc/gcc.c           |  2 +-
>> gcc/opts.c          |  1 +
>> 5 files changed, 53 insertions(+), 17 deletions(-)
>>
>>diff --git a/gcc/collect2.c b/gcc/collect2.c
>>index f8a5ce45994..efa652f7f82 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=bfd", 9) == 0
>>+		 && selected_linker != USE_LD_MAX)
>>+	  {
>
>This does not seem correct to me. You match -fuse-ld=bfd and then
>test other option values in the following block.

This is correct but I probably should use:

- strncmp (argv[i], "-fuse-ld=bfd", 9) == 0
+ strncmp (argv[i], "-fuse-ld=", 9) == 0


>>+	    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], "-fld-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,27 @@ 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 -fld-path= does not contain a slash, search for the command using
>>+	 the PATH environment variable.  */
>
>We also support file systems like Windows where the comment about a slash will be misleading.
>You can just mention relative vs. absolute path.

The behavior is modeled after
https://pubs.opengroup.org/onlinepubs/9699919799/utilities/V3_chap02.html Command Search and Execution

   e.  Otherwise, the command shall be searched for using the PATH environment variable as described in XBD Environment Variables :

is performed if "the command name does not contain any <slash> characters".

For your suggestion, I think 'word' can mean a relative path as well, along with 'rel\path' and 'rel/path'.

Should I say 

   If -fld-path= does not contain a path component separator (e.g. slash), search for the command using

?

>>+      if (lbasename (ld_path) == ld_path)
>
>Does it really return the same pointer? Maybe strcmp can be needed?

lbasename returns a const char * referencing a place of the string. 

lto-wrapper.s has a similar use case:

1542:  if (!dumppfx || lbasename (dumppfx) == dumppfx)

>>+	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 == 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);
>>@@ -1297,9 +1320,11 @@ main (int argc, char **argv)
>> #endif
>> 		}
>> 	      else if (!use_collect_ld
>>-		       && strncmp (arg, "-fuse-ld=", 9) == 0)
>>+		       && (strncmp (arg, "-fuse-ld=", 9) == 0 ||
>>+			   strncmp (arg, "-fld-path=", 10) == 0))
>> 		{
>>-		  /* Do not pass -fuse-ld={bfd|gold|lld} to the linker. */
>>+		  /* Do not pass -fuse-ld={bfd|gold|lld} or -fld-path= to the
>>+		     linker. */
>> 		  ld1--;
>> 		  ld2--;
>> 		}
>>diff --git a/gcc/common.opt b/gcc/common.opt
>>index df8af365d1b..d5dfbb5c9c6 100644
>>--- a/gcc/common.opt
>>+++ b/gcc/common.opt
>>@@ -2919,6 +2919,10 @@ fuse-ld=lld
>> Common Driver Negative(fuse-ld=lld)
>> Use the lld LLVM linker instead of the default linker.
>>+fld-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 e21d8a5217b..bfb6222f96b 100644
>>--- a/gcc/doc/invoke.texi
>>+++ b/gcc/doc/invoke.texi
>>@@ -14651,6 +14651,12 @@ 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 -fld-path=@var{linker}
>>+@opindex fld-path=linker
>>+Use the specified executable named @var{linker} instead of the default linker.
>>+If @var{linker} does not contain any slash character, the linker will be
>>+searched for using @env{PATH}.  This option overrides @option{-fuse-ld=}.
>
>The slash comment applies here as well.
>
>Martin

Ack. Will update when I get a clarification regarding the previous comment.

>>+
>> @cindex Libraries
>> @item -l@var{library}
>> @itemx -l @var{library}
>>diff --git a/gcc/gcc.c b/gcc/gcc.c
>>index c0eb3c10cfd..a57a3d0aec4 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=%*} %{fld-path=*:-fld-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 340d99434b3..c4507d53abc 100644
>>--- a/gcc/opts.c
>>+++ b/gcc/opts.c
>>@@ -2750,6 +2750,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_fld_path_:
>>     case OPT_fuse_linker_plugin:
>>       /* No-op. Used by the driver and passed to us because it starts with f.*/
>>       break;
>>-- 
>>2.27.0.383.g050319c2ae-goog
>>


More information about the Gcc-patches mailing list