On Thu, Jan 15, 2015 at 12:46 PM, Ilya Enkovich <enkovich....@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? + 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); Otherwise ok. Thanks, Richard. > Thanks, > Ilya > -- > gcc/ > > 2015-01-14 Ilya Enkovich <ilya.enkov...@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-14 Ilya Enkovich <ilya.enkov...@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..4fb60f0 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))) > + { > + std::string msg = "function cannot be instrumented: "; > + msg += reason; > + warning (OPT_Wchkp, msg.c_str (), 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. */