[arm-embedded][committed][PATCH 3/7] ARMv8-M Security Extension's cmse_nonsecure_entry: __acle_se label and bxns return

["Andre Vieira (lists)" <Andre.SimoesDiasVieira@arm.com>]

[-- Attachment #1: Type: text/plain, Size: 5265 bytes --]

On 27/10/16 10:55, Andre Vieira (lists) wrote:
> On 26/10/16 11:03, Kyrill Tkachov wrote:
>> Hi Andre,
>>
>> On 25/10/16 17:28, Andre Vieira (lists) wrote:
>>> On 25/07/16 14:23, Andre Vieira (lists) wrote:
>>>> This patch extends support for the ARMv8-M Security Extensions
>>>> 'cmse_nonsecure_entry' attribute in two ways:
>>>>
>>>> 1) Generate two labels for the function, the regular function name and
>>>> one with the function's name appended to '__acle_se_', this will trigger
>>>> the linker to create a secure gateway veneer for this entry function.
>>>> 2) Return from cmse_nonsecure_entry marked functions using bxns.
>>>>
>>>> See Section 5.4 of ARM®v8-M Security Extensions
>>>> (http://infocenter.arm.com/help/topic/com.arm.doc.ecm0359818/index.html).
>>>>
>>>>
>>>>
>>>> *** gcc/ChangeLog ***
>>>> 2016-07-25  Andre Vieira        <andre.simoesdiasvieira@arm.com>
>>>>              Thomas Preud'homme  <thomas.preudhomme@arm.com>
>>>>
>>>>          * config/arm/arm.c (use_return_insn): Change to return with 
>>>> bxns
>>>>          when cmse_nonsecure_entry.
>>>>          (output_return_instruction): Likewise.
>>>>          (arm_output_function_prologue): Likewise.
>>>>          (thumb_pop): Likewise.
>>>>          (thumb_exit): Likewise.
>>>>          (arm_function_ok_for_sibcall): Disable sibcall for entry
>>>> functions.
>>>>          (arm_asm_declare_function_name): New.
>>>>          * config/arm/arm-protos.h (arm_asm_declare_function_name): New.
>>>>          * config/arm/elf.h (ASM_DECLARE_FUNCTION_NAME): Redefine to
>>>>          use arm_asm_declare_function_name.
>>>>
>>>> *** gcc/testsuite/ChangeLog ***
>>>> 2016-07-25  Andre Vieira        <andre.simoesdiasvieira@arm.com>
>>>>              Thomas Preud'homme  <thomas.preudhomme@arm.com>
>>>>
>>>>          * gcc.target/arm/cmse/cmse-2.c: New.
>>>>          * gcc.target/arm/cmse/cmse-4.c: New.
>>>>
>>> Hi,
>>>
>>> Rebased previous patch on top of trunk as requested. No changes to
>>> ChangeLog.
>>>
>>> Cheers,
>>> Andre
>>
>> @@ -19919,6 +19932,42 @@ output_return_instruction (rtx operand, bool
>> really_return, bool reverse,
>>    return "";
>>  }
>>  
>> +/* Output in FILE asm statements needed to declare the NAME of the
>> function
>> +   defined by its DECL node.  */
>> +
>> +void
>> +arm_asm_declare_function_name (FILE *file, const char *name, tree decl)
>> +{
>> +  size_t cmse_name_len;
>> +  char *cmse_name = 0;
>> +  char cmse_prefix[] = "__acle_se_";
>> +
>> +  if (use_cmse && lookup_attribute ("cmse_nonsecure_entry",
>> +                    DECL_ATTRIBUTES (decl)))
>> +    {
>> +      cmse_name_len = sizeof (cmse_prefix) + strlen (name);
>> +      cmse_name = XALLOCAVEC (char, cmse_name_len);
>> +      snprintf (cmse_name, cmse_name_len, "%s%s", cmse_prefix, name);
>> +      targetm.asm_out.globalize_label (file, cmse_name);
>> +    }
>> +
>>
>> I think this definitely warrants a quick comment explaining why you're
>> adding
>> __acle_se_ to the function label
>>
>>  
>>  /* Scan INSN just before assembler is output for it.
>> @@ -25247,6 +25301,12 @@ thumb2_expand_return (bool simple_return)
>>  
>>    if (!simple_return && saved_regs_mask)
>>      {
>> +      /* TODO: Verify that this path is never taken for
>> cmse_nonsecure_entry
>> +     functions or adapt code to handle according to ACLE.  This path
>> should
>> +     not be reachable for cmse_nonsecure_entry functions though we prefer
>> +     to guard it for now to ensure that future code changes do not
>> silently
>> +     change this behavior.  */
>>
>> I think you mean s/guard/assert/
>>
>>  +      gcc_assert (!IS_CMSE_ENTRY (arm_current_func_type ()));
>>        if (num_regs == 1)
>>          {
>>            rtx par = gen_rtx_PARALLEL (VOIDmode, rtvec_alloc (2));
>>
>> This is ok with those changes.
>> Thanks,
>> Kyrill
>>
> Hi,
> 
> Reworked comments. Also got rid of a redundant 'if (cmse_name)' in
> 'arm_asm_declare_function_name'. No change to ChangeLogs.
> 
> Cheers,
> Andre
> 
Hi,

Backported this to the embedded-6-branch in revision r<revision>.

Cheers,
Andre

gcc/ChangeLog.arm:
2016-12-05  Andre Vieira  <andre.simoesdiasvieira@arm.com>

	Backport from mainline
	2016-12-02  Andre Vieira  <andre.simoesdiasvieira@arm.com>
		    Thomas Preud'homme	<thomas.preudhomme@arm.com>

	* config/arm/arm.c (use_return_insn): Change to return with  bxns
	when cmse_nonsecure_entry.
	(output_return_instruction): Likewise.
	(arm_output_function_prologue): Likewise.
	(thumb_pop): Likewise.
	(thumb_exit): Likewise.
	(thumb2_expand_return): Assert that entry functions always have simple
	returns.
	(arm_expand_epilogue): Handle entry functions.
	(arm_function_ok_for_sibcall): Disable sibcall for entry functions.
	(arm_asm_declare_function_name): New.
	* config/arm/arm-protos.h (arm_asm_declare_function_name): New.
	* config/arm/elf.h (ASM_DECLARE_FUNCTION_NAME): Redefine to
	use arm_asm_declare_function_name.

gcc/testsuite/ChangeLog.arm:
2016-12-05  Andre Vieira  <andre.simoesdiasvieira@arm.com>

	Backport from mainline
	2016-12-02  Andre Vieira  <andre.simoesdiasvieira@arm.com>
		    Thomas Preud'homme	<thomas.preudhomme@arm.com>

	* gcc.target/arm/cmse/cmse-4.c: New.
	* gcc.target/arm/cmse/cmse-9.c: New.
	* gcc.target/arm/cmse/cmse-10.c: New.



[-- Attachment #2: diff3 --]
[-- Type: text/plain, Size: 13128 bytes --]

diff --git a/gcc/ChangeLog.arm b/gcc/ChangeLog.arm
index c93007a6b814320f3a3fb283873e21347b4cd333..b443bd9a796b05a5ab26b0b72a6ad1c52b6cf8b8 100644
--- a/gcc/ChangeLog.arm
+++ b/gcc/ChangeLog.arm
@@ -4,6 +4,27 @@
 	2016-12-02  Andre Vieira  <andre.simoesdiasvieira@arm.com>
 		    Thomas Preud'homme	<thomas.preudhomme@arm.com>
 
+	* config/arm/arm.c (use_return_insn): Change to return with  bxns
+	when cmse_nonsecure_entry.
+	(output_return_instruction): Likewise.
+	(arm_output_function_prologue): Likewise.
+	(thumb_pop): Likewise.
+	(thumb_exit): Likewise.
+	(thumb2_expand_return): Assert that entry functions always have simple
+	returns.
+	(arm_expand_epilogue): Handle entry functions.
+	(arm_function_ok_for_sibcall): Disable sibcall for entry functions.
+	(arm_asm_declare_function_name): New.
+	* config/arm/arm-protos.h (arm_asm_declare_function_name): New.
+	* config/arm/elf.h (ASM_DECLARE_FUNCTION_NAME): Redefine to
+	use arm_asm_declare_function_name.
+
+2016-12-05  Andre Vieira        <andre.simoesdiasvieira@arm.com>
+
+	Backport from mainline
+	2016-12-02  Andre Vieira  <andre.simoesdiasvieira@arm.com>
+		    Thomas Preud'homme	<thomas.preudhomme@arm.com>
+
 	* config/arm/arm.c (arm_handle_cmse_nonsecure_entry): New.
 	(arm_attribute_table): Added cmse_nonsecure_entry
 	(arm_compute_func_type): Handle cmse_nonsecure_entry.
diff --git a/gcc/config/arm/arm-protos.h b/gcc/config/arm/arm-protos.h
index 8232b26a2e147a6ea20a39defae5ac7180adfc58..fb9e6f4c7f54f3f491381ba8e48ab14979bd0dc3 100644
--- a/gcc/config/arm/arm-protos.h
+++ b/gcc/config/arm/arm-protos.h
@@ -33,6 +33,7 @@ extern int arm_volatile_func (void);
 extern void arm_expand_prologue (void);
 extern void arm_expand_epilogue (bool);
 extern void arm_declare_function_name (FILE *, const char *, tree);
+extern void arm_asm_declare_function_name (FILE *, const char *, tree);
 extern void thumb2_expand_return (bool);
 extern const char *arm_strip_name_encoding (const char *);
 extern void arm_asm_output_labelref (FILE *, const char *);
diff --git a/gcc/config/arm/arm.c b/gcc/config/arm/arm.c
index 6eafba774542fb4b20f7b72fd353621a54ca5bc6..97502c74588439960c5dff39168020e02f8444f2 100644
--- a/gcc/config/arm/arm.c
+++ b/gcc/config/arm/arm.c
@@ -3834,6 +3834,11 @@ use_return_insn (int iscond, rtx sibling)
 	return 0;
     }
 
+  /* ARMv8-M nonsecure entry function need to use bxns to return and thus need
+     several instructions if anything needs to be popped.  */
+  if (saved_int_regs && IS_CMSE_ENTRY (func_type))
+    return 0;
+
   /* If there are saved registers but the LR isn't saved, then we need
      two instructions for the return.  */
   if (saved_int_regs && !(saved_int_regs & (1 << LR_REGNUM)))
@@ -6866,6 +6871,11 @@ arm_function_ok_for_sibcall (tree decl, tree exp)
   if (IS_INTERRUPT (func_type))
     return false;
 
+  /* ARMv8-M non-secure entry functions need to return with bxns which is only
+     generated for entry functions themselves.  */
+  if (IS_CMSE_ENTRY (arm_current_func_type ()))
+    return false;
+
   if (!VOID_TYPE_P (TREE_TYPE (DECL_RESULT (cfun->decl))))
     {
       /* Check that the return value locations are the same.  For
@@ -19715,6 +19725,7 @@ output_return_instruction (rtx operand, bool really_return, bool reverse,
 	 (e.g. interworking) then we can load the return address
 	 directly into the PC.  Otherwise we must load it into LR.  */
       if (really_return
+	  && !IS_CMSE_ENTRY (func_type)
 	  && (IS_INTERRUPT (func_type) || !TARGET_INTERWORK))
 	return_reg = reg_names[PC_REGNUM];
       else
@@ -19855,8 +19866,10 @@ output_return_instruction (rtx operand, bool really_return, bool reverse,
 	  break;
 
 	default:
+	  if (IS_CMSE_ENTRY (func_type))
+	    snprintf (instr, sizeof (instr), "bxns%s\t%%|lr", conditional);
 	  /* Use bx if it's available.  */
-	  if (arm_arch5 || arm_arch4t)
+	  else if (arm_arch5 || arm_arch4t)
 	    sprintf (instr, "bx%s\t%%|lr", conditional);
 	  else
 	    sprintf (instr, "mov%s\t%%|pc, %%|lr", conditional);
@@ -19869,6 +19882,44 @@ output_return_instruction (rtx operand, bool really_return, bool reverse,
   return "";
 }
 
+/* Output in FILE asm statements needed to declare the NAME of the function
+   defined by its DECL node.  */
+
+void
+arm_asm_declare_function_name (FILE *file, const char *name, tree decl)
+{
+  size_t cmse_name_len;
+  char *cmse_name = 0;
+  char cmse_prefix[] = "__acle_se_";
+
+  /* When compiling with ARMv8-M Security Extensions enabled, we should print an
+     extra function label for each function with the 'cmse_nonsecure_entry'
+     attribute.  This extra function label should be prepended with
+     '__acle_se_', telling the linker that it needs to create secure gateway
+     veneers for this function.  */
+  if (use_cmse && lookup_attribute ("cmse_nonsecure_entry",
+				    DECL_ATTRIBUTES (decl)))
+    {
+      cmse_name_len = sizeof (cmse_prefix) + strlen (name);
+      cmse_name = XALLOCAVEC (char, cmse_name_len);
+      snprintf (cmse_name, cmse_name_len, "%s%s", cmse_prefix, name);
+      targetm.asm_out.globalize_label (file, cmse_name);
+
+      ARM_DECLARE_FUNCTION_NAME (file, cmse_name, decl);
+      ASM_OUTPUT_TYPE_DIRECTIVE (file, cmse_name, "function");
+    }
+
+  ARM_DECLARE_FUNCTION_NAME (file, name, decl);
+  ASM_OUTPUT_TYPE_DIRECTIVE (file, name, "function");
+  ASM_DECLARE_RESULT (file, DECL_RESULT (decl));
+  ASM_OUTPUT_LABEL (file, name);
+
+  if (cmse_name)
+    ASM_OUTPUT_LABEL (file, cmse_name);
+
+  ARM_OUTPUT_FN_UNWIND (file, TRUE);
+}
+
 /* Write the function name into the code section, directly preceding
    the function prologue.
 
@@ -19918,10 +19969,6 @@ arm_output_function_prologue (FILE *f, HOST_WIDE_INT frame_size)
 {
   unsigned long func_type;
 
-  /* ??? Do we want to print some of the below anyway?  */
-  if (TARGET_THUMB1)
-    return;
-
   /* Sanity check.  */
   gcc_assert (!arm_ccfsm_state && !arm_target_insn);
 
@@ -19956,6 +20003,8 @@ arm_output_function_prologue (FILE *f, HOST_WIDE_INT frame_size)
     asm_fprintf (f, "\t%@ Nested: function declared inside another function.\n");
   if (IS_STACKALIGN (func_type))
     asm_fprintf (f, "\t%@ Stack Align: May be called with mis-aligned SP.\n");
+  if (IS_CMSE_ENTRY (func_type))
+    asm_fprintf (f, "\t%@ Non-secure entry function: called from non-secure code.\n");
 
   asm_fprintf (f, "\t%@ args = %d, pretend = %d, frame = %wd\n",
 	       crtl->args.size,
@@ -24031,8 +24080,8 @@ thumb_pop (FILE *f, unsigned long mask)
   if (mask & (1 << PC_REGNUM))
     {
       /* Catch popping the PC.  */
-      if (TARGET_INTERWORK || TARGET_BACKTRACE
-	  || crtl->calls_eh_return)
+      if (TARGET_INTERWORK || TARGET_BACKTRACE || crtl->calls_eh_return
+	  || IS_CMSE_ENTRY (arm_current_func_type ()))
 	{
 	  /* The PC is never poped directly, instead
 	     it is popped into r3 and then BX is used.  */
@@ -24093,7 +24142,10 @@ thumb_exit (FILE *f, int reg_containing_return_addr)
       if (crtl->calls_eh_return)
 	asm_fprintf (f, "\tadd\t%r, %r\n", SP_REGNUM, ARM_EH_STACKADJ_REGNUM);
 
-      asm_fprintf (f, "\tbx\t%r\n", reg_containing_return_addr);
+      if (IS_CMSE_ENTRY (arm_current_func_type ()))
+	asm_fprintf (f, "\tbxns\t%r\n", reg_containing_return_addr);
+      else
+	asm_fprintf (f, "\tbx\t%r\n", reg_containing_return_addr);
       return;
     }
   /* Otherwise if we are not supporting interworking and we have not created
@@ -24102,7 +24154,8 @@ thumb_exit (FILE *f, int reg_containing_return_addr)
   else if (!TARGET_INTERWORK
 	   && !TARGET_BACKTRACE
 	   && !is_called_in_ARM_mode (current_function_decl)
-	   && !crtl->calls_eh_return)
+	   && !crtl->calls_eh_return
+	   && !IS_CMSE_ENTRY (arm_current_func_type ()))
     {
       asm_fprintf (f, "\tpop\t{%r}\n", PC_REGNUM);
       return;
@@ -24325,7 +24378,10 @@ thumb_exit (FILE *f, int reg_containing_return_addr)
     asm_fprintf (f, "\tadd\t%r, %r\n", SP_REGNUM, ARM_EH_STACKADJ_REGNUM);
 
   /* Return to caller.  */
-  asm_fprintf (f, "\tbx\t%r\n", reg_containing_return_addr);
+  if (IS_CMSE_ENTRY (arm_current_func_type ()))
+    asm_fprintf (f, "\tbxns\t%r\n", reg_containing_return_addr);
+  else
+    asm_fprintf (f, "\tbx\t%r\n", reg_containing_return_addr);
 }
 \f
 /* Scan INSN just before assembler is output for it.
@@ -25211,6 +25267,12 @@ thumb2_expand_return (bool simple_return)
 
   if (!simple_return && saved_regs_mask)
     {
+      /* TODO: Verify that this path is never taken for cmse_nonsecure_entry
+	 functions or adapt code to handle according to ACLE.  This path should
+	 not be reachable for cmse_nonsecure_entry functions though we prefer
+	 to assert it for now to ensure that future code changes do not silently
+	 change this behavior.  */
+      gcc_assert (!IS_CMSE_ENTRY (arm_current_func_type ()));
       if (num_regs == 1)
         {
           rtx par = gen_rtx_PARALLEL (VOIDmode, rtvec_alloc (2));
@@ -25628,6 +25690,7 @@ arm_expand_epilogue (bool really_return)
 
       if (ARM_FUNC_TYPE (func_type) != ARM_FT_INTERWORKED
           && (TARGET_ARM || ARM_FUNC_TYPE (func_type) == ARM_FT_NORMAL)
+	  && !IS_CMSE_ENTRY (func_type)
           && !IS_STACKALIGN (func_type)
           && really_return
           && crtl->args.pretend_args_size == 0
diff --git a/gcc/config/arm/elf.h b/gcc/config/arm/elf.h
index bc4eb86f1da5beabf32647637eb87a3fc17a6c6a..03931eee7390ab4d09522e7d8bea10449719ddba 100644
--- a/gcc/config/arm/elf.h
+++ b/gcc/config/arm/elf.h
@@ -75,16 +75,7 @@
 
 /* We might need a ARM specific header to function declarations.  */
 #undef  ASM_DECLARE_FUNCTION_NAME
-#define ASM_DECLARE_FUNCTION_NAME(FILE, NAME, DECL)		\
-  do								\
-    {								\
-      ARM_DECLARE_FUNCTION_NAME (FILE, NAME, DECL);		\
-      ASM_OUTPUT_TYPE_DIRECTIVE (FILE, NAME, "function");	\
-      ASM_DECLARE_RESULT (FILE, DECL_RESULT (DECL));		\
-      ASM_OUTPUT_LABEL(FILE, NAME);				\
-      ARM_OUTPUT_FN_UNWIND (FILE, TRUE);			\
-    }								\
-  while (0)
+#define ASM_DECLARE_FUNCTION_NAME arm_asm_declare_function_name
 
 /* We might need an ARM specific trailer for function declarations.  */
 #undef  ASM_DECLARE_FUNCTION_SIZE
diff --git a/gcc/testsuite/ChangeLog.arm b/gcc/testsuite/ChangeLog.arm
index 30afd4b0fa9b6f2859a3f0c84485b40487f602e9..338ac2d0fbd6752ce5ca59557cea12e4996d607f 100644
--- a/gcc/testsuite/ChangeLog.arm
+++ b/gcc/testsuite/ChangeLog.arm
@@ -4,6 +4,16 @@
 	2016-12-02  Andre Vieira  <andre.simoesdiasvieira@arm.com>
 		    Thomas Preud'homme	<thomas.preudhomme@arm.com>
 
+	* gcc.target/arm/cmse/cmse-4.c: New.
+	* gcc.target/arm/cmse/cmse-9.c: New.
+	* gcc.target/arm/cmse/cmse-10.c: New.
+
+2016-12-05  Andre Vieira  <andre.simoesdiasvieira@arm.com>
+
+	Backport from mainline
+	2016-12-02  Andre Vieira  <andre.simoesdiasvieira@arm.com>
+		    Thomas Preud'homme	<thomas.preudhomme@arm.com>
+
 	* gcc.target/arm/cmse/cmse-3.c: New.
 
 2016-12-05  Andre Vieira  <andre.simoesdiasvieira@arm.com>
diff --git a/gcc/testsuite/gcc.target/arm/cmse/cmse-10.c b/gcc/testsuite/gcc.target/arm/cmse/cmse-10.c
new file mode 100644
index 0000000000000000000000000000000000000000..1a91ac39ee37ef20495e047b402d3f5edc60a613
--- /dev/null
+++ b/gcc/testsuite/gcc.target/arm/cmse/cmse-10.c
@@ -0,0 +1,9 @@
+/* { dg-do compile } */
+/* { dg-options "-mcmse" }  */
+
+void
+foo (void) {}
+
+/* { dg-final { scan-assembler-not "bxns" } } */
+/* { dg-final { scan-assembler "foo:" } } */
+/* { dg-final { scan-assembler-not "__acle_se_foo:" } } */
diff --git a/gcc/testsuite/gcc.target/arm/cmse/cmse-4.c b/gcc/testsuite/gcc.target/arm/cmse/cmse-4.c
new file mode 100644
index 0000000000000000000000000000000000000000..6f930ab04a1097c64097a4e003296bbe85733319
--- /dev/null
+++ b/gcc/testsuite/gcc.target/arm/cmse/cmse-4.c
@@ -0,0 +1,27 @@
+/* { dg-do compile } */
+/* { dg-options "-mcmse" }  */
+
+struct span {
+  int a, b;
+};
+
+extern int qux (void);
+
+void __attribute__ ((cmse_nonsecure_entry))
+foo (void) {}
+
+static void __attribute__ ((cmse_nonsecure_entry))
+bar (void) {} /* { dg-warning "has no effect on functions with static linkage" } */
+
+int __attribute__ ((cmse_nonsecure_entry))
+baz (void)
+{
+  return qux ();
+}
+
+/* { dg-final { scan-assembler-times "bxns" 2 } } */
+/* { dg-final { scan-assembler "foo:" } } */
+/* { dg-final { scan-assembler "__acle_se_foo:" } } */
+/* { dg-final { scan-assembler-not "__acle_se_bar:" } } */
+/* { dg-final { scan-assembler "baz:" } } */
+/* { dg-final { scan-assembler "__acle_se_baz:" } } */
diff --git a/gcc/testsuite/gcc.target/arm/cmse/cmse-9.c b/gcc/testsuite/gcc.target/arm/cmse/cmse-9.c
new file mode 100644
index 0000000000000000000000000000000000000000..1d97f0e1a37209bd129ffe96ce92a86bc2e0d5d1
--- /dev/null
+++ b/gcc/testsuite/gcc.target/arm/cmse/cmse-9.c
@@ -0,0 +1,12 @@
+/* { dg-do compile } */
+/* { dg-skip-if "Testing exclusion of -mcmse" { arm-*-* } { "-mcmse" } { "" } }  */
+
+
+int __attribute__ ((cmse_nonsecure_entry))
+foo (int a)
+{ /* { dg-warning "attribute ignored without -mcmse option" } */
+  return a + 1;
+}
+
+/* { dg-final { scan-assembler "foo:" } } */
+/* { dg-final { scan-assembler-not "__acle_se_foo:" } } */