RE: [PATCH][GCC][AArch64] Set default values for stack-clash and do basic validation in back-end. [Patch (5/6)]

[Tamar Christina <Tamar dot Christina at arm dot com>]

Hi All,

I'm looking for permission to backport this patch to the GCC-8 branch
to fix PR86486.

OK for backport?

Thanks,
Tamar

> -----Original Message-----
> From: James Greenhalgh <james.greenhalgh@arm.com>
> Sent: Tuesday, July 31, 2018 22:02
> To: Tamar Christina <Tamar.Christina@arm.com>
> Cc: gcc-patches@gcc.gnu.org; nd <nd@arm.com>; Richard Earnshaw
> <Richard.Earnshaw@arm.com>; Marcus Shawcroft
> <Marcus.Shawcroft@arm.com>
> Subject: Re: [PATCH][GCC][AArch64] Set default values for stack-clash and
> do basic validation in back-end. [Patch (5/6)]
> 
> On Tue, Jul 24, 2018 at 05:27:05AM -0500, Tamar Christina wrote:
> > Hi All,
> >
> > This patch is a cascade update from having to re-spin the configure patch
> (no# 4 in the series).
> >
> > This patch enforces that the default guard size for stack-clash
> > protection for
> > AArch64 be 64KB unless the user has overriden it via configure in
> > which case the user value is used as long as that value is within the valid
> range.
> >
> > It also does some basic validation to ensure that the guard size is
> > only 4KB or 64KB and also enforces that for aarch64 the stack-clash
> > probing interval is equal to the guard size.
> >
> > Bootstrapped Regtested on aarch64-none-linux-gnu and no issues.
> > Target was tested with stack clash on and off by default.
> >
> > Ok for trunk?
> 
> This is OK with the style changes below.
> 
> Thanks,
> James
> 
> > gcc/
> > 2018-07-24  Tamar Christina  <tamar.christina@arm.com>
> >
> > 	PR target/86486
> > 	* config/aarch64/aarch64.c (aarch64_override_options_internal):
> > 	Add validation for stack-clash parameters and set defaults.
> >
> > > -----Original Message-----
> > > From: Tamar Christina <tamar.christina@arm.com>
> > > Sent: Wednesday, July 11, 2018 12:23
> > > To: gcc-patches@gcc.gnu.org
> > > Cc: nd <nd@arm.com>; James Greenhalgh
> <James.Greenhalgh@arm.com>;
> > > Richard Earnshaw <Richard.Earnshaw@arm.com>; Marcus Shawcroft
> > > <Marcus.Shawcroft@arm.com>
> > > Subject: [PATCH][GCC][AArch64] Set default values for stack-clash
> > > and do basic validation in back-end. [Patch (5/6)]
> > >
> > > Hi All,
> > >
> > > This patch enforces that the default guard size for stack-clash
> > > protection for
> > > AArch64 be 64KB unless the user has overriden it via configure in
> > > which case the user value is used as long as that value is within the valid
> range.
> > >
> > > It also does some basic validation to ensure that the guard size is
> > > only 4KB or 64KB and also enforces that for aarch64 the stack-clash
> > > probing interval is equal to the guard size.
> > >
> > > Bootstrapped Regtested on aarch64-none-linux-gnu and no issues.
> > > Target was 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
> > > 	* config/aarch64/aarch64.c (aarch64_override_options_internal):
> > > 	Add validation for stack-clash parameters.
> > >
> > > --
> 
> > diff --git a/gcc/config/aarch64/aarch64.c
> > b/gcc/config/aarch64/aarch64.c index
> >
> e2c34cdfc96a1d3f99f7e4834c66a7551464a518..30c62c406e10793fe041d54c733
> 1
> > 6a6c8d7c229f 100644
> > --- a/gcc/config/aarch64/aarch64.c
> > +++ b/gcc/config/aarch64/aarch64.c
> > @@ -10916,6 +10916,37 @@ aarch64_override_options_internal (struct
> gcc_options *opts)
> >  			 opts->x_param_values,
> >  			 global_options_set.x_param_values);
> >
> > +  /* If the user hasn't change it via configure then set the default
> > + to 64 KB
> 
> s/change/changed/
> 
> > +     for the backend.  */
> > +  maybe_set_param_value
> (PARAM_STACK_CLASH_PROTECTION_GUARD_SIZE,
> > +			 DEFAULT_STK_CLASH_GUARD_SIZE == 0
> > +			   ? 16 : DEFAULT_STK_CLASH_GUARD_SIZE,
> > +			 opts->x_param_values,
> > +			 global_options_set.x_param_values);
> > +
> > +  /* Validate the guard size.  */
> > +  int guard_size = PARAM_VALUE
> > + (PARAM_STACK_CLASH_PROTECTION_GUARD_SIZE);
> > +  if (guard_size != 12 && guard_size != 16)
> > +      error ("only values 12 (4 KB) and 16 (64 KB) are supported for guard "
> 
> Formatting is wrong, two spaces to indent error.
> 
> > +	     "size.  Given value %d (%llu KB) is out of range.\n",
> 
> No \n on errors. s/out of range/invalid/
> 
> > +	     guard_size, (1ULL << guard_size) / 1024ULL);
> > +
> > +  /* Enforce that interval is the same size as size so the mid-end does the
> > +     right thing.  */
> > +  maybe_set_param_value
> (PARAM_STACK_CLASH_PROTECTION_PROBE_INTERVAL,
> > +			 guard_size,
> > +			 opts->x_param_values,
> > +			 global_options_set.x_param_values);
> > +
> > +  /* The maybe_set calls won't update the value if the user has explicitly
> set
> > +     one.  Which means we need to validate that probing interval and guard
> size
> > +     are equal.  */
> > +  int probe_interval
> > +    = PARAM_VALUE
> (PARAM_STACK_CLASH_PROTECTION_PROBE_INTERVAL);
> > +  if (guard_size != probe_interval)
> > +    error ("stack clash guard size '%d' must be equal to probing interval "
> > +	   "'%d'\n", guard_size, probe_interval);
> 
> No \n on errors.
> 
> > +
> >    /* Enable sw prefetching at specified optimization level for
> >       CPUS that have prefetch.  Lower optimization level threshold by 1
> >       when profiling is enabled.  */
> >