On 12/07/2022 16:45, Andrea Corallo via Gcc-patches wrote:
Richard Earnshaw <richard.earns...@foss.arm.com> writes:

On 28/04/2022 10:42, Andrea Corallo via Gcc-patches wrote:
This patch implements target feature macros when PACBTI is enabled
through the -march option or -mbranch-protection.  The target feature
macros __ARM_FEATURE_PAC_DEFAULT and __ARM_FEATURE_BTI_DEFAULT are
specified in ARM ACLE
<https://developer.arm.com/documentation/101028/0012/5--Feature-test-macros?lang=en>
__ARM_FEATURE_PAUTH and __ARM_FEATURE_BTI are specified in the
pull-request <https://github.com/ARM-software/acle/pull/55>.
Approved here
<https://gcc.gnu.org/pipermail/gcc-patches/2021-December/586334.html>.
gcc/ChangeLog:
        * config/arm/arm-c.c (arm_cpu_builtins): Define
        __ARM_FEATURE_BTI_DEFAULT, __ARM_FEATURE_PAC_DEFAULT,
        __ARM_FEATURE_PAUTH and __ARM_FEATURE_BTI.

This bit is OK.

gcc/testsuite/ChangeLog:
        * gcc.target/arm/acle/pacbti-m-predef-2.c: New test.
        * gcc.target/arm/acle/pacbti-m-predef-4.c: New test.
        * gcc.target/arm/acle/pacbti-m-predef-5.c: New test.


These are all execution tests.  I think we also need some compile-only
tests so that we get better coverage when the target does not directly
support PACBTI.

We also need some tests for the defines when targetting armv8-m.main
and some tests for checking __ARM_FEATURE_BTI and __ARM_FEATURE_PAC
(the tests here check only the '..._DEFAULT' macros.

Hi Richard & all,

please find attached the updated version of this patch.

Best Regards

   Andrea

gcc/ChangeLog:

        * config/arm/arm-c.c (arm_cpu_builtins): Define
        __ARM_FEATURE_BTI_DEFAULT, __ARM_FEATURE_PAC_DEFAULT,
        __ARM_FEATURE_PAUTH and __ARM_FEATURE_BTI.

gcc/testsuite/ChangeLog:

        * gcc.target/arm/acle/pacbti-m-predef-2.c: New test.
        * gcc.target/arm/acle/pacbti-m-predef-4.c: Likewise.
        * gcc.target/arm/acle/pacbti-m-predef-5.c: Likewise.
        * gcc.target/arm/acle/pacbti-m-predef-8.c: Likewise.
        * gcc.target/arm/acle/pacbti-m-predef-9.c: Likewise.
        * gcc.target/arm/acle/pacbti-m-predef-10.c: Likewise.
        * gcc.target/arm/acle/pacbti-m-predef-11.c: Likewise.
        * gcc.target/arm/acle/pacbti-m-predef-12.c: Likewise.


diff --git a/gcc/testsuite/gcc.target/arm/acle/pacbti-m-predef-10.c b/gcc/testsuite/gcc.target/arm/acle/pacbti-m-predef-10.c
new file mode 100644
index 00000000000..311cf572dd9
--- /dev/null
+++ b/gcc/testsuite/gcc.target/arm/acle/pacbti-m-predef-10.c
@@ -0,0 +1,10 @@
+/* { dg-do compile } */
+/* { dg-additional-options " -mbranch-protection=bti+pac-ret" } */

This is not enough. For example, if the testsuite is being run with "-march=armv6-m" as the testrun options, we'll get an error that will cause a test failure. You need to run a pre-test rule that validates that adding -mbranch-protection is safe.

+++ b/gcc/testsuite/gcc.target/arm/acle/pacbti-m-predef-11.c
@@ -0,0 +1,10 @@
+/* { dg-do compile } */
+/* { dg-options "-march=armv8.1-m.main+pacbti" } */

Similarly here, this would conflict with, for example, "-marm" as test options.

+++ b/gcc/testsuite/gcc.target/arm/acle/pacbti-m-predef-5.c
@@ -0,0 +1,24 @@
+
+/* { dg-do run } */

Blank line at the start of the test.

The other tests have similar issues.

R.

Reply via email to