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 <dnovi...@google.com> wrote: > On Wed, Dec 7, 2011 at 20:07, Han Shen <shen...@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.