On 24 Mar 15:06, Jakub Jelinek wrote:
> On Tue, Mar 24, 2015 at 12:22:27PM +0300, Ilya Enkovich wrote:
> > 2015-03-24 11:33 GMT+03:00 Jakub Jelinek <ja...@redhat.com>:
> > > On Thu, Mar 19, 2015 at 11:29:44AM +0300, Ilya Enkovich 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 (callee)
> > >> + skip_bounds = chkp_redirect_edge (e);
> > >
> > > I just want to say that I'm not really excited about all this compile time
> > > cost that is added everywhere unconditionally for chkp.
> > > I think much better would be to guard most of it with proper option check
> > > first and only do the more expensive part if the option has been used.
> >
> > Agree, overhead for not instrumented code should be minimized.
> > Unfortunately there is no option check I can use to guard chkp codes
> > due to LTO. Currently it is allowed to pass -fcheck-pointer-bounds for
> > IL generation and don't pass it for final code generation. I suppose I
> > may set this (or some new) flag if see instrumented node when read
> > cgraph and then use it to guard chkp related codes. Would it be
> > acceptable?
>
> The question is what you want to do in the LTO case for the different cases,
> in particular a TU compiled with -fcheck-pointer-bounds and LTO link without
> that, or TU compiled without -fcheck-pointer-bounds and LTO link with it.
> It could be handled as LTO incompatible option, where lto1 would error out
> if you try to mix -fcheck-pointer-bounds with -fno-check-pointer-bounds
> code, or e.g. similar to var-tracking, you could consider adjusting the IL
> upon LTO reading if if some TU has been built with -fcheck-pointer-bounds
> and the LTO link is -fno-check-pointer-bounds. Dunno what will happen
> with -fno-check-pointer-bounds TUs LTO linked with -fcheck-pointer-bounds.
> Or another possibility is to or in -fcheck-pointer-bounds from all TUs.
>
> > Maybe replace attribute usage with a new flag in tree_decl_with_vis
> > structure?
>
> Depends, might be better to stick it into cgraph_node instead, depends on
> whether you are querying it already early in the FEs or just during GIMPLE
> when the cgraph node should be created too.
>
> Jakub
Hi,
Here is an updated version of the patch. I added merge for
-fcheck-pointer-bounds option in LTO and used it as a guard for new code.
Testing is in progress. Does this version look OK?
Thanks,
Ilya
--
gcc/
2015-04-02 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-02 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..28b5996 100644
--- a/gcc/cgraph.c
+++ b/gcc/cgraph.c
@@ -1277,14 +1277,25 @@ cgraph_edge::redirect_call_stmt_to_callee (void)
{
cgraph_edge *e = this;
- tree decl = gimple_call_fndecl (e->call_stmt);
- tree lhs = gimple_call_lhs (e->call_stmt);
+ tree decl;
+ tree lhs;
gcall *new_stmt;
gimple_stmt_iterator gsi;
+ bool skip_bounds = false;
#ifdef ENABLE_CHECKING
cgraph_node *node;
#endif
+ /* 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);
+
+ decl = gimple_call_fndecl (e->call_stmt);
+ lhs = gimple_call_lhs (e->call_stmt);
+
if (e->speculative)
{
cgraph_edge *e2;
@@ -1390,7 +1401,8 @@ cgraph_edge::redirect_call_stmt_to_callee (void)
}
if (e->indirect_unknown_callee
- || decl == e->callee->decl)
+ || (decl == e->callee->decl
+ && !skip_bounds))
return e->call_stmt;
#ifdef ENABLE_CHECKING
@@ -1415,13 +1427,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 03f75b3..d6ed1f5 100644
--- a/gcc/tree-chkp.c
+++ b/gcc/tree-chkp.c
@@ -500,6 +500,71 @@ chkp_expand_bounds_reset_for_mem (tree mem, tree ptr)
expand_normal (bndstx);
}
+/* 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 86f3618..40e2489 100644
--- a/gcc/tree-chkp.h
+++ b/gcc/tree-chkp.h
@@ -54,5 +54,7 @@ extern void chkp_copy_bounds_for_assign (gimple assign,
extern bool chkp_gimple_call_builtin_p (gimple call,
enum built_in_function code);
extern void chkp_expand_bounds_reset_for_mem (tree mem, tree ptr);
+extern gcall *chkp_copy_call_skip_bounds (gcall *call);
+extern bool chkp_redirect_edge (cgraph_edge *e);
#endif /* GCC_TREE_CHKP_H */
