Message ID | 20150116094646.GA51557@msticlxl57.ims.intel.com |
---|---|
State | New |
Headers | show |
On Fri, Jan 16, 2015 at 11:41 AM, Ilya Enkovich <enkovich.gnu@gmail.com> wrote: > On 15 Jan 15:03, Richard Biener wrote: >> On Thu, Jan 15, 2015 at 2:51 PM, Ilya Enkovich <enkovich.gnu@gmail.com> wrote: >> > 2015-01-15 16:07 GMT+03:00 Richard Biener <richard.guenther@gmail.com>: >> >> On Thu, Jan 15, 2015 at 12:46 PM, Ilya Enkovich <enkovich.gnu@gmail.com> wrote: >> >>> Hi, >> >>> >> >>> This patch is to fix https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64363. Patch disables instrumentation for functions we cannot clone correctly due to labels. >> >>> >> >>> Bootstrapped and checked on x86_64-unknown-linux-gnu. OK for trunk? >> >> >> >> { >> >> + clone->remove_callees (); >> >> + clone->remove_all_references (); >> >> >> >> this looks like a spurious change? >> > >> > Seems bogus thunk creation here wasn't triggered before but this test >> > revealed it in addition to labels issue. >> > >> >> >> >> + std::string msg = "function cannot be instrumented: "; >> >> + msg += reason; >> >> + warning (OPT_Wchkp, msg.c_str (), node->decl); >> >> >> >> ick - please don't use std::string. SImply do >> >> >> >> warning (OPT_Wchkp, "function cannot be instrumented: %s", node->decl, reason); >> > >> > reason string has "%q+F" inside and therefore string concatenation is required. >> >> Then use >> >> if (warning (OPT_Wchkp, "function cannot be instrumented")) >> inform (reason); >> >> Richard. >> >> > Thanks, >> > Ilya >> > >> >> > > Here is a version with warning + inform. Thanks for review! Ok. Thanks, Richard. > Ilya > -- > gcc/ > > 2015-01-16 Ilya Enkovich <ilya.enkovich@intel.com> > > PR target/64363 > * ipa-chkp.h (chkp_instrumentable_p): New. > * ipa-chkp.c: Include tree-inline.h. > (chkp_instrumentable_p): New. > (chkp_maybe_create_clone): Use chkp_instrumentable_p. > Fix processing of not instrumentable functions. > (chkp_versioning): Use chkp_instrumentable_p. Warn about > not instrumentable functions. > * tree-chkp.c (chkp_add_bounds_to_call_stmt): Use > chkp_instrumentable_p. > * tree-inline.h (copy_forbidden): New. > * tree-inline.c (copy_forbidden): Not static anymore. > > gcc/testsuite/ > > 2015-01-16 Ilya Enkovich <ilya.enkovich@intel.com> > > PR target/64363 > * gcc.target/i386/chkp-label-address.c: New. > > > diff --git a/gcc/ipa-chkp.c b/gcc/ipa-chkp.c > index 30d511d..8e6612e 100644 > --- a/gcc/ipa-chkp.c > +++ b/gcc/ipa-chkp.c > @@ -50,6 +50,7 @@ along with GCC; see the file COPYING3. If not see > #include "lto-streamer.h" > #include "cgraph.h" > #include "tree-chkp.h" > +#include "tree-inline.h" > #include "ipa-chkp.h" > > /* Pointer Bounds Checker has two IPA passes to support code instrumentation. > @@ -401,6 +402,18 @@ chkp_maybe_clone_builtin_fndecl (tree fndecl) > return clone; > } > > +/* Return 1 if function FNDECL should be instrumented. */ > + > +bool > +chkp_instrumentable_p (tree fndecl) > +{ > + struct function *fn = DECL_STRUCT_FUNCTION (fndecl); > + return (!lookup_attribute ("bnd_legacy", DECL_ATTRIBUTES (fndecl)) > + && (!flag_chkp_instrument_marked_only > + || lookup_attribute ("bnd_instrument", DECL_ATTRIBUTES (fndecl))) > + && (!fn || !copy_forbidden (fn, fndecl))); > +} > + > /* Return clone created for instrumentation of NODE or NULL. */ > > cgraph_node * > @@ -483,10 +496,10 @@ chkp_maybe_create_clone (tree fndecl) > { > /* If function will not be instrumented, then it's instrumented > version is a thunk for the original. */ > - if (lookup_attribute ("bnd_legacy", DECL_ATTRIBUTES (fndecl)) > - || (flag_chkp_instrument_marked_only > - && !lookup_attribute ("bnd_instrument", DECL_ATTRIBUTES (fndecl)))) > + if (!chkp_instrumentable_p (fndecl)) > { > + clone->remove_callees (); > + clone->remove_all_references (); > clone->thunk.thunk_p = true; > clone->thunk.add_pointer_bounds_args = true; > clone->create_edge (node, NULL, 0, CGRAPH_FREQ_BASE); > @@ -532,7 +545,8 @@ chkp_maybe_create_clone (tree fndecl) > > /* Clone all thunks. */ > for (e = node->callers; e; e = e->next_caller) > - if (e->caller->thunk.thunk_p) > + if (e->caller->thunk.thunk_p > + && !e->caller->thunk.add_pointer_bounds_args) > { > struct cgraph_node *thunk > = chkp_maybe_create_clone (e->caller->decl); > @@ -578,6 +592,7 @@ static unsigned int > chkp_versioning (void) > { > struct cgraph_node *node; > + const char *reason; > > bitmap_obstack_initialize (NULL); > > @@ -587,14 +602,20 @@ chkp_versioning (void) > && !node->instrumented_version > && !node->alias > && !node->thunk.thunk_p > - && !lookup_attribute ("bnd_legacy", DECL_ATTRIBUTES (node->decl)) > - && (!flag_chkp_instrument_marked_only > - || lookup_attribute ("bnd_instrument", > - DECL_ATTRIBUTES (node->decl))) > && (!DECL_BUILT_IN (node->decl) > || (DECL_BUILT_IN_CLASS (node->decl) == BUILT_IN_NORMAL > && DECL_FUNCTION_CODE (node->decl) < BEGIN_CHKP_BUILTINS))) > - chkp_maybe_create_clone (node->decl); > + { > + if (chkp_instrumentable_p (node->decl)) > + chkp_maybe_create_clone (node->decl); > + else if ((reason = copy_forbidden (DECL_STRUCT_FUNCTION (node->decl), > + node->decl))) > + { > + if (warning_at (DECL_SOURCE_LOCATION (node->decl), OPT_Wchkp, > + "function cannot be instrumented")) > + inform (DECL_SOURCE_LOCATION (node->decl), reason, node->decl); > + } > + } > } > > /* Mark all aliases and thunks of functions with no instrumented > diff --git a/gcc/ipa-chkp.h b/gcc/ipa-chkp.h > index b087227..6708fe9 100644 > --- a/gcc/ipa-chkp.h > +++ b/gcc/ipa-chkp.h > @@ -23,5 +23,6 @@ along with GCC; see the file COPYING3. If not see > extern tree chkp_copy_function_type_adding_bounds (tree orig_type); > extern tree chkp_maybe_clone_builtin_fndecl (tree fndecl); > extern cgraph_node *chkp_maybe_create_clone (tree fndecl); > +extern bool chkp_instrumentable_p (tree fndecl); > > #endif /* GCC_IPA_CHKP_H */ > diff --git a/gcc/testsuite/gcc.target/i386/chkp-label-address.c b/gcc/testsuite/gcc.target/i386/chkp-label-address.c > new file mode 100644 > index 0000000..05963e2 > --- /dev/null > +++ b/gcc/testsuite/gcc.target/i386/chkp-label-address.c > @@ -0,0 +1,24 @@ > +/* { dg-do compile } */ > +/* { dg-require-effective-target mpx } */ > +/* { dg-options "-fcheck-pointer-bounds -mmpx -O2 -Wchkp" } */ > + > +#include <stdio.h> > + > +static int f1 () /* { dg-warning "function cannot be instrumented" "" } */ > +{ > + static int array = &&label_B - &&label_A; > + > + label_A: > + > + printf ("%d\n", array); > + > + label_B: > + > + return 0; > +} > + > +int f2 (int i) > +{ > + printf ("%d\n", i); > + return f1 (); > +} > diff --git a/gcc/tree-chkp.c b/gcc/tree-chkp.c > index c606040..f2e8c56 100644 > --- a/gcc/tree-chkp.c > +++ b/gcc/tree-chkp.c > @@ -1672,9 +1672,8 @@ chkp_add_bounds_to_call_stmt (gimple_stmt_iterator *gsi) > && DECL_FUNCTION_CODE (fndecl) == BUILT_IN_OBJECT_SIZE) > return; > > - /* Do nothing for calls to legacy functions. */ > - if (fndecl > - && lookup_attribute ("bnd_legacy", DECL_ATTRIBUTES (fndecl))) > + /* Do nothing for calls to not instrumentable functions. */ > + if (fndecl && !chkp_instrumentable_p (fndecl)) > return; > > /* Ignore CHKP_INIT_PTR_BOUNDS, CHKP_NULL_PTR_BOUNDS > diff --git a/gcc/tree-inline.c b/gcc/tree-inline.c > index 4a47fd2..8dbb55f 100644 > --- a/gcc/tree-inline.c > +++ b/gcc/tree-inline.c > @@ -3532,7 +3532,7 @@ has_label_address_in_static_1 (tree *nodep, int *walk_subtrees, void *fnp) > /* Determine if the function can be copied. If so return NULL. If > not return a string describng the reason for failure. */ > > -static const char * > +const char * > copy_forbidden (struct function *fun, tree fndecl) > { > const char *reason = fun->cannot_be_copied_reason; > diff --git a/gcc/tree-inline.h b/gcc/tree-inline.h > index 985d83b..f8b2ebf 100644 > --- a/gcc/tree-inline.h > +++ b/gcc/tree-inline.h > @@ -234,6 +234,7 @@ extern tree remap_type (tree type, copy_body_data *id); > extern gimple_seq copy_gimple_seq_and_replace_locals (gimple_seq seq); > extern bool debug_find_tree (tree, tree); > extern tree copy_fn (tree, tree&, tree&); > +extern const char *copy_forbidden (struct function *fun, tree fndecl); > > /* This is in tree-inline.c since the routine uses > data structures from the inliner. */
diff --git a/gcc/ipa-chkp.c b/gcc/ipa-chkp.c index 30d511d..8e6612e 100644 --- a/gcc/ipa-chkp.c +++ b/gcc/ipa-chkp.c @@ -50,6 +50,7 @@ along with GCC; see the file COPYING3. If not see #include "lto-streamer.h" #include "cgraph.h" #include "tree-chkp.h" +#include "tree-inline.h" #include "ipa-chkp.h" /* Pointer Bounds Checker has two IPA passes to support code instrumentation. @@ -401,6 +402,18 @@ chkp_maybe_clone_builtin_fndecl (tree fndecl) return clone; } +/* Return 1 if function FNDECL should be instrumented. */ + +bool +chkp_instrumentable_p (tree fndecl) +{ + struct function *fn = DECL_STRUCT_FUNCTION (fndecl); + return (!lookup_attribute ("bnd_legacy", DECL_ATTRIBUTES (fndecl)) + && (!flag_chkp_instrument_marked_only + || lookup_attribute ("bnd_instrument", DECL_ATTRIBUTES (fndecl))) + && (!fn || !copy_forbidden (fn, fndecl))); +} + /* Return clone created for instrumentation of NODE or NULL. */ cgraph_node * @@ -483,10 +496,10 @@ chkp_maybe_create_clone (tree fndecl) { /* If function will not be instrumented, then it's instrumented version is a thunk for the original. */ - if (lookup_attribute ("bnd_legacy", DECL_ATTRIBUTES (fndecl)) - || (flag_chkp_instrument_marked_only - && !lookup_attribute ("bnd_instrument", DECL_ATTRIBUTES (fndecl)))) + if (!chkp_instrumentable_p (fndecl)) { + clone->remove_callees (); + clone->remove_all_references (); clone->thunk.thunk_p = true; clone->thunk.add_pointer_bounds_args = true; clone->create_edge (node, NULL, 0, CGRAPH_FREQ_BASE); @@ -532,7 +545,8 @@ chkp_maybe_create_clone (tree fndecl) /* Clone all thunks. */ for (e = node->callers; e; e = e->next_caller) - if (e->caller->thunk.thunk_p) + if (e->caller->thunk.thunk_p + && !e->caller->thunk.add_pointer_bounds_args) { struct cgraph_node *thunk = chkp_maybe_create_clone (e->caller->decl); @@ -578,6 +592,7 @@ static unsigned int chkp_versioning (void) { struct cgraph_node *node; + const char *reason; bitmap_obstack_initialize (NULL); @@ -587,14 +602,20 @@ chkp_versioning (void) && !node->instrumented_version && !node->alias && !node->thunk.thunk_p - && !lookup_attribute ("bnd_legacy", DECL_ATTRIBUTES (node->decl)) - && (!flag_chkp_instrument_marked_only - || lookup_attribute ("bnd_instrument", - DECL_ATTRIBUTES (node->decl))) && (!DECL_BUILT_IN (node->decl) || (DECL_BUILT_IN_CLASS (node->decl) == BUILT_IN_NORMAL && DECL_FUNCTION_CODE (node->decl) < BEGIN_CHKP_BUILTINS))) - chkp_maybe_create_clone (node->decl); + { + if (chkp_instrumentable_p (node->decl)) + chkp_maybe_create_clone (node->decl); + else if ((reason = copy_forbidden (DECL_STRUCT_FUNCTION (node->decl), + node->decl))) + { + if (warning_at (DECL_SOURCE_LOCATION (node->decl), OPT_Wchkp, + "function cannot be instrumented")) + inform (DECL_SOURCE_LOCATION (node->decl), reason, node->decl); + } + } } /* Mark all aliases and thunks of functions with no instrumented diff --git a/gcc/ipa-chkp.h b/gcc/ipa-chkp.h index b087227..6708fe9 100644 --- a/gcc/ipa-chkp.h +++ b/gcc/ipa-chkp.h @@ -23,5 +23,6 @@ along with GCC; see the file COPYING3. If not see extern tree chkp_copy_function_type_adding_bounds (tree orig_type); extern tree chkp_maybe_clone_builtin_fndecl (tree fndecl); extern cgraph_node *chkp_maybe_create_clone (tree fndecl); +extern bool chkp_instrumentable_p (tree fndecl); #endif /* GCC_IPA_CHKP_H */ diff --git a/gcc/testsuite/gcc.target/i386/chkp-label-address.c b/gcc/testsuite/gcc.target/i386/chkp-label-address.c new file mode 100644 index 0000000..05963e2 --- /dev/null +++ b/gcc/testsuite/gcc.target/i386/chkp-label-address.c @@ -0,0 +1,24 @@ +/* { dg-do compile } */ +/* { dg-require-effective-target mpx } */ +/* { dg-options "-fcheck-pointer-bounds -mmpx -O2 -Wchkp" } */ + +#include <stdio.h> + +static int f1 () /* { dg-warning "function cannot be instrumented" "" } */ +{ + static int array = &&label_B - &&label_A; + + label_A: + + printf ("%d\n", array); + + label_B: + + return 0; +} + +int f2 (int i) +{ + printf ("%d\n", i); + return f1 (); +} diff --git a/gcc/tree-chkp.c b/gcc/tree-chkp.c index c606040..f2e8c56 100644 --- a/gcc/tree-chkp.c +++ b/gcc/tree-chkp.c @@ -1672,9 +1672,8 @@ chkp_add_bounds_to_call_stmt (gimple_stmt_iterator *gsi) && DECL_FUNCTION_CODE (fndecl) == BUILT_IN_OBJECT_SIZE) return; - /* Do nothing for calls to legacy functions. */ - if (fndecl - && lookup_attribute ("bnd_legacy", DECL_ATTRIBUTES (fndecl))) + /* Do nothing for calls to not instrumentable functions. */ + if (fndecl && !chkp_instrumentable_p (fndecl)) return; /* Ignore CHKP_INIT_PTR_BOUNDS, CHKP_NULL_PTR_BOUNDS diff --git a/gcc/tree-inline.c b/gcc/tree-inline.c index 4a47fd2..8dbb55f 100644 --- a/gcc/tree-inline.c +++ b/gcc/tree-inline.c @@ -3532,7 +3532,7 @@ has_label_address_in_static_1 (tree *nodep, int *walk_subtrees, void *fnp) /* Determine if the function can be copied. If so return NULL. If not return a string describng the reason for failure. */ -static const char * +const char * copy_forbidden (struct function *fun, tree fndecl) { const char *reason = fun->cannot_be_copied_reason; diff --git a/gcc/tree-inline.h b/gcc/tree-inline.h index 985d83b..f8b2ebf 100644 --- a/gcc/tree-inline.h +++ b/gcc/tree-inline.h @@ -234,6 +234,7 @@ extern tree remap_type (tree type, copy_body_data *id); extern gimple_seq copy_gimple_seq_and_replace_locals (gimple_seq seq); extern bool debug_find_tree (tree, tree); extern tree copy_fn (tree, tree&, tree&); +extern const char *copy_forbidden (struct function *fun, tree fndecl); /* This is in tree-inline.c since the routine uses data structures from the inliner. */