This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [AArch64] Accelerate -fstack-protector through pointer authentication extension


On 18/01/17 17:10, Jiong Wang wrote:
>> NOTE, this approach however requires DWARF change as the original LR
>> is signed,
>> the binary needs new libgcc to make sure c++ eh works correctly. 
>> Given this
>> acceleration already needs the user specify
>> -mstack-protector-dialect=pauth
>> which means the target platform largely should have install new
>> libgcc, otherwise
>> you can't utilize new pointer authentication features.
>>
>> gcc/
>> 2016-11-11  Jiong Wang  <jiong.wang@arm.com>
>>
>>         * config/aarch64/aarch64-opts.h
>> (aarch64_stack_protector_type): New
>>         enum.
>>         (aarch64_layout_frame): Swap callees and locals when
>>         -mstack-protector-dialect=pauth specified.
>>         (aarch64_expand_prologue): Use AARCH64_PAUTH_SSP_OR_RA_SIGN
>> instead
>>         of AARCH64_ENABLE_RETURN_ADDRESS_SIGN.
>>         (aarch64_expand_epilogue): Likewise.
>>         * config/aarch64/aarch64.md (*do_return): Likewise.
>>         (aarch64_override_options): Sanity check for ILP32 and
>> TARGET_PAUTH.
>>         * config/aarch64/aarch64.h (AARCH64_PAUTH_SSP_OPTION,
>> AARCH64_PAUTH_SSP,
>>         AARCH64_PAUTH_SSP_OR_RA_SIGN, LINK_SSP_SPEC): New defines.
>>         * config/aarch64/aarch64.opt (-mstack-protector-dialect=): New
>> option.
>>         * doc/invoke.texi (AArch64 Options): Documents
>>         -mstack-protector-dialect=.
>>
>                                                          Patch updated
> to migrate to TARGET_STACK_PROTECT_RUNTIME_ENABLED_P.
> 
> aarch64 cross check OK with the following options enabled on all testcases.
>     -fstack-protector-all -mstack-protector-pauth
> 
> OK for trunk?
>                                                                     gcc/
> 2017-01-18  Jiong Wang  <jiong.wang@arm.com>
>            * config/aarch64/aarch64-protos.h
>         (aarch64_pauth_stack_protector_enabled): New declaration.
>         * config/aarch64/aarch64.c (aarch64_layout_frame): Swap
> callee-save area
>         and locals area when aarch64_pauth_stack_protector_enabled
> returns true.
>         (aarch64_stack_protect_runtime_enabled): New function.
>         (aarch64_pauth_stack_protector_enabled): New function.
>         (aarch64_return_address_signing_enabled): Enabled by
>         aarch64_pauth_stack_protector_enabled.
>         (aarch64_override_options): Sanity check for
> -mstack-protector-pauth.
>         (TARGET_STACK_PROTECT_RUNTIME_ENABLED_P): Define.
>         * config/aarch64/aarch64.h (LINK_SSP_SPEC): Likewise.
>         * config/aarch64/aarch64.opt (-mstack-protector-pauth): New option.
>         * doc/invoke.texi (AArch64 Options): Documents
> -mstack-protector-pauth.
> 
> gcc/testsuite/
>         * gcc.target/aarch64/stack_protector_1.c: New test.
> 
> 
> 1.patch
> 
> 
> diff --git a/gcc/config/aarch64/aarch64-protos.h b/gcc/config/aarch64/aarch64-protos.h
> index 632dd4768d82c340ae4e9b4a93206743756c06e7..a3ad623eef498d00b52d24bf02a5748fad576c3d 100644
> --- a/gcc/config/aarch64/aarch64-protos.h
> +++ b/gcc/config/aarch64/aarch64-protos.h
> @@ -383,6 +383,7 @@ void aarch64_init_cumulative_args (CUMULATIVE_ARGS *, const_tree, rtx,
>  void aarch64_init_expanders (void);
>  void aarch64_init_simd_builtins (void);
>  void aarch64_emit_call_insn (rtx);
> +bool aarch64_pauth_stack_protector_enabled (void);
>  void aarch64_register_pragmas (void);
>  void aarch64_relayout_simd_types (void);
>  void aarch64_reset_previous_fndecl (void);
> diff --git a/gcc/config/aarch64/aarch64.h b/gcc/config/aarch64/aarch64.h
> index 3718ad1b3bf27c6bdb9e74831fd660e617cccbde..dd742d37ab6fc6fb5085e1c6b5d86d5ce1ce5f8a 100644
> --- a/gcc/config/aarch64/aarch64.h
> +++ b/gcc/config/aarch64/aarch64.h
> @@ -958,4 +958,11 @@ extern const char *host_detect_local_cpu (int argc, const char **argv);
>  extern tree aarch64_fp16_type_node;
>  extern tree aarch64_fp16_ptr_type_node;
>  
> +#ifndef TARGET_LIBC_PROVIDES_SSP
> +#define LINK_SSP_SPEC "%{!mstack-protector-pauth:\
> +			 %{fstack-protector|fstack-protector-all\
> +			   |fstack-protector-strong|fstack-protector-explicit:\
> +			   -lssp_nonshared -lssp}}"
> +#endif
> +

I don't think we want to suppress this.  PAUTH pased stack protections
isn't an all-or-nothing solution.  What if some object files are built
with traditional -fstack-protector code?

If the library isn't referenced by any of the input objects we won't
pull anything useful in from the library, so leaving it in the link list
should be harmless.


>  #endif /* GCC_AARCH64_H */
> diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c
> index 6451b08191cf1a44aed502930da8603111f6e8ca..461f7b59584af9315accaecc0256abc9a2df4350 100644
> --- a/gcc/config/aarch64/aarch64.c
> +++ b/gcc/config/aarch64/aarch64.c
> @@ -2884,8 +2884,28 @@ aarch64_layout_frame (void)
>    else if (cfun->machine->frame.wb_candidate1 != INVALID_REGNUM)
>      max_push_offset = 256;
>  
> -  if (cfun->machine->frame.frame_size < max_push_offset
> -      && crtl->outgoing_args_size == 0)
> +  /* Swap callee-save and local variables area to make callee-save which
> +     includes return address register X30/LR position above local variables
> +     that any local buffer overflow will override return address.  */
> +  if (aarch64_pauth_stack_protector_enabled ())
> +    {
> +      if (varargs_and_saved_regs_size < max_push_offset)
> +	/* stp reg1, reg2, [sp, -varargs_and_saved_regs_size]!.  */
> +	cfun->machine->frame.callee_adjust = varargs_and_saved_regs_size;
> +      else
> +	/* sub sp, sp, varargs_and_saved_regs_size.  */
> +	cfun->machine->frame.initial_adjust = varargs_and_saved_regs_size;
> +
> +      /* Any final_adjust needed.  */
> +      cfun->machine->frame.final_adjust
> +	= cfun->machine->frame.frame_size - varargs_and_saved_regs_size;
> +
> +      /* Point hard_fp_offset and locals_offset to correct offsets.  */
> +      cfun->machine->frame.hard_fp_offset = varargs_and_saved_regs_size;
> +      cfun->machine->frame.locals_offset = cfun->machine->frame.hard_fp_offset;
> +    }
> +  else if (cfun->machine->frame.frame_size < max_push_offset
> +	   && crtl->outgoing_args_size == 0)
>      {
>        /* Simple, small frame with no outgoing arguments:
>  	 stp reg1, reg2, [sp, -frame_size]!
> @@ -3117,6 +3137,26 @@ aarch64_gen_load_pair (machine_mode mode, rtx reg1, rtx mem1, rtx reg2,
>      }
>  }
>  
> +/* Implement the TARGET_STACK_PROTECT_RUNTIME_ENABLED_P.  */
> +
> +static bool
> +aarch64_stack_protect_runtime_enabled (void)
> +{
> +  return !(TARGET_AARCH64_SSP_PAUTH && TARGET_ARMV8_3);
> +}
> +
> +/* Return TRUE if pointer authentication accelerated -fstack-protector is
> +   enabled.  */
> +
> +bool
> +aarch64_pauth_stack_protector_enabled (void)
> +{
> +  /* Enable if this function is identified by GCC to be with buffer over flow
> +     risk and we are not using GCC's standard libssp runtime.  */
> +  return (crtl->stack_protect_guard
> +	  && !aarch64_stack_protect_runtime_enabled ());
> +}
> +
>  /* Return TRUE if return address signing should be enabled for one function,
>     otherwise return FALSE.  */
>  
> @@ -3127,10 +3167,12 @@ aarch64_return_address_signing_enabled (void)
>    gcc_assert (cfun->machine->frame.laid_out);
>  
>    /* If signing scope is AARCH64_FUNCTION_NON_LEAF, we only sign a leaf function
> -     if it's LR is pushed onto stack.  */
> -  return (aarch64_ra_sign_scope == AARCH64_FUNCTION_ALL
> -	  || (aarch64_ra_sign_scope == AARCH64_FUNCTION_NON_LEAF
> -	      && cfun->machine->frame.reg_offset[LR_REGNUM] >= 0));
> +     if it's LR is pushed onto stack.  Pointer authentication based stack
> +     protector also implies return address signing.  */
> +  return ((aarch64_ra_sign_scope == AARCH64_FUNCTION_ALL
> +	   || (aarch64_ra_sign_scope == AARCH64_FUNCTION_NON_LEAF
> +	       && cfun->machine->frame.reg_offset[LR_REGNUM] >= 0))
> +	  || aarch64_pauth_stack_protector_enabled ());
>  }

Do we really want to force signing of ALL leaf functions if using PAUTH
based signing?  Surely we only need to check those that might be
vulnerable to buffer overruns.

R.

>  
>  /* Emit code to save the callee-saved registers from register number START
> @@ -8944,6 +8986,14 @@ aarch64_override_options (void)
>    if (aarch64_ra_sign_scope != AARCH64_FUNCTION_NONE && TARGET_ILP32)
>      sorry ("Return address signing is only supported for -mabi=lp64");
>  
> +  if (TARGET_AARCH64_SSP_PAUTH && TARGET_ILP32)
> +    sorry ("Pointer authentication based -fstack-protector is only supported "
> +	   "on -mabi=lp64.");
> +
> +  if (TARGET_AARCH64_SSP_PAUTH && !TARGET_ARMV8_3)
> +    error ("Pointer authentication based -fstack-protector is only supported "
> +	   "on architecture with pointer authentication extension.");
> +
>    /* Make sure we properly set up the explicit options.  */
>    if ((aarch64_cpu_string && valid_cpu)
>         || (aarch64_tune_string && valid_tune))
> @@ -14859,6 +14909,10 @@ aarch64_libgcc_floating_mode_supported_p
>  #undef TARGET_SETUP_INCOMING_VARARGS
>  #define TARGET_SETUP_INCOMING_VARARGS aarch64_setup_incoming_varargs
>  
> +#undef TARGET_STACK_PROTECT_RUNTIME_ENABLED_P
> +#define TARGET_STACK_PROTECT_RUNTIME_ENABLED_P \
> +  aarch64_stack_protect_runtime_enabled
> +
>  #undef TARGET_STRUCT_VALUE_RTX
>  #define TARGET_STRUCT_VALUE_RTX   aarch64_struct_value_rtx
>  
> diff --git a/gcc/config/aarch64/aarch64.opt b/gcc/config/aarch64/aarch64.opt
> index 54368848bbb249949921a3018d927c4bd61b1fbd..b157e51fd37d9040ca0a63c1f823f2ad70c8c17e 100644
> --- a/gcc/config/aarch64/aarch64.opt
> +++ b/gcc/config/aarch64/aarch64.opt
> @@ -188,3 +188,7 @@ single precision and to 32 bits for double precision.
>  mverbose-cost-dump
>  Common Undocumented Var(flag_aarch64_verbose_cost)
>  Enables verbose cost model dummping in the debug dump files.
> +
> +mstack-protector-pauth
> +Target Report RejectNegative Mask(AARCH64_SSP_PAUTH) Save
> +Enable pointer authentication accelerated stack protector.
> diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
> index c75d4b3697bae8805e13168d47066a1d0dd28df7..4eaefb4fc5e4eee92ba429ca5d23d314aa6f3d29 100644
> --- a/gcc/doc/invoke.texi
> +++ b/gcc/doc/invoke.texi
> @@ -14052,6 +14052,11 @@ Permissible values are @samp{none}, which disables return address signing,
>  functions, and @samp{all}, which enables pointer signing for all functions.  The
>  default value is @samp{none}.
>  
> +@item -mstack-protector-pauth
> +@opindex mstack-protector-pauth
> +Use ARMv8.3-A Pointer Authentication Extension to accelerate GCC
> +-fstack-protector.
> +
>  @end table
>  
>  @subsubsection @option{-march} and @option{-mcpu} Feature Modifiers
> diff --git a/gcc/testsuite/gcc.target/aarch64/stack_protector_1.c b/gcc/testsuite/gcc.target/aarch64/stack_protector_1.c
> new file mode 100644
> index 0000000000000000000000000000000000000000..56741bb38f4a2e95b82f04fe8009bcd6b1eb3f26
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/aarch64/stack_protector_1.c
> @@ -0,0 +1,23 @@
> +/* { dg-do compile } */
> +/* { dg-options "-O2 -fstack-protector-all -mstack-protector-pauth" } */
> +
> +int foo (int);
> +
> +int __attribute__ ((target ("arch=armv8.3-a")))
> +func1 (int a, int b, int c)
> +{
> +  /* paciasp */
> +  return a + foo (b) + c;
> +  /* retaa */
> +}
> +
> +/* Shouldn't enable pauth acceleration as it's ARMv8.2-A.  */
> +
> +int __attribute__ ((target ("arch=armv8.2-a")))
> +func2 (int a, int b, int c)
> +{
> +  return a + foo (b) + c;
> +}
> +
> +/* { dg-final { scan-assembler-times "paciasp" 1 } } */
> +/* { dg-final { scan-assembler-times "retaa" 1 } } */
> 


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]