[PATCH] arm: Zero/Sign extends for CMSE security

[PATCH] arm: Zero/Sign extends for CMSE security

[Richard Ball]

[-- 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

Re: [PATCH] arm: Zero/Sign extends for CMSE security

[Richard Earnshaw (lists)]

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.

Re: [PATCH] arm: Zero/Sign extends for CMSE security

[Torbjorn SVENSSON]

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

Re: [PATCH] arm: Zero/Sign extends for CMSE security

[Richard Ball]

[-- 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

Re: [PATCH] arm: Zero/Sign extends for CMSE security

[Torbjorn SVENSSON]

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

Re: [PATCH] arm: Zero/Sign extends for CMSE security

[Richard Earnshaw (lists)]

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.

[PATCH] testsuite: Verify r0-r3 are extended with CMSE

[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

Re: [PATCH] testsuite: Verify r0-r3 are extended with CMSE

[Richard Earnshaw (lists)]

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.

Re: [PATCH] testsuite: Verify r0-r3 are extended with CMSE

[Torbjorn SVENSSON]

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

Re: [PATCH] testsuite: Verify r0-r3 are extended with CMSE

[Richard Earnshaw (lists)]

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.

[PATCH v2] testsuite: Verify r0-r3 are extended with CMSE

[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

Re: [PATCH v2] testsuite: Verify r0-r3 are extended with CMSE

[Torbjorn SVENSSON]

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
> +}

[PING] [PATCH v2] testsuite: Verify r0-r3 are extended with CMSE

[Torbjorn SVENSSON]

Hi,

I'm not sure if the previous "ok" from Richard on the v1 is enough for 
this or if there needs another approval.

Adding extra maintainers since Richard Earnshaw appears to be busy the 
past weeks.

Kind regards,
Torbjörn

On 2024-05-06 13:50, Torbjorn SVENSSON wrote:
> 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
>> +}