diff mbox

[PATCHv2,2/7,ARM,V8M] Handling ARMv8-M Security Extension's cmse_nonsecure_entry attribute

Message ID 580F8836.1050203@arm.com
State New
Headers show

Commit Message

Andre Vieira (lists) Oct. 25, 2016, 4:28 p.m. UTC
On 24/08/16 12:00, Andre Vieira (lists) wrote:
> On 25/07/16 14:21, Andre Vieira (lists) wrote:
>> This patch adds support for the ARMv8-M Security Extensions
>> 'cmse_nonsecure_entry' attribute. In this patch we implement the
>> attribute handling and diagnosis around the attribute. 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 (arm_handle_cmse_nonsecure_entry): New.
>>         (arm_attribute_table): Added cmse_nonsecure_entry
>>         (arm_compute_func_type): Handle cmse_nonsecure_entry.
>>         (cmse_func_args_or_return_in_stack): New.
>>         (arm_handle_cmse_nonsecure_entry): New.
>>         * config/arm/arm.h (ARM_FT_CMSE_ENTRY): New macro define.
>>         (IS_CMSE_ENTRY): Likewise.
>>
>> *** gcc/testsuite/ChangeLog ***
>> 2016-07-25  Andre Vieira        <andre.simoesdiasvieira@arm.com>
>>             Thomas Preud'homme  <thomas.preudhomme@arm.com>
>>
>>         * gcc.target/arm/cmse/cmse-3.c: New.
>>
> 
> Added more documentation as requested.
> 
> ----
> 
> This patch adds support for the ARMv8-M Security Extensions
> 'cmse_nonsecure_entry' attribute. In this patch we implement the
> attribute handling and diagnosis around the attribute. 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-xx  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.
>         (cmse_func_args_or_return_in_stack): New.
>         (arm_handle_cmse_nonsecure_entry): New.
>         * config/arm/arm.h (ARM_FT_CMSE_ENTRY): New macro define.
>         (IS_CMSE_ENTRY): Likewise.
>         * doc/extend.texi (ARM ARMv8-M Security Extensions): New attribute.
> 
> *** gcc/testsuite/ChangeLog ***
> 2016-07-xx  Andre Vieira        <andre.simoesdiasvieira@arm.com>
>             Thomas Preud'homme  <thomas.preudhomme@arm.com>
> 
>         * gcc.target/arm/cmse/cmse-3.c: New.
> 
Hi,

Rebased previous patch on top of trunk as requested. No changes to
ChangeLog.

Cheers,
Andre

Comments

Kyrill Tkachov Oct. 26, 2016, 9:33 a.m. UTC | #1
Hi Andre,

On 25/10/16 17:28, Andre Vieira (lists) wrote:
> On 24/08/16 12:00, Andre Vieira (lists) wrote:
>> On 25/07/16 14:21, Andre Vieira (lists) wrote:
>>> This patch adds support for the ARMv8-M Security Extensions
>>> 'cmse_nonsecure_entry' attribute. In this patch we implement the
>>> attribute handling and diagnosis around the attribute. 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 (arm_handle_cmse_nonsecure_entry): New.
>>>          (arm_attribute_table): Added cmse_nonsecure_entry
>>>          (arm_compute_func_type): Handle cmse_nonsecure_entry.
>>>          (cmse_func_args_or_return_in_stack): New.
>>>          (arm_handle_cmse_nonsecure_entry): New.
>>>          * config/arm/arm.h (ARM_FT_CMSE_ENTRY): New macro define.
>>>          (IS_CMSE_ENTRY): Likewise.
>>>
>>> *** gcc/testsuite/ChangeLog ***
>>> 2016-07-25  Andre Vieira        <andre.simoesdiasvieira@arm.com>
>>>              Thomas Preud'homme  <thomas.preudhomme@arm.com>
>>>
>>>          * gcc.target/arm/cmse/cmse-3.c: New.
>>>
>> Added more documentation as requested.
>>
>> ----
>>
>> This patch adds support for the ARMv8-M Security Extensions
>> 'cmse_nonsecure_entry' attribute. In this patch we implement the
>> attribute handling and diagnosis around the attribute. 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-xx  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.
>>          (cmse_func_args_or_return_in_stack): New.
>>          (arm_handle_cmse_nonsecure_entry): New.
>>          * config/arm/arm.h (ARM_FT_CMSE_ENTRY): New macro define.
>>          (IS_CMSE_ENTRY): Likewise.
>>          * doc/extend.texi (ARM ARMv8-M Security Extensions): New attribute.
>>
>> *** gcc/testsuite/ChangeLog ***
>> 2016-07-xx  Andre Vieira        <andre.simoesdiasvieira@arm.com>
>>              Thomas Preud'homme  <thomas.preudhomme@arm.com>
>>
>>          * gcc.target/arm/cmse/cmse-3.c: New.
>>
> Hi,
>
> Rebased previous patch on top of trunk as requested. No changes to
> ChangeLog.
>
> Cheers,
> Andre


@@ -6661,6 +6668,110 @@ arm_handle_notshared_attribute (tree *node,
  }
  #endif

+/* This function returns true if a function with declaration FNDECL, name
+   NAME and type FNTYPE uses the stack to pass arguments or return variables
+   and false otherwise.  This is used for functions with the attributes
+   'cmse_nonsecure_call' or 'cmse_nonsecure_entry' and this function will issue
+   diagnostic messages if the stack is used.  */
+

I think NAME is the name of the attribute, not the function. The comment here is
misleading, otherwise the error message in this function doesn't make sense.
Please fix the description.

+static bool
+cmse_func_args_or_return_in_stack (tree fndecl, tree name, tree fntype)
+{
+  function_args_iterator args_iter;
+  CUMULATIVE_ARGS args_so_far_v;
+  cumulative_args_t args_so_far;
+  bool first_param = true;
+  tree arg_type, prev_arg_type = NULL_TREE, ret_type;
+
+  /* Error out if any argument is passed on the stack.  */
+  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;
+      machine_mode arg_mode = TYPE_MODE (arg_type);
+
+      prev_arg_type = arg_type;
+      if (VOID_TYPE_P (arg_type))
+    continue;
+
+      if (!first_param)
+    arm_function_arg_advance (args_so_far, arg_mode, arg_type, true);
+      arg_rtx = arm_function_arg (args_so_far, arg_mode, arg_type, true);
+      if (!arg_rtx
+      || arm_arg_partial_bytes (args_so_far, arg_mode, arg_type, true))
+    {
+      error ("%qE attribute not available to functions with arguments "
+         "passed on the stack", name);
+      return true;
+    }
+      first_param = false;
+    }
+
+  /* Error out for variadic functions since we cannot control how many
+     arguments will be passed and thus stack could be used. stdarg_p () is not
+     used for the checking to avoid browsing arguments twice.  */
+  if (prev_arg_type != NULL_TREE && !VOID_TYPE_P (prev_arg_type))
+    {
+      error ("%qE attribute not available to functions with variable number "
+         "of arguments", name);
+      return true;
+    }
+
+  /* Error out if return value is passed on the stack.  */
+  ret_type = TREE_TYPE (fntype);
+  if (arm_return_in_memory (ret_type, fntype))
+    {
+      error ("%qE attribute not available to functions that return value on "
+         "the stack", name);
+      return true;
+    }
+  return false;
+}
+
+/* Called upon detection of the use of the cmse_nonsecure_entry attribute, this
+   function will check whether the attribute is allowed here and will add the
+   attribute to the function declaration tree or otherwise issue a warning.  */
+
+static tree
+arm_handle_cmse_nonsecure_entry (tree *node, tree name,
+                 tree /* args */,
+                 int /* flags */,
+                 bool *no_add_attrs)
+{
+  tree fndecl;
+
+  if (!use_cmse)
+    {
+      *no_add_attrs = true;
+      return NULL_TREE;
+    }

Do you also want to warn the user here that the attribute will be ignored?
This looks ok to me otherwise.

Thanks,
Kyrill
Andre Vieira (lists) Oct. 26, 2016, 4:28 p.m. UTC | #2
On 26/10/16 10:33, Kyrill Tkachov wrote:
> 
> +static tree
> +arm_handle_cmse_nonsecure_entry (tree *node, tree name,
> +                 tree /* args */,
> +                 int /* flags */,
> +                 bool *no_add_attrs)
> +{
> +  tree fndecl;
> +
> +  if (!use_cmse)
> +    {
> +      *no_add_attrs = true;
> +      return NULL_TREE;
> +    }
> 
> Do you also want to warn the user here that the attribute will be ignored?
> This looks ok to me otherwise.
> 

Can easily do and might be more user friendly. How about
"<attribute_name> attribute ignored without -mcmse option."

Cheers,
Andre
Kyrill Tkachov Oct. 26, 2016, 4:28 p.m. UTC | #3
On 26/10/16 17:28, Andre Vieira (lists) wrote:
> On 26/10/16 10:33, Kyrill Tkachov wrote:
>> +static tree
>> +arm_handle_cmse_nonsecure_entry (tree *node, tree name,
>> +                 tree /* args */,
>> +                 int /* flags */,
>> +                 bool *no_add_attrs)
>> +{
>> +  tree fndecl;
>> +
>> +  if (!use_cmse)
>> +    {
>> +      *no_add_attrs = true;
>> +      return NULL_TREE;
>> +    }
>>
>> Do you also want to warn the user here that the attribute will be ignored?
>> This looks ok to me otherwise.
>>
> Can easily do and might be more user friendly. How about
> "<attribute_name> attribute ignored without -mcmse option."

Yes, that's fine (without the full stop at the end)
Kyrill

> Cheers,
> Andre
>
diff mbox

Patch

diff --git a/gcc/config/arm/arm.h b/gcc/config/arm/arm.h
index a370dccdaa9fa4c980c1df11cb95a65cad16ac85..44d1ac45b03dec210e6986f103bf8588119a8aa8 100644
--- a/gcc/config/arm/arm.h
+++ b/gcc/config/arm/arm.h
@@ -1386,6 +1386,7 @@  enum reg_class
 #define ARM_FT_VOLATILE		(1 << 4) /* Does not return.  */
 #define ARM_FT_NESTED		(1 << 5) /* Embedded inside another func.  */
 #define ARM_FT_STACKALIGN	(1 << 6) /* Called with misaligned stack.  */
+#define ARM_FT_CMSE_ENTRY	(1 << 7) /* ARMv8-M non-secure entry function.  */
 
 /* Some macros to test these flags.  */
 #define ARM_FUNC_TYPE(t)	(t & ARM_FT_TYPE_MASK)
@@ -1394,6 +1395,7 @@  enum reg_class
 #define IS_NAKED(t)        	(t & ARM_FT_NAKED)
 #define IS_NESTED(t)       	(t & ARM_FT_NESTED)
 #define IS_STACKALIGN(t)       	(t & ARM_FT_STACKALIGN)
+#define IS_CMSE_ENTRY(t)	(t & ARM_FT_CMSE_ENTRY)
 
 
 /* Structure used to hold the function stack frame layout.  Offsets are
diff --git a/gcc/config/arm/arm.c b/gcc/config/arm/arm.c
index 44677c1bccad42c5ad603ea0951d62abcbd6f05d..996f917ae1dc8e16f219984738c3ac3f8b42f09f 100644
--- a/gcc/config/arm/arm.c
+++ b/gcc/config/arm/arm.c
@@ -135,6 +135,7 @@  static tree arm_handle_isr_attribute (tree *, tree, tree, int, bool *);
 #if TARGET_DLLIMPORT_DECL_ATTRIBUTES
 static tree arm_handle_notshared_attribute (tree *, tree, tree, int, bool *);
 #endif
+static tree arm_handle_cmse_nonsecure_entry (tree *, tree, tree, int, bool *);
 static void arm_output_function_epilogue (FILE *, HOST_WIDE_INT);
 static void arm_output_function_prologue (FILE *, HOST_WIDE_INT);
 static int arm_comp_type_attributes (const_tree, const_tree);
@@ -347,6 +348,9 @@  static const struct attribute_spec arm_attribute_table[] =
   { "notshared",    0, 0, false, true, false, arm_handle_notshared_attribute,
     false },
 #endif
+  /* ARMv8-M Security Extensions support.  */
+  { "cmse_nonsecure_entry", 0, 0, true, false, false,
+    arm_handle_cmse_nonsecure_entry, false },
   { NULL,           0, 0, false, false, false, NULL, false }
 };
 
@@ -3662,6 +3666,9 @@  arm_compute_func_type (void)
   else
     type |= arm_isr_value (TREE_VALUE (a));
 
+  if (lookup_attribute ("cmse_nonsecure_entry", attr))
+    type |= ARM_FT_CMSE_ENTRY;
+
   return type;
 }
 
@@ -6661,6 +6668,110 @@  arm_handle_notshared_attribute (tree *node,
 }
 #endif
 
+/* This function returns true if a function with declaration FNDECL, name
+   NAME and type FNTYPE uses the stack to pass arguments or return variables
+   and false otherwise.  This is used for functions with the attributes
+   'cmse_nonsecure_call' or 'cmse_nonsecure_entry' and this function will issue
+   diagnostic messages if the stack is used.  */
+
+static bool
+cmse_func_args_or_return_in_stack (tree fndecl, tree name, tree fntype)
+{
+  function_args_iterator args_iter;
+  CUMULATIVE_ARGS args_so_far_v;
+  cumulative_args_t args_so_far;
+  bool first_param = true;
+  tree arg_type, prev_arg_type = NULL_TREE, ret_type;
+
+  /* Error out if any argument is passed on the stack.  */
+  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;
+      machine_mode arg_mode = TYPE_MODE (arg_type);
+
+      prev_arg_type = arg_type;
+      if (VOID_TYPE_P (arg_type))
+	continue;
+
+      if (!first_param)
+	arm_function_arg_advance (args_so_far, arg_mode, arg_type, true);
+      arg_rtx = arm_function_arg (args_so_far, arg_mode, arg_type, true);
+      if (!arg_rtx
+	  || arm_arg_partial_bytes (args_so_far, arg_mode, arg_type, true))
+	{
+	  error ("%qE attribute not available to functions with arguments "
+		 "passed on the stack", name);
+	  return true;
+	}
+      first_param = false;
+    }
+
+  /* Error out for variadic functions since we cannot control how many
+     arguments will be passed and thus stack could be used.  stdarg_p () is not
+     used for the checking to avoid browsing arguments twice.  */
+  if (prev_arg_type != NULL_TREE && !VOID_TYPE_P (prev_arg_type))
+    {
+      error ("%qE attribute not available to functions with variable number "
+	     "of arguments", name);
+      return true;
+    }
+
+  /* Error out if return value is passed on the stack.  */
+  ret_type = TREE_TYPE (fntype);
+  if (arm_return_in_memory (ret_type, fntype))
+    {
+      error ("%qE attribute not available to functions that return value on "
+	     "the stack", name);
+      return true;
+    }
+  return false;
+}
+
+/* Called upon detection of the use of the cmse_nonsecure_entry attribute, this
+   function will check whether the attribute is allowed here and will add the
+   attribute to the function declaration tree or otherwise issue a warning.  */
+
+static tree
+arm_handle_cmse_nonsecure_entry (tree *node, tree name,
+				 tree /* args */,
+				 int /* flags */,
+				 bool *no_add_attrs)
+{
+  tree fndecl;
+
+  if (!use_cmse)
+    {
+      *no_add_attrs = true;
+      return NULL_TREE;
+    }
+
+  /* Ignore attribute for function types.  */
+  if (TREE_CODE (*node) != FUNCTION_DECL)
+    {
+      warning (OPT_Wattributes, "%qE attribute only applies to functions",
+	       name);
+      *no_add_attrs = true;
+      return NULL_TREE;
+    }
+
+  fndecl = *node;
+
+  /* Warn for static linkage functions.  */
+  if (!TREE_PUBLIC (fndecl))
+    {
+      warning (OPT_Wattributes, "%qE attribute has no effect on functions "
+	       "with static linkage", name);
+      *no_add_attrs = true;
+      return NULL_TREE;
+    }
+
+  *no_add_attrs |= cmse_func_args_or_return_in_stack (fndecl, name,
+						TREE_TYPE (fndecl));
+  return NULL_TREE;
+}
+
 /* Return 0 if the attributes for two types are incompatible, 1 if they
    are compatible, and 2 if they are nearly compatible (which causes a
    warning to be generated).  */
diff --git a/gcc/doc/extend.texi b/gcc/doc/extend.texi
index 3f6fc27950fafa2e7649deb9dc5db44737cbb691..e5b28032e5610b896908139ec285ce90a9dffd8a 100644
--- a/gcc/doc/extend.texi
+++ b/gcc/doc/extend.texi
@@ -12607,6 +12607,9 @@  Security Extensions: Requiremenets on Development Tools Engineering
 Specification, which can be found at
 @uref{http://infocenter.arm.com/help/topic/com.arm.doc.ecm0359818/ECM0359818_armv8m_security_extensions_reqs_on_dev_tools_1_0.pdf}.
 
+As part of the Security Extensions GCC implements a new function attribute
+@code{cmse_nonsecure_entry}.
+
 As part of the Security Extensions GCC implements the intrinsics below.  FPTR
 is used here to mean any function pointer type.
 
diff --git a/gcc/testsuite/gcc.target/arm/cmse/cmse-3.c b/gcc/testsuite/gcc.target/arm/cmse/cmse-3.c
new file mode 100644
index 0000000000000000000000000000000000000000..2c2920e1dc310106d83203eb51e1a68a275d0152
--- /dev/null
+++ b/gcc/testsuite/gcc.target/arm/cmse/cmse-3.c
@@ -0,0 +1,37 @@ 
+/* { dg-do compile } */
+/* { dg-options "-mcmse" }  */
+
+struct span {
+  int a, b;
+};
+struct span2 {
+  float a, b, c, d;
+};
+
+union test_union
+{
+  long long a;
+  int b;
+  struct span2 c;
+} test_union;
+
+void __attribute__ ((cmse_nonsecure_entry))
+foo (long long a, int b, long long c) {} /* { dg-error "not available to functions with arguments passed on the stack" } */
+
+void __attribute__ ((cmse_nonsecure_entry))
+bar (long long a, int b, struct span c) {} /* { dg-error "not available to functions with arguments passed on the stack" } */
+
+void __attribute__ ((cmse_nonsecure_entry))
+baz (int a, ...) {} /* { dg-error "not available to functions with variable number of arguments" } */
+
+struct span __attribute__ ((cmse_nonsecure_entry))
+qux (void) { /* { dg-error "not available to functions that return value on the stack" } */
+  struct span ret = {0, 0};
+  return ret;
+}
+
+void __attribute__ ((cmse_nonsecure_entry))
+norf (struct span2 a) {}
+
+void __attribute__ ((cmse_nonsecure_entry))
+foo2 (long long a, int b, union test_union c) {} /* { dg-error "not available to functions with arguments passed on the stack" } */