Re: [Patch 5/8, Arm, GCC] Implement target feature macros for PACBTI. [Was RE: [Patch 4/7, Arm. GCC] Implement target feature macros for PACBTI.]
On 28/10/2021 12:43, Tejas Belagod via Gcc-patches wrote: -Original Message- From: Richard Earnshaw Sent: Monday, October 11, 2021 2:58 PM To: Tejas Belagod ; gcc-patches@gcc.gnu.org Subject: Re: [Patch 4/7, Arm. GCC] Implement target feature macros for PACBTI. On 08/10/2021 13:18, Tejas Belagod via Gcc-patches wrote: Hi, This patch implements target feature macros when PACBTI is enabled through the -march option or -mbranch-protection. Tested on arm-none-eabi. OK for trunk? 2021-10-04 Tejas Belagod gcc/ChangeLog: * config/arm/arm-c.c (arm_cpu_builtins): Define __ARM_FEATURE_BTI_DEFAULT and __ARM_FEATURE_PAC_DEFAULT. 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. I presume the specification for this is ACLE - please say so rather than making me guess. Yes, sorry, very poor description on my part. Now fixed - please see patch description below for links to specific ACLE sections. + cpp_undef (pfile, "__ARM_FEATURE_BTI_DEFAULT"); + cpp_undef (pfile, "__ARM_FEATURE_PAC_DEFAULT"); + if (TARGET_HAVE_PACBTI) +{ + builtin_define_with_int_value ("__ARM_FEATURE_BTI_DEFAULT", +arm_enable_pacbti & 0x1); My reading of the ACLE specification would suggest this shouldn't be defined if it would have a value of 0, but that's not what this code does. I think it would be better to move this outside the TARGET_HAVE_PACBTI and use the def_or_undef approach. + builtin_define_with_int_value ("__ARM_FEATURE_PAC_DEFAULT", +arm_enable_pacbti >> 1); This one is less clear, could the value ever be zero? I guess exactly one of a-key and b-key must be defined and each has a separate bit. Now fixed according to what the arch specifies. For the M-profile, there's only one key which means when -mbranch-protection is invoked, bit 0 is always 1. +} + + Not more than one blank line at the end of a block. diff --git a/gcc/testsuite/gcc.target/arm/acle/pacbti-m-predef-2.c b/gcc/testsuite/gcc.target/arm/acle/pacbti-m-predef-2.c Given what I've said above, I think you need to also test that __ARM_FEATURE_BTI_DEFAULT is defined before testing the value (and emitting #error if it isn't). Fixed. 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). 2021-10-25 Tejas Belagod 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: New test. * gcc.target/arm/acle/pacbti-m-predef-5.c: New test. Tested the following configurations, OK for trunk? -mthumb/-march=armv8.1-m.main+pacbti/-mfloat-abi=soft -marm/-march=armv7-a/-mfpu=vfpv3-d16/-mfloat-abi=softfp mcmodel=small and tiny aarch64-none-linux-gnu native test and bootstrap Thanks, Tejas. OK. R.
[Patch 5/8, Arm, GCC] Implement target feature macros for PACBTI. [Was RE: [Patch 4/7, Arm. GCC] Implement target feature macros for PACBTI.]
> -Original Message- > From: Richard Earnshaw > Sent: Monday, October 11, 2021 2:58 PM > To: Tejas Belagod ; gcc-patches@gcc.gnu.org > Subject: Re: [Patch 4/7, Arm. GCC] Implement target feature macros for > PACBTI. > > On 08/10/2021 13:18, Tejas Belagod via Gcc-patches wrote: > > Hi, > > > > This patch implements target feature macros when PACBTI is enabled > > through the -march option or -mbranch-protection. > > > > Tested on arm-none-eabi. OK for trunk? > > > > 2021-10-04 Tejas Belagod > > > > gcc/ChangeLog: > > > > * config/arm/arm-c.c (arm_cpu_builtins): Define > > __ARM_FEATURE_BTI_DEFAULT and > __ARM_FEATURE_PAC_DEFAULT. > > > > 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. > > > > I presume the specification for this is ACLE - please say so rather than > making > me guess. > Yes, sorry, very poor description on my part. Now fixed - please see patch description below for links to specific ACLE sections. > > + cpp_undef (pfile, "__ARM_FEATURE_BTI_DEFAULT"); > + cpp_undef (pfile, "__ARM_FEATURE_PAC_DEFAULT"); > + if (TARGET_HAVE_PACBTI) > +{ > + builtin_define_with_int_value ("__ARM_FEATURE_BTI_DEFAULT", > + arm_enable_pacbti & 0x1); > > My reading of the ACLE specification would suggest this shouldn't be > defined if it would have a value of 0, but that's not what this code > does. I think it would be better to move this outside the > TARGET_HAVE_PACBTI and use the def_or_undef approach. > > + builtin_define_with_int_value ("__ARM_FEATURE_PAC_DEFAULT", > + arm_enable_pacbti >> 1); > > This one is less clear, could the value ever be zero? I guess exactly > one of a-key and b-key must be defined and each has a separate bit. > Now fixed according to what the arch specifies. For the M-profile, there's only one key which means when -mbranch-protection is invoked, bit 0 is always 1. > +} > + > + > > Not more than one blank line at the end of a block. > > > diff --git a/gcc/testsuite/gcc.target/arm/acle/pacbti-m-predef-2.c > b/gcc/testsuite/gcc.target/arm/acle/pacbti-m-predef-2.c > > > Given what I've said above, I think you need to also test that > __ARM_FEATURE_BTI_DEFAULT is defined before testing the value (and > emitting #error if it isn't). > Fixed. 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). 2021-10-25 Tejas Belagod 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: New test. * gcc.target/arm/acle/pacbti-m-predef-5.c: New test. Tested the following configurations, OK for trunk? -mthumb/-march=armv8.1-m.main+pacbti/-mfloat-abi=soft -marm/-march=armv7-a/-mfpu=vfpv3-d16/-mfloat-abi=softfp mcmodel=small and tiny aarch64-none-linux-gnu native test and bootstrap Thanks, Tejas. diff --git a/gcc/config/arm/arm-c.c b/gcc/config/arm/arm-c.c index cc7901bca8dc9c5c27ed6afc5bc26afd42689e6d..98d47ad4cc6e88aa7401429a809c555c5aadc15f 100644 --- a/gcc/config/arm/arm-c.c +++ b/gcc/config/arm/arm-c.c @@ -193,6 +193,24 @@ arm_cpu_builtins (struct cpp_reader* pfile) def_or_undef_macro (pfile, "__ARM_FEATURE_COMPLEX", TARGET_COMPLEX); def_or_undef_macro (pfile, "__ARM_32BIT_STATE", TARGET_32BIT); + def_or_undef_macro (pfile, "__ARM_FEATURE_PAUTH", TARGET_HAVE_PACBTI); + def_or_undef_macro (pfile, "__ARM_FEATURE_BTI", TARGET_HAVE_PACBTI); + def_or_undef_macro (pfile, "__ARM_FEATURE_BTI_DEFAULT", + aarch_enable_bti == 1); + + cpp_undef (pfile, "__ARM_FEATURE_PAC_DEFAULT"); + if (aarch_ra_sign_scope != AARCH_FUNCTION_NONE) + { +unsigned int pac = 1; + +gcc_assert (aarch_ra_sign_key == AARCH_KEY_A); + +if (aarch_ra_sign_scope == AARCH_FUNCTION_ALL) + pac |= 0x4; + +builtin_define_with_int_value ("__AR
Re: [Patch 4/7, Arm. GCC] Implement target feature macros for PACBTI.
On 08/10/2021 13:18, Tejas Belagod via Gcc-patches wrote: Hi, This patch implements target feature macros when PACBTI is enabled through the -march option or -mbranch-protection. Tested on arm-none-eabi. OK for trunk? 2021-10-04 Tejas Belagod gcc/ChangeLog: * config/arm/arm-c.c (arm_cpu_builtins): Define __ARM_FEATURE_BTI_DEFAULT and __ARM_FEATURE_PAC_DEFAULT. 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. I presume the specification for this is ACLE - please say so rather than making me guess. + cpp_undef (pfile, "__ARM_FEATURE_BTI_DEFAULT"); + cpp_undef (pfile, "__ARM_FEATURE_PAC_DEFAULT"); + if (TARGET_HAVE_PACBTI) +{ + builtin_define_with_int_value ("__ARM_FEATURE_BTI_DEFAULT", +arm_enable_pacbti & 0x1); My reading of the ACLE specification would suggest this shouldn't be defined if it would have a value of 0, but that's not what this code does. I think it would be better to move this outside the TARGET_HAVE_PACBTI and use the def_or_undef approach. + builtin_define_with_int_value ("__ARM_FEATURE_PAC_DEFAULT", +arm_enable_pacbti >> 1); This one is less clear, could the value ever be zero? I guess exactly one of a-key and b-key must be defined and each has a separate bit. +} + + Not more than one blank line at the end of a block. diff --git a/gcc/testsuite/gcc.target/arm/acle/pacbti-m-predef-2.c b/gcc/testsuite/gcc.target/arm/acle/pacbti-m-predef-2.c Given what I've said above, I think you need to also test that __ARM_FEATURE_BTI_DEFAULT is defined before testing the value (and emitting #error if it isn't). R.
[Patch 4/7, Arm. GCC] Implement target feature macros for PACBTI.
Hi, This patch implements target feature macros when PACBTI is enabled through the -march option or -mbranch-protection. Tested on arm-none-eabi. OK for trunk? 2021-10-04 Tejas Belagod gcc/ChangeLog: * config/arm/arm-c.c (arm_cpu_builtins): Define __ARM_FEATURE_BTI_DEFAULT and __ARM_FEATURE_PAC_DEFAULT. 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. diff --git a/gcc/config/arm/arm-c.c b/gcc/config/arm/arm-c.c index cc7901bca8dc9c5c27ed6afc5bc26afd42689e6d..00dc1c2f13f2023c2ba8d7b03038a4cdde068ef6 100644 --- a/gcc/config/arm/arm-c.c +++ b/gcc/config/arm/arm-c.c @@ -193,6 +193,17 @@ arm_cpu_builtins (struct cpp_reader* pfile) def_or_undef_macro (pfile, "__ARM_FEATURE_COMPLEX", TARGET_COMPLEX); def_or_undef_macro (pfile, "__ARM_32BIT_STATE", TARGET_32BIT); + cpp_undef (pfile, "__ARM_FEATURE_BTI_DEFAULT"); + cpp_undef (pfile, "__ARM_FEATURE_PAC_DEFAULT"); + if (TARGET_HAVE_PACBTI) +{ + builtin_define_with_int_value ("__ARM_FEATURE_BTI_DEFAULT", +arm_enable_pacbti & 0x1); + builtin_define_with_int_value ("__ARM_FEATURE_PAC_DEFAULT", +arm_enable_pacbti >> 1); +} + + cpp_undef (pfile, "__ARM_FEATURE_MVE"); if (TARGET_HAVE_MVE && TARGET_HAVE_MVE_FLOAT) { diff --git a/gcc/testsuite/gcc.target/arm/acle/pacbti-m-predef-2.c b/gcc/testsuite/gcc.target/arm/acle/pacbti-m-predef-2.c new file mode 100644 index ..7e8cdb2c5fc74dd22085fcac1f692229300a333a --- /dev/null +++ b/gcc/testsuite/gcc.target/arm/acle/pacbti-m-predef-2.c @@ -0,0 +1,16 @@ + +/* { dg-do run } */ +/* { dg-require-effective-target arm_pacbti_hw } */ +/* { dg-additional-options " -mbranch-protection=bti+pac-ret+b-key+leaf" } */ + +int +main() +{ + if (__ARM_FEATURE_BTI_DEFAULT != 1) +__builtin_abort (); + + if (__ARM_FEATURE_PAC_DEFAULT != 6) +__builtin_abort (); + + return 0; +} diff --git a/gcc/testsuite/gcc.target/arm/acle/pacbti-m-predef-4.c b/gcc/testsuite/gcc.target/arm/acle/pacbti-m-predef-4.c new file mode 100644 index ..41fdcf91a8ab789d055407ae3f8c151984660ee9 --- /dev/null +++ b/gcc/testsuite/gcc.target/arm/acle/pacbti-m-predef-4.c @@ -0,0 +1,16 @@ + +/* { dg-do run } */ +/* { dg-require-effective-target arm_pacbti_hw } */ +/* { dg-additional-options " -mbranch-protection=pac-ret+b-key" } */ + +int +main() +{ + if (__ARM_FEATURE_BTI_DEFAULT != 0) +__builtin_abort (); + + if (__ARM_FEATURE_PAC_DEFAULT != 2) +__builtin_abort (); + + return 0; +} diff --git a/gcc/testsuite/gcc.target/arm/acle/pacbti-m-predef-5.c b/gcc/testsuite/gcc.target/arm/acle/pacbti-m-predef-5.c new file mode 100644 index ..9527c9620a3a5c973b47a5f364ae290d975358c1 --- /dev/null +++ b/gcc/testsuite/gcc.target/arm/acle/pacbti-m-predef-5.c @@ -0,0 +1,16 @@ + +/* { dg-do run } */ +/* { dg-require-effective-target arm_pacbti_hw } */ +/* { dg-additional-options " -mbranch-protection=bti+pac-ret+leaf" } */ + +int +main() +{ + if (__ARM_FEATURE_BTI_DEFAULT != 1) +__builtin_abort (); + + if (__ARM_FEATURE_PAC_DEFAULT != 5) +__builtin_abort (); + + return 0; +}