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

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

[xur]

OK for google branches.


http://codereview.appspot.com/5461043/diff/16001/gcc/cfgexpand.c
File gcc/cfgexpand.c (right):

http://codereview.appspot.com/5461043/diff/16001/gcc/cfgexpand.c#newcode1597
gcc/cfgexpand.c:1597: contain an array or are arrays. */
"," before "or", and
two spaces before "*/".

http://codereview.appspot.com/5461043/diff/16001/gcc/cfgexpand.c#newcode1709
gcc/cfgexpand.c:1709: create_stack_guard ();
it's better to merge to the earlier if statement. something like:
   if (flag_stack_protect == 2
       || (flag_stack_protect == 3 && gen_stack_protect_signal)
       || (flag_stack_protect
  	  && (cfun->calls_alloca || has_protected_decls)))
      create_stack_guard ();

http://codereview.appspot.com/5461043/

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

[Han Shen(沈涵)]

Hi, ping?

Could someone take a look at this patch, it has already been reviewed
several rounds. I'm to submit it to gcc trunk.

Thanks,
-Han

On Wed, Jan 25, 2012 at 9:41 PM, <davidxl@google.com> wrote:
>
> ok for google branches with the above changes. Please continue to seek
> upstream approval.
>
> David
>
>
> http://codereview.appspot.com/5461043/diff/19001/gcc/doc/invoke.texi
> File gcc/doc/invoke.texi (right):
>
> http://codereview.appspot.com/5461043/diff/19001/gcc/doc/invoke.texi#newcode403
> gcc/doc/invoke.texi:403: +-fstack-protector-strong -fstack-protector-all
> -fstrict-aliasing @gol
> Switch the order of -fstack-protector-all and -fstack-proctor-strong (in
> alphabetic order)
>
> http://codereview.appspot.com/5461043/diff/19001/gcc/doc/invoke.texi#newcode8446
> gcc/doc/invoke.texi:8446: +@item -fstack-protector-strong
> Move this item after -fstack-protector-all
>
> http://codereview.appspot.com/5461043/

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

[davidxl]

ok for google branches with the above changes. Please continue to seek
upstream approval.

David


http://codereview.appspot.com/5461043/diff/19001/gcc/doc/invoke.texi
File gcc/doc/invoke.texi (right):

http://codereview.appspot.com/5461043/diff/19001/gcc/doc/invoke.texi#newcode403
gcc/doc/invoke.texi:403: +-fstack-protector-strong -fstack-protector-all
-fstrict-aliasing @gol
Switch the order of -fstack-protector-all and -fstack-proctor-strong (in
alphabetic order)

http://codereview.appspot.com/5461043/diff/19001/gcc/doc/invoke.texi#newcode8446
gcc/doc/invoke.texi:8446: +@item -fstack-protector-strong
Move this item after -fstack-protector-all

http://codereview.appspot.com/5461043/

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

[Han Shen(沈涵)]

Hi, David and Rong, thanks a lot! Modified code uploaded as patch 8
and is also included at the end of email body.

Ref - http://codereview.appspot.com/5461043

Regards,
-Han

====== Patch start
diff --git a/gcc/cfgexpand.c b/gcc/cfgexpand.c
index 6d31e90..131c1b9 100644
--- a/gcc/cfgexpand.c
+++ b/gcc/cfgexpand.c
@@ -1524,15 +1524,39 @@ 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_p (const_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_p (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.  */
   {
@@ -1565,6 +1589,23 @@ expand_used_vars (void)
 	}
     }

+  FOR_EACH_REFERENCED_VAR (cfun, var, rvi)
+    if (!is_global_var (var))
+      {
+	tree var_type = TREE_TYPE (var);
+	/* Examine local referenced variables that have their addresses taken,
+	   contain an array, or are arrays.  */
+	if (TREE_CODE (var) == VAR_DECL
+	    && (TREE_CODE (var_type) == ARRAY_TYPE
+		|| TREE_ADDRESSABLE (var)
+		|| (RECORD_OR_UNION_TYPE_P (var_type)
+		    && record_or_union_type_has_array_p (var_type))))
+	  {
+	    ++gen_stack_protect_signal;
+	    break;
+	  }
+      }
+
   /* At this point all variables on the local_decls with TREE_USED
      set are not associated with any block scope.  Lay them out.  */

@@ -1655,11 +1696,18 @@ expand_used_vars (void)
 	dump_stack_var_partition ();
     }

-  /* 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)))
+  /* Create stack guard, if
+     a) "-fstack-protector-all" - always;
+     b) "-fstack-protector-strong" - if there are arrays, memory
+     references to local variables, alloca used, or protected decls present;
+     c) "-fstack-protector" - if alloca used, or protected decls present  */
+  if (flag_stack_protect == 3  /* -fstack-protector-all  */
+      || (flag_stack_protect == 2  /* -fstack-protector-strong  */
+	  && (gen_stack_protect_signal || cfun->calls_alloca
+	      || has_protected_decls))
+      || (flag_stack_protector == 1  /* -fstack-protector  */
+	  && (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 ec1dbd1..b79b8cc 100644
--- a/gcc/common.opt
+++ b/gcc/common.opt
@@ -1835,8 +1835,12 @@ fstack-protector
 Common Report Var(flag_stack_protect, 1)
 Use propolice as a stack protection method

-fstack-protector-all
+fstack-protector-strong
 Common Report RejectNegative Var(flag_stack_protect, 2)
+Use a smart stack protection method for certain functions
+
+fstack-protector-all
+Common Report RejectNegative Var(flag_stack_protect, 3)
 Use a stack protection method for every function

 fstack-usage
diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
index e3d3789..607a7a5 100644
--- a/gcc/doc/invoke.texi
+++ b/gcc/doc/invoke.texi
@@ -400,8 +400,8 @@ Objective-C and Objective-C++ Dialects}.
 -fsel-sched-pipelining -fsel-sched-pipelining-outer-loops @gol
 -fshrink-wrap -fsignaling-nans -fsingle-precision-constant @gol
 -fsplit-ivs-in-unroller -fsplit-wide-types -fstack-protector @gol
--fstack-protector-all -fstrict-aliasing -fstrict-overflow @gol
--fthread-jumps -ftracer -ftree-bit-ccp @gol
+-fstack-protector-strong -fstack-protector-all -fstrict-aliasing @gol
+-fstrict-overflow -fthread-jumps -ftracer -ftree-bit-ccp @gol
 -ftree-builtin-call-dce -ftree-ccp -ftree-ch -ftree-copy-prop @gol
 -ftree-copyrename -ftree-dce -ftree-dominator-opts -ftree-dse @gol
 -ftree-forwprop -ftree-fre -ftree-loop-if-convert @gol
@@ -8443,6 +8443,12 @@ functions with buffers larger than 8 bytes.
The guards are initialized
 when a function is entered and then checked when the function exits.
 If a guard check fails, an error message is printed and the program exits.

+@item -fstack-protector-strong
+@opindex fstack-protector-strong
+Like @option{-fstack-protector} but includes additional functions to be
+protected - those that have local array definitions, or have references to
+local frame addresses.
+
 @item -fstack-protector-all
 @opindex fstack-protector-all
 Like @option{-fstack-protector} except that all functions are protected.
diff --git a/gcc/testsuite/g++.dg/fstack-protector-strong.C
b/gcc/testsuite/g++.dg/fstack-protector-strong.C
new file mode 100644
index 0000000..a4f0f81
--- /dev/null
+++ b/gcc/testsuite/g++.dg/fstack-protector-strong.C
@@ -0,0 +1,35 @@
+/* Test that stack protection is done on chosen functions. */
+
+/* { dg-do compile { target i?86-*-* x86_64-*-* } } */
+/* { dg-options "-O2 -fstack-protector-strong" } */
+
+class A
+{
+public:
+  A() {}
+  ~A() {}
+  void method();
+  int state;
+};
+
+/* Frame address exposed to A::method via "this". */
+int
+foo1 ()
+{
+  A a;
+  a.method ();
+  return a.state;
+}
+
+/* Possible destroying foo2's stack via &a. */
+int
+global_func (A& a);
+
+/* Frame address exposed to global_func. */
+int foo2 ()
+{
+  A a;
+  return global_func (a);
+}
+
+/* { dg-final { scan-assembler-times "stack_chk_fail" 2 } } */
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..5a5cf98
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/fstack-protector-strong.c
@@ -0,0 +1,135 @@
+/* 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));
+}
+
+int
+global2 (struct BB* pbb);
+
+/* Address taken on struct. */
+int
+foo10 ()
+{
+  struct BB bb;
+  int i;
+  bb.one = global2 (&bb);
+  for (i = 0; i < 10; ++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 } } */

===== patch end


On Tue, Jan 24, 2012 at 2:16 PM,  <davidxl@google.com> wrote:
>
> Also need to update doc/invoke.texi file for the new option.
>
>
>
>
>
> http://codereview.appspot.com/5461043/diff/16001/gcc/cfgexpand.c
> File gcc/cfgexpand.c (right):
>
> http://codereview.appspot.com/5461043/diff/16001/gcc/cfgexpand.c#newcode1531
> gcc/cfgexpand.c:1531: record_or_union_type_has_array (const_tree
> tree_type)
> Better add '_p' suffix to the predicate function name.
>
> http://codereview.appspot.com/5461043/diff/16001/gcc/cfgexpand.c#newcode1535
> gcc/cfgexpand.c:1535: for (f = fields; f; f = DECL_CHAIN (f))
> Add an empty line after declarations.
>
> http://codereview.appspot.com/5461043/diff/16001/gcc/cfgexpand.c#newcode1702diff --git a/gcc/cfgexpand.c b/gcc/cfgexpand.c
index 6d31e90..131c1b9 100644
--- a/gcc/cfgexpand.c
+++ b/gcc/cfgexpand.c
@@ -1524,15 +1524,39 @@ 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_p (const_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_p (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.  */
   {
@@ -1565,6 +1589,23 @@ expand_used_vars (void)
 	}
     }

+  FOR_EACH_REFERENCED_VAR (cfun, var, rvi)
+    if (!is_global_var (var))
+      {
+	tree var_type = TREE_TYPE (var);
+	/* Examine local referenced variables that have their addresses taken,
+	   contain an array, or are arrays.  */
+	if (TREE_CODE (var) == VAR_DECL
+	    && (TREE_CODE (var_type) == ARRAY_TYPE
+		|| TREE_ADDRESSABLE (var)
+		|| (RECORD_OR_UNION_TYPE_P (var_type)
+		    && record_or_union_type_has_array_p (var_type))))
+	  {
+	    ++gen_stack_protect_signal;
+	    break;
+	  }
+      }
+
   /* At this point all variables on the local_decls with TREE_USED
      set are not associated with any block scope.  Lay them out.  */

@@ -1655,11 +1696,18 @@ expand_used_vars (void)
 	dump_stack_var_partition ();
     }

-  /* 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)))
+  /* Create stack guard, if
+     a) "-fstack-protector-all" - always;
+     b) "-fstack-protector-strong" - if there are arrays, memory
+     references to local variables, alloca used, or protected decls present;
+     c) "-fstack-protector" - if alloca used, or protected decls present  */
+  if (flag_stack_protect == 3  /* -fstack-protector-all  */
+      || (flag_stack_protect == 2  /* -fstack-protector-strong  */
+	  && (gen_stack_protect_signal || cfun->calls_alloca
+	      || has_protected_decls))
+      || (flag_stack_protector == 1  /* -fstack-protector  */
+	  && (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 ec1dbd1..b79b8cc 100644
--- a/gcc/common.opt
+++ b/gcc/common.opt
@@ -1835,8 +1835,12 @@ fstack-protector
 Common Report Var(flag_stack_protect, 1)
 Use propolice as a stack protection method

-fstack-protector-all
+fstack-protector-strong
 Common Report RejectNegative Var(flag_stack_protect, 2)
+Use a smart stack protection method for certain functions
+
+fstack-protector-all
+Common Report RejectNegative Var(flag_stack_protect, 3)
 Use a stack protection method for every function

 fstack-usage
diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
index e3d3789..607a7a5 100644
--- a/gcc/doc/invoke.texi
+++ b/gcc/doc/invoke.texi
@@ -400,8 +400,8 @@ Objective-C and Objective-C++ Dialects}.
 -fsel-sched-pipelining -fsel-sched-pipelining-outer-loops @gol
 -fshrink-wrap -fsignaling-nans -fsingle-precision-constant @gol
 -fsplit-ivs-in-unroller -fsplit-wide-types -fstack-protector @gol
--fstack-protector-all -fstrict-aliasing -fstrict-overflow @gol
--fthread-jumps -ftracer -ftree-bit-ccp @gol
+-fstack-protector-strong -fstack-protector-all -fstrict-aliasing @gol
+-fstrict-overflow -fthread-jumps -ftracer -ftree-bit-ccp @gol
 -ftree-builtin-call-dce -ftree-ccp -ftree-ch -ftree-copy-prop @gol
 -ftree-copyrename -ftree-dce -ftree-dominator-opts -ftree-dse @gol
 -ftree-forwprop -ftree-fre -ftree-loop-if-convert @gol
@@ -8443,6 +8443,12 @@ functions with buffers larger than 8 bytes.
The guards are initialized
 when a function is entered and then checked when the function exits.
 If a guard check fails, an error message is printed and the program exits.

+@item -fstack-protector-strong
+@opindex fstack-protector-strong
+Like @option{-fstack-protector} but includes additional functions to be
+protected - those that have local array definitions, or have references to
+local frame addresses.
+
 @item -fstack-protector-all
 @opindex fstack-protector-all
 Like @option{-fstack-protector} except that all functions are protected.
diff --git a/gcc/testsuite/g++.dg/fstack-protector-strong.C
b/gcc/testsuite/g++.dg/fstack-protector-strong.C
new file mode 100644
index 0000000..a4f0f81
--- /dev/null
+++ b/gcc/testsuite/g++.dg/fstack-protector-strong.C
@@ -0,0 +1,35 @@
+/* Test that stack protection is done on chosen functions. */
+
+/* { dg-do compile { target i?86-*-* x86_64-*-* } } */
+/* { dg-options "-O2 -fstack-protector-strong" } */
+
+class A
+{
+public:
+  A() {}
+  ~A() {}
+  void method();
+  int state;
+};
+
+/* Frame address exposed to A::method via "this". */
+int
+foo1 ()
+{
+  A a;
+  a.method ();
+  return a.state;
+}
+
+/* Possible destroying foo2's stack via &a. */
+int
+global_func (A& a);
+
+/* Frame address exposed to global_func. */
+int foo2 ()
+{
+  A a;
+  return global_func (a);
+}
+
+/* { dg-final { scan-assembler-times "stack_chk_fail" 2 } } */
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..5a5cf98
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/fstack-protector-strong.c
@@ -0,0 +1,135 @@
+/* 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));
+}
+
+int
+global2 (struct BB* pbb);
+
+/* Address taken on struct. */
+int
+foo10 ()
+{
+  struct BB bb;
+  int i;
+  bb.one = global2 (&bb);
+  for (i = 0; i < 10; ++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 } } */

> gcc/cfgexpand.c:1702: if (flag_stack_protect == 2
> Add more descriptions. Better yet, fix the flag value mapping --
> protect_all-> 3, protect --> 2, and protect_strong-->1
>
> http://codereview.appspot.com/5461043/

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

[davidxl]

Also need to update doc/invoke.texi file for the new option.




http://codereview.appspot.com/5461043/diff/16001/gcc/cfgexpand.c
File gcc/cfgexpand.c (right):

http://codereview.appspot.com/5461043/diff/16001/gcc/cfgexpand.c#newcode1531
gcc/cfgexpand.c:1531: record_or_union_type_has_array (const_tree
tree_type)
Better add '_p' suffix to the predicate function name.

http://codereview.appspot.com/5461043/diff/16001/gcc/cfgexpand.c#newcode1535
gcc/cfgexpand.c:1535: for (f = fields; f; f = DECL_CHAIN (f))
Add an empty line after declarations.

http://codereview.appspot.com/5461043/diff/16001/gcc/cfgexpand.c#newcode1702
gcc/cfgexpand.c:1702: if (flag_stack_protect == 2
Add more descriptions. Better yet, fix the flag value mapping --
protect_all-> 3, protect --> 2, and protect_strong-->1

http://codereview.appspot.com/5461043/