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

Iain Sandoe iain@sandoe.co.uk
Fri Aug 20 10:13:09 GMT 2021


Hi Richard,

> On 20 Aug 2021, at 09:39, Richard Sandiford <richard.sandiford@arm.com> wrote:
> 
> Iain Sandoe <iain@sandoe.co.uk> writes:

>> 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:

thanks for a very helpful review :)

> Is the NO_PIE_FLAG just for completeness?

Well, the NO_PIE_FLAG is used (the flags that are passed down lose the distinction of stage1/post-stage1), 
it’s the BOOT_NO_PIE_FLAG that is not used in this initial patch - however, it is used when we consider
the enable_host_shared in a later patch.

The idea was to produce a general facility even if the first user of it did not need all elements.

>  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.

Yeah, perhaps trying to be general without a use-case, fails to test adequately.

… since the host shared stuff is now applied I’ll revise this consider that too so that both the
stage1 and post-stage1 cases should be exercised.

> 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)

OK. I have misgivings about this - the problem is that:

-fPIC -fno-PIE != -fno-PIE -fPIC,  which is not obvious to many folks - who expect that
the “last edition of a flag will be the one in force”.

So the PIE-ness and the PIC-ness are decoupled in the configury but they need to be
ordered specifically for targets that want PIC code by default (FWIW, I don’t think Darwin
is the only default-PIC case here, from discussions on irc).

Thus the mechanism I adopted was to ensure that the no-PIE setting could be adjusted
to make sure that PIC was enforced where needed (the probably non-obvious aspect of
this is that mdynamic-no-pic only happens to work because it doesn’t exist for ELF and
thus doesn’t get switched off by the no-PIE logic).  All very fragile, sadly.

In fact, (on Darwin) PIE implies PIC - it is one case where ELF semantics are at odds with
Mach-O in a non-easy manner (I have caught most cases in darwin_override_options, but
that obv. doesn’t work at configure-time since the bootstrap complier is outside our control).

I would be concerned that any decoupled flag could become reordered w.r.t the no-PIE
and break things again, but I guess we could assume that testing would catch that.

Trying to couple the PIC and PIE configs might be possible but I suspect it’s a deeper
rabbit hole than I have cycles to explore at the present.

> 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.

good catch; something that doesn’t trigger on darwin and linux testing but would be
reported the day after the patch was applied ;)
> 
> 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.

that’s a good idea it should make things more readable...
> 
> I think we should export them for the host too (via HOST_EXPORTS).

… that is done, in fact, it’s probably just not the most readable config fragment.
> 
> 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 :-)

I’ll make a revision to implement the changes to avoid bash-isms and the
simplification of the exports;

for switching to use of  CODE_MODEL_CFLAGS, I’m open to persuasion
that it can be done in a non-fragile manner.

thanks for the review,
Iain

> 
> 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])



More information about the Gcc-patches mailing list