Hi James
On 07/11/18 15:36, James Greenhalgh wrote:
> 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).
>
Updated patch attached. Return address signing is not supported for
ILP32 currently. This patch just follows that and hence the extra ILP32
check is added.
Thanks
Sudi
> 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. */
>
diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c
index b97d9e4deecf5ca33761dfd1008c39bb4b849881..e267d3441fd7f21105bfba339b69f2ecdb7595ae 100644
--- a/gcc/config/aarch64/aarch64.c
+++ b/gcc/config/aarch64/aarch64.c
@@ -11579,6 +11579,28 @@ 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
+ }
+
+ /* Return address signing is currently not supported for ILP32 targets. For
+ LP64 targets use the configured option in the absence of a command-line
+ option for -mbranch-protection. */
+ if (!TARGET_ILP32 && accepted_branch_protection_string == NULL)
+ {
+#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. */
diff --git a/gcc/configure b/gcc/configure
index 03461f1e27538a3a0791c2b61b0e75c3ff1a25be..a0f95106c22ee858bbf4516f14cd9d265dede272 100755
--- a/gcc/configure
+++ b/gcc/configure
@@ -947,6 +947,7 @@ with_plugin_ld
enable_gnu_indirect_function
enable_initfini_array
enable_comdat
+enable_standard_branch_protection
enable_fix_cortex_a53_835769
enable_fix_cortex_a53_843419
with_glibc_version
@@ -1677,6 +1678,14 @@ Optional Features:
--enable-initfini-array use .init_array/.fini_array sections
--enable-comdat enable COMDAT group support
+ --enable-standard-branch-protection
+ enable Branch Target Identification Mechanism and
+ Return Address Signing by default for AArch64
+ --disable-standard-branch-protection
+ disable Branch Target Identification Mechanism and
+ Return Address Signing by default for AArch64
+
+
--enable-fix-cortex-a53-835769
enable workaround for AArch64 Cortex-A53 erratum
835769 by default
@@ -18529,7 +18538,7 @@ else
lt_dlunknown=0; lt_dlno_uscore=1; lt_dlneed_uscore=2
lt_status=$lt_dlunknown
cat > conftest.$ac_ext <<_LT_EOF
-#line 18532 "configure"
+#line 18541 "configure"
#include "confdefs.h"
#if HAVE_DLFCN_H
@@ -18635,7 +18644,7 @@ else
lt_dlunknown=0; lt_dlno_uscore=1; lt_dlneed_uscore=2
lt_status=$lt_dlunknown
cat > conftest.$ac_ext <<_LT_EOF
-#line 18638 "configure"
+#line 18647 "configure"
#include "confdefs.h"
#if HAVE_DLFCN_H
@@ -24939,6 +24948,25 @@ $as_echo "#define HAVE_AS_SMALL_PIC_RELOCS 1" >>confdefs.h
fi
+ # Enable Branch Target Identification Mechanism and Return Address
+ # Signing by default.
+ # Check whether --enable-standard-branch-protection was given.
+if test "${enable_standard_branch_protection+set}" = set; then :
+ enableval=$enable_standard_branch_protection;
+ case $enableval in
+ yes)
+ tm_defines="${tm_defines} TARGET_ENABLE_BTI=1 TARGET_ENABLE_PAC_RET=1"
+ ;;
+ no)
+ ;;
+ *)
+ as_fn_error "'$enableval' is an invalid value for --enable-standard-branch-protection.\
+ Valid choices are 'yes' and 'no'." "$LINENO" 5
+ ;;
+ esac
+
+fi
+
# Enable default workaround for AArch64 Cortex-A53 erratum 835769.
# Check whether --enable-fix-cortex-a53-835769 was given.
if test "${enable_fix_cortex_a53_835769+set}" = set; then :
diff --git a/gcc/configure.ac b/gcc/configure.ac
index 1ac290e3c6e50f7da483ec95c4ae35faec765dab..12bbe86cb09b2c78dbc6d60d4bf74b89ce9fa5fe 100644
--- a/gcc/configure.ac
+++ b/gcc/configure.ac
@@ -3936,6 +3936,29 @@ case "$target" in
ldr x0, [[x2, #:gotpage_lo15:globalsym]]
],,[AC_DEFINE(HAVE_AS_SMALL_PIC_RELOCS, 1,
[Define if your assembler supports relocs needed by -fpic.])])
+ # Enable Branch Target Identification Mechanism and Return Address
+ # Signing by default.
+ AC_ARG_ENABLE(standard-branch-protection,
+ [
+AS_HELP_STRING([--enable-standard-branch-protection],
+ [enable Branch Target Identification Mechanism and Return Address Signing by default for AArch64])
+AS_HELP_STRING([--disable-standard-branch-protection],
+ [disable Branch Target Identification Mechanism and Return Address Signing by default for AArch64])
+ ],
+ [
+ case $enableval in
+ yes)
+ tm_defines="${tm_defines} TARGET_ENABLE_BTI=1 TARGET_ENABLE_PAC_RET=1"
+ ;;
+ no)
+ ;;
+ *)
+ AC_MSG_ERROR(['$enableval' is an invalid value for --enable-standard-branch-protection.\
+ Valid choices are 'yes' and 'no'.])
+ ;;
+ esac
+ ],
+ [])
# Enable default workaround for AArch64 Cortex-A53 erratum 835769.
AC_ARG_ENABLE(fix-cortex-a53-835769,
[
diff --git a/gcc/doc/install.texi b/gcc/doc/install.texi
index 6c7dac99743f98985dca8db56b5e8362b2d49860..a2ed628aefade62ee941976dcb31812c7541c166 100644
--- a/gcc/doc/install.texi
+++ b/gcc/doc/install.texi
@@ -3389,6 +3389,16 @@ The workaround is disabled by default if neither of
@option{--enable-fix-cortex-a53-843419} or
@option{--disable-fix-cortex-a53-843419} is given at configure time.
+To enable Branch Target Identification Mechanism and Return Address Signing by
+default at configure time use the @option{--enable-standard-branch-protection}
+option. This is equivalent to having @option{-mbranch-protection=standard}
+during compilation. This can be explicitly disabled during compilation by
+passing the @option{-mbranch-protection=none} option which turns off all
+types of branch protections. Conversely,
+@option{--disable-standard-branch-protection} will disable both the
+protections by default. This mechanism is turned off by default if neither
+of the options are given at configure time.
+
@html
<hr />
@end html
diff --git a/gcc/testsuite/gcc.target/aarch64/bti-1.c b/gcc/testsuite/gcc.target/aarch64/bti-1.c
index 575d01a5411a19dabcdb56b777e5d87d9703a848..67551859649f98541ac81ac24533a45b0c1afde2 100644
--- a/gcc/testsuite/gcc.target/aarch64/bti-1.c
+++ b/gcc/testsuite/gcc.target/aarch64/bti-1.c
@@ -1,6 +1,9 @@
/* { dg-do compile } */
/* -Os to create jump table. */
-/* { dg-options "-Os -mbranch-protection=standard" } */
+/* { dg-options "-Os" } */
+/* If configured with --enable-standard-branch-protection, don't use
+ command line option. */
+/* { dg-additional-options "-mbranch-protection=standard" { target { ! default_branch_protection } } } */
extern int f1 (void);
extern int f2 (void);
diff --git a/gcc/testsuite/gcc.target/aarch64/bti-2.c b/gcc/testsuite/gcc.target/aarch64/bti-2.c
index e50eef15c8936d00716c582e90b235add5da9136..85943c3d6415b010c858cb948221e33b0d30a310 100644
--- a/gcc/testsuite/gcc.target/aarch64/bti-2.c
+++ b/gcc/testsuite/gcc.target/aarch64/bti-2.c
@@ -1,6 +1,8 @@
/* { dg-do run } */
/* { dg-require-effective-target aarch64_bti_hw } */
-/* { dg-options "-mbranch-protection=standard" } */
+/* If configured with --enable-standard-branch-protection, don't use
+ command line option. */
+/* { dg-additional-options "-mbranch-protection=standard" { target { ! default_branch_protection } } } */
#include<stdio.h>
diff --git a/gcc/testsuite/lib/target-supports.exp b/gcc/testsuite/lib/target-supports.exp
index 65ff16817f5f88255148bb69d7018975465197dc..b66daffbaadb13029d0ed5f60452e90d979da57d 100644
--- a/gcc/testsuite/lib/target-supports.exp
+++ b/gcc/testsuite/lib/target-supports.exp
@@ -4286,6 +4286,11 @@ proc check_effective_target_aarch64_bti_hw { } {
} "-O2" ]
}
+# Return 1 if GCC was configured with --enable-standard-branch-protection
+proc check_effective_target_default_branch_protection { } {
+ return [check_configured_with "enable-standard-branch-protection"]
+}
+
# Return 1 if the target supports the ARMv8.1 Adv.SIMD extension, 0
# otherwise. The test is valid for AArch64 and ARM. Record the command
# line options needed.
