| Message ID | 20110207170518.1FCE71C686E@hpgntab-ubiq73.eem.corp.google.com |
|---|---|
| State | New |
| Headers | show |
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 > >
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.
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
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.
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
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