On 10 Apr 03:27, Jan Hubicka wrote:
> >
> > + /* We might propagate instrumented function pointer into
> > + not instrumented function and vice versa. In such a
> > + case we need to either fix function declaration or
> > + remove bounds from call statement. */
> > + if (flag_check_pointer_bounds && callee)
> > + skip_bounds = chkp_redirect_edge (e);
>
> I think this gets wrong the case where the edge is speculative and the new
> direct call needs adjustement. You probably need to do the right think in
> the if (e->speculative) branch so direct call is updated by indirect is not
> or at least give an explanation why this is not needed :)
>
> The speculative edge handling works in a way that the runtime conditoinal is
> built and then the edge is updated to the direct path, so perhaps
> you can just move all this after the ocnditoinal?
I think you are right, it should be OK to move it after apeculative call
processing.
>
> > diff --git a/gcc/lto-wrapper.c b/gcc/lto-wrapper.c
> > index 404cb68..ffb6ad7 100644
> > --- a/gcc/lto-wrapper.c
> > +++ b/gcc/lto-wrapper.c
> > @@ -273,6 +273,7 @@ merge_and_complain (struct cl_decoded_option
> > **decoded_options,
> > case OPT_fwrapv:
> > case OPT_fopenmp:
> > case OPT_fopenacc:
> > + case OPT_fcheck_pointer_bounds:
> > /* For selected options we can merge conservatively. */
> > for (j = 0; j < *decoded_options_count; ++j)
> > if ((*decoded_options)[j].opt_index == foption->opt_index)
> > @@ -503,6 +504,7 @@ append_compiler_options (obstack *argv_obstack, struct
> > cl_decoded_option *opts,
> > case OPT_Ofast:
> > case OPT_Og:
> > case OPT_Os:
> > + case OPT_fcheck_pointer_bounds:
>
> Hmm, will this make mixed units contaiing some check bounds and some
> non-check bounds to work?
> Perhaps these should be function specific? Does things like inlining bounds
> checked function into
> non-checked function work?
This actually should make it work better because solves a possible problem with
uninitialized static bounds data (chkp static constructors are generated only
when OPT_fcheck_pointer_bounds is passed).
Inlining of instrumentation thunks is not supported (similar to all other
thunks). But we may have a not instrumented call in an instrumented function
and do inlining for it.
>
> Otherwise the patch seems resonable.
> Honza
Here is a fixed version with chkp redirection moved. Bootstrap and testing
passed. Is it OK for trunk and later for GCC 5?
Thanks,
Ilya
--
gcc/
2015-04-14 Ilya Enkovich <ilya.enkov...@intel.com>
PR target/65527
* cgraph.c (cgraph_edge::redirect_call_stmt_to_callee): Add
redirection for instrumented calls.
* lto-wrapper.c (merge_and_complain): Merge -fcheck-pointer-bounds.
(append_compiler_options): Append -fcheck-pointer-bounds.
* tree-chkp.h (chkp_copy_call_skip_bounds): New.
(chkp_redirect_edge): New.
* tree-chkp.c (chkp_copy_call_skip_bounds): New.
(chkp_redirect_edge): New.
gcc/testsuite/
2015-04-14 Ilya Enkovich <ilya.enkov...@intel.com>
PR target/65527
* gcc.target/i386/mpx/chkp-fix-calls-1.c: New.
* gcc.target/i386/mpx/chkp-fix-calls-2.c: New.
* gcc.target/i386/mpx/chkp-fix-calls-3.c: New.
* gcc.target/i386/mpx/chkp-fix-calls-4.c: New.
diff --git a/gcc/cgraph.c b/gcc/cgraph.c
index 85531c8..38e71fc 100644
--- a/gcc/cgraph.c
+++ b/gcc/cgraph.c
@@ -1281,6 +1281,7 @@ cgraph_edge::redirect_call_stmt_to_callee (void)
tree lhs = gimple_call_lhs (e->call_stmt);
gcall *new_stmt;
gimple_stmt_iterator gsi;
+ bool skip_bounds = false;
#ifdef ENABLE_CHECKING
cgraph_node *node;
#endif
@@ -1389,8 +1390,16 @@ cgraph_edge::redirect_call_stmt_to_callee (void)
}
}
+ /* We might propagate instrumented function pointer into
+ not instrumented function and vice versa. In such a
+ case we need to either fix function declaration or
+ remove bounds from call statement. */
+ if (flag_check_pointer_bounds && e->callee)
+ skip_bounds = chkp_redirect_edge (e);
+
if (e->indirect_unknown_callee
- || decl == e->callee->decl)
+ || (decl == e->callee->decl
+ && !skip_bounds))
return e->call_stmt;
#ifdef ENABLE_CHECKING
@@ -1415,13 +1424,19 @@ cgraph_edge::redirect_call_stmt_to_callee (void)
}
}
- if (e->callee->clone.combined_args_to_skip)
+ if (e->callee->clone.combined_args_to_skip
+ || skip_bounds)
{
int lp_nr;
- new_stmt
- = gimple_call_copy_skip_args (e->call_stmt,
- e->callee->clone.combined_args_to_skip);
+ new_stmt = e->call_stmt;
+ if (e->callee->clone.combined_args_to_skip)
+ new_stmt
+ = gimple_call_copy_skip_args (new_stmt,
+ e->callee->clone.combined_args_to_skip);
+ if (skip_bounds)
+ new_stmt = chkp_copy_call_skip_bounds (new_stmt);
+
gimple_call_set_fndecl (new_stmt, e->callee->decl);
gimple_call_set_fntype (new_stmt, gimple_call_fntype (e->call_stmt));
diff --git a/gcc/lto-wrapper.c b/gcc/lto-wrapper.c
index 404cb68..ffb6ad7 100644
--- a/gcc/lto-wrapper.c
+++ b/gcc/lto-wrapper.c
@@ -273,6 +273,7 @@ merge_and_complain (struct cl_decoded_option
**decoded_options,
case OPT_fwrapv:
case OPT_fopenmp:
case OPT_fopenacc:
+ case OPT_fcheck_pointer_bounds:
/* For selected options we can merge conservatively. */
for (j = 0; j < *decoded_options_count; ++j)
if ((*decoded_options)[j].opt_index == foption->opt_index)
@@ -503,6 +504,7 @@ append_compiler_options (obstack *argv_obstack, struct
cl_decoded_option *opts,
case OPT_Ofast:
case OPT_Og:
case OPT_Os:
+ case OPT_fcheck_pointer_bounds:
break;
default:
diff --git a/gcc/testsuite/gcc.target/i386/mpx/chkp-fix-calls-1.c
b/gcc/testsuite/gcc.target/i386/mpx/chkp-fix-calls-1.c
new file mode 100644
index 0000000..cb4d229
--- /dev/null
+++ b/gcc/testsuite/gcc.target/i386/mpx/chkp-fix-calls-1.c
@@ -0,0 +1,16 @@
+/* { dg-do compile } */
+/* { dg-options "-O2 -fcheck-pointer-bounds -mmpx" } */
+
+#include "math.h"
+
+double
+test1 (double x, double y, double (*fn)(double, double))
+{
+ return fn (x, y);
+}
+
+double
+test2 (double x, double y)
+{
+ return test1 (x, y, copysign);
+}
diff --git a/gcc/testsuite/gcc.target/i386/mpx/chkp-fix-calls-2.c
b/gcc/testsuite/gcc.target/i386/mpx/chkp-fix-calls-2.c
new file mode 100644
index 0000000..951e7de
--- /dev/null
+++ b/gcc/testsuite/gcc.target/i386/mpx/chkp-fix-calls-2.c
@@ -0,0 +1,16 @@
+/* { dg-do compile } */
+/* { dg-options "-O3 -fcheck-pointer-bounds -mmpx -fno-inline" } */
+
+#include "math.h"
+
+double
+test1 (double x, double y, double (*fn)(double, double))
+{
+ return fn (x, y);
+}
+
+double
+test2 (double x, double y)
+{
+ return test1 (x, y, copysign);
+}
diff --git a/gcc/testsuite/gcc.target/i386/mpx/chkp-fix-calls-3.c
b/gcc/testsuite/gcc.target/i386/mpx/chkp-fix-calls-3.c
new file mode 100755
index 0000000..439f631
--- /dev/null
+++ b/gcc/testsuite/gcc.target/i386/mpx/chkp-fix-calls-3.c
@@ -0,0 +1,33 @@
+/* { dg-do compile } */
+/* { dg-options "-O2 -fexceptions -fcheck-pointer-bounds -mmpx" } */
+
+extern int f2 (const char*, int, ...);
+extern long int f3 (int *);
+extern void err (void) __attribute__((__error__("error")));
+
+extern __inline __attribute__ ((__always_inline__)) int
+f1 (int i, ...)
+{
+ if (__builtin_constant_p (i))
+ {
+ if (i)
+ err ();
+ return f2 ("", i);
+ }
+
+ return f2 ("", i);
+}
+
+int
+test ()
+{
+ int i;
+
+ if (f1 (0))
+ if (f3 (&i))
+ i = 0;
+
+ return i;
+}
+
+
diff --git a/gcc/testsuite/gcc.target/i386/mpx/chkp-fix-calls-4.c
b/gcc/testsuite/gcc.target/i386/mpx/chkp-fix-calls-4.c
new file mode 100644
index 0000000..1b7d703
--- /dev/null
+++ b/gcc/testsuite/gcc.target/i386/mpx/chkp-fix-calls-4.c
@@ -0,0 +1,17 @@
+/* { dg-do compile } */
+/* { dg-options "-Os -fcheck-pointer-bounds -mmpx" } */
+
+typedef void (func) (int *);
+
+static inline void
+bar (func f)
+{
+ int i;
+ f (&i);
+}
+
+void
+foo ()
+{
+ bar (0);
+}
diff --git a/gcc/tree-chkp.c b/gcc/tree-chkp.c
index 8c5a628..c2d9e94 100644
--- a/gcc/tree-chkp.c
+++ b/gcc/tree-chkp.c
@@ -529,6 +529,71 @@ chkp_insert_retbnd_call (tree bndval, tree retval,
return bndval;
}
+/* Build a GIMPLE_CALL identical to CALL but skipping bounds
+ arguments. */
+
+gcall *
+chkp_copy_call_skip_bounds (gcall *call)
+{
+ bitmap bounds;
+ unsigned i;
+
+ bitmap_obstack_initialize (NULL);
+ bounds = BITMAP_ALLOC (NULL);
+
+ for (i = 0; i < gimple_call_num_args (call); i++)
+ if (POINTER_BOUNDS_P (gimple_call_arg (call, i)))
+ bitmap_set_bit (bounds, i);
+
+ if (!bitmap_empty_p (bounds))
+ call = gimple_call_copy_skip_args (call, bounds);
+ gimple_call_set_with_bounds (call, false);
+
+ BITMAP_FREE (bounds);
+ bitmap_obstack_release (NULL);
+
+ return call;
+}
+
+/* Redirect edge E to the correct node according to call_stmt.
+ Return 1 if bounds removal from call_stmt should be done
+ instead of redirection. */
+
+bool
+chkp_redirect_edge (cgraph_edge *e)
+{
+ bool instrumented = false;
+ tree decl = e->callee->decl;
+
+ if (e->callee->instrumentation_clone
+ || chkp_function_instrumented_p (decl))
+ instrumented = true;
+
+ if (instrumented
+ && !gimple_call_with_bounds_p (e->call_stmt))
+ e->redirect_callee (cgraph_node::get_create (e->callee->orig_decl));
+ else if (!instrumented
+ && gimple_call_with_bounds_p (e->call_stmt)
+ && !chkp_gimple_call_builtin_p (e->call_stmt, BUILT_IN_CHKP_BNDCL)
+ && !chkp_gimple_call_builtin_p (e->call_stmt, BUILT_IN_CHKP_BNDCU)
+ && !chkp_gimple_call_builtin_p (e->call_stmt, BUILT_IN_CHKP_BNDSTX))
+ {
+ if (e->callee->instrumented_version)
+ e->redirect_callee (e->callee->instrumented_version);
+ else
+ {
+ tree args = TYPE_ARG_TYPES (TREE_TYPE (decl));
+ /* Avoid bounds removal if all args will be removed. */
+ if (!args || TREE_VALUE (args) != void_type_node)
+ return true;
+ else
+ gimple_call_set_with_bounds (e->call_stmt, false);
+ }
+ }
+
+ return false;
+}
+
/* Mark statement S to not be instrumented. */
static void
chkp_mark_stmt (gimple s)
diff --git a/gcc/tree-chkp.h b/gcc/tree-chkp.h
index 1bafe99..b5ab562 100644
--- a/gcc/tree-chkp.h
+++ b/gcc/tree-chkp.h
@@ -56,5 +56,7 @@ extern bool chkp_gimple_call_builtin_p (gimple call,
extern void chkp_expand_bounds_reset_for_mem (tree mem, tree ptr);
extern tree chkp_insert_retbnd_call (tree bndval, tree retval,
gimple_stmt_iterator *gsi);
+extern gcall *chkp_copy_call_skip_bounds (gcall *call);
+extern bool chkp_redirect_edge (cgraph_edge *e);
#endif /* GCC_TREE_CHKP_H */
