Fix bad interaction between sysroot and C++ include dir

Message ID 1946944.q1On1cTmB7@polaris
State New
Headers show
Series
  • Fix bad interaction between sysroot and C++ include dir
Related show

Commit Message

Eric Botcazou May 10, 2018, 9:36 a.m.
This fixes an annyoing regression introduced in 2012 by this change:
  https://codereview.appspot.com/5394041

The idea of the change is to interpolate the value passed to --with-sysroot in 
the value passed to --with-gxx-include-dir, so that you can change the sysroot 
at run time, i.e. pass --sysroot to the compiler, and have the gxx-include-dir 
automatically translated relatively to the new sysroot.

The original submission did just that:
  https://gcc.gnu.org/ml/gcc-patches/2011-11/msg01751.html
but it was probably done against a branch so the version eventually installed 
does the interpolation unconditionally, i.e. even when gxx-include-dir is the 
default C++ include path in the install tree.

The result is that, if you configure --with-sysroot with a value that happens 
to be identical to the start of the default C++ path in the install tree, the 
mechanism triggers and the C++ include path is completely broken if you pass 
--sysroot to the compiler.  IOW the same issue as the one meant to be fixed, 
but this time when --with-gxx-include-dir is not passed.

So the attached patch restores the original implementation by doing the 
interpolation only when both --with-sysroot and --with-gxx-include-dir are 
specified on the configure line.

Tested on x86-64/Linux, applied on the mainline as obvious.


2018-05-10  Eric Botcazou  <ebotcazou@adacore.com>

	* configure.ac (gcc_gxx_include_dir_add_sysroot): Set it to 1 only
	when --with-gxx-include-dir is also specified.
	* configure: Regenerate.

Comments

Thomas Schwinge Oct. 12, 2018, 8:35 p.m. | #1
Hi!

Belatedly...

On Thu, 10 May 2018 11:36:01 +0200, Eric Botcazou <ebotcazou@adacore.com> wrote:
> This fixes an annyoing regression introduced in 2012 by [...]

> The result is that, if you configure --with-sysroot with a value that happens 
> to be identical to the start of the default C++ path in the install tree, the 
> mechanism triggers and the C++ include path is completely broken if you pass 
> --sysroot to the compiler.  IOW the same issue as the one meant to be fixed, 
> but this time when --with-gxx-include-dir is not passed.
> 
> So the attached patch restores the original implementation by doing the 
> interpolation only when both --with-sysroot and --with-gxx-include-dir are 
> specified on the configure line.
> 
> Tested on x86-64/Linux, applied on the mainline as obvious.

Thanks for that.  I had found and (at least partly) analyzed the same,
but had not yet prepared a patch.  I'm confirming that this works as
expected when configuring with "--with-sysroot=" (that is, empty).

Can we get this onto release branches, too?


Grüße
 Thomas


> 2018-05-10  Eric Botcazou  <ebotcazou@adacore.com>
> 
> 	* configure.ac (gcc_gxx_include_dir_add_sysroot): Set it to 1 only
> 	when --with-gxx-include-dir is also specified.
> 	* configure: Regenerate.

> Index: configure.ac
> ===================================================================
> --- configure.ac	(revision 260071)
> +++ configure.ac	(working copy)
> @@ -205,6 +205,11 @@ no)	;;
>  *)	gcc_gxx_include_dir=$with_gxx_include_dir ;;
>  esac])
>  
> +# If both --with-sysroot and --with-gxx-include-dir are passed, we interpolate
> +# the former in the latter and, upon success, compute gcc_gxx_include_dir as
> +# relative to the sysroot.
> +gcc_gxx_include_dir_add_sysroot=0
> +
>  # This logic must match libstdc++-v3/acinclude.m4:GLIBCXX_EXPORT_INSTALL_INFO.
>  if test x${gcc_gxx_include_dir} = x; then
>    if test x${enable_version_specific_runtime_libs} = xyes; then
> @@ -216,15 +221,10 @@ if test x${gcc_gxx_include_dir} = x; the
>      fi
>      gcc_gxx_include_dir="\$(libsubdir)/\$(libsubdir_to_prefix)$libstdcxx_incdir"
>    fi
> -fi
> -
> -gcc_gxx_include_dir_add_sysroot=0
> -if test "${with_sysroot+set}" = set; then
> +elif test "${with_sysroot+set}" = set; then
>    gcc_gxx_without_sysroot=`expr "${gcc_gxx_include_dir}" : "${with_sysroot}"'\(.*\)'`
>    if test "${gcc_gxx_without_sysroot}"; then
> -    if test x${with_sysroot} != x/; then
> -      gcc_gxx_include_dir="${gcc_gxx_without_sysroot}"
> -    fi
> +    gcc_gxx_include_dir="${gcc_gxx_without_sysroot}"
>      gcc_gxx_include_dir_add_sysroot=1
>    fi
>  fi
Eric Botcazou Oct. 12, 2018, 10:29 p.m. | #2
> Thanks for that.  I had found and (at least partly) analyzed the same,
> but had not yet prepared a patch.  I'm confirming that this works as
> expected when configuring with "--with-sysroot=" (that is, empty).

You're welcome.

> Can we get this onto release branches, too?

That's an oldish regression so it may qualify, but needs RM approval IMO.

Patch

Index: configure.ac
===================================================================
--- configure.ac	(revision 260071)
+++ configure.ac	(working copy)
@@ -205,6 +205,11 @@  no)	;;
 *)	gcc_gxx_include_dir=$with_gxx_include_dir ;;
 esac])
 
+# If both --with-sysroot and --with-gxx-include-dir are passed, we interpolate
+# the former in the latter and, upon success, compute gcc_gxx_include_dir as
+# relative to the sysroot.
+gcc_gxx_include_dir_add_sysroot=0
+
 # This logic must match libstdc++-v3/acinclude.m4:GLIBCXX_EXPORT_INSTALL_INFO.
 if test x${gcc_gxx_include_dir} = x; then
   if test x${enable_version_specific_runtime_libs} = xyes; then
@@ -216,15 +221,10 @@  if test x${gcc_gxx_include_dir} = x; the
     fi
     gcc_gxx_include_dir="\$(libsubdir)/\$(libsubdir_to_prefix)$libstdcxx_incdir"
   fi
-fi
-
-gcc_gxx_include_dir_add_sysroot=0
-if test "${with_sysroot+set}" = set; then
+elif test "${with_sysroot+set}" = set; then
   gcc_gxx_without_sysroot=`expr "${gcc_gxx_include_dir}" : "${with_sysroot}"'\(.*\)'`
   if test "${gcc_gxx_without_sysroot}"; then
-    if test x${with_sysroot} != x/; then
-      gcc_gxx_include_dir="${gcc_gxx_without_sysroot}"
-    fi
+    gcc_gxx_include_dir="${gcc_gxx_without_sysroot}"
     gcc_gxx_include_dir_add_sysroot=1
   fi
 fi