diff mbox

[PATCHv3,5/7,ARM,V8M] Handling ARMv8-M Security Extension's cmse_nonsecure_call attribute

Message ID 583EC08E.3000403@arm.com
State New
Headers show

Commit Message

Andre Vieira (lists) Nov. 30, 2016, 12:05 p.m. UTC
Hi,

I got a bug report against the old version of this patch and fixed it
here. This had to do with GCC optimizations sharing types with and
without the 'cmse_nonsecure_call' attribute.  The patch now no longer
sets the main variant, this didn't seem to do what I thought it did.
Instead the patch now creates distinct type copies for every declared
pointer that eventually points to the function type with the attribute,
it will also create a distinct copy for the function type itself.
Another change in this patch was to make 'arm_comp_type_attributes', the
ARM implementation of TARGET_COMP_TYPE_ATTRIBUTES, deny compatibility
between function types with the attribute and without.

I added a test case to test the issue solved with these changes.

*** gcc/ChangeLog ***
2016-11-xx  Andre Vieira        <andre.simoesdiasvieira@arm.com>
            Thomas Preud'homme  <thomas.preudhomme@arm.com>

        * config/arm/arm.c (gimplify.h): New include.
        (arm_handle_cmse_nonsecure_call): New.
        (arm_attribute_table): Added cmse_nonsecure_call.
        (arm_comp_type_attributes): Deny compatibility of function types
with
        without the cmse_nonsecure_call attribute.
        * doc/extend.texi (ARM ARMv8-M Security Extensions): New attribute.

*** gcc/testsuite/ChangeLog ***
2016-11-xx  Andre Vieira        <andre.simoesdiasvieira@arm.com>
            Thomas Preud'homme  <thomas.preudhomme@arm.com>

        * gcc.target/arm/cmse/cmse-3.c: Add tests.
        * gcc.target/arm/cmse/cmse-4.c: Add tests.
        * gcc.target/arm/cmse/cmse-15.c: New.


Cheers,
Andre

Comments

Kyrill Tkachov Nov. 30, 2016, 5:22 p.m. UTC | #1
On 30/11/16 12:05, Andre Vieira (lists) wrote:
> Hi,
>
> I got a bug report against the old version of this patch and fixed it
> here. This had to do with GCC optimizations sharing types with and
> without the 'cmse_nonsecure_call' attribute.  The patch now no longer
> sets the main variant, this didn't seem to do what I thought it did.
> Instead the patch now creates distinct type copies for every declared
> pointer that eventually points to the function type with the attribute,
> it will also create a distinct copy for the function type itself.
> Another change in this patch was to make 'arm_comp_type_attributes', the
> ARM implementation of TARGET_COMP_TYPE_ATTRIBUTES, deny compatibility
> between function types with the attribute and without.
>
> I added a test case to test the issue solved with these changes.

Ok.
Thanks,
Kyrill

> *** gcc/ChangeLog ***
> 2016-11-xx  Andre Vieira        <andre.simoesdiasvieira@arm.com>
>              Thomas Preud'homme  <thomas.preudhomme@arm.com>
>
>          * config/arm/arm.c (gimplify.h): New include.
>          (arm_handle_cmse_nonsecure_call): New.
>          (arm_attribute_table): Added cmse_nonsecure_call.
>          (arm_comp_type_attributes): Deny compatibility of function types
> with
>          without the cmse_nonsecure_call attribute.
>          * doc/extend.texi (ARM ARMv8-M Security Extensions): New attribute.
>
> *** gcc/testsuite/ChangeLog ***
> 2016-11-xx  Andre Vieira        <andre.simoesdiasvieira@arm.com>
>              Thomas Preud'homme  <thomas.preudhomme@arm.com>
>
>          * gcc.target/arm/cmse/cmse-3.c: Add tests.
>          * gcc.target/arm/cmse/cmse-4.c: Add tests.
>          * gcc.target/arm/cmse/cmse-15.c: New.
>
>
> Cheers,
> Andre
diff mbox

Patch

diff --git a/gcc/config/arm/arm.c b/gcc/config/arm/arm.c
index 9509b0e25c19f35ed312efa1d86efc18a0db7674..d1160d5001830beea981df7454f09daf7fac2c5a 100644
--- a/gcc/config/arm/arm.c
+++ b/gcc/config/arm/arm.c
@@ -63,6 +63,7 @@ 
 #include "tm-constrs.h"
 #include "rtl-iter.h"
 #include "optabs-libfuncs.h"
+#include "gimplify.h"
 
 /* This file should be included last.  */
 #include "target-def.h"
@@ -137,6 +138,7 @@  static tree arm_handle_isr_attribute (tree *, tree, tree, int, bool *);
 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 tree arm_handle_cmse_nonsecure_call (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);
@@ -348,6 +350,8 @@  static const struct attribute_spec arm_attribute_table[] =
   /* ARMv8-M Security Extensions support.  */
   { "cmse_nonsecure_entry", 0, 0, true, false, false,
     arm_handle_cmse_nonsecure_entry, false },
+  { "cmse_nonsecure_call", 0, 0, true, false, false,
+    arm_handle_cmse_nonsecure_call, true },
   { NULL,           0, 0, false, false, false, NULL, false }
 };
 
@@ -6753,6 +6757,78 @@  arm_handle_cmse_nonsecure_entry (tree *node, tree name,
   return NULL_TREE;
 }
 
+
+/* Called upon detection of the use of the cmse_nonsecure_call attribute, this
+   function will check whether the attribute is allowed here and will add the
+   attribute to the function type tree or otherwise issue a diagnostic.  The
+   reason we check this at declaration time is to only allow the use of the
+   attribute with declarations of function pointers and not function
+   declarations.  This function checks NODE is of the expected type and issues
+   diagnostics otherwise using NAME.  If it is not of the expected type
+   *NO_ADD_ATTRS will be set to true.  */
+
+static tree
+arm_handle_cmse_nonsecure_call (tree *node, tree name,
+				 tree /* args */,
+				 int /* flags */,
+				 bool *no_add_attrs)
+{
+  tree decl = NULL_TREE, fntype = NULL_TREE;
+  tree main_variant, type;
+
+  if (!use_cmse)
+    {
+      *no_add_attrs = true;
+      warning (OPT_Wattributes, "%qE attribute ignored without -mcmse option.",
+	       name);
+      return NULL_TREE;
+    }
+
+  if (TREE_CODE (*node) == VAR_DECL || TREE_CODE (*node) == TYPE_DECL)
+    {
+      decl = *node;
+      fntype = TREE_TYPE (decl);
+    }
+
+  while (fntype != NULL_TREE && TREE_CODE (fntype) == POINTER_TYPE)
+    fntype = TREE_TYPE (fntype);
+
+  if (!decl || TREE_CODE (fntype) != FUNCTION_TYPE)
+    {
+	warning (OPT_Wattributes, "%qE attribute only applies to base type of a "
+		 "function pointer", name);
+	*no_add_attrs = true;
+	return NULL_TREE;
+    }
+
+  *no_add_attrs |= cmse_func_args_or_return_in_stack (NULL, name, fntype);
+
+  if (*no_add_attrs)
+    return NULL_TREE;
+
+  /* Prevent trees being shared among function types with and without
+     cmse_nonsecure_call attribute.  */
+  type = TREE_TYPE (decl);
+
+  type = build_distinct_type_copy (type);
+  TREE_TYPE (decl) = type;
+  fntype = type;
+
+  while (TREE_CODE (fntype) != FUNCTION_TYPE)
+    {
+      type = fntype;
+      fntype = TREE_TYPE (fntype);
+      fntype = build_distinct_type_copy (fntype);
+      TREE_TYPE (type) = fntype;
+    }
+
+  /* Construct a type attribute and add it to the function type.  */
+  tree attrs = tree_cons (get_identifier ("cmse_nonsecure_call"), NULL_TREE,
+			  TYPE_ATTRIBUTES (fntype));
+  TYPE_ATTRIBUTES (fntype) = attrs;
+  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).  */
@@ -6793,6 +6869,14 @@  arm_comp_type_attributes (const_tree type1, const_tree type2)
   if (l1 != l2)
     return 0;
 
+  l1 = lookup_attribute ("cmse_nonsecure_call",
+			 TYPE_ATTRIBUTES (type1)) != NULL;
+  l2 = lookup_attribute ("cmse_nonsecure_call",
+			 TYPE_ATTRIBUTES (type2)) != NULL;
+
+  if (l1 != l2)
+    return 0;
+
   return 1;
 }
 
diff --git a/gcc/doc/extend.texi b/gcc/doc/extend.texi
index e8d9d90c0ac98a2b50aad60edf01e16dfa480db7..e8d8deefca87eacbdde6042fc1885c0d0a85a5b8 100644
--- a/gcc/doc/extend.texi
+++ b/gcc/doc/extend.texi
@@ -12622,8 +12622,8 @@  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 two new function attributes:
+@code{cmse_nonsecure_entry} and @code{cmse_nonsecure_call}.
 
 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-15.c b/gcc/testsuite/gcc.target/arm/cmse/cmse-15.c
new file mode 100644
index 0000000000000000000000000000000000000000..4e9ace1f3f33b8a8653797e29ca62eb3dd7ae918
--- /dev/null
+++ b/gcc/testsuite/gcc.target/arm/cmse/cmse-15.c
@@ -0,0 +1,72 @@ 
+/* { dg-do compile } */
+/* { dg-options "-mcmse" } */
+
+int __attribute__ ((cmse_nonsecure_call)) (*ns_foo) (void);
+int (*s_bar) (void);
+int __attribute__ ((cmse_nonsecure_call)) (**ns_foo2) (void);
+int (**s_bar2) (void);
+
+typedef int __attribute__ ((cmse_nonsecure_call)) ns_foo_t (void);
+typedef int s_bar_t (void);
+typedef int __attribute__ ((cmse_nonsecure_call)) (* ns_foo_ptr) (void);
+typedef int (*s_bar_ptr) (void);
+
+int nonsecure0 (ns_foo_t * ns_foo_p)
+{
+  return ns_foo_p ();
+}
+
+int nonsecure1 (ns_foo_t ** ns_foo_p)
+{
+  return (*ns_foo_p) ();
+}
+
+int nonsecure2 (ns_foo_ptr ns_foo_p)
+{
+  return ns_foo_p ();
+}
+int nonsecure3 (ns_foo_ptr * ns_foo_p)
+{
+  return (*ns_foo_p) ();
+}
+
+int secure0 (s_bar_t * s_bar_p)
+{
+  return s_bar_p ();
+}
+
+int secure1 (s_bar_t ** s_bar_p)
+{
+  return (*s_bar_p) ();
+}
+
+int secure2 (s_bar_ptr s_bar_p)
+{
+  return s_bar_p ();
+}
+
+int secure3 (s_bar_ptr * s_bar_p)
+{
+  return (*s_bar_p) ();
+}
+
+int nonsecure4 (void)
+{
+  return ns_foo ();
+}
+
+int nonsecure5 (void)
+{
+  return (*ns_foo2) ();
+}
+
+int secure4 (void)
+{
+  return s_bar ();
+}
+
+int secure5 (void)
+{
+  return (*s_bar2) ();
+}
+/* { dg-final { scan-assembler-times "bl\\s+__gnu_cmse_nonsecure_call" 6 } } */
diff --git a/gcc/testsuite/gcc.target/arm/cmse/cmse-3.c b/gcc/testsuite/gcc.target/arm/cmse/cmse-3.c
index 2c2920e1dc310106d83203eb51e1a68a275d0152..7f92a4c28b3333e4c8fdc256211f3ed74a383cd4 100644
--- a/gcc/testsuite/gcc.target/arm/cmse/cmse-3.c
+++ b/gcc/testsuite/gcc.target/arm/cmse/cmse-3.c
@@ -35,3 +35,11 @@  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" } */
+
+typedef void __attribute__ ((cmse_nonsecure_call)) bar2 (long long a, int b, long long c); /* { dg-error "not available to functions with arguments passed on the stack" } */
+
+typedef void __attribute__ ((cmse_nonsecure_call)) baz2 (long long a, int b, struct span c); /* { dg-error "not available to functions with arguments passed on the stack" } */
+
+typedef struct span __attribute__ ((cmse_nonsecure_call)) qux2 (void); /* { dg-error "not available to functions that return value on the stack" } */
+
+typedef void __attribute__ ((cmse_nonsecure_call)) norf2 (int a, ...); /* { dg-error "not available to functions with variable number of arguments" } */
diff --git a/gcc/testsuite/gcc.target/arm/cmse/cmse-4.c b/gcc/testsuite/gcc.target/arm/cmse/cmse-4.c
index 6f930ab04a1097c64097a4e003296bbe85733319..d0999a4181ac022b115ae20cf3e1c8bf78f6becf 100644
--- a/gcc/testsuite/gcc.target/arm/cmse/cmse-4.c
+++ b/gcc/testsuite/gcc.target/arm/cmse/cmse-4.c
@@ -19,9 +19,16 @@  baz (void)
   return qux ();
 }
 
+void __attribute__ ((cmse_nonsecure_call))
+quux (void) {} /* { dg-warning "attribute only applies to base type of a function pointer" } */
+
+int __attribute__ ((cmse_nonsecure_call)) norf; /* { dg-warning "attribute only applies to base type of a function pointer" } */
+
 /* { 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:" } } */
+/* { dg-final { scan-assembler-not "__acle_se_quux:" } } */
+/* { dg-final { scan-assembler-not "__acle_se_norf:" } } */