diff mbox series

configure: Avoid unnecessary constraints on executables for $build.

Message ID 743F3094-16FF-46CB-9564-6BA7B610E3EA@sandoe.co.uk
State New
Headers show
Series configure: Avoid unnecessary constraints on executables for $build. | expand

Commit Message

Iain Sandoe Sept. 7, 2021, 8:11 p.m. UTC
Hi Folks,

So, looking through the various email threads and the PR, I think that
what has happened is :

As the PR points out, our existing PCH model does not work if the compiler
executable is PIE - which manifests on platforms like Darwin (which is PIE
by default) or Linux when configured —enable-default-pie.

H.J’s original patch forces no-PIE onto the compiler executables, and
because of shared code on $host also to the driver etc.

However, the patch also forces no-PIE onto executables that run on
$build (e.g. the generators etc) which is not needed (and breaks bootstrap
for at least one case, albeit one not often tested).

Marcus, observed that there was no separation in the treatment of $build
and $host, and a follow-on patch was applied that made the no-PIE change
to $build distinct.

However, IMHO, the correct change there would really be to remove the
code applying no-PIE to $build (where it is not required).

The patch below makes this change and thus fixes the bootstrap regression.

Testing - all supported languages :

x86_64, powerpc64le, powerpc64, aarch64 - linux
x64_64, powerpc, i686 - darwin
aix (nop, since ASLR is not used)
solaris, thanks to Rainer (since the machine on the cfarm doesn’t support
PIE).

All Linux platforms configured —enable-default-pie (without which one
cannot observe this anyway).

crosses and “canadians” (actually all those here are $target=$host).

$build = x86_64-linux, $host=$target = aarch64-linux
$build = powerpcle-linux $host=$target = x86_64-linux
$build= x86_64-darwin, $target = powerpc-darwin
$build = x86_64-darwin $host=$target=aarch64-darwin20

The seems to be a problem in building the native-crossed libgo, but that
is not a result of this patch.  Other than this - nothing untoward is observed
and I’ve manually checked that the $build exes are PIE and the $host ones
are not...

OK for master, and eventually backports?
Iain


[1] https://gcc.gnu.org/bugzilla/show_bug.cgi?id=71934
[2] https://gcc.gnu.org/pipermail/gcc-patches/2015-October/432180.html
[3] https://gcc.gnu.org/pipermail/gcc-patches/2021-August/578169.html

—— commit log

The executables for GCC's c-family compilers must be built with no-PIE
because they use PCH and the current model for this requires that the
exe is always lauched at the same address.  Since the other language
compilers share code with the c-family this constraint is also applied
to them.

However, the executables that run on $build (generators, and parsers
for md and def files) need not have any such constraint; they do not
consume PCH files.

This change simplifies the configuration and Makefile content by
removing the code enforcing no-PIE on these exes.  This also fixes a
bootstrap issue with some Darwin versions and clang as the bootstrap
compiler,  where -no-PIE causes the correct relocation model to be
switched off leading to invalid user-space code.

Signed-off-by: Iain Sandoe <iain@sandoe.co.uk>

gcc/ChangeLog:

	* Makefile.in: Remove variables related to applying no-PIE
	to the exes on $build.
	* configure: Regenerate.
	* configure.ac: Remove configuration related to applying
	no-PIE to the exes on $build.
---
 gcc/Makefile.in  |  7 -------
 gcc/configure    | 18 ++----------------
 gcc/configure.ac | 10 ----------
 3 files changed, 2 insertions(+), 33 deletions(-)

--

Comments

Richard Biener Sept. 8, 2021, 6:35 a.m. UTC | #1
On Tue, Sep 7, 2021 at 10:11 PM Iain Sandoe <iain@sandoe.co.uk> wrote:
>
> Hi Folks,
>
> So, looking through the various email threads and the PR, I think that
> what has happened is :
>
> As the PR points out, our existing PCH model does not work if the compiler
> executable is PIE - which manifests on platforms like Darwin (which is PIE
> by default) or Linux when configured —enable-default-pie.
>
> H.J’s original patch forces no-PIE onto the compiler executables, and
> because of shared code on $host also to the driver etc.
>
> However, the patch also forces no-PIE onto executables that run on
> $build (e.g. the generators etc) which is not needed (and breaks bootstrap
> for at least one case, albeit one not often tested).
>
> Marcus, observed that there was no separation in the treatment of $build
> and $host, and a follow-on patch was applied that made the no-PIE change
> to $build distinct.
>
> However, IMHO, the correct change there would really be to remove the
> code applying no-PIE to $build (where it is not required).
>
> The patch below makes this change and thus fixes the bootstrap regression.
>
> Testing - all supported languages :
>
> x86_64, powerpc64le, powerpc64, aarch64 - linux
> x64_64, powerpc, i686 - darwin
> aix (nop, since ASLR is not used)
> solaris, thanks to Rainer (since the machine on the cfarm doesn’t support
> PIE).
>
> All Linux platforms configured —enable-default-pie (without which one
> cannot observe this anyway).
>
> crosses and “canadians” (actually all those here are $target=$host).
>
> $build = x86_64-linux, $host=$target = aarch64-linux
> $build = powerpcle-linux $host=$target = x86_64-linux
> $build= x86_64-darwin, $target = powerpc-darwin
> $build = x86_64-darwin $host=$target=aarch64-darwin20
>
> The seems to be a problem in building the native-crossed libgo, but that
> is not a result of this patch.  Other than this - nothing untoward is observed
> and I’ve manually checked that the $build exes are PIE and the $host ones
> are not...
>
> OK for master, and eventually backports?

OK for trunk, I think it warrants quite some soaking time before considering
backports.

Thanks,
Richard.

> Iain
>
>
> [1] https://gcc.gnu.org/bugzilla/show_bug.cgi?id=71934
> [2] https://gcc.gnu.org/pipermail/gcc-patches/2015-October/432180.html
> [3] https://gcc.gnu.org/pipermail/gcc-patches/2021-August/578169.html
>
> —— commit log
>
> The executables for GCC's c-family compilers must be built with no-PIE
> because they use PCH and the current model for this requires that the
> exe is always lauched at the same address.  Since the other language
> compilers share code with the c-family this constraint is also applied
> to them.
>
> However, the executables that run on $build (generators, and parsers
> for md and def files) need not have any such constraint; they do not
> consume PCH files.
>
> This change simplifies the configuration and Makefile content by
> removing the code enforcing no-PIE on these exes.  This also fixes a
> bootstrap issue with some Darwin versions and clang as the bootstrap
> compiler,  where -no-PIE causes the correct relocation model to be
> switched off leading to invalid user-space code.
>
> Signed-off-by: Iain Sandoe <iain@sandoe.co.uk>
>
> gcc/ChangeLog:
>
>         * Makefile.in: Remove variables related to applying no-PIE
>         to the exes on $build.
>         * configure: Regenerate.
>         * configure.ac: Remove configuration related to applying
>         no-PIE to the exes on $build.
> ---
>  gcc/Makefile.in  |  7 -------
>  gcc/configure    | 18 ++----------------
>  gcc/configure.ac | 10 ----------
>  3 files changed, 2 insertions(+), 33 deletions(-)
>
> diff --git a/gcc/Makefile.in b/gcc/Makefile.in
> index f0c560fe45b..d0c5ca214c9 100644
> --- a/gcc/Makefile.in
> +++ b/gcc/Makefile.in
> @@ -799,13 +799,8 @@ DIR = ../gcc
>  # Native compiler for the build machine and its switches.
>  CC_FOR_BUILD = @CC_FOR_BUILD@
>  CXX_FOR_BUILD = @CXX_FOR_BUILD@
> -NO_PIE_CFLAGS_FOR_BUILD = @NO_PIE_CFLAGS_FOR_BUILD@
> -NO_PIE_FLAG_FOR_BUILD = @NO_PIE_FLAG_FOR_BUILD@
>  BUILD_CFLAGS= @BUILD_CFLAGS@ $(GENERATOR_CFLAGS) -DGENERATOR_FILE
>  BUILD_CXXFLAGS = @BUILD_CXXFLAGS@ $(GENERATOR_CFLAGS) -DGENERATOR_FILE
> -BUILD_NO_PIE_CFLAGS = @BUILD_NO_PIE_CFLAGS@
> -BUILD_CFLAGS += $(BUILD_NO_PIE_CFLAGS)
> -BUILD_CXXFLAGS += $(BUILD_NO_PIE_CFLAGS)
>
>  # Native compiler that we use.  This may be C++ some day.
>  COMPILER_FOR_BUILD = $(CXX_FOR_BUILD)
> @@ -817,8 +812,6 @@ BUILD_LINKERFLAGS = $(BUILD_CXXFLAGS)
>
>  # Native linker and preprocessor flags.  For x-fragment overrides.
>  BUILD_LDFLAGS=@BUILD_LDFLAGS@
> -BUILD_NO_PIE_FLAG = @BUILD_NO_PIE_FLAG@
> -BUILD_LDFLAGS += $(BUILD_NO_PIE_FLAG)
>  BUILD_CPPFLAGS= -I. -I$(@D) -I$(srcdir) -I$(srcdir)/$(@D) \
>                 -I$(srcdir)/../include @INCINTL@ $(CPPINC) $(CPPFLAGS)
>
> diff --git a/gcc/configure b/gcc/configure
> index 500e3f68215..87d7b9c435b 100755
> --- a/gcc/configure
> +++ b/gcc/configure
> @@ -753,10 +753,6 @@ FGREP
>  SED
>  LIBTOOL
>  collect2
> -NO_PIE_FLAG_FOR_BUILD
> -NO_PIE_CFLAGS_FOR_BUILD
> -BUILD_NO_PIE_FLAG
> -BUILD_NO_PIE_CFLAGS
>  STMP_FIXINC
>  BUILD_LDFLAGS
>  BUILD_CXXFLAGS
> @@ -13336,24 +13332,14 @@ BUILD_CXXFLAGS='$(ALL_CXXFLAGS)'
>  BUILD_LDFLAGS='$(LDFLAGS)'
>  STMP_FIXINC=stmp-fixinc
>
> -BUILD_NO_PIE_CFLAGS='$(NO_PIE_CFLAGS)'
> -BUILD_NO_PIE_FLAG='$(NO_PIE_FLAG)'
> -
>  # And these apply if build != host, or we are generating coverage data
>  if test x$build != x$host || test "x$coverage_flags" != x
>  then
>      BUILD_CFLAGS='$(INTERNAL_CFLAGS) $(T_CFLAGS) $(CFLAGS-$@) $(CFLAGS_FOR_BUILD)'
>      BUILD_CXXFLAGS='$(INTERNAL_CFLAGS) $(T_CFLAGS) $(CFLAGS-$@) $(CXXFLAGS_FOR_BUILD)'
>      BUILD_LDFLAGS='$(LDFLAGS_FOR_BUILD)'
> -
> -    NO_PIE_CFLAGS_FOR_BUILD=${NO_PIE_CFLAGS_FOR_BUILD-${NO_PIE_CFLAGS}}
> -    NO_PIE_FLAG_FOR_BUILD=${NO_PIE_FLAG_FOR_BUILD-${NO_PIE_FLAG}}
> -    BUILD_NO_PIE_CFLAGS='$(NO_PIE_CFLAGS_FOR_BUILD)'
> -    BUILD_NO_PIE_FLAG='$(NO_PIE_FLAG_FOR_BUILD)'
>  fi
>
> -
> -
>  # Expand extra_headers to include complete path.
>  # This substitutes for lots of t-* files.
>  extra_headers_list=
> @@ -19480,7 +19466,7 @@ else
>    lt_dlunknown=0; lt_dlno_uscore=1; lt_dlneed_uscore=2
>    lt_status=$lt_dlunknown
>    cat > conftest.$ac_ext <<_LT_EOF
> -#line 19483 "configure"
> +#line 19469 "configure"
>  #include "confdefs.h"
>
>  #if HAVE_DLFCN_H
> @@ -19586,7 +19572,7 @@ else
>    lt_dlunknown=0; lt_dlno_uscore=1; lt_dlneed_uscore=2
>    lt_status=$lt_dlunknown
>    cat > conftest.$ac_ext <<_LT_EOF
> -#line 19589 "configure"
> +#line 19575 "configure"
>  #include "confdefs.h"
>
>  #if HAVE_DLFCN_H
> diff --git a/gcc/configure.ac b/gcc/configure.ac
> index 6f768e02aa4..3ba78aa9dee 100644
> --- a/gcc/configure.ac
> +++ b/gcc/configure.ac
> @@ -2473,23 +2473,13 @@ BUILD_CXXFLAGS='$(ALL_CXXFLAGS)' AC_SUBST(BUILD_CXXFLAGS)
>  BUILD_LDFLAGS='$(LDFLAGS)'     AC_SUBST(BUILD_LDFLAGS)
>  STMP_FIXINC=stmp-fixinc                AC_SUBST(STMP_FIXINC)
>
> -BUILD_NO_PIE_CFLAGS='$(NO_PIE_CFLAGS)' AC_SUBST(BUILD_NO_PIE_CFLAGS)
> -BUILD_NO_PIE_FLAG='$(NO_PIE_FLAG)' AC_SUBST(BUILD_NO_PIE_FLAG)
> -
>  # And these apply if build != host, or we are generating coverage data
>  if test x$build != x$host || test "x$coverage_flags" != x
>  then
>      BUILD_CFLAGS='$(INTERNAL_CFLAGS) $(T_CFLAGS) $(CFLAGS-$@) $(CFLAGS_FOR_BUILD)'
>      BUILD_CXXFLAGS='$(INTERNAL_CFLAGS) $(T_CFLAGS) $(CFLAGS-$@) $(CXXFLAGS_FOR_BUILD)'
>      BUILD_LDFLAGS='$(LDFLAGS_FOR_BUILD)'
> -
> -    NO_PIE_CFLAGS_FOR_BUILD=${NO_PIE_CFLAGS_FOR_BUILD-${NO_PIE_CFLAGS}}
> -    NO_PIE_FLAG_FOR_BUILD=${NO_PIE_FLAG_FOR_BUILD-${NO_PIE_FLAG}}
> -    BUILD_NO_PIE_CFLAGS='$(NO_PIE_CFLAGS_FOR_BUILD)'
> -    BUILD_NO_PIE_FLAG='$(NO_PIE_FLAG_FOR_BUILD)'
>  fi
> -AC_SUBST(NO_PIE_CFLAGS_FOR_BUILD)
> -AC_SUBST(NO_PIE_FLAG_FOR_BUILD)
>
>  # Expand extra_headers to include complete path.
>  # This substitutes for lots of t-* files.
> --
>
Iain Sandoe Oct. 28, 2021, 3:44 p.m. UTC | #2
Hi Richard,

> On 8 Sep 2021, at 07:35, Richard Biener <richard.guenther@gmail.com> wrote:
> 
> On Tue, Sep 7, 2021 at 10:11 PM Iain Sandoe <iain@sandoe.co.uk> wrote:
>> 

>> So, looking through the various email threads and the PR, I think that
>> what has happened is :
>> 
>> As the PR points out, our existing PCH model does not work if the compiler
>> executable is PIE - which manifests on platforms like Darwin (which is PIE
>> by default) or Linux when configured —enable-default-pie.
>> 
>> H.J’s original patch forces no-PIE onto the compiler executables, and
>> because of shared code on $host also to the driver etc.

>> OK for master, and eventually backports?
> 
> OK for trunk, I think it warrants quite some soaking time before considering
> backports.

It’s been on master for quite some time now (and presumably several cycles of
everyone’s CI) without any reports of problems,  it would be good to get this at
least onto 11 and 10 (since that is the last version we can bootstrap with c++98).

OK for backports now?
thanks
Iain
Richard Biener Oct. 29, 2021, 7:25 a.m. UTC | #3
On Thu, Oct 28, 2021 at 5:44 PM Iain Sandoe <iain@sandoe.co.uk> wrote:
>
> Hi Richard,
>
> > On 8 Sep 2021, at 07:35, Richard Biener <richard.guenther@gmail.com> wrote:
> >
> > On Tue, Sep 7, 2021 at 10:11 PM Iain Sandoe <iain@sandoe.co.uk> wrote:
> >>
>
> >> So, looking through the various email threads and the PR, I think that
> >> what has happened is :
> >>
> >> As the PR points out, our existing PCH model does not work if the compiler
> >> executable is PIE - which manifests on platforms like Darwin (which is PIE
> >> by default) or Linux when configured —enable-default-pie.
> >>
> >> H.J’s original patch forces no-PIE onto the compiler executables, and
> >> because of shared code on $host also to the driver etc.
>
> >> OK for master, and eventually backports?
> >
> > OK for trunk, I think it warrants quite some soaking time before considering
> > backports.
>
> It’s been on master for quite some time now (and presumably several cycles of
> everyone’s CI) without any reports of problems,  it would be good to get this at
> least onto 11 and 10 (since that is the last version we can bootstrap with c++98).
>
> OK for backports now?

OK.

> thanks
> Iain
>
diff mbox series

Patch

diff --git a/gcc/Makefile.in b/gcc/Makefile.in
index f0c560fe45b..d0c5ca214c9 100644
--- a/gcc/Makefile.in
+++ b/gcc/Makefile.in
@@ -799,13 +799,8 @@  DIR = ../gcc
 # Native compiler for the build machine and its switches.
 CC_FOR_BUILD = @CC_FOR_BUILD@
 CXX_FOR_BUILD = @CXX_FOR_BUILD@
-NO_PIE_CFLAGS_FOR_BUILD = @NO_PIE_CFLAGS_FOR_BUILD@
-NO_PIE_FLAG_FOR_BUILD = @NO_PIE_FLAG_FOR_BUILD@
 BUILD_CFLAGS= @BUILD_CFLAGS@ $(GENERATOR_CFLAGS) -DGENERATOR_FILE
 BUILD_CXXFLAGS = @BUILD_CXXFLAGS@ $(GENERATOR_CFLAGS) -DGENERATOR_FILE
-BUILD_NO_PIE_CFLAGS = @BUILD_NO_PIE_CFLAGS@
-BUILD_CFLAGS += $(BUILD_NO_PIE_CFLAGS)
-BUILD_CXXFLAGS += $(BUILD_NO_PIE_CFLAGS)
 
 # Native compiler that we use.  This may be C++ some day.
 COMPILER_FOR_BUILD = $(CXX_FOR_BUILD)
@@ -817,8 +812,6 @@  BUILD_LINKERFLAGS = $(BUILD_CXXFLAGS)
 
 # Native linker and preprocessor flags.  For x-fragment overrides.
 BUILD_LDFLAGS=@BUILD_LDFLAGS@
-BUILD_NO_PIE_FLAG = @BUILD_NO_PIE_FLAG@
-BUILD_LDFLAGS += $(BUILD_NO_PIE_FLAG)
 BUILD_CPPFLAGS= -I. -I$(@D) -I$(srcdir) -I$(srcdir)/$(@D) \
 		-I$(srcdir)/../include @INCINTL@ $(CPPINC) $(CPPFLAGS)
 
diff --git a/gcc/configure b/gcc/configure
index 500e3f68215..87d7b9c435b 100755
--- a/gcc/configure
+++ b/gcc/configure
@@ -753,10 +753,6 @@  FGREP
 SED
 LIBTOOL
 collect2
-NO_PIE_FLAG_FOR_BUILD
-NO_PIE_CFLAGS_FOR_BUILD
-BUILD_NO_PIE_FLAG
-BUILD_NO_PIE_CFLAGS
 STMP_FIXINC
 BUILD_LDFLAGS
 BUILD_CXXFLAGS
@@ -13336,24 +13332,14 @@  BUILD_CXXFLAGS='$(ALL_CXXFLAGS)'
 BUILD_LDFLAGS='$(LDFLAGS)'
 STMP_FIXINC=stmp-fixinc
 
-BUILD_NO_PIE_CFLAGS='$(NO_PIE_CFLAGS)'
-BUILD_NO_PIE_FLAG='$(NO_PIE_FLAG)'
-
 # And these apply if build != host, or we are generating coverage data
 if test x$build != x$host || test "x$coverage_flags" != x
 then
     BUILD_CFLAGS='$(INTERNAL_CFLAGS) $(T_CFLAGS) $(CFLAGS-$@) $(CFLAGS_FOR_BUILD)'
     BUILD_CXXFLAGS='$(INTERNAL_CFLAGS) $(T_CFLAGS) $(CFLAGS-$@) $(CXXFLAGS_FOR_BUILD)'
     BUILD_LDFLAGS='$(LDFLAGS_FOR_BUILD)'
-
-    NO_PIE_CFLAGS_FOR_BUILD=${NO_PIE_CFLAGS_FOR_BUILD-${NO_PIE_CFLAGS}}
-    NO_PIE_FLAG_FOR_BUILD=${NO_PIE_FLAG_FOR_BUILD-${NO_PIE_FLAG}}
-    BUILD_NO_PIE_CFLAGS='$(NO_PIE_CFLAGS_FOR_BUILD)'
-    BUILD_NO_PIE_FLAG='$(NO_PIE_FLAG_FOR_BUILD)'
 fi
 
-
-
 # Expand extra_headers to include complete path.
 # This substitutes for lots of t-* files.
 extra_headers_list=
@@ -19480,7 +19466,7 @@  else
   lt_dlunknown=0; lt_dlno_uscore=1; lt_dlneed_uscore=2
   lt_status=$lt_dlunknown
   cat > conftest.$ac_ext <<_LT_EOF
-#line 19483 "configure"
+#line 19469 "configure"
 #include "confdefs.h"
 
 #if HAVE_DLFCN_H
@@ -19586,7 +19572,7 @@  else
   lt_dlunknown=0; lt_dlno_uscore=1; lt_dlneed_uscore=2
   lt_status=$lt_dlunknown
   cat > conftest.$ac_ext <<_LT_EOF
-#line 19589 "configure"
+#line 19575 "configure"
 #include "confdefs.h"
 
 #if HAVE_DLFCN_H
diff --git a/gcc/configure.ac b/gcc/configure.ac
index 6f768e02aa4..3ba78aa9dee 100644
--- a/gcc/configure.ac
+++ b/gcc/configure.ac
@@ -2473,23 +2473,13 @@  BUILD_CXXFLAGS='$(ALL_CXXFLAGS)' AC_SUBST(BUILD_CXXFLAGS)
 BUILD_LDFLAGS='$(LDFLAGS)'	AC_SUBST(BUILD_LDFLAGS)
 STMP_FIXINC=stmp-fixinc		AC_SUBST(STMP_FIXINC)
 
-BUILD_NO_PIE_CFLAGS='$(NO_PIE_CFLAGS)' AC_SUBST(BUILD_NO_PIE_CFLAGS)
-BUILD_NO_PIE_FLAG='$(NO_PIE_FLAG)' AC_SUBST(BUILD_NO_PIE_FLAG)
-
 # And these apply if build != host, or we are generating coverage data
 if test x$build != x$host || test "x$coverage_flags" != x
 then
     BUILD_CFLAGS='$(INTERNAL_CFLAGS) $(T_CFLAGS) $(CFLAGS-$@) $(CFLAGS_FOR_BUILD)'
     BUILD_CXXFLAGS='$(INTERNAL_CFLAGS) $(T_CFLAGS) $(CFLAGS-$@) $(CXXFLAGS_FOR_BUILD)'
     BUILD_LDFLAGS='$(LDFLAGS_FOR_BUILD)'
-
-    NO_PIE_CFLAGS_FOR_BUILD=${NO_PIE_CFLAGS_FOR_BUILD-${NO_PIE_CFLAGS}}
-    NO_PIE_FLAG_FOR_BUILD=${NO_PIE_FLAG_FOR_BUILD-${NO_PIE_FLAG}}
-    BUILD_NO_PIE_CFLAGS='$(NO_PIE_CFLAGS_FOR_BUILD)'
-    BUILD_NO_PIE_FLAG='$(NO_PIE_FLAG_FOR_BUILD)'
 fi
-AC_SUBST(NO_PIE_CFLAGS_FOR_BUILD)
-AC_SUBST(NO_PIE_FLAG_FOR_BUILD)
 
 # Expand extra_headers to include complete path.
 # This substitutes for lots of t-* files.