On Fri, Nov 02, 2018 at 01:38:46PM -0500, Sudakshina Das wrote:
> Hi
> 
> This patch is part of a series that enables ARMv8.5-A in GCC and
> adds Branch Target Identification Mechanism.
> (https://developer.arm.com/products/architecture/cpu-architecture/a-profile/exploration-tools)
> 
> This patch is adding a new configure option for enabling and return
> address signing by default with --enable-standard-branch-protection.
> This is equivalent to -mbranch-protection=standard which would
> imply -mbranch-protection=pac-ret+bti.
> 
> Bootstrapped and regression tested with aarch64-none-linux-gnu with
> and without the configure option turned on.
> Also tested on aarch64-none-elf with and without configure option with a
> BTI enabled aem. Only 2 regressions and these were because newlib
> requires patches to protect hand coded libraries with BTI.
> 
> Is this ok for trunk?

With a tweak to the comment above your changes in aarch64.c, yes this is OK.

> *** gcc/ChangeLog ***
> 
> 2018-xx-xx  Sudakshina Das  <sudi....@arm.com>
> 
>       * config/aarch64/aarch64.c (aarch64_override_options): Add case to check
>       configure option to set BTI and Return Address Signing.
>       * configure.ac: Add --enable-standard-branch-protection and
>       --disable-standard-branch-protection.
>       * configure: Regenerated.
>       * doc/install.texi: Document the same.
> 
> *** gcc/testsuite/ChangeLog ***
> 
> 2018-xx-xx  Sudakshina Das  <sudi....@arm.com>
> 
>       * gcc.target/aarch64/bti-1.c: Update test to not add command
>       line option when configure with bti.
>       * gcc.target/aarch64/bti-2.c: Likewise.
>       * lib/target-supports.exp
>       (check_effective_target_default_branch_protection):
>       Add configure check for --enable-standard-branch-protection.
> 

> diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c
> index 
> 12a55a640de4fdc5df21d313c7ea6841f1daf3f2..a1a5b7b464eaa2ce67ac66d9aea837159590aa07
>  100644
> --- a/gcc/config/aarch64/aarch64.c
> +++ b/gcc/config/aarch64/aarch64.c
> @@ -11558,6 +11558,26 @@ aarch64_override_options (void)
>    if (!selected_tune)
>      selected_tune = selected_cpu;
>  
> +  if (aarch64_enable_bti == 2)
> +    {
> +#ifdef TARGET_ENABLE_BTI
> +      aarch64_enable_bti = 1;
> +#else
> +      aarch64_enable_bti = 0;
> +#endif
> +    }
> +
> +  /* No command-line option yet.  */

This is too broad. Can you narrow this down to which command line option this
relates to, and what the expected default behaviours are (for both LP64 and
ILP32).

Thanks,
James

> +  if (accepted_branch_protection_string == NULL && !TARGET_ILP32)
> +    {
> +#ifdef TARGET_ENABLE_PAC_RET
> +      aarch64_ra_sign_scope = AARCH64_FUNCTION_NON_LEAF;
> +      aarch64_ra_sign_key = AARCH64_KEY_A;
> +#else
> +      aarch64_ra_sign_scope = AARCH64_FUNCTION_NONE;
> +#endif
> +    }
> +
>  #ifndef HAVE_AS_MABI_OPTION
>    /* The compiler may have been configured with 2.23.* binutils, which does
>       not have support for ILP32.  */

Reply via email to