driver: Use <triple>-as/ld as final fallback instead of as/ld for cross

Message ID 20240521121109.3089022-1-syq@gcc.gnu.org
State New
Headers
Series driver: Use <triple>-as/ld as final fallback instead of as/ld for cross |

Checks

Context Check Description
linaro-tcwg-bot/tcwg_gcc_build--master-arm success Testing passed
linaro-tcwg-bot/tcwg_gcc_build--master-aarch64 success Testing passed
linaro-tcwg-bot/tcwg_gcc_check--master-arm success Testing passed
linaro-tcwg-bot/tcwg_gcc_check--master-aarch64 success Testing passed

Commit Message

YunQiang Su May 21, 2024, 12:11 p.m. UTC
  If `find_a_program` cannot find `as/ld` 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 before fallback to native as/ld.

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

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

Comments

Andrew Pinski May 21, 2024, 12:23 p.m. UTC | #1
On Tue, May 21, 2024 at 5:12 AM YunQiang Su <syq@gcc.gnu.org> wrote:
>
> If `find_a_program` cannot find `as/ld` 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 before fallback to native as/ld.
>
> This patch is derivatived from Debian's patch:
>   gcc-search-prefixed-as-ld.diff
>
> gcc
>         * gcc.cc(execute): Looks for <triple>-as/ld before fallback
>         to native as/ld.
> ---
>  gcc/gcc.cc | 21 +++++++++++++++++++++
>  1 file changed, 21 insertions(+)
>
> diff --git a/gcc/gcc.cc b/gcc/gcc.cc
> index 830a4700a87..8a1bdb5e3e2 100644
> --- a/gcc/gcc.cc
> +++ b/gcc/gcc.cc
> @@ -3293,6 +3293,27 @@ 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], "as")
> +                   || !strcmp (commands[0].argv[0], "ld")))
> +       {
> +         string = XNEWVEC (char, strlen (commands[0].argv[0]) + 2
> +                                 + strlen (DEFAULT_REAL_TARGET_MACHINE));
> +         strcpy (string, DEFAULT_REAL_TARGET_MACHINE);
> +         strcat (string, "-");
> +         strcat (string, commands[0].argv[0]);
> +         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);

I think this should be handled under find_a_program instead of
execute. That should simplify things slightly.
You should also most likely use concat here instead of
XNEWVEC/strcpy/strcat which will also simplify the code.
Like string = concat (DEFAULT_REAL_TARGET_MACHINE, "-", commands[0].prog);

I think this should be done for more than just as/ld but also objcopy
(which is used for gsplit-dwarf).
Is there a reason why you are needing to try to execute with
"--version" as an argument here?

Thanks,
Andrew Pinski

> +         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
>
  
YunQiang Su May 21, 2024, 12:42 p.m. UTC | #2
Andrew Pinski <pinskia@gmail.com> 于2024年5月21日周二 20:23写道:
>
> On Tue, May 21, 2024 at 5:12 AM YunQiang Su <syq@gcc.gnu.org> wrote:
> >
> > If `find_a_program` cannot find `as/ld` 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 before fallback to native as/ld.
> >
> > This patch is derivatived from Debian's patch:
> >   gcc-search-prefixed-as-ld.diff
> >
> > gcc
> >         * gcc.cc(execute): Looks for <triple>-as/ld before fallback
> >         to native as/ld.
> > ---
> >  gcc/gcc.cc | 21 +++++++++++++++++++++
> >  1 file changed, 21 insertions(+)
> >
> > diff --git a/gcc/gcc.cc b/gcc/gcc.cc
> > index 830a4700a87..8a1bdb5e3e2 100644
> > --- a/gcc/gcc.cc
> > +++ b/gcc/gcc.cc
> > @@ -3293,6 +3293,27 @@ 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], "as")
> > +                   || !strcmp (commands[0].argv[0], "ld")))
> > +       {
> > +         string = XNEWVEC (char, strlen (commands[0].argv[0]) + 2
> > +                                 + strlen (DEFAULT_REAL_TARGET_MACHINE));
> > +         strcpy (string, DEFAULT_REAL_TARGET_MACHINE);
> > +         strcat (string, "-");
> > +         strcat (string, commands[0].argv[0]);
> > +         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);
>
> I think this should be handled under find_a_program instead of
> execute. That should simplify things slightly.

Maybe. But it seems that they are two different problems.
`find_a_program` won't try to find any as/ld from user path dirs,
such as /usr/bin

My patch tries to resolve the problem: if  `find_a_program` fails to find
any usable ld/as, then let's fallback to /usr/bin/<triple>-as
instead of /usr/bin/as.

Yes, we should also make `find_a_program` look for  <triple>-as
from its search path, while I guess it should be done by another patch.

> You should also most likely use concat here instead of
> XNEWVEC/strcpy/strcat which will also simplify the code.
> Like string = concat (DEFAULT_REAL_TARGET_MACHINE, "-", commands[0].prog);
>
> I think this should be done for more than just as/ld but also objcopy
> (which is used for gsplit-dwarf).
> Is there a reason why you are needing to try to execute with
> "--version" as an argument here?
>

I try to make it possible to fallback to system's ld/as, if
<triple>-as/ld doesn't exist.
With `--version` args, I have a test to <triple>-as/ld.

> Thanks,
> Andrew Pinski
>
> > +         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

diff --git a/gcc/gcc.cc b/gcc/gcc.cc
index 830a4700a87..8a1bdb5e3e2 100644
--- a/gcc/gcc.cc
+++ b/gcc/gcc.cc
@@ -3293,6 +3293,27 @@  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], "as")
+		    || !strcmp (commands[0].argv[0], "ld")))
+	{
+	  string = XNEWVEC (char, strlen (commands[0].argv[0]) + 2
+				  + strlen (DEFAULT_REAL_TARGET_MACHINE));
+	  strcpy (string, DEFAULT_REAL_TARGET_MACHINE);
+	  strcat (string, "-");
+	  strcat (string, commands[0].argv[0]);
+	  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++)