Hi Jeff, > -----Original Message----- > From: Tamar Christina <tamar.christ...@arm.com> > Sent: Thursday, July 12, 2018 18:45 > To: Jeff Law <l...@redhat.com> > Cc: gcc-patches@gcc.gnu.org; nd <n...@arm.com>; > jos...@codesourcery.com; bonz...@gnu.org; d...@redhat.com; > nero...@gcc.gnu.org; aol...@redhat.com; ralf.wildenh...@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.christ...@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 > > --
