Hi Jeff, 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 patch defines a new macro DEFAULT_STK_CLASH_GUARD_SIZE which targets need to use explicitly is they want to support this configure flag and values that users may have set. 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-24 Tamar Christina <tamar.christ...@arm.com> PR target/86486 * configure.ac: Add stack-clash-protection-guard-size. * config.in (DEFAULT_STK_CLASH_GUARD_SIZE): New. * params.def: Update comment for guard-size. * configure: Regenerate. > -----Original Message----- > From: Jeff Law <l...@redhat.com> > Sent: Monday, July 23, 2018 23:19 > To: Tamar Christina <tamar.christ...@arm.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)] > > On 07/20/2018 09:39 AM, Tamar Christina wrote: > >> > >> On 07/20/2018 05:03 AM, Tamar Christina wrote: > >>>> Understood. Thanks for verifying. I wonder if we could just bury > >>>> this entirely in the aarch64 config files and not expose the > >>>> default into > >> params.def? > >>>> > >>> > >>> Burying it in config.gcc isn't ideal because if your C runtime is > >>> configurable (like uClibc) it means you have to go and modify this > >>> file every time you change something. If the argument is against > >>> having these defines in the params and not the configure flag itself > >>> then I > >> can just have an aarch64 specific configure flag and just use the > >> created define directly in the AArch64 back-end. > >> Not config.gcc, but in a .h/.c file for the target. > >> > >> If we leave the generic option, but override the default in the target > >> files. > >> Would that work? > > > > So leaving the generic configure option? Yes that would work too. The > > only downside is that if we have want to do any validation on the > > value at configure time it would need to be manually kept in sync with > > those in params.def. Or we'd just have to not do any checking at > > configure time. This would mean you can get to the end of your build and > only when you try to use the compiler would it complain. > > > > Both aren't a real deal breaker to me. > > > > Shall I then just leave the configure flag but remove the params plumbing? > Yea, I think any sanity check essentially has to move to when the compiler > runs. We can always return to param removal at a later point. > > Can you post an updated patch? Note I'm on PTO starting Wed for a week. > If you post it tomorrow I'll try to take a look before I disappear. > > jeff
diff --git a/gcc/config.in b/gcc/config.in index 2856e72d627df537a301a6c7ab6b5bbb75f6b43f..f3b301ef5afdaf0db8865e11601980f19ea0b3dd 100644 --- a/gcc/config.in +++ b/gcc/config.in @@ -55,6 +55,12 @@ #endif +/* Define to larger than zero set the default stack clash protector size. */ +#ifndef USED_FOR_TARGET +#undef DEFAULT_STK_CLASH_GUARD_SIZE +#endif + + /* Define if you want to use __cxa_atexit, rather than atexit, to register C++ destructors for local statics and global objects. This is essential for fully standards-compliant handling of destructors, but requires diff --git a/gcc/configure b/gcc/configure index 60d373982fd38fe51c285e2b02941754d1b833d6..42ec5b536bee90adb319d172eb7cca1a363a87b6 100755 --- a/gcc/configure +++ b/gcc/configure @@ -905,6 +905,7 @@ enable_valgrind_annotations with_stabs enable_multilib enable_multiarch +with_stack_clash_protection_guard_size enable___cxa_atexit enable_decimal_float enable_fixed_point @@ -1724,6 +1725,9 @@ Optional Packages: --with-gnu-as arrange to work with GNU as --with-as arrange to use the specified as (full pathname) --with-stabs arrange to use stabs instead of host debug format + --with-stack-clash-protection-guard-size=size + Set the default stack clash protection guard size + for specific targets. --with-dwarf2 force the default debug format to be DWARF 2 --with-specs=SPECS add SPECS to driver command-line processing --with-pkgversion=PKG Use PKG in the version string in place of "GCC" @@ -7436,6 +7440,35 @@ $as_echo "$enable_multiarch$ma_msg_suffix" >&6; } +# default stack clash protection guard size +# Please keep these in sync with params.def. +stk_clash_min=12 +stk_clash_max=30 +stk_clash_default=12 + +# Keep the default value when the option is not used to 0, this allows us to +# distinguish between the cases where the user specifially set a value via +# configure and when the normal default value is used. + +# Check whether --with-stack-clash-protection-guard-size was given. +if test "${with_stack_clash_protection_guard_size+set}" = set; then : + withval=$with_stack_clash_protection_guard_size; DEFAULT_STK_CLASH_GUARD_SIZE="$with_stack_clash_protection_guard_size" +else + DEFAULT_STK_CLASH_GUARD_SIZE=0 +fi + +if test $DEFAULT_STK_CLASH_GUARD_SIZE -ne 0 \ + && (test $DEFAULT_STK_CLASH_GUARD_SIZE -lt $stk_clash_min \ + || test $DEFAULT_STK_CLASH_GUARD_SIZE -gt $stk_clash_max); then + as_fn_error "Invalid value $DEFAULT_STK_CLASH_GUARD_SIZE for --with-stack-clash-protection-guard-size. Must be between $stk_clash_min and $stk_clash_max." "$LINENO" 5 +fi + + +cat >>confdefs.h <<_ACEOF +#define DEFAULT_STK_CLASH_GUARD_SIZE $DEFAULT_STK_CLASH_GUARD_SIZE +_ACEOF + + # Enable __cxa_atexit for C++. # Check whether --enable-__cxa_atexit was given. if test "${enable___cxa_atexit+set}" = set; then : @@ -18448,7 +18481,7 @@ else lt_dlunknown=0; lt_dlno_uscore=1; lt_dlneed_uscore=2 lt_status=$lt_dlunknown cat > conftest.$ac_ext <<_LT_EOF -#line 18451 "configure" +#line 18484 "configure" #include "confdefs.h" #if HAVE_DLFCN_H @@ -18554,7 +18587,7 @@ else lt_dlunknown=0; lt_dlno_uscore=1; lt_dlneed_uscore=2 lt_status=$lt_dlunknown cat > conftest.$ac_ext <<_LT_EOF -#line 18557 "configure" +#line 18590 "configure" #include "confdefs.h" #if HAVE_DLFCN_H diff --git a/gcc/configure.ac b/gcc/configure.ac index 010ecd2ccf609ded1f4d2849a2acc13aba43b55b..2ed1806ed87aca3b451ccd4b0848b50e934b75a9 100644 --- a/gcc/configure.ac +++ b/gcc/configure.ac @@ -811,6 +811,30 @@ AC_MSG_RESULT($enable_multiarch$ma_msg_suffix) AC_SUBST(with_cpu) AC_SUBST(with_float) +# default stack clash protection guard size +# Please keep these in sync with params.def. +stk_clash_min=12 +stk_clash_max=30 +stk_clash_default=12 + +# Keep the default value when the option is not used to 0, this allows us to +# distinguish between the cases where the user specifially set a value via +# configure and when the normal default value is used. +AC_ARG_WITH(stack-clash-protection-guard-size, +[AS_HELP_STRING([--with-stack-clash-protection-guard-size=size], +[Set the default stack clash protection guard size for specific targets.])], +[DEFAULT_STK_CLASH_GUARD_SIZE="$with_stack_clash_protection_guard_size"], [DEFAULT_STK_CLASH_GUARD_SIZE=0]) +if test $DEFAULT_STK_CLASH_GUARD_SIZE -ne 0 \ + && (test $DEFAULT_STK_CLASH_GUARD_SIZE -lt $stk_clash_min \ + || test $DEFAULT_STK_CLASH_GUARD_SIZE -gt $stk_clash_max); then + AC_MSG_ERROR(m4_normalize([ + Invalid value $DEFAULT_STK_CLASH_GUARD_SIZE for --with-stack-clash-protection-guard-size. \ + Must be between $stk_clash_min and $stk_clash_max.])) +fi + +AC_DEFINE_UNQUOTED(DEFAULT_STK_CLASH_GUARD_SIZE, $DEFAULT_STK_CLASH_GUARD_SIZE, + [Define to larger than zero set the default stack clash protector size.]) + # Enable __cxa_atexit for C++. AC_ARG_ENABLE(__cxa_atexit, [AS_HELP_STRING([--enable-__cxa_atexit], [enable __cxa_atexit for C++])], diff --git a/gcc/params.def b/gcc/params.def index a3906c268814bdc3a813416a60b9f666d1568f0a..b968a3fdbb7aef5074a15c3da8613679825aba2f 100644 --- a/gcc/params.def +++ b/gcc/params.def @@ -213,6 +213,7 @@ DEFPARAM(PARAM_STACK_FRAME_GROWTH, "Maximal stack frame growth due to inlining (in percent).", 1000, 0, 0) +/* Keep these up to date with those in configure.ac. */ DEFPARAM(PARAM_STACK_CLASH_PROTECTION_GUARD_SIZE, "stack-clash-protection-guard-size", "Size of the stack guard expressed as a power of two.",