This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

RE: [PATCH][GCC][front-end][build-machinery][opt-framework] Allow setting of stack-clash via configure options. [Patch (4/6)]


Hi Jeff,

> -----Original Message-----
> From: Tamar Christina <Tamar.Christina@arm.com>
> Sent: Thursday, July 12, 2018 18:45
> To: Jeff Law <law@redhat.com>
> Cc: gcc-patches@gcc.gnu.org; nd <nd@arm.com>;
> joseph@codesourcery.com; bonzini@gnu.org; dj@redhat.com;
> neroden@gcc.gnu.org; aoliva@redhat.com; Ralf.Wildenhues@gmx.de
> Subject: Re: [PATCH][GCC][front-end][build-machinery][opt-framework]
> Allow setting of stack-clash via configure options. [Patch (4/6)]
> 
> Hi Jeff,
> 
> The 07/11/2018 20:21, Jeff Law wrote:
> > On 07/11/2018 05:22 AM, Tamar Christina wrote:
> > > Hi All,
> > >
> > > This patch defines a configure option to allow the setting of the
> > > default guard size via configure flags when building the target.
> > >
> > > The new flag is:
> > >
> > >  * --with-stack-clash-protection-guard-size=<num>
> > >
> > > The value of configured based params are set very early on and allow
> > > the target to validate or reject the values as it sees fit.
> > >
> > > To do this the values for the parameter get set by configure through CPP
> defines.
> > > In case the back-end wants to know if a value was set or not the
> > > original default value is also passed down as a define.
> > >
> > > This allows a target to check if a param was changed by the user at
> configure time.
> > >
> > > Bootstrapped Regtested on aarch64-none-linux-gnu, x86_64-pc-linux-
> gnu and no issues.
> > > Both targets were tested with stack clash on and off by default.
> > >
> > > Ok for trunk?
> > >
> > > Thanks,
> > > Tamar
> > >
> > > gcc/
> > > 2018-07-11  Tamar Christina  <tamar.christina@arm.com>
> > >
> > > 	PR target/86486
> > > 	* configure.ac: Add stack-clash-protection-guard-size.
> > > 	* config.in (DEFAULT_STK_CLASH_GUARD_SIZE,
> STK_CLASH_GUARD_SIZE_DEFAULT,
> > > 	STK_CLASH_GUARD_SIZE_MAX, STK_CLASH_GUARD_SIZE_MIN):
> New.
> > > 	* params.def (PARAM_STACK_CLASH_PROTECTION_GUARD_SIZE):
> Use it.
> > > 	* configure: Regenerate.
> > > 	* Makefile.in (params.list, params.options): Add include dir for CPP.
> > > 	* params-list.h: Include auto-host.h
> > > 	* params-options.h: Likewise.
> > >
> > Something seems wrong here.
> >
> > What's the purpose of including auto-host in params-list and
> > params-options?  It seems like you're putting a property of the target
> > (guard size) into the wrong place (auto-host.h).
> >
> 
> The reason for this is because there's a test gcc.dg/params/blocksort-part.c
> that uses these params-options to generate test cases to perform parameter
> validation. However because now the params.def file can contain a CPP
> macro these would then fail.
> 
> CPP is already called to create params-options and params-list so the easiest
> way to fix this test was just to include auto-host which would get it the values
> from configure.
> 
> This test is probably not needed anymore after my second patch series as
> parameters are validated by the front-end now, so they can never go out of
> range.
> 
> > It's also a bit unclear to me why this is necessary at all.  Are we
> > planning to support both the 4k and 64k guards?  My goal (once the
> > guard was configurable) was never for supporting multiple sizes on a
> > target but instead to allow experimentation to find the right default.
> >

Having talked to people I believe we do need to support both 4k and 64k guards.
For the Linux/Glibc world it wouldn't matter much, either 4 or 64k would do, though Glibc has settled on 64k pages.

However other systems like (open/free)BSD or musl based systems do not want
64k pages but want 4k ones.  So we're ending up having to support both as a compromise.

Regards,
Tamar

> 
> I will get back to you on this one.
> 
> Thanks,
> Tamar
> 
> > Jeff
> 
> --

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]