[trunk] RFA: translate built-in include paths for sysroot

[cgd, 2011-06-26 06:08:45]

The setup:

Configuring a toolchain targeting x86-64 GNU Linux (Ubuntu Lucid), as a
cross-compiler.  Using a sysroot to provide the Lucid headers+libraries,
with the sysroot path being within the GCC install tree.  Want to use the
Lucid system libstdc++ and headers, which means that I'm not
building/installing libstdc++-v3.

So, configuring with:
  --with-sysroot="$SYSROOT"
  --disable-libstdc++-v3 \
  --with-gxx-include-dir="$SYSROOT/usr/include/c++/4.4" \
(among other options).

Hoping to support two usage models with this configuration, w.r.t. use of
the sysroot:

(1) somebody installs the sysroot in the normal location relative to the
GCC install, and relocates the whole bundle (sysroot+GCC).  This works
great AFAICT, GCC finds its includes (including the C++ includes) thanks
to the add_standard_paths iprefix handling.

(2) somebody installs the sysroot in a non-standard location, and uses
--sysroot to try to access it.  This works fine for the C headers, but
doesn't work.

For the C headers, add_standard_paths prepends the sysroot location to
the /usr/include path (since that's what's specified in cppdefault.c for
that path).  It doesn't do the same for the C++ include path, though
(again, as specified in cppdefault.c).

add_standard_paths doesn't attempt to relocate built-in include paths that
start with the compiled-in sysroot location (e.g., the g++ include dir, in
this case).  This isn't surprising really: normally you either prepend the
sysroot location or you don't (as specified by cppdefault.c); none of the
built-in paths normally *start* with the sysroot location and need to be
relocated.  However, in this odd-ball case of trying to use the C++ headers
from the sysroot, one of the paths *does* need to be relocated in this way.


The following patch adds code to add_standard_paths to do the sysroot
translation (for oddball configurations like mine).


Tested before/after on a x86_64-unknown-linux-gnu system, with
--enable-languages=all, RUNTESTFLAGS='--target_board=unix\{-m32,-m64\}'.
No changes in test results.  (!! usually there are some, only dates this time!)
Also built my special cross configuration, that worked as expected.


OK for trunk?



chris
---
[gcc/ChangeLog]
2011-06-25  Chris Demetriou  <cgd@google.com>

	* cppdefault.h (cpp_TARGET_SYSTEM_ROOT): New variable.
	(cpp_TARGET_SYSTEM_ROOT_len): Likewise.
	* cppdefault.c (cpp_TARGET_SYSTEM_ROOT): Likewise.
	(cpp_TARGET_SYSTEM_ROOT_len): Likewise.
	* incpath.c (add_standard_paths): Relocate paths that start
	with the compiled-in sysroot path, if a sysroot is given.

Index: incpath.c
===================================================================
--- incpath.c	(revision 175395)
+++ incpath.c	(working copy)
@@ -133,6 +133,30 @@ add_standard_paths (const char *sysroot, const cha
   int relocated = cpp_relocated();
   size_t len;
 
+  if (sysroot && (len = cpp_TARGET_SYSTEM_ROOT_len) != 0)
+    {
+      /* Look for directories that start with the compiled-in sysroot prefix.
+        "Translate" them, i.e. replace /usr/local/target/sys-root... with
+	 SYSROOT and search them first.  */
+      for (p = cpp_include_defaults; p->fname; p++)
+	{
+	  if (!p->cplusplus || cxx_stdinc)
+	    {
+              /* If we're going to add the sysroot to a path, then it
+                 is not expected to start with the path to the sysroot.  */
+	      if (p->add_sysroot)
+		continue;
+	      if (!filename_ncmp (p->fname, cpp_TARGET_SYSTEM_ROOT, len))
+		{
+		  char *str = concat (sysroot, p->fname + len, NULL);
+		  if (p->multilib && imultilib)
+		    str = concat (str, dir_separator_str, imultilib, NULL);
+		  add_path (str, SYSTEM, p->cxx_aware, false);
+		}
+	    }
+	}
+    }
+
   if (iprefix && (len = cpp_GCC_INCLUDE_DIR_len) != 0)
     {
       /* Look for directories that start with the standard prefix.
Index: cppdefault.c
===================================================================
--- cppdefault.c	(revision 175395)
+++ cppdefault.c	(working copy)
@@ -117,6 +117,15 @@ const char cpp_EXEC_PREFIX[] = STANDARD_EXEC_PREFI
 /* This value is set by cpp_relocated at runtime */
 const char *gcc_exec_prefix;
 
+/* The configured target system root (sysroot).  */
+#ifdef TARGET_SYSTEM_ROOT
+const char cpp_TARGET_SYSTEM_ROOT[] = TARGET_SYSTEM_ROOT;
+const size_t cpp_TARGET_SYSTEM_ROOT_len = sizeof TARGET_SYSTEM_ROOT - 1;
+#else
+const char cpp_TARGET_SYSTEM_ROOT[] = "";
+const size_t cpp_TARGET_SYSTEM_ROOT_len = 0;
+#endif
+
 /* Return true if the toolchain is relocated.  */
 bool
 cpp_relocated (void)
Index: cppdefault.h
===================================================================
--- cppdefault.h	(revision 175395)
+++ cppdefault.h	(working copy)
@@ -63,6 +63,11 @@ extern const char cpp_EXEC_PREFIX[];
 /* The run-time execution prefix.  This is typically the lib/gcc
    subdirectory of the actual installation.  */
 extern const char *gcc_exec_prefix;
+/* The configure-time target system root (sysroot) directory, or empty
+   string if none.  */
+extern const char cpp_TARGET_SYSTEM_ROOT[];
+/* The length of the configure-time target system root directory.  */
+extern const size_t cpp_TARGET_SYSTEM_ROOT_len;
 
 /* Return true if the toolchain is relocated.  */
 bool cpp_relocated (void);

--
This patch is available for review at http://codereview.appspot.com/4641076

[joseph_codesourcery.com, 2011-06-26 14:28:16]

On Sat, 25 Jun 2011, Chris Demetriou wrote:

> For the C headers, add_standard_paths prepends the sysroot location to
> the /usr/include path (since that's what's specified in cppdefault.c for
> that path).  It doesn't do the same for the C++ include path, though
> (again, as specified in cppdefault.c).

It seems to me that what's really wanted here is to change the add_sysroot 
flag for the C++ path (all of the C++ paths?), rather than adding special 
code to detect paths starting with the sysroot and reinterpret them.  And 
so making configure detect when the --with-gxx-include-dir setting starts 
with the sysroot and adjusting the flag accordingly in that case would be 
a cleaner solution - that way it would be obvious that the semantics of 
the relocation are exactly the same as for other sysrooted paths, whereas 
it isn't when the relocation goes through different code paths in the 
compiler.

-- 
Joseph S. Myers
joseph@codesourcery.com

[cgd, 2011-06-26 17:38:24]

On Sun, Jun 26, 2011 at 07:28, Joseph S. Myers <joseph@codesourcery.com> wrote:
> It seems to me that what's really wanted here is to change the add_sysroot
> flag for the C++ path (all of the C++ paths?), rather than adding special
> code to detect paths starting with the sysroot and reinterpret them.

I considered doing that, I wasn't sure what (if any) implications that
would have on the way gcc normally builds + configures / how it works
when other people use flags like --with-gxx-include-dir.

I couldn't think of *harm*, but it seemed to me that my change was
least likely to cause harm to existing working paths.
(That doesn't mean that it's the right change, just the one that I
thought I could understand best.  8-)


> And
> so making configure detect when the --with-gxx-include-dir setting starts
> with the sysroot and adjusting the flag accordingly in that case would be
> a cleaner solution - that way it would be obvious that the semantics of
> the relocation are exactly the same as for other sysrooted paths, whereas
> it isn't when the relocation goes through different code paths in the
> compiler.

yeah, i can do that.


thanks for the quick look.



chris