* [PATCH] arm: Zero/Sign extends for CMSE security
@ 2024-04-24 15:55 Richard Ball
2024-04-25 10:01 ` Richard Earnshaw (lists)
2024-04-25 11:47 ` Torbjorn SVENSSON
0 siblings, 2 replies; 12+ messages in thread
From: Richard Ball @ 2024-04-24 15:55 UTC (permalink / raw)
To: gcc-patches, Richard Earnshaw, Richard Sandiford,
Marcus Shawcroft, Kyrylo Tkachov
[-- Attachment #1: Type: text/plain, Size: 744 bytes --]
This patch makes the following changes:
1) When calling a secure function from non-secure code then any arguments
smaller than 32-bits that are passed in registers are zero- or sign-extended.
2) After a non-secure function returns into secure code then any return value
smaller than 32-bits that is passed in a register is zero- or sign-extended.
This patch addresses the following CVE-2024-0151.
gcc/ChangeLog:
PR target/114837
* config/arm/arm.cc (cmse_nonsecure_call_inline_register_clear):
Add zero/sign extend.
(arm_expand_prologue): Add zero/sign extend.
gcc/testsuite/ChangeLog:
* gcc.target/arm/cmse/extend-param.c: New test.
* gcc.target/arm/cmse/extend-return.c: New test.
[-- Attachment #2: CMSEvulnerability.patch --]
[-- Type: text/x-patch, Size: 7607 bytes --]
diff --git a/gcc/config/arm/arm.cc b/gcc/config/arm/arm.cc
index 0217abc218d60956ce727e6d008d46b9176dddc5..ea0c963a4d67ecd70e1571624e84dfe46d757df9 100644
--- a/gcc/config/arm/arm.cc
+++ b/gcc/config/arm/arm.cc
@@ -19210,6 +19210,30 @@ cmse_nonsecure_call_inline_register_clear (void)
end_sequence ();
emit_insn_before (seq, insn);
+ /* The AAPCS requires the callee to widen integral types narrower
+ than 32 bits to the full width of the register; but when handling
+ calls to non-secure space, we cannot trust the callee to have
+ correctly done so. So forcibly re-widen the result here. */
+ tree ret_type = TREE_TYPE (fntype);
+ if ((TREE_CODE (ret_type) == INTEGER_TYPE
+ || TREE_CODE (ret_type) == ENUMERAL_TYPE
+ || TREE_CODE (ret_type) == BOOLEAN_TYPE)
+ && known_lt (GET_MODE_SIZE (TYPE_MODE (ret_type)), 4))
+ {
+ machine_mode ret_mode = TYPE_MODE (ret_type);
+ rtx extend;
+ if (TYPE_UNSIGNED (ret_type))
+ extend = gen_rtx_ZERO_EXTEND (SImode,
+ gen_rtx_REG (ret_mode, R0_REGNUM));
+ else
+ extend = gen_rtx_SIGN_EXTEND (SImode,
+ gen_rtx_REG (ret_mode, R0_REGNUM));
+ emit_insn_after (gen_rtx_SET (gen_rtx_REG (SImode, R0_REGNUM),
+ extend), insn);
+
+ }
+
+
if (TARGET_HAVE_FPCXT_CMSE)
{
rtx_insn *last, *pop_insn, *after = insn;
@@ -23652,6 +23676,51 @@ arm_expand_prologue (void)
ip_rtx = gen_rtx_REG (SImode, IP_REGNUM);
+ /* The AAPCS requires the callee to widen integral types narrower
+ than 32 bits to the full width of the register; but when handling
+ calls to non-secure space, we cannot trust the callee to have
+ correctly done so. So forcibly re-widen the result here. */
+ if (IS_CMSE_ENTRY (func_type))
+ {
+ function_args_iterator args_iter;
+ CUMULATIVE_ARGS args_so_far_v;
+ cumulative_args_t args_so_far;
+ bool first_param = true;
+ tree arg_type;
+ tree fndecl = current_function_decl;
+ tree fntype = TREE_TYPE (fndecl);
+ arm_init_cumulative_args (&args_so_far_v, fntype, NULL_RTX, fndecl);
+ args_so_far = pack_cumulative_args (&args_so_far_v);
+ FOREACH_FUNCTION_ARGS (fntype, arg_type, args_iter)
+ {
+ rtx arg_rtx;
+
+ if (VOID_TYPE_P (arg_type))
+ break;
+
+ function_arg_info arg (arg_type, /*named=*/true);
+ if (!first_param)
+ /* We should advance after processing the argument and pass
+ the argument we're advancing past. */
+ arm_function_arg_advance (args_so_far, arg);
+ first_param = false;
+ arg_rtx = arm_function_arg (args_so_far, arg);
+ gcc_assert (REG_P (arg_rtx));
+ if ((TREE_CODE (arg_type) == INTEGER_TYPE
+ || TREE_CODE (arg_type) == ENUMERAL_TYPE
+ || TREE_CODE (arg_type) == BOOLEAN_TYPE)
+ && known_lt (GET_MODE_SIZE (GET_MODE (arg_rtx)), 4))
+ {
+ if (TYPE_UNSIGNED (arg_type))
+ emit_set_insn (gen_rtx_REG (SImode, REGNO (arg_rtx)),
+ gen_rtx_ZERO_EXTEND (SImode, arg_rtx));
+ else
+ emit_set_insn (gen_rtx_REG (SImode, REGNO (arg_rtx)),
+ gen_rtx_SIGN_EXTEND (SImode, arg_rtx));
+ }
+ }
+ }
+
if (IS_STACKALIGN (func_type))
{
rtx r0, r1;
diff --git a/gcc/testsuite/gcc.target/arm/cmse/extend-param.c b/gcc/testsuite/gcc.target/arm/cmse/extend-param.c
new file mode 100644
index 0000000000000000000000000000000000000000..01fac7862385f871f3ecc246ede95eea180be025
--- /dev/null
+++ b/gcc/testsuite/gcc.target/arm/cmse/extend-param.c
@@ -0,0 +1,96 @@
+/* { dg-do compile } */
+/* { dg-options "-mcmse" } */
+/* { dg-final { check-function-bodies "**" "" "" } } */
+
+#include <arm_cmse.h>
+#include <stdbool.h>
+
+#define ARRAY_SIZE (256)
+char array[ARRAY_SIZE];
+
+enum offset
+{
+ zero = 0,
+ one = 1,
+ two = 2
+};
+
+/*
+**__acle_se_unsignSecureFunc:
+** ...
+** uxtb r0, r0
+** ...
+*/
+__attribute__((cmse_nonsecure_entry)) char unsignSecureFunc (unsigned char index) {
+ if (index >= ARRAY_SIZE)
+ return 0;
+ return array[index];
+}
+
+/*
+**__acle_se_signSecureFunc:
+** ...
+** sxtb r0, r0
+** ...
+*/
+__attribute__((cmse_nonsecure_entry)) char signSecureFunc (signed char index) {
+ if (index >= ARRAY_SIZE)
+ return 0;
+ return array[index];
+}
+
+/*
+**__acle_se_shortUnsignSecureFunc:
+** ...
+** uxth r0, r0
+** ...
+*/
+__attribute__((cmse_nonsecure_entry)) char shortUnsignSecureFunc (unsigned short index) {
+ if (index >= ARRAY_SIZE)
+ return 0;
+ return array[index];
+}
+
+/*
+**__acle_se_shortSignSecureFunc:
+** ...
+** sxth r0, r0
+** ...
+*/
+__attribute__((cmse_nonsecure_entry)) char shortSignSecureFunc (signed short index) {
+ if (index >= ARRAY_SIZE)
+ return 0;
+ return array[index];
+}
+
+/*
+**__acle_se_enumSecureFunc:
+** ...
+** uxtb r0, r0
+** ...
+*/
+__attribute__((cmse_nonsecure_entry)) char enumSecureFunc (enum offset index) {
+
+ // Compiler may optimize away bounds check as value is an unsigned char.
+
+ // According to AAPCS caller will zero extend to ensure value is < 256.
+
+ if (index >= ARRAY_SIZE)
+ return 0;
+ return array[index];
+
+}
+
+/*
+**__acle_se_boolSecureFunc:
+** ...
+** uxtb r0, r0
+** ...
+*/
+__attribute__((cmse_nonsecure_entry)) char boolSecureFunc (bool index) {
+
+ if (index >= ARRAY_SIZE)
+ return 0;
+ return array[index];
+
+}
\ No newline at end of file
diff --git a/gcc/testsuite/gcc.target/arm/cmse/extend-return.c b/gcc/testsuite/gcc.target/arm/cmse/extend-return.c
new file mode 100644
index 0000000000000000000000000000000000000000..cf731ed33df7e6dc101320c1970016f01b14c59a
--- /dev/null
+++ b/gcc/testsuite/gcc.target/arm/cmse/extend-return.c
@@ -0,0 +1,92 @@
+/* { dg-do compile } */
+/* { dg-options "-mcmse" } */
+/* { dg-final { check-function-bodies "**" "" "" } } */
+
+#include <arm_cmse.h>
+#include <stdbool.h>
+
+enum offset
+{
+ zero = 0,
+ one = 1,
+ two = 2
+};
+
+typedef unsigned char __attribute__ ((cmse_nonsecure_call)) ns_unsign_foo_t (void);
+typedef signed char __attribute__ ((cmse_nonsecure_call)) ns_sign_foo_t (void);
+typedef unsigned short __attribute__ ((cmse_nonsecure_call)) ns_short_unsign_foo_t (void);
+typedef signed short __attribute__ ((cmse_nonsecure_call)) ns_short_sign_foo_t (void);
+typedef enum offset __attribute__ ((cmse_nonsecure_call)) ns_enum_foo_t (void);
+typedef bool __attribute__ ((cmse_nonsecure_call)) ns_bool_foo_t (void);
+
+/*
+**unsignNonsecure0:
+** ...
+** bl __gnu_cmse_nonsecure_call
+** uxtb r0, r0
+** ...
+*/
+unsigned char unsignNonsecure0 (ns_unsign_foo_t * ns_foo_p)
+{
+ return ns_foo_p ();
+}
+
+/*
+**signNonsecure0:
+** ...
+** bl __gnu_cmse_nonsecure_call
+** sxtb r0, r0
+** ...
+*/
+signed char signNonsecure0 (ns_sign_foo_t * ns_foo_p)
+{
+ return ns_foo_p ();
+}
+
+/*
+**shortUnsignNonsecure0:
+** ...
+** bl __gnu_cmse_nonsecure_call
+** uxth r0, r0
+** ...
+*/
+unsigned short shortUnsignNonsecure0 (ns_short_unsign_foo_t * ns_foo_p)
+{
+ return ns_foo_p ();
+}
+
+/*
+**shortSignNonsecure0:
+** ...
+** bl __gnu_cmse_nonsecure_call
+** sxth r0, r0
+** ...
+*/
+signed short shortSignNonsecure0 (ns_short_sign_foo_t * ns_foo_p)
+{
+ return ns_foo_p ();
+}
+
+/*
+**enumNonsecure0:
+** ...
+** bl __gnu_cmse_nonsecure_call
+** uxtb r0, r0
+** ...
+*/
+unsigned char __attribute__((noipa)) enumNonsecure0 (ns_enum_foo_t * ns_foo_p)
+{
+ return ns_foo_p ();
+}
+
+/*
+**boolNonsecure0:
+** ...
+** bl __gnu_cmse_nonsecure_call
+** uxtb r0, r0
+** ...
+*/
+unsigned char boolNonsecure0 (ns_bool_foo_t * ns_foo_p)
+{
+ return ns_foo_p ();
+}
\ No newline at end of file
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] arm: Zero/Sign extends for CMSE security
2024-04-24 15:55 [PATCH] arm: Zero/Sign extends for CMSE security Richard Ball
@ 2024-04-25 10:01 ` Richard Earnshaw (lists)
2024-04-25 11:47 ` Torbjorn SVENSSON
1 sibling, 0 replies; 12+ messages in thread
From: Richard Earnshaw (lists) @ 2024-04-25 10:01 UTC (permalink / raw)
To: Richard Ball, gcc-patches, Richard Sandiford, Marcus Shawcroft,
Kyrylo Tkachov
On 24/04/2024 16:55, Richard Ball wrote:
> This patch makes the following changes:
>
> 1) When calling a secure function from non-secure code then any arguments
> smaller than 32-bits that are passed in registers are zero- or sign-extended.
> 2) After a non-secure function returns into secure code then any return value
> smaller than 32-bits that is passed in a register is zero- or sign-extended.
>
> This patch addresses the following CVE-2024-0151.
>
> gcc/ChangeLog:
> PR target/114837
> * config/arm/arm.cc (cmse_nonsecure_call_inline_register_clear):
> Add zero/sign extend.
> (arm_expand_prologue): Add zero/sign extend.
>
> gcc/testsuite/ChangeLog:
>
> * gcc.target/arm/cmse/extend-param.c: New test.
> * gcc.target/arm/cmse/extend-return.c: New test.
OK. And OK to backport to active branches.
R.
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] arm: Zero/Sign extends for CMSE security
2024-04-24 15:55 [PATCH] arm: Zero/Sign extends for CMSE security Richard Ball
2024-04-25 10:01 ` Richard Earnshaw (lists)
@ 2024-04-25 11:47 ` Torbjorn SVENSSON
2024-04-25 14:25 ` Richard Ball
1 sibling, 1 reply; 12+ messages in thread
From: Torbjorn SVENSSON @ 2024-04-25 11:47 UTC (permalink / raw)
To: Richard Ball, gcc-patches, Richard Earnshaw, Richard Sandiford,
Marcus Shawcroft, Kyrylo Tkachov
Hi,
On 2024-04-24 17:55, Richard Ball wrote:
> This patch makes the following changes:
>
> 1) When calling a secure function from non-secure code then any arguments
> smaller than 32-bits that are passed in registers are zero- or sign-extended.
> 2) After a non-secure function returns into secure code then any return value
> smaller than 32-bits that is passed in a register is zero- or sign-extended.
>
> This patch addresses the following CVE-2024-0151.
>
> gcc/ChangeLog:
> PR target/114837
> * config/arm/arm.cc (cmse_nonsecure_call_inline_register_clear):
> Add zero/sign extend.
> (arm_expand_prologue): Add zero/sign extend.
>
> gcc/testsuite/ChangeLog:
>
> * gcc.target/arm/cmse/extend-param.c: New test.
> * gcc.target/arm/cmse/extend-return.c: New test.
I think it would make sense that there is at least one test case that
takes 2 or more arguments to ensure that not only the first argument is
extended. WDYT?
Kind regards,
Torbjörn
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] arm: Zero/Sign extends for CMSE security
2024-04-25 11:47 ` Torbjorn SVENSSON
@ 2024-04-25 14:25 ` Richard Ball
2024-04-26 8:39 ` Torbjorn SVENSSON
0 siblings, 1 reply; 12+ messages in thread
From: Richard Ball @ 2024-04-25 14:25 UTC (permalink / raw)
To: Torbjorn SVENSSON, Richard Earnshaw, gcc-patches
[-- Attachment #1: Type: text/plain, Size: 1826 bytes --]
Hi Torbjorn,
Thanks very much for the comments.
I think given that the code that handles this, is within a FOREACH_FUNCTION_ARGS loop.
It seems a fairly safe assumption that if the code works for one that it will work for all.
To go back and add extra tests to me seems a little overkill.
Kind Regards,
Richard Ball
________________________________
From: Torbjorn SVENSSON <torbjorn.svensson@foss.st.com>
Sent: 25 April 2024 12:47
To: Richard Ball <Richard.Ball@arm.com>; gcc-patches@gcc.gnu.org <gcc-patches@gcc.gnu.org>; Richard Earnshaw <Richard.Earnshaw@arm.com>; Richard Sandiford <Richard.Sandiford@arm.com>; Marcus Shawcroft <Marcus.Shawcroft@arm.com>; Kyrylo Tkachov <Kyrylo.Tkachov@arm.com>
Subject: Re: [PATCH] arm: Zero/Sign extends for CMSE security
Hi,
On 2024-04-24 17:55, Richard Ball wrote:
> This patch makes the following changes:
>
> 1) When calling a secure function from non-secure code then any arguments
> smaller than 32-bits that are passed in registers are zero- or sign-extended.
> 2) After a non-secure function returns into secure code then any return value
> smaller than 32-bits that is passed in a register is zero- or sign-extended.
>
> This patch addresses the following CVE-2024-0151.
>
> gcc/ChangeLog:
> PR target/114837
> * config/arm/arm.cc (cmse_nonsecure_call_inline_register_clear):
> Add zero/sign extend.
> (arm_expand_prologue): Add zero/sign extend.
>
> gcc/testsuite/ChangeLog:
>
> * gcc.target/arm/cmse/extend-param.c: New test.
> * gcc.target/arm/cmse/extend-return.c: New test.
I think it would make sense that there is at least one test case that
takes 2 or more arguments to ensure that not only the first argument is
extended. WDYT?
Kind regards,
Torbjörn
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] arm: Zero/Sign extends for CMSE security
2024-04-25 14:25 ` Richard Ball
@ 2024-04-26 8:39 ` Torbjorn SVENSSON
2024-04-26 9:19 ` Richard Earnshaw (lists)
0 siblings, 1 reply; 12+ messages in thread
From: Torbjorn SVENSSON @ 2024-04-26 8:39 UTC (permalink / raw)
To: Richard Ball, Richard Earnshaw, gcc-patches
Hi,
On 2024-04-25 16:25, Richard Ball wrote:
> Hi Torbjorn,
>
> Thanks very much for the comments.
> I think given that the code that handles this, is within a
> FOREACH_FUNCTION_ARGS loop.
> It seems a fairly safe assumption that if the code works for one that it
> will work for all.
> To go back and add extra tests to me seems a little overkill.
For verifying that the implementation does the right thing now, no, but
for verifying against future regressions, then yes.
So, from a regression point of view, I think it makes sense to have the
check that more than the first argument is managed properly.
Kind regards,
Torbjörn
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] arm: Zero/Sign extends for CMSE security
2024-04-26 8:39 ` Torbjorn SVENSSON
@ 2024-04-26 9:19 ` Richard Earnshaw (lists)
2024-04-27 14:13 ` [PATCH] testsuite: Verify r0-r3 are extended with CMSE Torbjörn SVENSSON
0 siblings, 1 reply; 12+ messages in thread
From: Richard Earnshaw (lists) @ 2024-04-26 9:19 UTC (permalink / raw)
To: Torbjorn SVENSSON, Richard Ball, gcc-patches
On 26/04/2024 09:39, Torbjorn SVENSSON wrote:
> Hi,
>
> On 2024-04-25 16:25, Richard Ball wrote:
>> Hi Torbjorn,
>>
>> Thanks very much for the comments.
>> I think given that the code that handles this, is within a FOREACH_FUNCTION_ARGS loop.
>> It seems a fairly safe assumption that if the code works for one that it will work for all.
>> To go back and add extra tests to me seems a little overkill.
>
> For verifying that the implementation does the right thing now, no, but for verifying against future regressions, then yes.
>
> So, from a regression point of view, I think it makes sense to have the check that more than the first argument is managed properly.
>
> Kind regards,
> Torbjörn
Feel free to post some additional tests, Torbjorn.
R.
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH] testsuite: Verify r0-r3 are extended with CMSE
2024-04-26 9:19 ` Richard Earnshaw (lists)
@ 2024-04-27 14:13 ` Torbjörn SVENSSON
2024-04-30 15:11 ` Richard Earnshaw (lists)
0 siblings, 1 reply; 12+ messages in thread
From: Torbjörn SVENSSON @ 2024-04-27 14:13 UTC (permalink / raw)
To: gcc-patches; +Cc: Richard.Earnshaw, Richard.Ball, Torbjörn SVENSSON
Add regression test to the existing zero/sign extend tests for CMSE to
verify that r0, r1, r2 and r3 are properly extended, not just r0.
Test is done using -O0 to ensure the instructions are in a predictable
order.
gcc/testsuite/ChangeLog:
* gcc.target/arm/cmse/extend-param.c: Add regression test.
Signed-off-by: Torbjörn SVENSSON <torbjorn.svensson@foss.st.com>
---
.../gcc.target/arm/cmse/extend-param.c | 20 ++++++++++++++++++-
1 file changed, 19 insertions(+), 1 deletion(-)
diff --git a/gcc/testsuite/gcc.target/arm/cmse/extend-param.c b/gcc/testsuite/gcc.target/arm/cmse/extend-param.c
index 01fac786238..b8b8ecbff56 100644
--- a/gcc/testsuite/gcc.target/arm/cmse/extend-param.c
+++ b/gcc/testsuite/gcc.target/arm/cmse/extend-param.c
@@ -93,4 +93,22 @@ __attribute__((cmse_nonsecure_entry)) char boolSecureFunc (bool index) {
return 0;
return array[index];
-}
\ No newline at end of file
+}
+
+/*
+**__acle_se_boolCharShortEnumSecureFunc:
+** ...
+** uxtb r0, r0
+** uxtb r1, r1
+** uxth r2, r2
+** uxtb r3, r3
+** ...
+*/
+__attribute__((cmse_nonsecure_entry,optimize(0))) char boolCharShortEnumSecureFunc (bool a, unsigned char b, unsigned short c, enum offset d) {
+
+ size_t index = a + b + c + d;
+ if (index >= ARRAY_SIZE)
+ return 0;
+ return array[index];
+
+}
--
2.25.1
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] testsuite: Verify r0-r3 are extended with CMSE
2024-04-27 14:13 ` [PATCH] testsuite: Verify r0-r3 are extended with CMSE Torbjörn SVENSSON
@ 2024-04-30 15:11 ` Richard Earnshaw (lists)
2024-04-30 15:37 ` Torbjorn SVENSSON
0 siblings, 1 reply; 12+ messages in thread
From: Richard Earnshaw (lists) @ 2024-04-30 15:11 UTC (permalink / raw)
To: Torbjörn SVENSSON, gcc-patches; +Cc: Richard.Ball
On 27/04/2024 15:13, Torbjörn SVENSSON wrote:
> Add regression test to the existing zero/sign extend tests for CMSE to
> verify that r0, r1, r2 and r3 are properly extended, not just r0.
>
> Test is done using -O0 to ensure the instructions are in a predictable
> order.
>
> gcc/testsuite/ChangeLog:
>
> * gcc.target/arm/cmse/extend-param.c: Add regression test.
>
> Signed-off-by: Torbjörn SVENSSON <torbjorn.svensson@foss.st.com>
> ---
> .../gcc.target/arm/cmse/extend-param.c | 20 ++++++++++++++++++-
> 1 file changed, 19 insertions(+), 1 deletion(-)
>
> diff --git a/gcc/testsuite/gcc.target/arm/cmse/extend-param.c b/gcc/testsuite/gcc.target/arm/cmse/extend-param.c
> index 01fac786238..b8b8ecbff56 100644
> --- a/gcc/testsuite/gcc.target/arm/cmse/extend-param.c
> +++ b/gcc/testsuite/gcc.target/arm/cmse/extend-param.c
> @@ -93,4 +93,22 @@ __attribute__((cmse_nonsecure_entry)) char boolSecureFunc (bool index) {
> return 0;
> return array[index];
>
> -}
> \ No newline at end of file
> +}
> +
> +/*
> +**__acle_se_boolCharShortEnumSecureFunc:
> +** ...
> +** uxtb r0, r0
> +** uxtb r1, r1
> +** uxth r2, r2
> +** uxtb r3, r3
> +** ...
> +*/
> +__attribute__((cmse_nonsecure_entry,optimize(0))) char boolCharShortEnumSecureFunc (bool a, unsigned char b, unsigned short c, enum offset d) {
> +
> + size_t index = a + b + c + d;
> + if (index >= ARRAY_SIZE)
> + return 0;
> + return array[index];
> +
> +}
Ok, but please can you add '-fshort-enums' to dg-options to ensure this test still behaves correctly if run with a different default (I missed that last time around).
R.
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] testsuite: Verify r0-r3 are extended with CMSE
2024-04-30 15:11 ` Richard Earnshaw (lists)
@ 2024-04-30 15:37 ` Torbjorn SVENSSON
2024-04-30 16:50 ` Richard Earnshaw (lists)
0 siblings, 1 reply; 12+ messages in thread
From: Torbjorn SVENSSON @ 2024-04-30 15:37 UTC (permalink / raw)
To: Richard Earnshaw (lists), gcc-patches; +Cc: Richard.Ball
On 2024-04-30 17:11, Richard Earnshaw (lists) wrote:
> On 27/04/2024 15:13, Torbjörn SVENSSON wrote:
>> Add regression test to the existing zero/sign extend tests for CMSE to
>> verify that r0, r1, r2 and r3 are properly extended, not just r0.
>>
>> Test is done using -O0 to ensure the instructions are in a predictable
>> order.
>>
>> gcc/testsuite/ChangeLog:
>>
>> * gcc.target/arm/cmse/extend-param.c: Add regression test.
>>
>> Signed-off-by: Torbjörn SVENSSON <torbjorn.svensson@foss.st.com>
>> ---
>> .../gcc.target/arm/cmse/extend-param.c | 20 ++++++++++++++++++-
>> 1 file changed, 19 insertions(+), 1 deletion(-)
>>
>> diff --git a/gcc/testsuite/gcc.target/arm/cmse/extend-param.c b/gcc/testsuite/gcc.target/arm/cmse/extend-param.c
>> index 01fac786238..b8b8ecbff56 100644
>> --- a/gcc/testsuite/gcc.target/arm/cmse/extend-param.c
>> +++ b/gcc/testsuite/gcc.target/arm/cmse/extend-param.c
>> @@ -93,4 +93,22 @@ __attribute__((cmse_nonsecure_entry)) char boolSecureFunc (bool index) {
>> return 0;
>> return array[index];
>>
>> -}
>> \ No newline at end of file
>> +}
>> +
>> +/*
>> +**__acle_se_boolCharShortEnumSecureFunc:
>> +** ...
>> +** uxtb r0, r0
>> +** uxtb r1, r1
>> +** uxth r2, r2
>> +** uxtb r3, r3
>> +** ...
>> +*/
>> +__attribute__((cmse_nonsecure_entry,optimize(0))) char boolCharShortEnumSecureFunc (bool a, unsigned char b, unsigned short c, enum offset d) {
>> +
>> + size_t index = a + b + c + d;
>> + if (index >= ARRAY_SIZE)
>> + return 0;
>> + return array[index];
>> +
>> +}
>
> Ok, but please can you add '-fshort-enums' to dg-options to ensure this test still behaves correctly if run with a different default (I missed that last time around).
Ok, I'll add that to extend-param.c. Do you want me to also add it to
the extend-return.c test case?
Kind regards,
Torbjörn
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] testsuite: Verify r0-r3 are extended with CMSE
2024-04-30 15:37 ` Torbjorn SVENSSON
@ 2024-04-30 16:50 ` Richard Earnshaw (lists)
2024-05-02 10:50 ` [PATCH v2] " Torbjörn SVENSSON
0 siblings, 1 reply; 12+ messages in thread
From: Richard Earnshaw (lists) @ 2024-04-30 16:50 UTC (permalink / raw)
To: Torbjorn SVENSSON, gcc-patches; +Cc: Richard.Ball
On 30/04/2024 16:37, Torbjorn SVENSSON wrote:
>
>
> On 2024-04-30 17:11, Richard Earnshaw (lists) wrote:
>> On 27/04/2024 15:13, Torbjörn SVENSSON wrote:
>>> Add regression test to the existing zero/sign extend tests for CMSE to
>>> verify that r0, r1, r2 and r3 are properly extended, not just r0.
>>>
>>> Test is done using -O0 to ensure the instructions are in a predictable
>>> order.
>>>
>>> gcc/testsuite/ChangeLog:
>>>
>>> * gcc.target/arm/cmse/extend-param.c: Add regression test.
>>>
>>> Signed-off-by: Torbjörn SVENSSON <torbjorn.svensson@foss.st.com>
>>> ---
>>> .../gcc.target/arm/cmse/extend-param.c | 20 ++++++++++++++++++-
>>> 1 file changed, 19 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/gcc/testsuite/gcc.target/arm/cmse/extend-param.c b/gcc/testsuite/gcc.target/arm/cmse/extend-param.c
>>> index 01fac786238..b8b8ecbff56 100644
>>> --- a/gcc/testsuite/gcc.target/arm/cmse/extend-param.c
>>> +++ b/gcc/testsuite/gcc.target/arm/cmse/extend-param.c
>>> @@ -93,4 +93,22 @@ __attribute__((cmse_nonsecure_entry)) char boolSecureFunc (bool index) {
>>> return 0;
>>> return array[index];
>>> -}
>>> \ No newline at end of file
>>> +}
>>> +
>>> +/*
>>> +**__acle_se_boolCharShortEnumSecureFunc:
>>> +** ...
>>> +** uxtb r0, r0
>>> +** uxtb r1, r1
>>> +** uxth r2, r2
>>> +** uxtb r3, r3
>>> +** ...
>>> +*/
>>> +__attribute__((cmse_nonsecure_entry,optimize(0))) char boolCharShortEnumSecureFunc (bool a, unsigned char b, unsigned short c, enum offset d) {
>>> +
>>> + size_t index = a + b + c + d;
>>> + if (index >= ARRAY_SIZE)
>>> + return 0;
>>> + return array[index];
>>> +
>>> +}
>>
>> Ok, but please can you add '-fshort-enums' to dg-options to ensure this test still behaves correctly if run with a different default (I missed that last time around).
>
> Ok, I'll add that to extend-param.c. Do you want me to also add it to the extend-return.c test case?
>
> Kind regards,
> Torbjörn
Yes please, if it has the same issue.
R.
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH v2] testsuite: Verify r0-r3 are extended with CMSE
2024-04-30 16:50 ` Richard Earnshaw (lists)
@ 2024-05-02 10:50 ` Torbjörn SVENSSON
2024-05-06 11:50 ` Torbjorn SVENSSON
0 siblings, 1 reply; 12+ messages in thread
From: Torbjörn SVENSSON @ 2024-05-02 10:50 UTC (permalink / raw)
To: gcc-patches
Cc: Richard.Earnshaw, Richard.Ball, yvan.roux, Torbjörn SVENSSON
Add regression test to the existing zero/sign extend tests for CMSE to
verify that r0, r1, r2 and r3 are properly extended, not just r0.
boolCharShortEnumSecureFunc test is done using -O0 to ensure the
instructions are in a predictable order.
gcc/testsuite/ChangeLog:
* gcc.target/arm/cmse/extend-param.c: Add regression test. Add
-fshort-enums.
* gcc.target/arm/cmse/extend-return.c: Add -fshort-enums option.
Signed-off-by: Torbjörn SVENSSON <torbjorn.svensson@foss.st.com>
---
.../gcc.target/arm/cmse/extend-param.c | 21 +++++++++++++++----
.../gcc.target/arm/cmse/extend-return.c | 4 ++--
2 files changed, 19 insertions(+), 6 deletions(-)
diff --git a/gcc/testsuite/gcc.target/arm/cmse/extend-param.c b/gcc/testsuite/gcc.target/arm/cmse/extend-param.c
index 01fac786238..d01ef87e0be 100644
--- a/gcc/testsuite/gcc.target/arm/cmse/extend-param.c
+++ b/gcc/testsuite/gcc.target/arm/cmse/extend-param.c
@@ -1,5 +1,5 @@
/* { dg-do compile } */
-/* { dg-options "-mcmse" } */
+/* { dg-options "-mcmse -fshort-enums" } */
/* { dg-final { check-function-bodies "**" "" "" } } */
#include <arm_cmse.h>
@@ -78,7 +78,6 @@ __attribute__((cmse_nonsecure_entry)) char enumSecureFunc (enum offset index) {
if (index >= ARRAY_SIZE)
return 0;
return array[index];
-
}
/*
@@ -88,9 +87,23 @@ __attribute__((cmse_nonsecure_entry)) char enumSecureFunc (enum offset index) {
** ...
*/
__attribute__((cmse_nonsecure_entry)) char boolSecureFunc (bool index) {
-
if (index >= ARRAY_SIZE)
return 0;
return array[index];
+}
-}
\ No newline at end of file
+/*
+**__acle_se_boolCharShortEnumSecureFunc:
+** ...
+** uxtb r0, r0
+** uxtb r1, r1
+** uxth r2, r2
+** uxtb r3, r3
+** ...
+*/
+__attribute__((cmse_nonsecure_entry,optimize(0))) char boolCharShortEnumSecureFunc (bool a, unsigned char b, unsigned short c, enum offset d) {
+ size_t index = a + b + c + d;
+ if (index >= ARRAY_SIZE)
+ return 0;
+ return array[index];
+}
diff --git a/gcc/testsuite/gcc.target/arm/cmse/extend-return.c b/gcc/testsuite/gcc.target/arm/cmse/extend-return.c
index cf731ed33df..081de0d699f 100644
--- a/gcc/testsuite/gcc.target/arm/cmse/extend-return.c
+++ b/gcc/testsuite/gcc.target/arm/cmse/extend-return.c
@@ -1,5 +1,5 @@
/* { dg-do compile } */
-/* { dg-options "-mcmse" } */
+/* { dg-options "-mcmse -fshort-enums" } */
/* { dg-final { check-function-bodies "**" "" "" } } */
#include <arm_cmse.h>
@@ -89,4 +89,4 @@ unsigned char __attribute__((noipa)) enumNonsecure0 (ns_enum_foo_t * ns_foo_p)
unsigned char boolNonsecure0 (ns_bool_foo_t * ns_foo_p)
{
return ns_foo_p ();
-}
\ No newline at end of file
+}
--
2.25.1
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v2] testsuite: Verify r0-r3 are extended with CMSE
2024-05-02 10:50 ` [PATCH v2] " Torbjörn SVENSSON
@ 2024-05-06 11:50 ` Torbjorn SVENSSON
0 siblings, 0 replies; 12+ messages in thread
From: Torbjorn SVENSSON @ 2024-05-06 11:50 UTC (permalink / raw)
To: gcc-patches, Richard.Earnshaw; +Cc: Richard.Ball, yvan.roux
Hi,
Forgot to mention when I sent the patch that I would like to commit it
to the following branches:
- releases/gcc-11
- releases/gcc-12
- releases/gcc-13
- releases/gcc-14
- trunk
Kind regards,
Torbjörn
On 2024-05-02 12:50, Torbjörn SVENSSON wrote:
> Add regression test to the existing zero/sign extend tests for CMSE to
> verify that r0, r1, r2 and r3 are properly extended, not just r0.
>
> boolCharShortEnumSecureFunc test is done using -O0 to ensure the
> instructions are in a predictable order.
>
> gcc/testsuite/ChangeLog:
>
> * gcc.target/arm/cmse/extend-param.c: Add regression test. Add
> -fshort-enums.
> * gcc.target/arm/cmse/extend-return.c: Add -fshort-enums option.
>
> Signed-off-by: Torbjörn SVENSSON <torbjorn.svensson@foss.st.com>
> ---
> .../gcc.target/arm/cmse/extend-param.c | 21 +++++++++++++++----
> .../gcc.target/arm/cmse/extend-return.c | 4 ++--
> 2 files changed, 19 insertions(+), 6 deletions(-)
>
> diff --git a/gcc/testsuite/gcc.target/arm/cmse/extend-param.c b/gcc/testsuite/gcc.target/arm/cmse/extend-param.c
> index 01fac786238..d01ef87e0be 100644
> --- a/gcc/testsuite/gcc.target/arm/cmse/extend-param.c
> +++ b/gcc/testsuite/gcc.target/arm/cmse/extend-param.c
> @@ -1,5 +1,5 @@
> /* { dg-do compile } */
> -/* { dg-options "-mcmse" } */
> +/* { dg-options "-mcmse -fshort-enums" } */
> /* { dg-final { check-function-bodies "**" "" "" } } */
>
> #include <arm_cmse.h>
> @@ -78,7 +78,6 @@ __attribute__((cmse_nonsecure_entry)) char enumSecureFunc (enum offset index) {
> if (index >= ARRAY_SIZE)
> return 0;
> return array[index];
> -
> }
>
> /*
> @@ -88,9 +87,23 @@ __attribute__((cmse_nonsecure_entry)) char enumSecureFunc (enum offset index) {
> ** ...
> */
> __attribute__((cmse_nonsecure_entry)) char boolSecureFunc (bool index) {
> -
> if (index >= ARRAY_SIZE)
> return 0;
> return array[index];
> +}
>
> -}
> \ No newline at end of file
> +/*
> +**__acle_se_boolCharShortEnumSecureFunc:
> +** ...
> +** uxtb r0, r0
> +** uxtb r1, r1
> +** uxth r2, r2
> +** uxtb r3, r3
> +** ...
> +*/
> +__attribute__((cmse_nonsecure_entry,optimize(0))) char boolCharShortEnumSecureFunc (bool a, unsigned char b, unsigned short c, enum offset d) {
> + size_t index = a + b + c + d;
> + if (index >= ARRAY_SIZE)
> + return 0;
> + return array[index];
> +}
> diff --git a/gcc/testsuite/gcc.target/arm/cmse/extend-return.c b/gcc/testsuite/gcc.target/arm/cmse/extend-return.c
> index cf731ed33df..081de0d699f 100644
> --- a/gcc/testsuite/gcc.target/arm/cmse/extend-return.c
> +++ b/gcc/testsuite/gcc.target/arm/cmse/extend-return.c
> @@ -1,5 +1,5 @@
> /* { dg-do compile } */
> -/* { dg-options "-mcmse" } */
> +/* { dg-options "-mcmse -fshort-enums" } */
> /* { dg-final { check-function-bodies "**" "" "" } } */
>
> #include <arm_cmse.h>
> @@ -89,4 +89,4 @@ unsigned char __attribute__((noipa)) enumNonsecure0 (ns_enum_foo_t * ns_foo_p)
> unsigned char boolNonsecure0 (ns_bool_foo_t * ns_foo_p)
> {
> return ns_foo_p ();
> -}
> \ No newline at end of file
> +}
^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2024-05-06 11:50 UTC | newest]
Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-04-24 15:55 [PATCH] arm: Zero/Sign extends for CMSE security Richard Ball
2024-04-25 10:01 ` Richard Earnshaw (lists)
2024-04-25 11:47 ` Torbjorn SVENSSON
2024-04-25 14:25 ` Richard Ball
2024-04-26 8:39 ` Torbjorn SVENSSON
2024-04-26 9:19 ` Richard Earnshaw (lists)
2024-04-27 14:13 ` [PATCH] testsuite: Verify r0-r3 are extended with CMSE Torbjörn SVENSSON
2024-04-30 15:11 ` Richard Earnshaw (lists)
2024-04-30 15:37 ` Torbjorn SVENSSON
2024-04-30 16:50 ` Richard Earnshaw (lists)
2024-05-02 10:50 ` [PATCH v2] " Torbjörn SVENSSON
2024-05-06 11:50 ` Torbjorn SVENSSON
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).