This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [4.7][google] Adding a new option -fstack-protector-strong. (issue5461043)


Hi, Andrew and Richard, check via referenced vars is much easier, thanks!

Updated patches attached at EOM, also uploaded to
http://codereview.appspot.com/5461043

Hi, Diego, that's good suggestion. I'm glad to send this for trunk at
the next stage 1.

-Han

Updated patches ================
diff --git a/gcc/cfgexpand.c b/gcc/cfgexpand.c
index 8684721..ae76441 100644
--- a/gcc/cfgexpand.c
+++ b/gcc/cfgexpand.c
@@ -1507,15 +1507,34 @@ estimated_stack_frame_size (struct cgraph_node *node)
   return size;
 }

+/* Helper routine to check if a record or union contains an array field. */
+
+static int record_or_union_type_has_array(tree tree_type) {
+  tree fields = TYPE_FIELDS(tree_type);
+  tree f;
+  for (f = fields; f; f = DECL_CHAIN (f)) {
+    if (TREE_CODE(f) == FIELD_DECL) {
+      tree field_type = TREE_TYPE(f);
+      if (RECORD_OR_UNION_TYPE_P(field_type))
+        return record_or_union_type_has_array(field_type);
+      if (TREE_CODE(field_type) == ARRAY_TYPE)
+        return 1;
+    }
+  }
+  return 0;
+}
+
 /* Expand all variables used in the function.  */

 static void
 expand_used_vars (void)
 {
   tree var, outer_block = DECL_INITIAL (current_function_decl);
+  referenced_var_iterator rvi;
   VEC(tree,heap) *maybe_local_decls = NULL;
   unsigned i;
   unsigned len;
+  int gen_stack_protect_signal = 0;

   /* Compute the phase of the stack frame for this function.  */
   {
@@ -1548,6 +1567,20 @@ expand_used_vars (void)
 	}
     }

+  FOR_EACH_REFERENCED_VAR(DECL_STRUCT_FUNCTION(current_function_decl),
var, rvi)
+    if (!is_global_var(var) && TREE_ADDRESSABLE(var))
+      ++gen_stack_protect_signal;
+
+  /* Examine local variable declaration. */
+  if (!gen_stack_protect_signal)
+    FOR_EACH_LOCAL_DECL (cfun, i, var)
+      if (TREE_CODE(var) == VAR_DECL && !is_global_var(var)) {
+        tree var_type = TREE_TYPE(var);
+        gen_stack_protect_signal += (TREE_CODE(var_type) == ARRAY_TYPE) ||
+          (RECORD_OR_UNION_TYPE_P(var_type) &&
+           record_or_union_type_has_array(var_type));
+      }
+
   /* At this point all variables on the local_decls with TREE_USED
      set are not associated with any block scope.  Lay them out.  */

@@ -1640,9 +1673,13 @@ expand_used_vars (void)

   /* There are several conditions under which we should create a
      stack guard: protect-all, alloca used, protected decls present.  */
-  if (flag_stack_protect == 2
-      || (flag_stack_protect
-	  && (cfun->calls_alloca || has_protected_decls)))
+  if (flag_stack_protect == 2 /* -fstack-protector-all */
+      || (flag_stack_protect == 1 /* -fstack-protector */
+	  && (cfun->calls_alloca || has_protected_decls))) {
+    create_stack_guard ();
+  } else if (flag_stack_protect == 3 && /* -fstack-protector-strong */
+             (gen_stack_protect_signal ||
+              cfun->calls_alloca || has_protected_decls))
     create_stack_guard ();

   /* Assign rtl to each variable based on these partitions.  */
diff --git a/gcc/common.opt b/gcc/common.opt
index 55d3f2d..1ad9717 100644
--- a/gcc/common.opt
+++ b/gcc/common.opt
@@ -1848,6 +1848,10 @@ fstack-protector-all
 Common Report RejectNegative Var(flag_stack_protect, 2)
 Use a stack protection method for every function

+fstack-protector-strong
+Common Report RejectNegative Var(flag_stack_protect, 3)
+Use a smart stack protection method for certain functions
+
 fstack-usage
 Common RejectNegative Var(flag_stack_usage)
 Output stack usage information on a per-function basis
diff --git a/gcc/testsuite/gcc.dg/fstack-protector-strong.c
b/gcc/testsuite/gcc.dg/fstack-protector-strong.c
new file mode 100644
index 0000000..44225f5
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/fstack-protector-strong.c
@@ -0,0 +1,110 @@
+/* Test that stack protection is done on chosen functions. */
+
+/* { dg-do compile { target i?86-*-* x86_64-*-* } } */
+/* { dg-options "-O2 -fstack-protector-strong" } */
+
+#include<string.h>
+#include<stdlib.h>
+
+extern int g0;
+extern int *pg0;
+int goo(int *);
+int hoo(int);
+
+/* Function frame address escaped function call. */
+int foo1()
+{
+  int i;
+  return goo(&i);
+}
+
+struct ArrayStruct {
+  int a;
+  int array[10];
+};
+
+struct AA {
+  int b;
+  struct ArrayStruct as;
+};
+
+/* Function frame contains array. */
+int foo2() {
+  struct AA aa;
+  int i;
+  for (i = 0; i < 10; ++i) {
+    aa.as.array[i] = i * (i-1) + i / 2;
+  }
+  return aa.as.array[5];
+}
+
+/* Address computation based on a function frame address. */
+int foo3() {
+  int a;
+  int *p;
+  p = &a + 5;
+  return goo(p);
+}
+
+/* Address cast based on a function frame address. */
+int foo4() {
+  int a;
+  return goo(g0 << 2 ? (int *)(3 * (long)(void *)(&a)) : 0);
+
+}
+
+/* Address cast based on a local array. */
+int foo5() {
+  short array[10];
+  return goo((int *)(array + 5));
+}
+
+struct BB {
+  int one;
+  int two;
+  int three;
+};
+
+/* Address computaton based on a function frame address.*/
+int foo6() {
+  struct BB bb;
+  return goo(&bb.one + sizeof(int));
+}
+
+/* Function frame address escaped via global variable. */
+int foo7() {
+  int a;
+  pg0 = &a;
+  goo(pg0);
+  return *pg0;
+}
+
+/* Check that this covers -fstack-protector. */
+int foo8() {
+  char base[100];
+  memcpy((void *)base, (const void *)pg0, 105);
+  return (int)(base[32]);
+}
+
+/* Check that this covers -fstack-protector. */
+int foo9() {
+  char* p = alloca(100);
+  return goo((int *)(p + 50));
+}
+
+/* Address taken on struct. */
+int foo10() {
+  struct BB bb;
+  int i;
+  memset(&bb, 5, sizeof bb);
+  for (i = 0; i < 10; ++i) {
+    bb.one = i;
+    bb.two = bb.one + bb.two;
+    bb.three = bb.one + bb.two + bb.three;
+  }
+  return bb.three;
+}
+
+/* { dg-final { scan-assembler-times "stack_chk_fail" 10 } } */
+
+
================================

On Thu, Dec 8, 2011 at 5:18 AM, Diego Novillo <dnovillo@google.com> wrote:
> On Wed, Dec 7, 2011 at 20:07, Han Shen <shenhan@google.com> wrote:
>
>> Status - implemented internally, to be up-streamed or merged to google branch only.
>
> Why would you not consider sending this for trunk at the next stage 1?
>
> (patch review in progress)
>
> Diego.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]