Patchwork [google] Auto-detect suitable default behaviour for prefix canonicalization

login
register
mail settings
Submitter Simon Baldwin
Date Feb. 7, 2011, 5:05 p.m.
Message ID <20110207170518.1FCE71C686E@hpgntab-ubiq73.eem.corp.google.com>
Download mbox | patch
Permalink /patch/82119/
State New
Headers show

Comments

Simon Baldwin - Feb. 7, 2011, 5:05 p.m.
Auto-detect suitable default behaviour for prefix canonicalization.

Current gcc offers -no-canonical-prefixes to turn off realpath() for prefixes
generated from the path used to address the gcc driver.  This allows gcc to
work in "symlink farm" installations, where every file in gcc is actually a
symlink to its real contents.  However, the flag has to be given explicitly.
If not, the default is to use realpath() to create prefixes and the result
is usually failure to find cc1[plus], f951, etc.

This patch adds a check for a file as a way to auto-detect whether prefix
canonicalization is appropriate or not.  Detection can be overridden by
using the -[no-]canonical-prefixes flags.

The patch also completes the fix for PR/29931, adding code that covers the
unadopted portion of this PR's attached patch.

Primarily needed for google/integration.  OK?

Secondarily, equally applicable to trunk.  OK there also?

gcc/ChangeLog.google:
2011-02-07  Simon Baldwin  <simonb@google.com>

	PR driver/29931
	* doc/invoke.texi: Adjust -[no-]canonical-prefixes documentation.
	* gcc.c (display_help): Help text for -[no-]canonical-prefixes.
	(driver_handle_option): Ignore OPT_canonical_prefixes.
	(process_command): Handle OPT_[no_]canonical_prefixes, auto-detect
	suitable default prefix canonicalization mode.
	* common.opt (canonical-prefixes): New flag.

Google ref: 40029, 38719
Diego Novillo - Feb. 7, 2011, 5:27 p.m.
On Mon, Feb 7, 2011 at 12:05, Simon Baldwin <simonb@google.com> wrote:
> Auto-detect suitable default behaviour for prefix canonicalization.
>
> Current gcc offers -no-canonical-prefixes to turn off realpath() for prefixes
> generated from the path used to address the gcc driver.  This allows gcc to
> work in "symlink farm" installations, where every file in gcc is actually a
> symlink to its real contents.  However, the flag has to be given explicitly.
> If not, the default is to use realpath() to create prefixes and the result
> is usually failure to find cc1[plus], f951, etc.
>
> This patch adds a check for a file as a way to auto-detect whether prefix
> canonicalization is appropriate or not.  Detection can be overridden by
> using the -[no-]canonical-prefixes flags.
>
> The patch also completes the fix for PR/29931, adding code that covers the
> unadopted portion of this PR's attached patch.
>
> Primarily needed for google/integration.  OK?

OK.

> Secondarily, equally applicable to trunk.  OK there also?

Added Joseph to the CC list for his review.


Diego.

>
> gcc/ChangeLog.google:
> 2011-02-07  Simon Baldwin  <simonb@google.com>
>
>        PR driver/29931
>        * doc/invoke.texi: Adjust -[no-]canonical-prefixes documentation.
>        * gcc.c (display_help): Help text for -[no-]canonical-prefixes.
>        (driver_handle_option): Ignore OPT_canonical_prefixes.
>        (process_command): Handle OPT_[no_]canonical_prefixes, auto-detect
>        suitable default prefix canonicalization mode.
>        * common.opt (canonical-prefixes): New flag.
>
> Google ref: 40029, 38719
>
>
> Index: gcc/doc/invoke.texi
> ===================================================================
> --- gcc/doc/invoke.texi (revision 169786)
> +++ gcc/doc/invoke.texi (working copy)
> @@ -1348,11 +1348,20 @@ gcc -c -Q -O2 --help=optimizers > /tmp/O
>  diff /tmp/O2-opts /tmp/O3-opts | grep enabled
>  @end smallexample
>
> +@item -canonical-prefixes
> +@opindex canonical-prefixes
> +Always expand any symbolic links, resolve references to @samp{/../}
> +or @samp{/./}, and make the path absolute when generating a relative
> +prefix.
> +
>  @item -no-canonical-prefixes
>  @opindex no-canonical-prefixes
> -Do not expand any symbolic links, resolve references to @samp{/../}
> +Never expand any symbolic links, resolve references to @samp{/../}
>  or @samp{/./}, or make the path absolute when generating a relative
> -prefix.
> +prefix.  If neither @option{-canonical-prefixes} nor
> +@option{-canonical-prefixes} is given, GCC tries to set an appropriate
> +default by looking for a target-specific subdirectory alongside the
> +directory containing the compiler driver.
>
>  @item --version
>  @opindex version
> Index: gcc/gcc.c
> ===================================================================
> --- gcc/gcc.c   (revision 169786)
> +++ gcc/gcc.c   (working copy)
> @@ -2937,7 +2937,7 @@ display_help (void)
>   fputs (_("  -save-temps              Do not delete intermediate files\n"), stdout);
>   fputs (_("  -save-temps=<arg>        Do not delete intermediate files\n"), stdout);
>   fputs (_("\
> -  -no-canonical-prefixes   Do not canonicalize paths when building relative\n\
> +  -[no-]canonical-prefixes Specify the path canonicalization for relative\n\
>                            prefixes to other gcc components\n"), stdout);
>   fputs (_("  -pipe                    Use pipes rather than intermediate files\n"), stdout);
>   fputs (_("  -time                    Time the execution of each subprocess\n"), stdout);
> @@ -3346,6 +3346,7 @@ driver_handle_option (struct gcc_options
>                     decoded->orig_option_with_args_text);
>       break;
>
> +    case OPT_canonical_prefixes:
>     case OPT_no_canonical_prefixes:
>       /* Already handled as a special case, so ignored here.  */
>       do_save = false;
> @@ -3523,20 +3524,47 @@ process_command (unsigned int decoded_op
>        }
>     }
>
> -  /* Handle any -no-canonical-prefixes flag early, to assign the function
> +  /* Handle any -[no-]canonical-prefixes flags early, to assign the function
>      that builds relative prefixes.  This function creates default search
>      paths that are needed later in normal option handling.  */
>
>   for (j = 1; j < decoded_options_count; j++)
>     {
> -      if (decoded_options[j].opt_index == OPT_no_canonical_prefixes)
> -       {
> -         get_relative_prefix = make_relative_prefix_ignore_links;
> -         break;
> -       }
> +      if (decoded_options[j].opt_index == OPT_canonical_prefixes)
> +       get_relative_prefix = make_relative_prefix;
> +      else if (decoded_options[j].opt_index == OPT_no_canonical_prefixes)
> +       get_relative_prefix = make_relative_prefix_ignore_links;
>     }
>   if (! get_relative_prefix)
> -    get_relative_prefix = make_relative_prefix;
> +    {
> +      /* If no canonical prefixes flags, try to set a suitable default.
> +
> +        If the gcc driver is a symlink we differentiate two cases: a single
> +        symlink to the driver needing canonicalization; and a symlink farm
> +        with every file in the gcc installation a symlink to its actual
> +        content, for which we don't want canonicalization.  To choose, we
> +        check for a target-specific file relative to the canonicalized path
> +        to the gcc driver.  If found, assume canonicalizing is appropriate,
> +        otherwise don't canonicalize.
> +
> +        If the driver is not a symlink, make_relative_prefix_ignore_links
> +        and make_relative_prefix act equivalently.  */
> +
> +      char *search_prefix = make_relative_prefix (decoded_options[0].arg,
> +                                                 standard_bindir_prefix,
> +                                                 standard_exec_prefix);
> +      if (search_prefix)
> +       {
> +         char *search = concat (search_prefix, spec_machine, NULL);
> +         if (access (search, R_OK) == 0)
> +           get_relative_prefix = make_relative_prefix;
> +         free (search);
> +         free (search_prefix);
> +       }
> +
> +      if (! get_relative_prefix)
> +       get_relative_prefix = make_relative_prefix_ignore_links;
> +    }
>
>   /* Set up the default search paths.  If there is no GCC_EXEC_PREFIX,
>      see if we can create it from the pathname specified in
> Index: gcc/common.opt
> ===================================================================
> --- gcc/common.opt      (revision 169786)
> +++ gcc/common.opt      (working copy)
> @@ -272,6 +272,9 @@ Driver Joined Alias(L)
>  -no-canonical-prefixes
>  Driver Alias(no-canonical-prefixes)
>
> +-canonical-prefixes
> +Driver Alias(canonical-prefixes)
> +
>  -no-standard-libraries
>  Driver Alias(nostdlib)
>
> @@ -2205,6 +2208,9 @@ Driver
>  no-canonical-prefixes
>  Driver
>
> +canonical-prefixes
> +Driver
> +
>  nodefaultlibs
>  Driver
>
>
Joseph S. Myers - Feb. 8, 2011, 6:09 p.m.
On Mon, 7 Feb 2011, Simon Baldwin wrote:

> +      char *search_prefix = make_relative_prefix (decoded_options[0].arg,
> +						  standard_bindir_prefix,
> +						  standard_exec_prefix);
> +      if (search_prefix)
> +	{
> +	  char *search = concat (search_prefix, spec_machine, NULL);
> +	  if (access (search, R_OK) == 0)
> +	    get_relative_prefix = make_relative_prefix;

Are you sure this will be present in a native installation?  As I 
understand it you're looking for $exec_prefix/$target_noncanonical, and I 
don't see that in my native installs.

In general the new option seems safer than the autodetection; certainly I 
don't think the autodetection is safe for 4.6 at this stage.  So it may be 
useful to split this into one patch for the new option, and then a 
separate follow-up patch to autodetect things.
Simon Baldwin - Feb. 9, 2011, 10:43 a.m.
On 8 February 2011 19:09, Joseph S. Myers <joseph@codesourcery.com> wrote:
>
> On Mon, 7 Feb 2011, Simon Baldwin wrote:
>
> > +      char *search_prefix = make_relative_prefix (decoded_options[0].arg,
> > +                                               standard_bindir_prefix,
> > +                                               standard_exec_prefix);
> > +      if (search_prefix)
> > +     {
> > +       char *search = concat (search_prefix, spec_machine, NULL);
> > +       if (access (search, R_OK) == 0)
> > +         get_relative_prefix = make_relative_prefix;
>
> Are you sure this will be present in a native installation?  As I
> understand it you're looking for $exec_prefix/$target_noncanonical, and I
> don't see that in my native installs.

Thanks for the note.

Do you have any suggestions offhand for specific ways to handle this
case?  I wanted to use find_a_file here to better match what the rest
of the driver does, but find_a_file relies on global variables that
have not yet been set.

--S

--
Google UK Limited | Registered Office: Belgrave House, 76 Buckingham
Palace Road, London SW1W 9TQ | Registered in England Number: 3977902
Joseph S. Myers - Feb. 9, 2011, 5:35 p.m.
On Wed, 9 Feb 2011, Simon Baldwin wrote:

> On 8 February 2011 19:09, Joseph S. Myers <joseph@codesourcery.com> wrote:
> >
> > On Mon, 7 Feb 2011, Simon Baldwin wrote:
> >
> > > +      char *search_prefix = make_relative_prefix (decoded_options[0].arg,
> > > +                                               standard_bindir_prefix,
> > > +                                               standard_exec_prefix);
> > > +      if (search_prefix)
> > > +     {
> > > +       char *search = concat (search_prefix, spec_machine, NULL);
> > > +       if (access (search, R_OK) == 0)
> > > +         get_relative_prefix = make_relative_prefix;
> >
> > Are you sure this will be present in a native installation?  As I
> > understand it you're looking for $exec_prefix/$target_noncanonical, and I
> > don't see that in my native installs.
> 
> Thanks for the note.
> 
> Do you have any suggestions offhand for specific ways to handle this
> case?  I wanted to use find_a_file here to better match what the rest
> of the driver does, but find_a_file relies on global variables that
> have not yet been set.

I don't have any suggestions here (presumably those global variable values 
will depend on the choice made by this code so there isn't any easy 
reordering) beyond the point I made earlier that this patch has a 
straightforward part, adding a new option, and a risky (and it now seems 
tricky to get right) part, adding autodetection, and separating the two 
would be useful.
Simon Baldwin - Feb. 10, 2011, 5:36 p.m.
On 9 February 2011 18:35, Joseph S. Myers <joseph@codesourcery.com> wrote:
>
> On Wed, 9 Feb 2011, Simon Baldwin wrote:
>
> > On 8 February 2011 19:09, Joseph S. Myers <joseph@codesourcery.com> wrote:
> > >
> > > On Mon, 7 Feb 2011, Simon Baldwin wrote:
> > >
> > > > +      char *search_prefix = make_relative_prefix (decoded_options[0].arg,
> > > > +                                               standard_bindir_prefix,
> > > > +                                               standard_exec_prefix);
> > > > +      if (search_prefix)
> > > > +     {
> > > > +       char *search = concat (search_prefix, spec_machine, NULL);
> > > > +       if (access (search, R_OK) == 0)
> > > > +         get_relative_prefix = make_relative_prefix;
> > >
> > > Are you sure this will be present in a native installation?  As I
> > > understand it you're looking for $exec_prefix/$target_noncanonical, and I
> > > don't see that in my native installs.
> >
> > Thanks for the note.
> >
> > Do you have any suggestions offhand for specific ways to handle this
> > case?  I wanted to use find_a_file here to better match what the rest
> > of the driver does, but find_a_file relies on global variables that
> > have not yet been set.
>
> I don't have any suggestions here (presumably those global variable values
> will depend on the choice made by this code so there isn't any easy
> reordering) beyond the point I made earlier that this patch has a
> straightforward part, adding a new option, and a risky (and it now seems
> tricky to get right) part, adding autodetection, and separating the two
> would be useful.

The more I think about it, the more cases I can come up with where
autodetection might be inaccurate.  I'm not sure it is possible to do
it reliably for every installation.  Maybe a stable default set by
either a configure flag or in a specs file would be preferable.

--
Google UK Limited | Registered Office: Belgrave House, 76 Buckingham
Palace Road, London SW1W 9TQ | Registered in England Number: 3977902

Patch

Index: gcc/doc/invoke.texi
===================================================================
--- gcc/doc/invoke.texi	(revision 169786)
+++ gcc/doc/invoke.texi	(working copy)
@@ -1348,11 +1348,20 @@  gcc -c -Q -O2 --help=optimizers > /tmp/O
 diff /tmp/O2-opts /tmp/O3-opts | grep enabled
 @end smallexample
 
+@item -canonical-prefixes
+@opindex canonical-prefixes
+Always expand any symbolic links, resolve references to @samp{/../}
+or @samp{/./}, and make the path absolute when generating a relative
+prefix.
+
 @item -no-canonical-prefixes
 @opindex no-canonical-prefixes
-Do not expand any symbolic links, resolve references to @samp{/../}
+Never expand any symbolic links, resolve references to @samp{/../}
 or @samp{/./}, or make the path absolute when generating a relative
-prefix.
+prefix.  If neither @option{-canonical-prefixes} nor
+@option{-canonical-prefixes} is given, GCC tries to set an appropriate
+default by looking for a target-specific subdirectory alongside the
+directory containing the compiler driver.
 
 @item --version
 @opindex version
Index: gcc/gcc.c
===================================================================
--- gcc/gcc.c	(revision 169786)
+++ gcc/gcc.c	(working copy)
@@ -2937,7 +2937,7 @@  display_help (void)
   fputs (_("  -save-temps              Do not delete intermediate files\n"), stdout);
   fputs (_("  -save-temps=<arg>        Do not delete intermediate files\n"), stdout);
   fputs (_("\
-  -no-canonical-prefixes   Do not canonicalize paths when building relative\n\
+  -[no-]canonical-prefixes Specify the path canonicalization for relative\n\
                            prefixes to other gcc components\n"), stdout);
   fputs (_("  -pipe                    Use pipes rather than intermediate files\n"), stdout);
   fputs (_("  -time                    Time the execution of each subprocess\n"), stdout);
@@ -3346,6 +3346,7 @@  driver_handle_option (struct gcc_options
 		     decoded->orig_option_with_args_text);
       break;
 
+    case OPT_canonical_prefixes:
     case OPT_no_canonical_prefixes:
       /* Already handled as a special case, so ignored here.  */
       do_save = false;
@@ -3523,20 +3524,47 @@  process_command (unsigned int decoded_op
 	}
     }
 
-  /* Handle any -no-canonical-prefixes flag early, to assign the function
+  /* Handle any -[no-]canonical-prefixes flags early, to assign the function
      that builds relative prefixes.  This function creates default search
      paths that are needed later in normal option handling.  */
 
   for (j = 1; j < decoded_options_count; j++)
     {
-      if (decoded_options[j].opt_index == OPT_no_canonical_prefixes)
-	{
-	  get_relative_prefix = make_relative_prefix_ignore_links;
-	  break;
-	}
+      if (decoded_options[j].opt_index == OPT_canonical_prefixes)
+	get_relative_prefix = make_relative_prefix;
+      else if (decoded_options[j].opt_index == OPT_no_canonical_prefixes)
+	get_relative_prefix = make_relative_prefix_ignore_links;
     }
   if (! get_relative_prefix)
-    get_relative_prefix = make_relative_prefix;
+    {
+      /* If no canonical prefixes flags, try to set a suitable default.
+
+	 If the gcc driver is a symlink we differentiate two cases: a single
+	 symlink to the driver needing canonicalization; and a symlink farm
+	 with every file in the gcc installation a symlink to its actual
+	 content, for which we don't want canonicalization.  To choose, we
+	 check for a target-specific file relative to the canonicalized path
+	 to the gcc driver.  If found, assume canonicalizing is appropriate,
+	 otherwise don't canonicalize.
+
+	 If the driver is not a symlink, make_relative_prefix_ignore_links
+	 and make_relative_prefix act equivalently.  */
+
+      char *search_prefix = make_relative_prefix (decoded_options[0].arg,
+						  standard_bindir_prefix,
+						  standard_exec_prefix);
+      if (search_prefix)
+	{
+	  char *search = concat (search_prefix, spec_machine, NULL);
+	  if (access (search, R_OK) == 0)
+	    get_relative_prefix = make_relative_prefix;
+	  free (search);
+	  free (search_prefix);
+	}
+
+      if (! get_relative_prefix)
+	get_relative_prefix = make_relative_prefix_ignore_links;
+    }
 
   /* Set up the default search paths.  If there is no GCC_EXEC_PREFIX,
      see if we can create it from the pathname specified in
Index: gcc/common.opt
===================================================================
--- gcc/common.opt	(revision 169786)
+++ gcc/common.opt	(working copy)
@@ -272,6 +272,9 @@  Driver Joined Alias(L)
 -no-canonical-prefixes
 Driver Alias(no-canonical-prefixes)
 
+-canonical-prefixes
+Driver Alias(canonical-prefixes)
+
 -no-standard-libraries
 Driver Alias(nostdlib)
 
@@ -2205,6 +2208,9 @@  Driver
 no-canonical-prefixes
 Driver
 
+canonical-prefixes
+Driver
+
 nodefaultlibs
 Driver