Re: [PATCH 3/7, GCC, ARM, V8M] ARMv8-M Security Extension's cmse_nonsecure_entry: __acle_se label and bxns return

[Kyrill Tkachov]

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.simoesdiasvie...@arm.com>
             Thomas Preud'homme  <thomas.preudho...@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.simoesdiasvie...@arm.com>
             Thomas Preud'homme  <thomas.preudho...@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