Re: [PATCH] configure: Allow a host makefile fragment to override PIE flag settings.

[Richard Sandiford <richard.sandiford@arm.com>]

Iain Sandoe <iain@sandoe.co.uk> writes:
> Hi,
>
> This concerns the settings of flags (using the host makefile fragment) for
> tools that will run on the host.
>
> At present the (no)PIE flags are computed in gcc/configure but it is not
> possible to override them (either from higher level Makefile or from the
> command line).  Secondly the ordering of placement of flags assumes
> ELF semantics are OK for ordering of -fno-PIE and -fPIC.
>
> For Darwin, this introduces problems if fno-PIE causes PIC to be switched
> off and the bootstrap compiler does not support mdynamic-no-pic (which
> is the case when we bootstrap a 32b toolchain with clang).  This causes
> the host files to be built '-static' which is not legal for user-space
> code, and the build terminates with illegal relocations (so that bootstrap
> fails).
>
> This patch:
>
> 1. Allows the computed PIE flags to be overridden by the top level
>    configure.
>
> 2. Allows a host fragment to set values on the basis of the configured
>    host platform/version.
>
> 3. Sets suitable values for the Darwin cases that currently fail.
>
> tested on i686,powerpc,x86-64-darwin, x86_64, powerpc64-linux,
> OK for master?
>
> thanks
> Iain
>
> Signed-off-by: Iain Sandoe <iain@sandoe.co.uk>
>
> ChangeLog:
>
> 	* Makefile.in: Regenerated.
> 	* Makefile.tpl: Add PIE flags to HOST_EXPORTS and
> 	POST_STAGE1_HOST_EXPORTS as specified by the host makefile
> 	fragment.
>
> config/ChangeLog:
>
> 	* mh-darwin: Specify suitable PIE/PIC flags for 32b Darwin
> 	hosts.
>
> gcc/ChangeLog:
>
> 	* configure: Regenerate.
> 	* configure.ac: Allow top level configure to override assumed
> 	PIE flags.
> ---
>  Makefile.in      | 14 ++++++++++++++
>  Makefile.tpl     | 14 ++++++++++++++
>  config/mh-darwin | 20 ++++++++++++++++----
>  gcc/configure    |  4 ++--
>  gcc/configure.ac |  4 ++--
>  5 files changed, 48 insertions(+), 8 deletions(-)
>
> diff --git a/Makefile.tpl b/Makefile.tpl
> index 9adf4f94728..9523be5a761 100644
> --- a/Makefile.tpl
> +++ b/Makefile.tpl
> @@ -576,6 +576,20 @@ all:
>  @host_makefile_frag@
>  ###
>  
> +# Allow host makefile fragment to override PIE settings.
> +ifneq ($(STAGE1_NO_PIE_CFLAGS),)
> +  HOST_EXPORTS += export NO_PIE_CFLAGS="$(STAGE1_NO_PIE_CFLAGS)";
> +endif
> +ifneq ($(STAGE1_NO_PIE_FLAG),)
> +  HOST_EXPORTS += export NO_PIE_FLAG="$(STAGE1_NO_PIE_FLAG)";
> +endif
> +ifneq ($(BOOT_NO_PIE_CFLAGS),)
> +  POSTSTAGE1_HOST_EXPORTS += export NO_PIE_CFLAGS="$(BOOT_NO_PIE_CFLAGS)";
> +endif
> +ifneq ($(BOOT_NO_PIE_FLAG),)
> +  POSTSTAGE1_HOST_EXPORTS += export NO_PIE_FLAG="$(BOOT_NO_PIE_FLAG)";
> +endif

Not really my area, but:

Is the NO_PIE_FLAG just for completeness?  It didn't look like the
patch overrode that.  Might be easier to leave it out if so, to avoid
any risk of accidentally having an empty variable.

Maybe it would be easier to have the makefile fragments determine
something like CODE_MODEL_CFLAGS, which can be "-fPIC", "-mdynamic-no-pic",
etc., and use:

COMPILER += $(NO_PIE_CFLAGS) $(CODE_MODEL_CFLAGS)

This avoids mixing the autoconf-derived variable with the host override.

I think we need to stick to the traditional sh

  foo=X; export foo

style used elsewhere.  "export foo=X" is a bash extension.

It also might be better to have:

  STAGE1_CODE_MODEL_CFLAGS =
  BOOT_CODE_MODEL_CFLAGS =
  CODE_MODEL_CFLAGS =

in Makefile.tpl (just before the config/ includes) and then override
them in mh-darwin.  We can then include the exports unconditionally,
like with other variables.

I think we should export them for the host too (via HOST_EXPORTS).

We could add {STAGE1,BOOT}_CODE_MODEL_CFLAGS to the main
{STAGE1,BOOT}_{C,CXX}FLAGS in Makefile.tpl.  It looks like
that might simplify some of the existing mh-darwin code a bit.

Like I say, not my area, so feel free to ignore :-)

Thanks,
Richard

> +
>  # This is the list of directories that may be needed in RPATH_ENVVAR
>  # so that programs built for the target machine work.
>  TARGET_LIB_PATH = [+ FOR target_modules +][+
> diff --git a/config/mh-darwin b/config/mh-darwin
> index b72835ae953..d3c357a0574 100644
> --- a/config/mh-darwin
> +++ b/config/mh-darwin
> @@ -7,6 +7,13 @@
>  # We use Werror, since some versions of clang report unknown command line flags
>  # as a warning only.
>  
> +# In addition, all versions of clang released to date treat -fno-PIE in -m32
> +# compilations  as switching PIC code off too, which means that -fno-PIE,
> +# without -mdynamic-no-pic produces broken relocations (and we cannot enable
> +# -mdynamic-no-pic because the inverse setting doesn't work).  To work around
> +# this, we need to ensure that the no-PIE option is followed by -fPIE, when
> +# the compiler does not support mdynamic-no-pic.
> +
>  # We only need to determine this for the host tool used to build stage1 (or a
>  # non-bootstrapped compiler), later stages will be built by GCC which supports
>  # the required flags.
> @@ -24,23 +31,28 @@ endif
>  @if gcc-bootstrap
>  ifeq (${BOOTSTRAP_TOOL_CAN_USE_MDYNAMIC_NO_PIC},true)
>  STAGE1_CFLAGS += -mdynamic-no-pic
> +STAGE1_NO_PIE_CFLAGS = -fno-PIE
>  else
>  STAGE1_CFLAGS += -fPIC
> +STAGE1_NO_PIE_CFLAGS = -fno-PIE -fPIC
>  endif
>  ifeq (${host_shared},no)
>  # Add -mdynamic-no-pic to later stages when we know it is built with GCC.
>  BOOT_CFLAGS += -mdynamic-no-pic
> +BOOT_NO_PIE_CFLAGS = -fno-PIE
> +else
> +BOOT_NO_PIE_CFLAGS = -fno-PIE -fPIC
>  endif
>  @endif gcc-bootstrap
>  
>  @unless gcc-bootstrap
>  ifeq (${BOOTSTRAP_TOOL_CAN_USE_MDYNAMIC_NO_PIC},true)
> -# FIXME: we should also enable this for cross and non-bootstrap builds but
> -# that needs amendment to libcc1.
> -# CFLAGS += -mdynamic-no-pic
> -# CXXFLAGS += -mdynamic-no-pic
> +CFLAGS += -mdynamic-no-pic
> +CXXFLAGS += -mdynamic-no-pic
> +STAGE1_NO_PIE_CFLAGS = -fno-PIE
>  else
>  CFLAGS += -fPIC
>  CXXFLAGS += -fPIC
> +STAGE1_NO_PIE_CFLAGS = -fno-PIE -fPIC
>  endif
>  @endunless gcc-bootstrap
>
> diff --git a/gcc/configure.ac b/gcc/configure.ac
> index ad8fa5a4604..fad9b27879e 100644
> --- a/gcc/configure.ac
> +++ b/gcc/configure.ac
> @@ -7580,7 +7580,7 @@ AC_CACHE_CHECK([for -fno-PIE option],
>       [gcc_cv_c_no_fpie=no])
>     CXXFLAGS="$saved_CXXFLAGS"])
>  if test "$gcc_cv_c_no_fpie" = "yes"; then
> -  NO_PIE_CFLAGS="-fno-PIE"
> +  NO_PIE_CFLAGS=${NO_PIE_CFLAGS-"-fno-PIE"}
>  fi
>  AC_SUBST([NO_PIE_CFLAGS])
>  
> @@ -7594,7 +7594,7 @@ AC_CACHE_CHECK([for -no-pie option],
>       [gcc_cv_no_pie=no])
>     LDFLAGS="$saved_LDFLAGS"])
>  if test "$gcc_cv_no_pie" = "yes"; then
> -  NO_PIE_FLAG="-no-pie"
> +  NO_PIE_FLAG=${NO_PIE_FLAG-"-no-pie"}
>  fi
>  AC_SUBST([NO_PIE_FLAG])