diff mbox series

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

Message ID 20200522044207.yzano7rtgybapizb@google.com
State New
Headers show
Series [v3] Add -fuse-ld= to specify an arbitrary executable as the linker | expand

Commit Message

Fangrui Song May 22, 2020, 4:42 a.m. UTC 
On 2020-05-21, Martin Liška wrote:
>On 5/21/20 1:52 AM, Fangrui Song wrote:
>>The above issues motivated me to touch this line in PATCH v2.
>>Dropped in PATCH v2.
>
>Thank you for the updated patch.
>The patch is fine except coding style issues:
>
>$ ./contrib/check_GNU_style.py /tmp/0001-Add-fuse-ld-to-specify-an-arbitrary-executable-as-th.patch
>=== ERROR type #1: blocks of 8 spaces should be replaced with tabs (4 error(s)) ===
>gcc/collect2.c:1135:0:████████ld_file_name = find_a_file(&cpath, ld_suffixes[selected_linker], X_OK);
>gcc/collect2.c:1137:0:████████ for `ld' (if native linking) or `TARGET-ld' (if cross).  */
>gcc/collect2.c:1139:0:████████ld_file_name =
>gcc/collect2.c:1140:0:████████    find_a_file(&path, full_ld_suffixes[selected_linker], X_OK);
>
>=== ERROR type #2: there should be exactly one space between function name and parenthesis (2 error(s)) ===
>gcc/collect2.c:1135:34:        ld_file_name = find_a_file(&cpath, ld_suffixes[selected_linker], X_OK);
>gcc/collect2.c:1140:23:            find_a_file(&path, full_ld_suffixes[selected_linker], X_OK);

Thank you. I did not know ./contrib/check_GNU_style.py

PATCH v3 fixes the above style issues and checks both cpath and path
which appears to make more sense (and compatible with clang).

>=== ERROR type #3: trailing operator (1 error(s)) ===
>gcc/collect2.c:1139:21:        ld_file_name =

but I can't fix this one because joining two lines will break the 80-column rule.
>
>Note that I can't approve the patch, please wait for a reviewer that can approve it.
>
>Martin

I'll wait for a reviewer to approve and apply it.

Comments

Martin Liška May 25, 2020, 9:57 a.m. UTC | #1 
On 5/22/20 6:42 AM, Fangrui Song wrote:
> but I can't fix this one because joining two lines will break the 80-column rule.

What about this:

diff --git a/gcc/collect2.c b/gcc/collect2.c
index cc57a20e08b..e5b54b080f7 100644
--- a/gcc/collect2.c
+++ b/gcc/collect2.c
@@ -1138,8 +1138,8 @@ main (int argc, char **argv)
        /* 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);
+	ld_file_name
+	  = find_a_file (&path, full_ld_suffixes[selected_linker], X_OK);
      }
  
  #ifdef REAL_NM_FILE_NAME

Apart from that, the patch is fine.

Martin
Fangrui Song May 29, 2020, 1:10 a.m. UTC | #2 
On 2020-05-25, Martin Liška wrote:
>On 5/22/20 6:42 AM, Fangrui Song wrote:
>>but I can't fix this one because joining two lines will break the 80-column rule.
>
>What about this:
>
>diff --git a/gcc/collect2.c b/gcc/collect2.c
>index cc57a20e08b..e5b54b080f7 100644
>--- a/gcc/collect2.c
>+++ b/gcc/collect2.c
>@@ -1138,8 +1138,8 @@ main (int argc, char **argv)
>       /* 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);
>+	ld_file_name
>+	  = find_a_file (&path, full_ld_suffixes[selected_linker], X_OK);
>     }
> #ifdef REAL_NM_FILE_NAME
>
>Apart from that, the patch is fine.
>
>Martin

Adding people who may be able to approve and commit on my behalf.

This formatting issue seems small enough. Hopefully a maintainer can do
it for me.
Martin Liška June 30, 2020, 12:51 p.m. UTC | #3 
PING^1

On 5/29/20 3:10 AM, Fangrui Song wrote:
> On 2020-05-25, Martin Liška wrote:
>> On 5/22/20 6:42 AM, Fangrui Song wrote:
>>> but I can't fix this one because joining two lines will break the 80-column rule.
>>
>> What about this:
>>
>> diff --git a/gcc/collect2.c b/gcc/collect2.c
>> index cc57a20e08b..e5b54b080f7 100644
>> --- a/gcc/collect2.c
>> +++ b/gcc/collect2.c
>> @@ -1138,8 +1138,8 @@ main (int argc, char **argv)
>>       /* 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);
>> +    ld_file_name
>> +      = find_a_file (&path, full_ld_suffixes[selected_linker], X_OK);
>>     }
>> #ifdef REAL_NM_FILE_NAME
>>
>> Apart from that, the patch is fine.
>>
>> Martin
> 
> Adding people who may be able to approve and commit on my behalf.
> 
> This formatting issue seems small enough. Hopefully a maintainer can do
> it for me.
Fangrui Song June 30, 2020, 3:32 p.m. UTC | #4 
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)?

On Tue, Jun 30, 2020 at 6:04 AM Martin Liška <mliska@suse.cz> wrote:
>
> PING^1
>
> On 5/29/20 3:10 AM, Fangrui Song wrote:
> > On 2020-05-25, Martin Liška wrote:
> >> On 5/22/20 6:42 AM, Fangrui Song wrote:
> >>> but I can't fix this one because joining two lines will break the 80-column rule.
> >>
> >> What about this:
> >>
> >> diff --git a/gcc/collect2.c b/gcc/collect2.c
> >> index cc57a20e08b..e5b54b080f7 100644
> >> --- a/gcc/collect2.c
> >> +++ b/gcc/collect2.c
> >> @@ -1138,8 +1138,8 @@ main (int argc, char **argv)
> >>       /* 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);
> >> +    ld_file_name
> >> +      = find_a_file (&path, full_ld_suffixes[selected_linker], X_OK);
> >>     }
> >> #ifdef REAL_NM_FILE_NAME
> >>
> >> Apart from that, the patch is fine.
> >>
> >> Martin
> >
> > Adding people who may be able to approve and commit on my behalf.
> >
> > This formatting issue seems small enough. Hopefully a maintainer can do
> > it for me.
>
Martin Liška July 1, 2020, 7:13 a.m. UTC | #5 
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

> 
> On Tue, Jun 30, 2020 at 6:04 AM Martin Liška <mliska@suse.cz> wrote:
>>
>> PING^1
>>
>> On 5/29/20 3:10 AM, Fangrui Song wrote:
>>> On 2020-05-25, Martin Liška wrote:
>>>> On 5/22/20 6:42 AM, Fangrui Song wrote:
>>>>> but I can't fix this one because joining two lines will break the 80-column rule.
>>>>
>>>> What about this:
>>>>
>>>> diff --git a/gcc/collect2.c b/gcc/collect2.c
>>>> index cc57a20e08b..e5b54b080f7 100644
>>>> --- a/gcc/collect2.c
>>>> +++ b/gcc/collect2.c
>>>> @@ -1138,8 +1138,8 @@ main (int argc, char **argv)
>>>>        /* 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);
>>>> +    ld_file_name
>>>> +      = find_a_file (&path, full_ld_suffixes[selected_linker], X_OK);
>>>>      }
>>>> #ifdef REAL_NM_FILE_NAME
>>>>
>>>> Apart from that, the patch is fine.
>>>>
>>>> Martin
>>>
>>> Adding people who may be able to approve and commit on my behalf.
>>>
>>> This formatting issue seems small enough. Hopefully a maintainer can do
>>> it for me.
>>
> 
>
Fangrui Song July 2, 2020, 4:59 a.m. UTC | #6 
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.

>>
>>On Tue, Jun 30, 2020 at 6:04 AM Martin Liška <mliska@suse.cz> wrote:
>>>
>>>PING^1
>>>
>>>On 5/29/20 3:10 AM, Fangrui Song wrote:
>>>>On 2020-05-25, Martin Liška wrote:
>>>>>On 5/22/20 6:42 AM, Fangrui Song wrote:
>>>>>>but I can't fix this one because joining two lines will break the 80-column rule.
>>>>>
>>>>>What about this:
>>>>>
>>>>>diff --git a/gcc/collect2.c b/gcc/collect2.c
>>>>>index cc57a20e08b..e5b54b080f7 100644
>>>>>--- a/gcc/collect2.c
>>>>>+++ b/gcc/collect2.c
>>>>>@@ -1138,8 +1138,8 @@ main (int argc, char **argv)
>>>>>       /* 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);
>>>>>+    ld_file_name
>>>>>+      = find_a_file (&path, full_ld_suffixes[selected_linker], X_OK);
>>>>>     }
>>>>>#ifdef REAL_NM_FILE_NAME
>>>>>
>>>>>Apart from that, the patch is fine.
>>>>>
>>>>>Martin
>>>>
>>>>Adding people who may be able to approve and commit on my behalf.
>>>>
>>>>This formatting issue seems small enough. Hopefully a maintainer can do
>>>>it for me.
>>>
>>
>>
>
diff mbox series

Patch

From 1fea80498891db3fef831bd65f231603951e5f14 Mon Sep 17 00:00:00 2001
From: Fangrui Song <maskray@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 a relative path (relative to a COMPILER_PATH
directory or a PATH directory) or an absolute 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/collect2.c      | 32 ++++++++++++++++++++++++--------
 gcc/common.opt      |  4 ++++
 gcc/doc/invoke.texi |  4 ++++
 gcc/opts.c          |  1 +
 4 files changed, 33 insertions(+), 8 deletions(-)

diff --git a/gcc/collect2.c b/gcc/collect2.c
index f8a5ce45994..cc57a20e08b 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 *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
@@ -967,6 +968,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_MAX;
+	  }
 	else if (strncmp (argv[i], "-o", 2) == 0)
 	  {
 	    /* Parse the output filename if it's given so that we can make
@@ -1117,14 +1123,24 @@  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)
+    {
+      ld_file_name = find_a_file (&cpath, use_ld, X_OK);
+      if (ld_file_name == 0)
+	ld_file_name = find_a_file (&path, use_ld, X_OK);
+    }
+  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);
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.27.0.rc0.183.gde8f92d652-goog