public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [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).