Re: PATCH: Correct path relocation logic

[Richard Sandiford <richard at codesourcery dot com>]

Mark Mitchell <mark@codesourcery.com> writes:
> This patch corrects the patch relocation performed in c-incpath.c.
> The purpose of this logic is to adjust header file search paths when
> the compiler installation is relocated.  However, this logic was only
> checking for paths inside $prefix/lib/gcc, when, really, it should be
> checking for paths inside $prefix.
>
> Tested on x86_64-unknown-linux-gnu, with the usual tests, and by
> looking at the output of -v for a relocated compiler.  Applied on the
> mainline.
...
>    -DPREFIX=\"$(prefix)\" \
> +  -DSTANDARD_EXEC_PREFIX=\"$(libdir)/gcc\" \
...
> -	      /* If the compiler is relocated, and this is a configured 
> -		 prefix relative path, then we use gcc_exec_prefix instead 
> -		 of the configured prefix.  */
> -	      str = concat (gcc_exec_prefix, p->fname
> -			      + cpp_PREFIX_len, NULL);
> + 	      static const char *relocated_prefix;
> +	      /* If this path starts with the configure-time prefix, 
> +		 but the compiler has been relocated, replace it 
> +		 with the run-time prefix.  The run-time exec prefix
> +		 is GCC_EXEC_PREFIX.  Compute the path from there back
> +		 to the toplevel prefix.  */
> +	      if (!relocated_prefix)
> +		{
> +		  char *dummy;
> +		  /* Make relative prefix expects the first argument
> +		     to be a program, not a directory.  */
> +		  dummy = concat (gcc_exec_prefix, "dummy", NULL);
> +		  relocated_prefix 
> +		    = make_relative_prefix (dummy,
> +					    cpp_EXEC_PREFIX,
> +					    cpp_PREFIX);
> +		}
> +	      str = concat (relocated_prefix,
> +			    p->fname + cpp_PREFIX_len, 
> +			    NULL);

This causes a regression for me.  make_relative_prefix treats
directory components as "everything up to and including the next
directory separators", and in this call, cpp_PREFIX is "$prefix"
and cpp_EXEC_PREFIX is "$prefix/lib/gcc".  So we have:

    cpp_PREFIX = "/foo/bar"
    cpp_EXEC_PREFIX = "/foo/bar/lib/gcc"
    dummy = "/frob/a/b/c/dummy"

or, alternatively:

    cpp_PREFIX = "/foo/bar/"
    cpp_EXEC_PREFIX = "/foo/bar//lib/gcc"
    dummy = "/frob/a/b/c/dummy"

In these cases, only the "/" and "foo/" components of cpp_PREFIX
and cpp_EXEC_PREFIX match, so make_relative_prefix will return:

    "/frob/a/b/c/../../../../bar/lib/gcc"

rather than the expected:

    "/frob/a/b/c/../../../lib/gcc"

So if you use --with-gxx-includedir='${prefix}'/foo,
and then move the installed toolchain, g++ will only be able
to find the C++ headers if the new installation directory is
also called "bar".

I think make_relative_prefix's behaviour is by design (it copes,
for example, with cases where "prefix" is actually a binary) and
other users make sure that directories are terminated with
directory separators.

Bootstrapped & regression-tested on x86_64-linux-gnu.  Also tested on
the 4.1 vxworks port that originally showed the problem.  I also build
a mipsel-elf compiler before and after the patch, configured with
--with-gxx-include-dir='${prefix}'/foo, and verified that while the
before compiler couldn't compile C++ hello world from a new location,
the after compiler could.  OK to install?

Richard


gcc/
	* Makefile.in (PREPROCESSOR_DEFINES): Add directory terminators
	to PREFIX and STANDARD_PREFIX.

Index: gcc/Makefile.in
===================================================================
--- gcc/Makefile.in	(revision 165665)
+++ gcc/Makefile.in	(working copy)
@@ -3033,8 +3033,8 @@ PREPROCESSOR_DEFINES = \
   -DLOCAL_INCLUDE_DIR=\"$(local_includedir)\" \
   -DCROSS_INCLUDE_DIR=\"$(CROSS_SYSTEM_HEADER_DIR)\" \
   -DTOOL_INCLUDE_DIR=\"$(gcc_tooldir)/include\" \
-  -DPREFIX=\"$(prefix)\" \
-  -DSTANDARD_EXEC_PREFIX=\"$(libdir)/gcc\" \
+  -DPREFIX=\"$(prefix)/\" \
+  -DSTANDARD_EXEC_PREFIX=\"$(libdir)/gcc/\" \
   @TARGET_SYSTEM_ROOT_DEFINE@
 
 cppdefault.o: cppdefault.c $(CONFIG_H) $(SYSTEM_H) coretypes.h $(TM_H) \