[PATCH v2 1/2] driver: Use <triple>-as/ld/objcopy as final fallback instead of native ones for cross

[PATCH v2 1/2] driver: Use <triple>-as/ld/objcopy as final fallback instead of native ones for cross

[YunQiang Su]

If `find_a_program` cannot find `as/ld/objcopy` and we are a cross toolchain,
the final fallback is `as/ld` of system.  In fact, we can have a try with
<triple>-as/ld/objcopy before fallback to native as/ld/objcopy.

This patch is derivatived from Debian's patch:
  gcc-search-prefixed-as-ld.diff

gcc
	* gcc.cc(execute): Looks for <triple>-as/ld/objcopy before fallback
	to native as/ld/objcopy.
---
 gcc/gcc.cc | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/gcc/gcc.cc b/gcc/gcc.cc
index 830a4700a87..3dc6348d761 100644
--- a/gcc/gcc.cc
+++ b/gcc/gcc.cc
@@ -3293,6 +3293,26 @@ execute (void)
       string = find_a_program(commands[0].prog);
       if (string)
 	commands[0].argv[0] = string;
+      else if (*cross_compile != '0'
+		&& !strcmp (commands[0].argv[0], commands[0].prog)
+		&& (!strcmp (commands[0].prog, "as")
+		    || !strcmp (commands[0].prog, "ld")
+		    || !strcmp (commands[0].prog, "objcopy")))
+	{
+	  string = concat (DEFAULT_REAL_TARGET_MACHINE, "-",
+				commands[0].prog, NULL);
+	  const char *string_args[] = {string, "--version", NULL};
+	  int exit_status = 0;
+	  int err = 0;
+	  const char *errmsg = pex_one (PEX_SEARCH, string,
+			  CONST_CAST (char **, string_args), string,
+			  NULL, NULL, &exit_status, &err);
+	  if (errmsg == NULL && exit_status == 0 && err == 0)
+	    {
+	      commands[0].argv[0] = string;
+	      commands[0].prog = string;
+	    }
+	}
     }
 
   for (n_commands = 1, i = 0; argbuf.iterate (i, &arg); i++)
-- 
2.39.2

[PATCH v2 2/2] driver: Search <triple>-as/ld/objcopy before non-triple ones

[YunQiang Su]

When looking for as/ld/objcopy, `find_a_program/file_at_path` only
try to find the raw name, but won't find the one with <triple>-
prefix.

This patch is derivatived from Debian's patch:
    gcc-search-prefixed-as-ld.diff

gcc
	* gcc.cc(for_each_path): Add more space for <triple>-.
	(file_at_path): Search <triple>-as/ld/objcopy before
	non-triple ones.
---
 gcc/gcc.cc | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/gcc/gcc.cc b/gcc/gcc.cc
index 3dc6348d761..0fa2eafea84 100644
--- a/gcc/gcc.cc
+++ b/gcc/gcc.cc
@@ -2820,6 +2820,8 @@ for_each_path (const struct path_prefix *paths,
 	{
 	  len = paths->max_len + extra_space + 1;
 	  len += MAX (MAX (suffix_len, multi_os_dir_len), multiarch_len);
+	  /* triplet prefix for as, ld.  */
+	  len += MAX (strlen (DEFAULT_REAL_TARGET_MACHINE), multiarch_len) + 2;
 	  path = XNEWVEC (char, len);
 	}
 
@@ -3033,6 +3035,17 @@ file_at_path (char *path, void *data)
   struct file_at_path_info *info = (struct file_at_path_info *) data;
   size_t len = strlen (path);
 
+  /* search for the <triple>-as / -ld / objcopy first.  */
+  if (! strcmp (info->name, "as") || ! strcmp (info->name, "ld")
+	|| ! strcmp (info->name, "objcopy"))
+    {
+      struct file_at_path_info prefix_info = *info;
+      prefix_info.name = concat (DEFAULT_REAL_TARGET_MACHINE, "-",
+				info->name, NULL);
+      prefix_info.name_len = strlen (prefix_info.name);
+      if (file_at_path (path, &prefix_info))
+	return path;
+    }
   memcpy (path + len, info->name, info->name_len);
   len += info->name_len;
 
-- 
2.39.2

Re: [PATCH v2 1/2] driver: Use <triple>-as/ld/objcopy as final fallback instead of native ones for cross

[YunQiang Su]

YunQiang Su <syq@gcc.gnu.org> 于2024年5月22日周三 17:54写道：
>
> If `find_a_program` cannot find `as/ld/objcopy` and we are a cross toolchain,
> the final fallback is `as/ld` of system.  In fact, we can have a try with
> <triple>-as/ld/objcopy before fallback to native as/ld/objcopy.
>
> This patch is derivatived from Debian's patch:
>   gcc-search-prefixed-as-ld.diff
>
> gcc
>         * gcc.cc(execute): Looks for <triple>-as/ld/objcopy before fallback
>         to native as/ld/objcopy.

ping. OK for the trunk?

> ---
>  gcc/gcc.cc | 20 ++++++++++++++++++++
>  1 file changed, 20 insertions(+)
>
> diff --git a/gcc/gcc.cc b/gcc/gcc.cc
> index 830a4700a87..3dc6348d761 100644
> --- a/gcc/gcc.cc
> +++ b/gcc/gcc.cc
> @@ -3293,6 +3293,26 @@ execute (void)
>        string = find_a_program(commands[0].prog);
>        if (string)
>         commands[0].argv[0] = string;
> +      else if (*cross_compile != '0'
> +               && !strcmp (commands[0].argv[0], commands[0].prog)
> +               && (!strcmp (commands[0].prog, "as")
> +                   || !strcmp (commands[0].prog, "ld")
> +                   || !strcmp (commands[0].prog, "objcopy")))
> +       {
> +         string = concat (DEFAULT_REAL_TARGET_MACHINE, "-",
> +                               commands[0].prog, NULL);
> +         const char *string_args[] = {string, "--version", NULL};
> +         int exit_status = 0;
> +         int err = 0;
> +         const char *errmsg = pex_one (PEX_SEARCH, string,
> +                         CONST_CAST (char **, string_args), string,
> +                         NULL, NULL, &exit_status, &err);
> +         if (errmsg == NULL && exit_status == 0 && err == 0)
> +           {
> +             commands[0].argv[0] = string;
> +             commands[0].prog = string;
> +           }
> +       }
>      }
>
>    for (n_commands = 1, i = 0; argbuf.iterate (i, &arg); i++)
> --
> 2.39.2
>

Re: [PATCH v2 1/2] driver: Use <triple>-as/ld/objcopy as final fallback instead of native ones for cross

[Richard Sandiford]

YunQiang Su <syq@gcc.gnu.org> writes:
> If `find_a_program` cannot find `as/ld/objcopy` and we are a cross toolchain,
> the final fallback is `as/ld` of system.  In fact, we can have a try with
> <triple>-as/ld/objcopy before fallback to native as/ld/objcopy.
>
> This patch is derivatived from Debian's patch:
>   gcc-search-prefixed-as-ld.diff

I'm probably making you repeat a previous discussion, sorry, but could
you describe the use case in more detail?  The current approach to
handling cross toolchains has been used for many years.  Presumably
this patch is supporting a different way of organising things,
but I wasn't sure from the description what it was.

AIUI, we currently assume that cross as, ld and objcopy will be
installed under those names in $prefix/$target_alias/bin (aka $tooldir/bin).
E.g.:

   bin/aarch64-elf-as = aarch64-elf/bin/as

GCC should then find as in aarch64-elf/bin.

Is that not true in your case?

To be clear, I'm not saying the patch is wrong.  I'm just trying to
understand why the patch is needed.

Thanks,
Richard

>
> gcc
> 	* gcc.cc(execute): Looks for <triple>-as/ld/objcopy before fallback
> 	to native as/ld/objcopy.
> ---
>  gcc/gcc.cc | 20 ++++++++++++++++++++
>  1 file changed, 20 insertions(+)
>
> diff --git a/gcc/gcc.cc b/gcc/gcc.cc
> index 830a4700a87..3dc6348d761 100644
> --- a/gcc/gcc.cc
> +++ b/gcc/gcc.cc
> @@ -3293,6 +3293,26 @@ execute (void)
>        string = find_a_program(commands[0].prog);
>        if (string)
>  	commands[0].argv[0] = string;
> +      else if (*cross_compile != '0'
> +		&& !strcmp (commands[0].argv[0], commands[0].prog)
> +		&& (!strcmp (commands[0].prog, "as")
> +		    || !strcmp (commands[0].prog, "ld")
> +		    || !strcmp (commands[0].prog, "objcopy")))
> +	{
> +	  string = concat (DEFAULT_REAL_TARGET_MACHINE, "-",
> +				commands[0].prog, NULL);
> +	  const char *string_args[] = {string, "--version", NULL};
> +	  int exit_status = 0;
> +	  int err = 0;
> +	  const char *errmsg = pex_one (PEX_SEARCH, string,
> +			  CONST_CAST (char **, string_args), string,
> +			  NULL, NULL, &exit_status, &err);
> +	  if (errmsg == NULL && exit_status == 0 && err == 0)
> +	    {
> +	      commands[0].argv[0] = string;
> +	      commands[0].prog = string;
> +	    }
> +	}
>      }
>  
>    for (n_commands = 1, i = 0; argbuf.iterate (i, &arg); i++)

Re: [PATCH v2 1/2] driver: Use <triple>-as/ld/objcopy as final fallback instead of native ones for cross

[YunQiang Su]

Richard Sandiford <richard.sandiford@arm.com> 于2024年5月29日周三 05:28写道：
>
> YunQiang Su <syq@gcc.gnu.org> writes:
> > If `find_a_program` cannot find `as/ld/objcopy` and we are a cross toolchain,
> > the final fallback is `as/ld` of system.  In fact, we can have a try with
> > <triple>-as/ld/objcopy before fallback to native as/ld/objcopy.
> >
> > This patch is derivatived from Debian's patch:
> >   gcc-search-prefixed-as-ld.diff
>
> I'm probably making you repeat a previous discussion, sorry, but could
> you describe the use case in more detail?  The current approach to
> handling cross toolchains has been used for many years.  Presumably
> this patch is supporting a different way of organising things,
> but I wasn't sure from the description what it was.
>
> AIUI, we currently assume that cross as, ld and objcopy will be
> installed under those names in $prefix/$target_alias/bin (aka $tooldir/bin).
> E.g.:
>
>    bin/aarch64-elf-as = aarch64-elf/bin/as
>
> GCC should then find as in aarch64-elf/bin.
>
> Is that not true in your case?
>

Yes. This patch is only about the final fallback. I mean aarch64-elf/bin/as
still has higher priority than bin/aarch64-elf-as.

In the current code, we find gas with:
    /prefix/aarch64-elf/bin/as > $PATH/as

And this patch a new one between them:
    /prefix/aarch64-elf/bin/as > $PATH/aarch64-elf-as > $PATH/as

> To be clear, I'm not saying the patch is wrong.  I'm just trying to
> understand why the patch is needed.
>

Yes. If gcc is configured correctly, it is not so useful.
In some case for some lazy user, it may be useful,
for example, the binutils installed into different prefix with libc etc.

For example, binutils is installed into /usr/aarch64-elf/bin, while
libc is installed into /usr/local/aarch64-elf/.

> Thanks,
> Richard
>
> >
> > gcc
> >       * gcc.cc(execute): Looks for <triple>-as/ld/objcopy before fallback
> >       to native as/ld/objcopy.
> > ---
> >  gcc/gcc.cc | 20 ++++++++++++++++++++
> >  1 file changed, 20 insertions(+)
> >
> > diff --git a/gcc/gcc.cc b/gcc/gcc.cc
> > index 830a4700a87..3dc6348d761 100644
> > --- a/gcc/gcc.cc
> > +++ b/gcc/gcc.cc
> > @@ -3293,6 +3293,26 @@ execute (void)
> >        string = find_a_program(commands[0].prog);
> >        if (string)
> >       commands[0].argv[0] = string;
> > +      else if (*cross_compile != '0'
> > +             && !strcmp (commands[0].argv[0], commands[0].prog)
> > +             && (!strcmp (commands[0].prog, "as")
> > +                 || !strcmp (commands[0].prog, "ld")
> > +                 || !strcmp (commands[0].prog, "objcopy")))
> > +     {
> > +       string = concat (DEFAULT_REAL_TARGET_MACHINE, "-",
> > +                             commands[0].prog, NULL);
> > +       const char *string_args[] = {string, "--version", NULL};
> > +       int exit_status = 0;
> > +       int err = 0;
> > +       const char *errmsg = pex_one (PEX_SEARCH, string,
> > +                       CONST_CAST (char **, string_args), string,
> > +                       NULL, NULL, &exit_status, &err);
> > +       if (errmsg == NULL && exit_status == 0 && err == 0)
> > +         {
> > +           commands[0].argv[0] = string;
> > +           commands[0].prog = string;
> > +         }
> > +     }
> >      }
> >
> >    for (n_commands = 1, i = 0; argbuf.iterate (i, &arg); i++)

Re: [PATCH v2 1/2] driver: Use <triple>-as/ld/objcopy as final fallback instead of native ones for cross

[YunQiang Su]

YunQiang Su <syq@gcc.gnu.org> 于2024年5月29日周三 10:02写道：
>
> Richard Sandiford <richard.sandiford@arm.com> 于2024年5月29日周三 05:28写道：
> >
> > YunQiang Su <syq@gcc.gnu.org> writes:
> > > If `find_a_program` cannot find `as/ld/objcopy` and we are a cross toolchain,
> > > the final fallback is `as/ld` of system.  In fact, we can have a try with
> > > <triple>-as/ld/objcopy before fallback to native as/ld/objcopy.
> > >
> > > This patch is derivatived from Debian's patch:
> > >   gcc-search-prefixed-as-ld.diff
> >
> > I'm probably making you repeat a previous discussion, sorry, but could
> > you describe the use case in more detail?  The current approach to
> > handling cross toolchains has been used for many years.  Presumably
> > this patch is supporting a different way of organising things,
> > but I wasn't sure from the description what it was.
> >
> > AIUI, we currently assume that cross as, ld and objcopy will be
> > installed under those names in $prefix/$target_alias/bin (aka $tooldir/bin).
> > E.g.:
> >
> >    bin/aarch64-elf-as = aarch64-elf/bin/as
> >
> > GCC should then find as in aarch64-elf/bin.
> >
> > Is that not true in your case?
> >
>
> Yes. This patch is only about the final fallback. I mean aarch64-elf/bin/as
> still has higher priority than bin/aarch64-elf-as.
>
> In the current code, we find gas with:
>     /prefix/aarch64-elf/bin/as > $PATH/as
>
> And this patch a new one between them:
>     /prefix/aarch64-elf/bin/as > $PATH/aarch64-elf-as > $PATH/as
>
> > To be clear, I'm not saying the patch is wrong.  I'm just trying to
> > understand why the patch is needed.
> >
>
> Yes. If gcc is configured correctly, it is not so useful.
> In some case for some lazy user, it may be useful,
> for example, the binutils installed into different prefix with libc etc.
>
> For example, binutils is installed into /usr/aarch64-elf/bin, while
> libc is installed into /usr/local/aarch64-elf/.
>

Any idea about it? Is it a use case making sense?

Re: [PATCH v2 1/2] driver: Use <triple>-as/ld/objcopy as final fallback instead of native ones for cross

[Richard Sandiford]

YunQiang Su <syq@gcc.gnu.org> writes:
> YunQiang Su <syq@gcc.gnu.org> 于2024年5月29日周三 10:02写道：
>>
>> Richard Sandiford <richard.sandiford@arm.com> 于2024年5月29日周三 05:28写道：
>> >
>> > YunQiang Su <syq@gcc.gnu.org> writes:
>> > > If `find_a_program` cannot find `as/ld/objcopy` and we are a cross toolchain,
>> > > the final fallback is `as/ld` of system.  In fact, we can have a try with
>> > > <triple>-as/ld/objcopy before fallback to native as/ld/objcopy.
>> > >
>> > > This patch is derivatived from Debian's patch:
>> > >   gcc-search-prefixed-as-ld.diff
>> >
>> > I'm probably making you repeat a previous discussion, sorry, but could
>> > you describe the use case in more detail?  The current approach to
>> > handling cross toolchains has been used for many years.  Presumably
>> > this patch is supporting a different way of organising things,
>> > but I wasn't sure from the description what it was.
>> >
>> > AIUI, we currently assume that cross as, ld and objcopy will be
>> > installed under those names in $prefix/$target_alias/bin (aka $tooldir/bin).
>> > E.g.:
>> >
>> >    bin/aarch64-elf-as = aarch64-elf/bin/as
>> >
>> > GCC should then find as in aarch64-elf/bin.
>> >
>> > Is that not true in your case?
>> >
>>
>> Yes. This patch is only about the final fallback. I mean aarch64-elf/bin/as
>> still has higher priority than bin/aarch64-elf-as.
>>
>> In the current code, we find gas with:
>>     /prefix/aarch64-elf/bin/as > $PATH/as
>>
>> And this patch a new one between them:
>>     /prefix/aarch64-elf/bin/as > $PATH/aarch64-elf-as > $PATH/as
>>
>> > To be clear, I'm not saying the patch is wrong.  I'm just trying to
>> > understand why the patch is needed.
>> >
>>
>> Yes. If gcc is configured correctly, it is not so useful.
>> In some case for some lazy user, it may be useful,
>> for example, the binutils installed into different prefix with libc etc.
>>
>> For example, binutils is installed into /usr/aarch64-elf/bin, while
>> libc is installed into /usr/local/aarch64-elf/.
>>
>
> Any idea about it? Is it a use case making sense?

Yeah, I think it makes sense.  GCC and binutils are separate packages.
Users could cherry-pick a GCC installation and a separate binutils
installation rather than bundling them together into a single
toolchain.  And not everyone will have permission to change $tooldir.

So I agree we should support searching the user's path for an
as/ld/etc. based on the tool prefix.  Unfortunately, I don't think
I understand the code & constraints well enough to do a review.

In particular, it seems unfortunate that we need to do a trial
subcommand invocation before committing to the prefixed name.
And, if we continue to search for "as" in the user's path as a fallback,
it's not 100% obvious that "${triple}-as" later in the path should trump
"as" earlier in the path.

In some ways, it seems more consistent to do the replacement without
first doing a trial invocation.  But I don't know whether that would
break existing use cases.  (To be clear, I wouldn't feel comfortable
approving a patch to do that without buy-in from other maintainers.)

Thanks,
Richard

Re: [PATCH v2 1/2] driver: Use <triple>-as/ld/objcopy as final fallback instead of native ones for cross

[YunQiang Su]

Richard Sandiford <richard.sandiford@arm.com> 于2024年6月6日周四 17:54写道：
>
> YunQiang Su <syq@gcc.gnu.org> writes:
> > YunQiang Su <syq@gcc.gnu.org> 于2024年5月29日周三 10:02写道：
> >>
> >> Richard Sandiford <richard.sandiford@arm.com> 于2024年5月29日周三 05:28写道：
> >> >
> >> > YunQiang Su <syq@gcc.gnu.org> writes:
> >> > > If `find_a_program` cannot find `as/ld/objcopy` and we are a cross toolchain,
> >> > > the final fallback is `as/ld` of system.  In fact, we can have a try with
> >> > > <triple>-as/ld/objcopy before fallback to native as/ld/objcopy.
> >> > >
> >> > > This patch is derivatived from Debian's patch:
> >> > >   gcc-search-prefixed-as-ld.diff
> >> >
> >> > I'm probably making you repeat a previous discussion, sorry, but could
> >> > you describe the use case in more detail?  The current approach to
> >> > handling cross toolchains has been used for many years.  Presumably
> >> > this patch is supporting a different way of organising things,
> >> > but I wasn't sure from the description what it was.
> >> >
> >> > AIUI, we currently assume that cross as, ld and objcopy will be
> >> > installed under those names in $prefix/$target_alias/bin (aka $tooldir/bin).
> >> > E.g.:
> >> >
> >> >    bin/aarch64-elf-as = aarch64-elf/bin/as
> >> >
> >> > GCC should then find as in aarch64-elf/bin.
> >> >
> >> > Is that not true in your case?
> >> >
> >>
> >> Yes. This patch is only about the final fallback. I mean aarch64-elf/bin/as
> >> still has higher priority than bin/aarch64-elf-as.
> >>
> >> In the current code, we find gas with:
> >>     /prefix/aarch64-elf/bin/as > $PATH/as
> >>
> >> And this patch a new one between them:
> >>     /prefix/aarch64-elf/bin/as > $PATH/aarch64-elf-as > $PATH/as
> >>
> >> > To be clear, I'm not saying the patch is wrong.  I'm just trying to
> >> > understand why the patch is needed.
> >> >
> >>
> >> Yes. If gcc is configured correctly, it is not so useful.
> >> In some case for some lazy user, it may be useful,
> >> for example, the binutils installed into different prefix with libc etc.
> >>
> >> For example, binutils is installed into /usr/aarch64-elf/bin, while
> >> libc is installed into /usr/local/aarch64-elf/.
> >>
> >
> > Any idea about it? Is it a use case making sense?
>
> Yeah, I think it makes sense.  GCC and binutils are separate packages.
> Users could cherry-pick a GCC installation and a separate binutils
> installation rather than bundling them together into a single
> toolchain.  And not everyone will have permission to change $tooldir.
>
> So I agree we should support searching the user's path for an
> as/ld/etc. based on the tool prefix.  Unfortunately, I don't think
> I understand the code & constraints well enough to do a review.
>
> In particular, it seems unfortunate that we need to do a trial
> subcommand invocation before committing to the prefixed name.
> And, if we continue to search for "as" in the user's path as a fallback,
> it's not 100% obvious that "${triple}-as" later in the path should trump
> "as" earlier in the path.
>
> In some ways, it seems more consistent to do the replacement without
> first doing a trial invocation.  But I don't know whether that would
> break existing use cases.  (To be clear, I wouldn't feel comfortable

Yes. This is also my worry as some users may set $PATH manually
to a path which contains target `as`, such as
   export PATH="/usr/aarch64-linux-gnu/bin:$PATH"

> approving a patch to do that without buy-in from other maintainers.)
>
> Thanks,
> Richard