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.d...@tallertechnologies.com>
Andres Tiraboschi <andres.tirabos...@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.
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();
}
/* 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;
+}
