| Message ID | 20180711112251.GA15936@arm.com |
|---|---|
| State | New |
| Headers | show |
| Series | [AArch64] Set default values for stack-clash and do basic validation in back-end. [Patch (5/6)] | expand |
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? Thanks, Tamar 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..30c62c406e10793fe041d54c73316a6c8d7c229f 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 + 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 " + "size. Given value %d (%llu KB) is out of range.\n", + 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); + /* Enable sw prefetching at specified optimization level for CPUS that have prefetch. Lower optimization level threshold by 1 when profiling is enabled. */
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..30c62c406e10793fe041d54c73316a6c8d7c229f 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. */ >
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. */ > >
diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c index ae3c2fb85256b1e95e2242f3f16a027e918ba368..e62d8a92ff53128e5e10ffd3b52eb8898869b756 100644 --- a/gcc/config/aarch64/aarch64.c +++ b/gcc/config/aarch64/aarch64.c @@ -10905,6 +10905,36 @@ 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 + for the backend. */ + if (DEFAULT_STK_CLASH_GUARD_SIZE == 0) + maybe_set_param_value (PARAM_STACK_CLASH_PROTECTION_GUARD_SIZE, 16, + 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 " + "size. Given value %d (%llu KB) is out of range.\n", + 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); + /* Enable sw prefetching at specified optimization level for CPUS that have prefetch. Lower optimization level threshold by 1 when profiling is enabled. */