| Message ID | a4a947fa-970f-45a3-15db-5a84e2698d8b@foss.arm.com |
|---|---|
| State | New |
| Headers | show |
On Fri, 11 Nov 2016, Jiong Wang wrote: > In this new patch, I introduced a new target macro for SSP to allow one > backend GCC's default SSP runtime generation be disabled. I see no reason this needs to be a target macro rather than a hook. New target macros are discouraged and should only be added for good reasons (e.g. used in target-side code; used in defining contents of an enum or some other such thing that can't be runtime-conditional; forms part of an existing family of target macros). -- Joseph S. Myers joseph@codesourcery.com
On 11/11/2016 11:41 AM, Jiong Wang wrote: > On 24/10/16 16:22, Jeff Law wrote: > >> Asserting couldn't hurt. I'd much rather have the compiler issue an >> error, ICE or somesuch than silently not generate a call to the stack >> protector fail routine. > > Hi Jeff, > > I have just send out the other patch which accelerates > -fstack-protector on > AArch64, more background information there at: > > https://gcc.gnu.org/ml/gcc-patches/2016-11/msg01168.html Confirms what I suspected :-) > > Previous, I was emptying three target insns/hooks, and was relying GCC to > optimize all remaining SSP runtime stuff out. I am thinking it's > better and > safer that GCC allow one backend to disable the default SSP runtime > cleanly, > so the backend does't need to rely on optimization level, and libssp > is not > needed at all optimization level. > > In this new patch, I introduced a new target macro for SSP to allow one > backend GCC's default SSP runtime generation be disabled. > > How does this looks to you? > > Thanks. > > gcc/ > 2016-11-11 Jiong Wang <jiong.wang@arm.com> > * function.c (expand_function_end): Guard stack_protect_epilogue > with > ENABLE_DEFAULT_SSP_RUNTIME. > * cfgexpand.c (pass_expand::execute): Likewise guard for > stack_protect_prologue. > * defaults.h (ENABLE_DEFAULT_SSP_RUNTIME): New macro. Default > set to 1. > * doc/tm.texi.in (Misc): Documents ENABLE_DEFAULT_SSP_RUNTIME. > * doc/tm.texi: Regenerate. > Like Joseph, I think this should be a hook rather than a new target macro. I do think it's closer to the right track though (separation of access to the guard from the rest of the SSP runtime bits). Jeff
diff --git a/gcc/cfgexpand.c b/gcc/cfgexpand.c index 130a16b1d7d06c4ec9e31439037ffcbcbd0e085f..99f055d2db622f7acd393a223b3968be12b6235f 100644 --- a/gcc/cfgexpand.c +++ b/gcc/cfgexpand.c @@ -6343,7 +6343,7 @@ pass_expand::execute (function *fun) /* Initialize the stack_protect_guard field. This must happen after the call to __main (if any) so that the external decl is initialized. */ - if (crtl->stack_protect_guard) + if (crtl->stack_protect_guard && ENABLE_DEFAULT_SSP_RUNTIME) stack_protect_prologue (); expand_phi_nodes (&SA); diff --git a/gcc/defaults.h b/gcc/defaults.h index af8fe916be49e745c842d992a5af372c46ec2fe3..ec5e52c9761e3e5aee5274c54628157d0bde1808 100644 --- a/gcc/defaults.h +++ b/gcc/defaults.h @@ -1404,6 +1404,14 @@ see the files COPYING3 and COPYING.RUNTIME respectively. If not, see # define DEFAULT_FLAG_SSP 0 #endif +/* Supply a default definition of ENABLE_DEFAULT_SSP_RUNTIME. GCC use this to + decide whether stack_protect_prologue and stack_protect_epilogue based + runtime support should be generated. */ + +#ifndef ENABLE_DEFAULT_SSP_RUNTIME +#define ENABLE_DEFAULT_SSP_RUNTIME 1 +#endif + /* Provide default values for the macros controlling stack checking. */ /* The default is neither full builtin stack checking... */ diff --git a/gcc/doc/tm.texi b/gcc/doc/tm.texi index 586626062435f3718cfae84c6aab3024d08d79d7..64d20bc493470221286b6248354f0d6122405cb6 100644 --- a/gcc/doc/tm.texi +++ b/gcc/doc/tm.texi @@ -10487,6 +10487,14 @@ The default implementation does nothing. @c prevent bad page break with this line Here are several miscellaneous parameters. +@defmac ENABLE_DEFAULT_SSP_RUNTIME +Define this boolean macro to indicate whether or not your architecture +use GCC default stack protector runtime. If this macro is set to false, +stack_protect_prologue and stack_protect_epilogue based runtime support will be +generated, otherwise GCC assumes your architecture generates private runtime +support. This macro is default set to true. +@end defmac + @defmac HAS_LONG_COND_BRANCH Define this boolean macro to indicate whether or not your architecture has conditional branches that can span all of memory. It is used in diff --git a/gcc/doc/tm.texi.in b/gcc/doc/tm.texi.in index da133a4b7010533d85d5bb9a850b91e8a80ce1ca..729c76fa182076828a5819ab391b4f61fb80a771 100644 --- a/gcc/doc/tm.texi.in +++ b/gcc/doc/tm.texi.in @@ -7499,6 +7499,14 @@ c_register_addr_space ("__ea", ADDR_SPACE_EA); @c prevent bad page break with this line Here are several miscellaneous parameters. +@defmac ENABLE_DEFAULT_SSP_RUNTIME +Define this boolean macro to indicate whether or not your architecture +use GCC default stack protector runtime. If this macro is set to false, +stack_protect_prologue and stack_protect_epilogue based runtime support will be +generated, otherwise GCC assumes your architecture generates private runtime +support. This macro is default set to true. +@end defmac + @defmac HAS_LONG_COND_BRANCH Define this boolean macro to indicate whether or not your architecture has conditional branches that can span all of memory. It is used in diff --git a/gcc/function.c b/gcc/function.c index 53bad8736e9ef251347d23d40bc0ab767a979bc7..9dce8929590f6cb06155a540e33960c2cf0e3b16 100644 --- a/gcc/function.c +++ b/gcc/function.c @@ -5624,7 +5624,7 @@ expand_function_end (void) emit_insn (gen_blockage ()); /* If stack protection is enabled for this function, check the guard. */ - if (crtl->stack_protect_guard) + if (crtl->stack_protect_guard && ENABLE_DEFAULT_SSP_RUNTIME) stack_protect_epilogue (); /* If we had calls to alloca, and this machine needs
On 24/10/16 16:22, Jeff Law wrote: > Asserting couldn't hurt. I'd much rather have the compiler issue an error, ICE or somesuch than silently not generate a call to the stack protector fail routine. Hi Jeff, I have just send out the other patch which accelerates -fstack-protector on AArch64, more background information there at: https://gcc.gnu.org/ml/gcc-patches/2016-11/msg01168.html Previous, I was emptying three target insns/hooks, and was relying GCC to optimize all remaining SSP runtime stuff out. I am thinking it's better and safer that GCC allow one backend to disable the default SSP runtime cleanly, so the backend does't need to rely on optimization level, and libssp is not needed at all optimization level. In this new patch, I introduced a new target macro for SSP to allow one backend GCC's default SSP runtime generation be disabled. How does this looks to you? Thanks. gcc/ 2016-11-11 Jiong Wang <jiong.wang@arm.com> * function.c (expand_function_end): Guard stack_protect_epilogue with ENABLE_DEFAULT_SSP_RUNTIME. * cfgexpand.c (pass_expand::execute): Likewise guard for stack_protect_prologue. * defaults.h (ENABLE_DEFAULT_SSP_RUNTIME): New macro. Default set to 1. * doc/tm.texi.in (Misc): Documents ENABLE_DEFAULT_SSP_RUNTIME. * doc/tm.texi: Regenerate.