diff mbox series

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

Message ID 20200702193449.cz6bhplp5kvdmkkd@google.com
State New
Headers show
Series Add -fld-path= to specify an arbitrary executable as the linker | expand

Commit Message

Fangrui Song July 2, 2020, 7:34 p.m. UTC
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.

Comments

Martin Liška July 3, 2020, 1:06 p.m. UTC | #1
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.

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

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

Does it really return the same pointer? Maybe strcmp can be needed?

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

> +
>  @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
>
Fangrui Song July 3, 2020, 5:18 p.m. UTC | #2
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
>>
Martin Liška July 7, 2020, 10:16 a.m. UTC | #3
On 7/3/20 7:18 PM, Fāng-ruì Sòng wrote:
> 
> 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

Yes, that would be much better.

> 
> 
>>> +        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
> 
> ?

Seems file to me.

> 
>>> +      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:

Fine then.

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

Thanks,
Martin

> 
>>> +
>>> @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
>>>
diff mbox series

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)
+	  {
+	    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.  */
+      if (lbasename (ld_path) == ld_path)
+	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=}.
+
 @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