[PATCH] Add security_sensitive attribute to clean function stack and regs.

[PATCH] Add security_sensitive attribute to clean function stack and regs.

[Marcos Díaz]

[-- Attachment #1: Type: text/plain, Size: 1687 bytes --]

Hi,
   the attached patch adds a new attribute 'security_sensitive' for functions.
The idea was discussed in PR middle-end/69976.
This attribute makes gcc to emit clean up code at the function's epilogue.
This clean-up code cleans the stack used by this function and that isn't
needed anymore. It also cleans used registers. It only works in x86_64.
Please, review the discussion here:
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=69976
since we had some doubts with the implementation.

We also added some test-cases and ran all tests in x86_64.
We think this isn't a bug-fix but a new feature.

Changelog
2016-03-21  Marcos Diaz  <marcos.diaz@tallertechnologies.com>
   Andres Tiraboschi  <andres.tiraboschi@tallertechnologies.com>

PR tree-optimization/69820
* config/i386/i386-protos.h: Add ix86_clear_regs_emit and
ix86_sec_sensitive_attr_p
* config/i386/i386.c: (ix86_sec_sensitive_attr_p): New function
(ix86_using_red_zone): now take into account if the function has the new
attribute.
(is_preserved_reg): New function.
(is_integer_reg): New function.
(is_used_as_ret): New function.
(reg_to_string): New function.
(clear_reg_emit): New function.
(ix86_clear_regs_emit): New function.
(ix86_expand_epilogue): Added code to emit clean up code only when
security_sensitive attribute is set.
(ix86_handle_security_sensitive_attribute): New function.
(ix86_attribute_table): Added new attribute.
* config/i386/i386.md: (UNSPECV_CLRSTACK): New unspecv.
(UNSPECV_CLRREGS): New unspecv.
(return): Conditionally emit cleaning regs code.
(simple_return): Likewise
(clear_regs): New insn.
(clear_stack): New insn.
* doc/extend.texi: Added description for new security_sensitive attribute.

[-- Attachment #2: secSensitive.diff --]
[-- Type: text/plain, Size: 12022 bytes --]

diff --git a/gcc/config/i386/i386-protos.h b/gcc/config/i386/i386-protos.h
index e4652f3..b69aa59 100644
--- a/gcc/config/i386/i386-protos.h
+++ b/gcc/config/i386/i386-protos.h
@@ -32,6 +32,8 @@ extern HOST_WIDE_INT ix86_initial_elimination_offset (int, int);
 extern void ix86_expand_prologue (void);
 extern void ix86_maybe_emit_epilogue_vzeroupper (void);
 extern void ix86_expand_epilogue (int);
+extern void ix86_clear_regs_emit (rtx*);
+extern bool ix86_sec_sensitive_attr_p(void);
 extern void ix86_expand_split_stack_prologue (void);
 
 extern void ix86_output_addr_vec_elt (FILE *, int);
diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c
index 3d8dbc4..7c58d6d 100644
--- a/gcc/config/i386/i386.c
+++ b/gcc/config/i386/i386.c
@@ -3690,12 +3690,19 @@ make_pass_stv (gcc::context *ctxt)
   return new pass_stv (ctxt);
 }
 
+bool ix86_sec_sensitive_attr_p(void)
+{
+  return lookup_attribute ("security_sensitive", DECL_ATTRIBUTES (cfun->decl));
+}
+
+
 /* Return true if a red-zone is in use.  */
 
 static inline bool
 ix86_using_red_zone (void)
 {
-  return TARGET_RED_ZONE && !TARGET_64BIT_MS_ABI;
+  return TARGET_RED_ZONE && !TARGET_64BIT_MS_ABI
+	&& !ix86_sec_sensitive_attr_p();
 }
 \f
 /* Return a string that documents the current -m options.  The caller is
@@ -13325,6 +13332,144 @@ ix86_emit_restore_sse_regs_using_mov (HOST_WIDE_INT cfa_offset,
       }
 }
 
+/* Returns true iff regno is a register that must be preserved in a function.*/
+static inline bool
+is_preserved_reg(const unsigned int regno)
+{
+  return (regno == BX_REG)
+	 || (regno == BP_REG)
+	 || (regno == SP_REG)
+	 || (regno == R12_REG)
+	 || (regno == R13_REG)
+	 || (regno == R14_REG)
+	 || (regno == R15_REG);
+}
+
+/* Returns true iff regno is an integer register.*/
+static inline bool is_integer_reg(unsigned int regno)
+{
+  return (regno <= 7u) ||
+     ((FIRST_REX_INT_REG <= regno) && (regno <= LAST_REX_INT_REG));
+}
+
+/* Returns true iff regno is used to return in the current function.*/
+static inline bool
+is_used_as_ret(const unsigned int reg_number)
+{
+  bool is_ret = ix86_function_value_regno_p(reg_number);
+  if (is_ret)
+    {
+      rtx ret = ix86_function_value
+		(TREE_TYPE (DECL_RESULT (cfun->decl)), cfun->decl, true);
+      if ((REG_P(ret))
+	  && (is_integer_reg(REGNO(ret)))
+	  && (is_integer_reg(reg_number))
+	 )
+	{
+	  if ((GET_MODE(ret) == TImode) || (GET_MODE(ret) == CDImode))
+	    is_ret = (reg_number == AX_REG) || (reg_number == DX_REG);
+	  else
+	    is_ret = (reg_number == AX_REG);
+	}
+      else if ((REG_P(ret)))
+	is_ret = REGNO(ret) == reg_number;
+      else
+	{
+	  // Is parallel
+	  const unsigned int len = XVECLEN(ret, 0);
+	  unsigned int j = 0u;
+	  bool found = false;
+	  while (!found && j < len)
+	    {
+	      const rtx explist = XVECEXP(ret, 0, j);
+	      const rtx ret_reg = XEXP(explist, 0);
+	      found = REGNO(ret_reg) == reg_number;
+	      ++j;
+	    }
+	  is_ret = found;
+	}
+
+    }
+  return is_ret;
+}
+
+// Make this big enough to store any instruction
+#define MAX_CLEAR_STRING_SIZE 50
+
+/* Adds to str in pos position the neame of the register regno.*/
+static size_t
+reg_to_string(const unsigned int regno, size_t pos, char * const str)
+{
+  str[pos++] = '%';
+  str[pos++] = '%';
+  if (regno <= STACK_POINTER_REGNUM)
+    str[pos++] = 'e';
+
+  strncpy(str + pos, reg_names[regno], MAX_CLEAR_STRING_SIZE - pos);
+  pos += strnlen(reg_names[regno], MAX_CLEAR_STRING_SIZE - pos);
+  return pos;
+}
+
+/*Emits an instruction to clear regno register.*/
+static void
+clear_reg_emit(const unsigned int regno, rtx *operands)
+{
+  static char output[MAX_CLEAR_STRING_SIZE];
+  size_t size = 0u;
+  bool emit = true;
+  if (regno <= STACK_POINTER_REGNUM)
+    {
+      static const char clear_ins[] = "xorl\t";
+      size = strnlen(clear_ins, sizeof(clear_ins));
+      strncpy(output, clear_ins, size);
+    }
+  else if ((FIRST_REX_INT_REG <= regno) && (regno <= LAST_REX_INT_REG))
+    {
+      static const char clear_ins[] = "xorq\t";
+      size = strnlen(clear_ins, sizeof(clear_ins));
+      strncpy(output, clear_ins, size);
+    }
+  else if ( ((FIRST_SSE_REG <= regno) && (regno <= LAST_SSE_REG)) ||
+	    ((FIRST_REX_SSE_REG <= regno) && (regno <= LAST_REX_SSE_REG)) )
+    {
+      static const char clear_ins[] = "xorps\t";
+      size = strnlen(clear_ins, sizeof(clear_ins));
+      strncpy(output, clear_ins, size);
+    }
+  else if ((FIRST_MMX_REG <= regno) && (regno <= LAST_MMX_REG))
+    {
+      static const char clear_ins[] = "pxor\t";
+      size = strnlen(clear_ins, sizeof(clear_ins));
+      strncpy(output, clear_ins, size);
+    }
+  else
+    emit = false;
+
+  if (emit)
+    {
+      size = reg_to_string (regno, size, output);
+      gcc_assert (size < MAX_CLEAR_STRING_SIZE - 2);
+      output[size++] = ',';
+      output[size++] = ' ';
+      size = reg_to_string (regno, size, output);
+      gcc_assert (size < MAX_CLEAR_STRING_SIZE);
+      output[size] = '\0';
+      output_asm_insn (output, operands);
+    }
+}
+
+/* Finds which registers can be cleared and emits code to do that.*/
+void ix86_clear_regs_emit (rtx *operands)
+{
+  for (unsigned int j = 0u; j <= LAST_REX_SSE_REG; ++j)
+    {
+      const bool is_ret = is_used_as_ret(j);
+      const bool is_pres = is_preserved_reg(j);
+      if (df_hard_reg_used_p(j) && !is_pres && !is_ret)
+	clear_reg_emit(j, operands);
+    }
+}
+
 /* Restore function stack, frame, and registers.  */
 
 void
@@ -13335,6 +13480,8 @@ ix86_expand_epilogue (int style)
   struct ix86_frame frame;
   bool restore_regs_via_mov;
   bool using_drap;
+  const bool security_sensitive_attribute =
+	ix86_sec_sensitive_attr_p();
 
   ix86_finalize_stack_realign_flags ();
   ix86_compute_frame_layout (&frame);
@@ -13620,6 +13767,17 @@ ix86_expand_epilogue (int style)
   else
     ix86_add_queued_cfa_restore_notes (get_last_insn ());
 
+  /* If "security_sensitive" attribute enabled, emit code to clear the stack.*/
+  if (security_sensitive_attribute)
+    {
+      const int stack_size_to_clear = frame.stack_pointer_offset
+				      - m->fs.sp_offset;
+      const int size_in_words = stack_size_to_clear / UNITS_PER_WORD;
+      if (stack_size_to_clear)
+	emit_insn(gen_clear_stack(GEN_INT(stack_size_to_clear),
+        	  GEN_INT(size_in_words)));
+    }
+
   /* Sibcall epilogues don't want a return instruction.  */
   if (style == 0)
     {
@@ -13627,6 +13785,10 @@ ix86_expand_epilogue (int style)
       return;
     }
 
+  /* We emit cleaning registers code only in non sibcalls*/
+  if (security_sensitive_attribute)
+    emit_insn(gen_clear_regs());
+
   if (crtl->args.pops_args && crtl->args.size)
     {
       rtx popc = GEN_INT (crtl->args.pops_args);
@@ -44532,6 +44694,32 @@ ix86_handle_fndecl_attribute (tree *node, tree name, tree, int,
   return NULL_TREE;
 }
 
+static tree
+ix86_handle_security_sensitive_attribute (tree *node, tree name, tree, int,
+                              bool *no_add_attrs)
+{
+  if (TREE_CODE (*node) != FUNCTION_DECL)
+    {
+      warning (OPT_Wattributes, "%qE attribute only applies to functions",
+               name);
+      *no_add_attrs = true;
+    }
+  else if (!TARGET_64BIT || ix86_cfun_abi () == MS_ABI)
+    {
+      warning (OPT_Wattributes, "%qE attribute only valid for 64bits not MS ABI",
+               name);
+      *no_add_attrs = true;
+    }
+  else
+    {
+      DECL_ATTRIBUTES (*node)
+        = tree_cons (get_identifier ("noinline"),
+                     NULL_TREE, DECL_ATTRIBUTES (*node));
+      DECL_UNINLINABLE (*node) = 1;
+    }
+  return NULL_TREE;
+}
+
 static bool
 ix86_ms_bitfield_layout_p (const_tree record_type)
 {
@@ -48742,6 +48930,8 @@ static const struct attribute_spec ix86_attribute_table[] =
     false },
   { "callee_pop_aggregate_return", 1, 1, false, true, true,
     ix86_handle_callee_pop_aggregate_return, true },
+  { "security_sensitive", 0, 0, true, false, false,
+    ix86_handle_security_sensitive_attribute, false},
   /* End element.  */
   { NULL,        0, 0, false, false, false, NULL, false }
 };
diff --git a/gcc/config/i386/i386.md b/gcc/config/i386/i386.md
index eed43b1..7c2ccc0 100644
--- a/gcc/config/i386/i386.md
+++ b/gcc/config/i386/i386.md
@@ -270,6 +270,10 @@
 
   ;; For RDPKRU and WRPKRU support
   UNSPECV_PKU
+
+  ;; For security_sensitive attribute
+  UNSPECV_CLRSTACK
+  UNSPECV_CLRREGS
 ])
 
 ;; Constants to represent rounding modes in the ROUND instruction
@@ -12341,6 +12345,8 @@
   [(simple_return)]
   "ix86_can_use_return_insn_p ()"
 {
+  if (ix86_sec_sensitive_attr_p())
+    emit_insn(gen_clear_regs());
   if (crtl->args.pops_args)
     {
       rtx popc = GEN_INT (crtl->args.pops_args);
@@ -12361,6 +12367,8 @@
   [(simple_return)]
   "!TARGET_SEH && !ix86_static_chain_on_stack"
 {
+  if (ix86_sec_sensitive_attr_p())
+    emit_insn(gen_clear_regs());
   if (crtl->args.pops_args)
     {
       rtx popc = GEN_INT (crtl->args.pops_args);
@@ -12572,6 +12580,23 @@
   DONE;
 })
 
+(define_insn "clear_regs"
+  [(unspec_volatile [(const_int 0)] UNSPECV_CLRREGS)]
+  "reload_completed"
+  "*
+  {
+    ix86_clear_regs_emit (operands);
+    return \"\";
+  }"
+)
+
+(define_insn "clear_stack"
+  [(unspec_volatile [(match_operand 0 "const_int_operand" "")
+  (match_operand 1 "const_int_operand" "")] UNSPECV_CLRSTACK)]
+  "TARGET_64BIT"
+  "movq\t%%rax, %%rsi\n\txorl\t%%eax, %%eax\n\tmovq\t%%rsp, %%rdi\n\tsubq\t%0, %%rdi\n\tmovl\t%1, %%ecx\n\trep stosq\n\tmovq\t%%rsi, %%rax"
+)
+
 (define_insn_and_split "eh_return_internal"
   [(eh_return)]
   ""
diff --git a/gcc/doc/extend.texi b/gcc/doc/extend.texi
index 8fddb34..73d9f2d 100644
--- a/gcc/doc/extend.texi
+++ b/gcc/doc/extend.texi
@@ -5216,6 +5216,14 @@ this function attribute to make GCC generate the ``hot-patching'' function
 prologue used in Win32 API functions in Microsoft Windows XP Service Pack 2
 and newer.
 
+@item security_sensitive
+@cindex @code{security_sensitive} function attribute, x86
+
+On x86_64 targets, you can use this function attribute to make GCC generate
+stack and registers cleaning code, after the execution of this function.
+Note that this will enable the attribute @code{noinline}, and will make the
+function not to use the red zone.
+
 @item regparm (@var{number})
 @cindex @code{regparm} function attribute, x86
 @cindex functions that are passed arguments in registers on x86-32
diff --git a/gcc/testsuite/gcc.target/i386/security_sensitive_attr-asm.c b/gcc/testsuite/gcc.target/i386/security_sensitive_attr-asm.c
new file mode 100644
index 0000000..b54374a
--- /dev/null
+++ b/gcc/testsuite/gcc.target/i386/security_sensitive_attr-asm.c
@@ -0,0 +1,14 @@
+/* { dg-do compile { target { ! ia32 } } } */
+/* { dg-options "-O2" } */
+/* { dg-final { scan-assembler-times "rep stosq" 1 } }*/
+/* { dg-final { scan-assembler-times "xorl\\t%esi, %esi"  1 } } */
+
+void read_secret(char* buf, unsigned int size);
+int
+  __attribute__((security_sensitive))
+check_password (void)
+{
+  char buf[16];
+  read_secret (buf, 16);
+  return 1;
+}
\ No newline at end of file
diff --git a/gcc/testsuite/gcc.target/i386/security_sensitive_attr.c b/gcc/testsuite/gcc.target/i386/security_sensitive_attr.c
new file mode 100644
index 0000000..3fadd14
--- /dev/null
+++ b/gcc/testsuite/gcc.target/i386/security_sensitive_attr.c
@@ -0,0 +1,39 @@
+/* { dg-do run { target { ! ia32 } } } */
+/* { dg-options "-O2" } */
+
+#include <string.h>
+
+void __attribute__((noinline)) read_secret (char *out, size_t size)
+{
+  strncpy (out, "secret password", size);
+}
+
+int
+  __attribute__((security_sensitive))
+check_password (void)
+{
+  char buf[16];
+  read_secret (buf, 16);
+  return 1;
+}
+
+/* Attempt to access the secret still on the stack.  */
+
+int
+  __attribute__((noinline))
+steal_password (void)
+{
+  char buf[16];
+  if (strstr(buf, "password"))
+    return 1;
+  return 0;
+}
+
+int main (int argc, const char **argv)
+{
+  if (!check_password ())
+    return 1;
+
+  if (steal_password ())
+    return 2;
+}

Re: [PATCH] Add security_sensitive attribute to clean function stack and regs.

[Marcos Díaz]

(Ping and changelog fix)

On Tue, Mar 22, 2016 at 11:15 AM, Marcos Díaz
<marcos.diaz@tallertechnologies.com> wrote:
> Hi,
>    the attached patch adds a new attribute 'security_sensitive' for functions.
> The idea was discussed in PR middle-end/69976.
> This attribute makes gcc to emit clean up code at the function's epilogue.
> This clean-up code cleans the stack used by this function and that isn't
> needed anymore. It also cleans used registers. It only works in x86_64.
> Please, review the discussion here:
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=69976
> since we had some doubts with the implementation.
>
> We also added some test-cases and ran all tests in x86_64.
> We think this isn't a bug-fix but a new feature.
>
> Changelog
> 2016-03-21  Marcos Diaz  <marcos.diaz@tallertechnologies.com>
>    Andres Tiraboschi  <andres.tiraboschi@tallertechnologies.com>
>
> PR tree-optimization/69820
This line should've been  PR middle-end/69976
> * config/i386/i386-protos.h: Add ix86_clear_regs_emit and
> ix86_sec_sensitive_attr_p
> * config/i386/i386.c: (ix86_sec_sensitive_attr_p): New function
> (ix86_using_red_zone): now take into account if the function has the new
> attribute.
> (is_preserved_reg): New function.
> (is_integer_reg): New function.
> (is_used_as_ret): New function.
> (reg_to_string): New function.
> (clear_reg_emit): New function.
> (ix86_clear_regs_emit): New function.
> (ix86_expand_epilogue): Added code to emit clean up code only when
> security_sensitive attribute is set.
> (ix86_handle_security_sensitive_attribute): New function.
> (ix86_attribute_table): Added new attribute.
> * config/i386/i386.md: (UNSPECV_CLRSTACK): New unspecv.
> (UNSPECV_CLRREGS): New unspecv.
> (return): Conditionally emit cleaning regs code.
> (simple_return): Likewise
> (clear_regs): New insn.
> (clear_stack): New insn.
> * doc/extend.texi: Added description for new security_sensitive attribute.

Changelog
2016-03-21  Marcos Diaz  <marcos.diaz@tallertechnologies.com>
   Andres Tiraboschi  <andres.tiraboschi@tallertechnologies.com>

PR middle-end/69976
* config/i386/i386-protos.h: Add ix86_clear_regs_emit and
ix86_sec_sensitive_attr_p
* config/i386/i386.c: (ix86_sec_sensitive_attr_p): New function
(ix86_using_red_zone): now take into account if the function has the new
attribute.
(is_preserved_reg): New function.
(is_integer_reg): New function.
(is_used_as_ret): New function.
(reg_to_string): New function.
(clear_reg_emit): New function.
(ix86_clear_regs_emit): New function.
(ix86_expand_epilogue): Added code to emit clean up code only when
security_sensitive attribute is set.
(ix86_handle_security_sensitive_attribute): New function.
(ix86_attribute_table): Added new attribute.
* config/i386/i386.md: (UNSPECV_CLRSTACK): New unspecv.
(UNSPECV_CLRREGS): New unspecv.
(return): Conditionally emit cleaning regs code.
(simple_return): Likewise
(clear_regs): New insn.
(clear_stack): New insn.
* doc/extend.texi: Added description for new security_sensitive attribute.

Re: [PATCH] Add security_sensitive attribute to clean function stack and regs.

[Marcos Díaz]

On Mon, Mar 28, 2016 at 3:05 PM, Marcos Díaz
<marcos.diaz@tallertechnologies.com> wrote:
> (Ping and changelog fix)
>
> On Tue, Mar 22, 2016 at 11:15 AM, Marcos Díaz
> <marcos.diaz@tallertechnologies.com> wrote:
>> Hi,
>>    the attached patch adds a new attribute 'security_sensitive' for functions.
>> The idea was discussed in PR middle-end/69976.
>> This attribute makes gcc to emit clean up code at the function's epilogue.
>> This clean-up code cleans the stack used by this function and that isn't
>> needed anymore. It also cleans used registers. It only works in x86_64.
>> Please, review the discussion here:
>> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=69976
>> since we had some doubts with the implementation.
>>
>> We also added some test-cases and ran all tests in x86_64.
>> We think this isn't a bug-fix but a new feature.
>>
>> Changelog
>> 2016-03-21  Marcos Diaz  <marcos.diaz@tallertechnologies.com>
>>    Andres Tiraboschi  <andres.tiraboschi@tallertechnologies.com>
>>
>> PR tree-optimization/69820
> This line should've been  PR middle-end/69976
>> * config/i386/i386-protos.h: Add ix86_clear_regs_emit and
>> ix86_sec_sensitive_attr_p
>> * config/i386/i386.c: (ix86_sec_sensitive_attr_p): New function
>> (ix86_using_red_zone): now take into account if the function has the new
>> attribute.
>> (is_preserved_reg): New function.
>> (is_integer_reg): New function.
>> (is_used_as_ret): New function.
>> (reg_to_string): New function.
>> (clear_reg_emit): New function.
>> (ix86_clear_regs_emit): New function.
>> (ix86_expand_epilogue): Added code to emit clean up code only when
>> security_sensitive attribute is set.
>> (ix86_handle_security_sensitive_attribute): New function.
>> (ix86_attribute_table): Added new attribute.
>> * config/i386/i386.md: (UNSPECV_CLRSTACK): New unspecv.
>> (UNSPECV_CLRREGS): New unspecv.
>> (return): Conditionally emit cleaning regs code.
>> (simple_return): Likewise
>> (clear_regs): New insn.
>> (clear_stack): New insn.
>> * doc/extend.texi: Added description for new security_sensitive attribute.
>
> Changelog
> 2016-03-21  Marcos Diaz  <marcos.diaz@tallertechnologies.com>
>    Andres Tiraboschi  <andres.tiraboschi@tallertechnologies.com>
>
> PR middle-end/69976
> * config/i386/i386-protos.h: Add ix86_clear_regs_emit and
> ix86_sec_sensitive_attr_p
> * config/i386/i386.c: (ix86_sec_sensitive_attr_p): New function
> (ix86_using_red_zone): now take into account if the function has the new
> attribute.
> (is_preserved_reg): New function.
> (is_integer_reg): New function.
> (is_used_as_ret): New function.
> (reg_to_string): New function.
> (clear_reg_emit): New function.
> (ix86_clear_regs_emit): New function.
> (ix86_expand_epilogue): Added code to emit clean up code only when
> security_sensitive attribute is set.
> (ix86_handle_security_sensitive_attribute): New function.
> (ix86_attribute_table): Added new attribute.
> * config/i386/i386.md: (UNSPECV_CLRSTACK): New unspecv.
> (UNSPECV_CLRREGS): New unspecv.
> (return): Conditionally emit cleaning regs code.
> (simple_return): Likewise
> (clear_regs): New insn.
> (clear_stack): New insn.
> * doc/extend.texi: Added description for new security_sensitive attribute.

ping^2

Re: [PATCH] Add security_sensitive attribute to clean function stack and regs.

[Bernd Schmidt]

On 03/22/2016 03:15 PM, Marcos Díaz wrote:
>     the attached patch adds a new attribute 'security_sensitive' for functions.
> The idea was discussed in PR middle-end/69976.

The first question would be: I see several submissions from folks 
@tallertechnologies.com. Do you and your employer have copyright 
assignments on file with the FSF?

The patch violates gcc coding guidelines in many ways. I'll point out 
some issues, but in general you are expected to familiarize yourself 
with the requirements first. Please review the entire patch yourself for 
such issues.

> +bool ix86_sec_sensitive_attr_p(void)

Break line after return types.
>   {
> -  return TARGET_RED_ZONE && !TARGET_64BIT_MS_ABI;
> +  return TARGET_RED_ZONE && !TARGET_64BIT_MS_ABI
> +	&& !ix86_sec_sensitive_attr_p();

Wrap multi-line expressions in parentheses to get correct formatting 
from the editor.

> +  return (regno == BX_REG)
> +	 || (regno == BP_REG)
> +	 || (regno == SP_REG)
> +	 || (regno == R12_REG)
> +	 || (regno == R13_REG)
> +	 || (regno == R14_REG)
> +	 || (regno == R15_REG);

Same here but also remove unnecessary parentheses (in many places in 
this patch).

> +}
> +
> +/* Returns true iff regno is an integer register.*/
> +static inline bool is_integer_reg(unsigned int regno)

Space before open parens.

> +      rtx ret = ix86_function_value
> +		(TREE_TYPE (DECL_RESULT (cfun->decl)), cfun->decl, true);

Probably better to split this line after one of the commas.

> +	  // Is parallel

Use C style comments consistently.
> +/* Adds to str in pos position the neame of the register regno.*/

Always two spaces after a period, including at the end of a comment.

In general it would be ideal if this functionality was not x86 specific. 
I could imagine that thread_prologue_and_epilogue_insns could do this in 
a machine-independent fashion.


Bernd