[CHKP] Support returned bounds in thunks expand

[CHKP] Support returned bounds in thunks expand

[Ilya Enkovich]

Hi,

Currentl we loose returned bounds when functions are merged.  This patch fixes it by adding returne bounds support for cgraph_node::expand_thunk.  Bootstrapped and tested on x86_64-unknown-linux-gnu.  OK for trunk?

Thanks,
Ilya
--
gcc/

2015-03-06  Ilya Enkovich  <ilya.enkovich@intel.com>

	* cgraphunit.c (cgraph_node::expand_thunk): Build returned
	bounds for instrumented functions.

gcc/testsuite/

2015-03-06  Ilya Enkovich  <ilya.enkovich@intel.com>

	* gcc/testsuite/gcc.target/i386/thunk-retbnd.c: New.


diff --git a/gcc/cgraphunit.c b/gcc/cgraphunit.c
index e640907..fc38e67 100644
--- a/gcc/cgraphunit.c
+++ b/gcc/cgraphunit.c
@@ -1581,6 +1581,7 @@ cgraph_node::expand_thunk (bool output_asm_thunks, bool force_gimple_thunk)
       int i;
       tree resdecl;
       tree restmp = NULL;
+      tree resbnd = NULL;
 
       gcall *call;
       greturn *ret;
@@ -1697,6 +1698,21 @@ cgraph_node::expand_thunk (bool output_asm_thunks, bool force_gimple_thunk)
       gsi_insert_after (&bsi, call, GSI_NEW_STMT);
       if (!alias_is_noreturn)
 	{
+	  if (instrumentation_clone
+	      && !DECL_BY_REFERENCE (resdecl)
+	      && restmp
+	      && BOUNDED_P (restmp))
+	    {
+	      tree fn = targetm.builtin_chkp_function (BUILT_IN_CHKP_BNDRET);
+	      gcall *retbnd = gimple_build_call (fn, 1, restmp);
+
+	      resbnd = create_tmp_reg (pointer_bounds_type_node, "retbnd");
+	      gimple_call_set_lhs (retbnd, resbnd);
+
+	      gsi_insert_after (&bsi, retbnd, GSI_NEW_STMT);
+	      create_edge (get_create (fn), retbnd, callees->count, callees->frequency);
+	    }
+
 	  if (restmp && !this_adjusting
 	      && (fixed_offset || virtual_offset))
 	    {
@@ -1766,6 +1782,7 @@ cgraph_node::expand_thunk (bool output_asm_thunks, bool force_gimple_thunk)
 	    ret = gimple_build_return (restmp);
 	  else
 	    ret = gimple_build_return (resdecl);
+	  gimple_return_set_retbnd (ret, resbnd);
 
 	  gsi_insert_after (&bsi, ret, GSI_NEW_STMT);
 	}
diff --git a/gcc/testsuite/gcc.target/i386/thunk-retbnd.c b/gcc/testsuite/gcc.target/i386/thunk-retbnd.c
new file mode 100644
index 0000000..d9bd031
--- /dev/null
+++ b/gcc/testsuite/gcc.target/i386/thunk-retbnd.c
@@ -0,0 +1,19 @@
+/* { dg-do compile } */
+/* { dg-require-effective-target mpx } */
+/* { dg-options "-O2 -fcheck-pointer-bounds -mmpx -fdump-tree-optimized" } */
+/* { dg-final { scan-tree-dump-times "return &glob," 2 "optimized" } } */
+/* { dg-final { cleanup-tree-dump "optimized" } } */
+
+int glob;
+
+int *
+test1 (void)
+{
+  return &glob;
+}
+
+int *
+test2 (void)
+{
+  return test1 ();
+}

Re: [CHKP] Support returned bounds in thunks expand

[Ilya Enkovich]

Ping

2015-03-10 13:12 GMT+03:00 Ilya Enkovich <enkovich.gnu@gmail.com>:
> Hi,
>
> Currentl we loose returned bounds when functions are merged.  This patch fixes it by adding returne bounds support for cgraph_node::expand_thunk.  Bootstrapped and tested on x86_64-unknown-linux-gnu.  OK for trunk?
>
> Thanks,
> Ilya
> --
> gcc/
>
> 2015-03-06  Ilya Enkovich  <ilya.enkovich@intel.com>
>
>         * cgraphunit.c (cgraph_node::expand_thunk): Build returned
>         bounds for instrumented functions.
>
> gcc/testsuite/
>
> 2015-03-06  Ilya Enkovich  <ilya.enkovich@intel.com>
>
>         * gcc/testsuite/gcc.target/i386/thunk-retbnd.c: New.
>
>
> diff --git a/gcc/cgraphunit.c b/gcc/cgraphunit.c
> index e640907..fc38e67 100644
> --- a/gcc/cgraphunit.c
> +++ b/gcc/cgraphunit.c
> @@ -1581,6 +1581,7 @@ cgraph_node::expand_thunk (bool output_asm_thunks, bool force_gimple_thunk)
>        int i;
>        tree resdecl;
>        tree restmp = NULL;
> +      tree resbnd = NULL;
>
>        gcall *call;
>        greturn *ret;
> @@ -1697,6 +1698,21 @@ cgraph_node::expand_thunk (bool output_asm_thunks, bool force_gimple_thunk)
>        gsi_insert_after (&bsi, call, GSI_NEW_STMT);
>        if (!alias_is_noreturn)
>         {
> +         if (instrumentation_clone
> +             && !DECL_BY_REFERENCE (resdecl)
> +             && restmp
> +             && BOUNDED_P (restmp))
> +           {
> +             tree fn = targetm.builtin_chkp_function (BUILT_IN_CHKP_BNDRET);
> +             gcall *retbnd = gimple_build_call (fn, 1, restmp);
> +
> +             resbnd = create_tmp_reg (pointer_bounds_type_node, "retbnd");
> +             gimple_call_set_lhs (retbnd, resbnd);
> +
> +             gsi_insert_after (&bsi, retbnd, GSI_NEW_STMT);
> +             create_edge (get_create (fn), retbnd, callees->count, callees->frequency);
> +           }
> +
>           if (restmp && !this_adjusting
>               && (fixed_offset || virtual_offset))
>             {
> @@ -1766,6 +1782,7 @@ cgraph_node::expand_thunk (bool output_asm_thunks, bool force_gimple_thunk)
>             ret = gimple_build_return (restmp);
>           else
>             ret = gimple_build_return (resdecl);
> +         gimple_return_set_retbnd (ret, resbnd);
>
>           gsi_insert_after (&bsi, ret, GSI_NEW_STMT);
>         }
> diff --git a/gcc/testsuite/gcc.target/i386/thunk-retbnd.c b/gcc/testsuite/gcc.target/i386/thunk-retbnd.c
> new file mode 100644
> index 0000000..d9bd031
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/i386/thunk-retbnd.c
> @@ -0,0 +1,19 @@
> +/* { dg-do compile } */
> +/* { dg-require-effective-target mpx } */
> +/* { dg-options "-O2 -fcheck-pointer-bounds -mmpx -fdump-tree-optimized" } */
> +/* { dg-final { scan-tree-dump-times "return &glob," 2 "optimized" } } */
> +/* { dg-final { cleanup-tree-dump "optimized" } } */
> +
> +int glob;
> +
> +int *
> +test1 (void)
> +{
> +  return &glob;
> +}
> +
> +int *
> +test2 (void)
> +{
> +  return test1 ();
> +}

Re: [CHKP] Support returned bounds in thunks expand

[Jeff Law]

On 04/02/2015 08:49 AM, Ilya Enkovich wrote:
> Ping
>
> 2015-03-10 13:12 GMT+03:00 Ilya Enkovich <enkovich.gnu@gmail.com>:
>> Hi,
>>
>> Currentl we loose returned bounds when functions are merged.  This patch fixes it by adding returne bounds support for cgraph_node::expand_thunk.  Bootstrapped and tested on x86_64-unknown-linux-gnu.  OK for trunk?
>>
>> Thanks,
>> Ilya
>> --
>> gcc/
>>
>> 2015-03-06  Ilya Enkovich  <ilya.enkovich@intel.com>
>>
>>          * cgraphunit.c (cgraph_node::expand_thunk): Build returned
>>          bounds for instrumented functions.
>>
>> gcc/testsuite/
>>
>> 2015-03-06  Ilya Enkovich  <ilya.enkovich@intel.com>
>>
>>          * gcc/testsuite/gcc.target/i386/thunk-retbnd.c: New.
I really dislike the amount of gimple and bounded pointer knowledge in 
this code.

It seems like a significant modularity violation and while you didn't 
introduce the gimple stuff, we probably shouldn't be making it worse.

Is it possible to let this code build up the thunk, then pass it off as 
a whole to the chkp code to add the instrumentation, particularly for 
the return value?

ALso, is this critical for stage4?  It looks like this is strictly a 
QofI change, correct?

jeff

Re: [CHKP] Support returned bounds in thunks expand

[Jan Hubicka]

> Ping
> 
> 2015-03-10 13:12 GMT+03:00 Ilya Enkovich <enkovich.gnu@gmail.com>:
> > Hi,
> >
> > Currentl we loose returned bounds when functions are merged.  This patch fixes it by adding returne bounds support for cgraph_node::expand_thunk.  Bootstrapped and tested on x86_64-unknown-linux-gnu.  OK for trunk?
> > 2015-03-06  Ilya Enkovich  <ilya.enkovich@intel.com>
> >
> >         * cgraphunit.c (cgraph_node::expand_thunk): Build returned
> >         bounds for instrumented functions.

I think if the extra bultin call is needed here, the same andling needs to be
added to ipa-split.  We really ought to unify that code - it is surprisingly
difficult to produce a wrapper call these days.
> > +         if (instrumentation_clone
> > +             && !DECL_BY_REFERENCE (resdecl)
> > +             && restmp
> > +             && BOUNDED_P (restmp))
> > +           {
> > +             tree fn = targetm.builtin_chkp_function (BUILT_IN_CHKP_BNDRET);
> > +             gcall *retbnd = gimple_build_call (fn, 1, restmp);
> > +
> > +             resbnd = create_tmp_reg (pointer_bounds_type_node, "retbnd");
> > +             gimple_call_set_lhs (retbnd, resbnd);
> > +
> > +             gsi_insert_after (&bsi, retbnd, GSI_NEW_STMT);
> > +             create_edge (get_create (fn), retbnd, callees->count, callees->frequency);
> > +           }

I am not sure why we need to check here:  Originaly we have two bounded functions, A and B.
We turn function B to a wrapper of function A. If callers of bounded functions need
to call a builtin, I would expect all callers of B to do it already, so I would expect
that wrapper is safe here?

Or do we mix instrumented and non-instrumented versions somehow?

Addding code handling return value is going to turn the call to non-tailcall, so you probably
want to drop the tailcall flag, too. 

Honza

Re: [CHKP] Support returned bounds in thunks expand

[Ilya Enkovich]

2015-04-02 22:42 GMT+03:00 Jeff Law <law@redhat.com>:
> On 04/02/2015 08:49 AM, Ilya Enkovich wrote:
>>
>> Ping
>>
>> 2015-03-10 13:12 GMT+03:00 Ilya Enkovich <enkovich.gnu@gmail.com>:
>>>
>>> Hi,
>>>
>>> Currentl we loose returned bounds when functions are merged.  This patch
>>> fixes it by adding returne bounds support for cgraph_node::expand_thunk.
>>> Bootstrapped and tested on x86_64-unknown-linux-gnu.  OK for trunk?
>>>
>>> Thanks,
>>> Ilya
>>> --
>>> gcc/
>>>
>>> 2015-03-06  Ilya Enkovich  <ilya.enkovich@intel.com>
>>>
>>>          * cgraphunit.c (cgraph_node::expand_thunk): Build returned
>>>          bounds for instrumented functions.
>>>
>>> gcc/testsuite/
>>>
>>> 2015-03-06  Ilya Enkovich  <ilya.enkovich@intel.com>
>>>
>>>          * gcc/testsuite/gcc.target/i386/thunk-retbnd.c: New.
>
> I really dislike the amount of gimple and bounded pointer knowledge in this
> code.
>
> It seems like a significant modularity violation and while you didn't
> introduce the gimple stuff, we probably shouldn't be making it worse.
>
> Is it possible to let this code build up the thunk, then pass it off as a
> whole to the chkp code to add the instrumentation, particularly for the
> return value?

OK, will rework the patch.

>
> ALso, is this critical for stage4?  It looks like this is strictly a QofI
> change, correct?

Actually having just a tail call in a function we don't lose bounds
and I don't expect it affects QofI. But we get instrumented function
with no returned bounds for a pointer which triggers an assert
somewhere (don't remember exact place). I'll revisit the original
problem and will probably make a simpler stability fix for now,
leaving thunk modification for stage 1.

Thanks,
Ilya

>
> jeff
>

Re: [CHKP] Support returned bounds in thunks expand

[Ilya Enkovich]

2015-04-02 23:55 GMT+03:00 Jan Hubicka <hubicka@ucw.cz>:
>> Ping
>>
>> 2015-03-10 13:12 GMT+03:00 Ilya Enkovich <enkovich.gnu@gmail.com>:
>> > Hi,
>> >
>> > Currentl we loose returned bounds when functions are merged.  This patch fixes it by adding returne bounds support for cgraph_node::expand_thunk.  Bootstrapped and tested on x86_64-unknown-linux-gnu.  OK for trunk?
>> > 2015-03-06  Ilya Enkovich  <ilya.enkovich@intel.com>
>> >
>> >         * cgraphunit.c (cgraph_node::expand_thunk): Build returned
>> >         bounds for instrumented functions.
>
> I think if the extra bultin call is needed here, the same andling needs to be
> added to ipa-split.  We really ought to unify that code - it is surprisingly
> difficult to produce a wrapper call these days.

Yep, there is such code in place. It is done by
insert_bndret_call_after. You are right, I should use the same
interface for both these cases.

>> > +         if (instrumentation_clone
>> > +             && !DECL_BY_REFERENCE (resdecl)
>> > +             && restmp
>> > +             && BOUNDED_P (restmp))
>> > +           {
>> > +             tree fn = targetm.builtin_chkp_function (BUILT_IN_CHKP_BNDRET);
>> > +             gcall *retbnd = gimple_build_call (fn, 1, restmp);
>> > +
>> > +             resbnd = create_tmp_reg (pointer_bounds_type_node, "retbnd");
>> > +             gimple_call_set_lhs (retbnd, resbnd);
>> > +
>> > +             gsi_insert_after (&bsi, retbnd, GSI_NEW_STMT);
>> > +             create_edge (get_create (fn), retbnd, callees->count, callees->frequency);
>> > +           }
>
> I am not sure why we need to check here:  Originaly we have two bounded functions, A and B.
> We turn function B to a wrapper of function A. If callers of bounded functions need
> to call a builtin, I would expect all callers of B to do it already, so I would expect
> that wrapper is safe here?

The problem with instrumented call is that instrumented function
returns two values and call lhs gets only the first one. Thus we
generate bndret call to get the second one to build own return with
two values.

>
> Or do we mix instrumented and non-instrumented versions somehow?
>
> Addding code handling return value is going to turn the call to non-tailcall, so you probably
> want to drop the tailcall flag, too.

No, function still return what the last call return, thus may keep tailcall.

Thanks,
Ilya

>
> Honza

Re: [CHKP] Support returned bounds in thunks expand

[Jan Hubicka]

> 
> Yep, there is such code in place. It is done by
> insert_bndret_call_after. You are right, I should use the same
> interface for both these cases.

That would be nice indeed.  We should separate out and unify the code for porducing
a wrapper call next stage1 (as Jeff mentions too).
> 
> >> > +         if (instrumentation_clone
> >> > +             && !DECL_BY_REFERENCE (resdecl)
> >> > +             && restmp
> >> > +             && BOUNDED_P (restmp))
> >> > +           {
> >> > +             tree fn = targetm.builtin_chkp_function (BUILT_IN_CHKP_BNDRET);
> >> > +             gcall *retbnd = gimple_build_call (fn, 1, restmp);
> >> > +
> >> > +             resbnd = create_tmp_reg (pointer_bounds_type_node, "retbnd");
> >> > +             gimple_call_set_lhs (retbnd, resbnd);
> >> > +
> >> > +             gsi_insert_after (&bsi, retbnd, GSI_NEW_STMT);
> >> > +             create_edge (get_create (fn), retbnd, callees->count, callees->frequency);
> >> > +           }
> >
> > I am not sure why we need to check here:  Originaly we have two bounded functions, A and B.
> > We turn function B to a wrapper of function A. If callers of bounded functions need
> > to call a builtin, I would expect all callers of B to do it already, so I would expect
> > that wrapper is safe here?
> 
> The problem with instrumented call is that instrumented function
> returns two values and call lhs gets only the first one. Thus we
> generate bndret call to get the second one to build own return with
> two values.

I see, patch is OK then (preferably merging as much as possible with ipa-split)

Honza
> 
> >
> > Or do we mix instrumented and non-instrumented versions somehow?
> >
> > Addding code handling return value is going to turn the call to non-tailcall, so you probably
> > want to drop the tailcall flag, too.
> 
> No, function still return what the last call return, thus may keep tailcall.
> 
> Thanks,
> Ilya
> 
> >
> > Honza

Re: [CHKP] Support returned bounds in thunks expand

[Ilya Enkovich]

> > 
> > The problem with instrumented call is that instrumented function
> > returns two values and call lhs gets only the first one. Thus we
> > generate bndret call to get the second one to build own return with
> > two values.
> 
> I see, patch is OK then (preferably merging as much as possible with ipa-split)
> 
> Honza

Here is a refactored version with common code moved to tree-chkp.c.  Bootstrapped and tested on x86_64-unknown-linux-gnu.  Does it look OK?

Thanks,
Ilya
--
gcc/

2015-04-07  Ilya Enkovich  <ilya.enkovich@intel.com>

	* tree-chkp.h (chkp_insert_retbnd_call): New.
	* tree-chkp.c (chkp_insert_retbnd_call): New.
	* ipa-split.c (insert_bndret_call_after): Remove.
	(split_function): Use chkp_insert_retbnd_call.
	* cgraphunit.c (cgraph_node::expand_thunk): Build returned
	bounds for instrumented functions.

gcc/testsuite/

2015-04-07  Ilya Enkovich  <ilya.enkovich@intel.com>

	* gcc/testsuite/gcc.target/i386/thunk-retbnd.c: New.


diff --git a/gcc/cgraphunit.c b/gcc/cgraphunit.c
index 8ac92e1..ca34c31 100644
--- a/gcc/cgraphunit.c
+++ b/gcc/cgraphunit.c
@@ -1581,6 +1581,7 @@ cgraph_node::expand_thunk (bool output_asm_thunks, bool force_gimple_thunk)
       int i;
       tree resdecl;
       tree restmp = NULL;
+      tree resbnd = NULL;
 
       gcall *call;
       greturn *ret;
@@ -1697,6 +1698,17 @@ cgraph_node::expand_thunk (bool output_asm_thunks, bool force_gimple_thunk)
       gsi_insert_after (&bsi, call, GSI_NEW_STMT);
       if (!alias_is_noreturn)
 	{
+	  if (instrumentation_clone
+	      && !DECL_BY_REFERENCE (resdecl)
+	      && restmp
+	      && BOUNDED_P (restmp))
+	    {
+	      resbnd = chkp_insert_retbnd_call (NULL, restmp, &bsi);
+	      create_edge (get_create (gimple_call_fndecl (gsi_stmt (bsi))),
+			   as_a <gcall *> (gsi_stmt (bsi)),
+			   callees->count, callees->frequency);
+	    }
+
 	  if (restmp && !this_adjusting
 	      && (fixed_offset || virtual_offset))
 	    {
@@ -1766,6 +1778,7 @@ cgraph_node::expand_thunk (bool output_asm_thunks, bool force_gimple_thunk)
 	    ret = gimple_build_return (restmp);
 	  else
 	    ret = gimple_build_return (resdecl);
+	  gimple_return_set_retbnd (ret, resbnd);
 
 	  gsi_insert_after (&bsi, ret, GSI_NEW_STMT);
 	}
diff --git a/gcc/ipa-split.c b/gcc/ipa-split.c
index 5d5db0e..f4bd8d7 100644
--- a/gcc/ipa-split.c
+++ b/gcc/ipa-split.c
@@ -1230,20 +1230,6 @@ find_split_points (basic_block return_bb, int overall_time, int overall_size)
   BITMAP_FREE (current.ssa_names_to_pass);
 }
 
-/* Build and insert initialization of returned bounds RETBND
-   for returned value RETVAL.  Statements are inserted after
-   a statement pointed by GSI and GSI is modified to point to
-   the last inserted statement.  */
-
-static void
-insert_bndret_call_after (tree retbnd, tree retval, gimple_stmt_iterator *gsi)
-{
-  tree fndecl = targetm.builtin_chkp_function (BUILT_IN_CHKP_BNDRET);
-  gimple bndret = gimple_build_call (fndecl, 1, retval);
-  gimple_call_set_lhs (bndret, retbnd);
-  gsi_insert_after (gsi, bndret, GSI_CONTINUE_LINKING);
-}
-
 /* Split function at SPLIT_POINT.  */
 
 static void
@@ -1650,7 +1636,7 @@ split_function (basic_block return_bb, struct split_point *split_point,
 		    }
 		  /* Build bndret call to obtain returned bounds.  */
 		  if (retbnd)
-		    insert_bndret_call_after (retbnd, retval, &gsi);
+		    chkp_insert_retbnd_call (retbnd, retval, &gsi);
 		  gimple_call_set_lhs (call, retval);
 		  update_stmt (call);
 		}
@@ -1700,7 +1686,7 @@ split_function (basic_block return_bb, struct split_point *split_point,
           gsi_insert_after (&gsi, call, GSI_NEW_STMT);
 	  /* Build bndret call to obtain returned bounds.  */
 	  if (retbnd)
-	    insert_bndret_call_after (retbnd, retval, &gsi);
+	    chkp_insert_retbnd_call (retbnd, retval, &gsi);
 	  if (tsan_func_exit_call)
 	    gsi_insert_after (&gsi, tsan_func_exit_call, GSI_NEW_STMT);
 	  ret = gimple_build_return (retval);
diff --git a/gcc/testsuite/gcc.target/i386/thunk-retbnd.c b/gcc/testsuite/gcc.target/i386/thunk-retbnd.c
new file mode 100644
index 0000000..d9bd031
--- /dev/null
+++ b/gcc/testsuite/gcc.target/i386/thunk-retbnd.c
@@ -0,0 +1,19 @@
+/* { dg-do compile } */
+/* { dg-require-effective-target mpx } */
+/* { dg-options "-O2 -fcheck-pointer-bounds -mmpx -fdump-tree-optimized" } */
+/* { dg-final { scan-tree-dump-times "return &glob," 2 "optimized" } } */
+/* { dg-final { cleanup-tree-dump "optimized" } } */
+
+int glob;
+
+int *
+test1 (void)
+{
+  return &glob;
+}
+
+int *
+test2 (void)
+{
+  return test1 ();
+}
diff --git a/gcc/tree-chkp.c b/gcc/tree-chkp.c
index 03f75b3..541af29 100644
--- a/gcc/tree-chkp.c
+++ b/gcc/tree-chkp.c
@@ -500,6 +500,35 @@ chkp_expand_bounds_reset_for_mem (tree mem, tree ptr)
   expand_normal (bndstx);
 }
 
+/* Build retbnd call for returned value RETVAL.
+
+   If BNDVAL is not NULL then result is stored
+   in it.  Otherwise a temporary is created to
+   hold returned value.
+
+   GSI points to a position for a retbnd call
+   and is set to created stmt.
+
+   Cgraph edge is created for a new call if
+   UPDATE_EDGE is 1.
+
+   Obtained bounds are returned.  */
+tree
+chkp_insert_retbnd_call (tree bndval, tree retval,
+			 gimple_stmt_iterator *gsi)
+{
+  gimple call;
+
+  if (!bndval)
+    bndval = create_tmp_reg (pointer_bounds_type_node, "retbnd");
+
+  call = gimple_build_call (chkp_ret_bnd_fndecl, 1, retval);
+  gimple_call_set_lhs (call, bndval);
+  gsi_insert_after (gsi, call, GSI_CONTINUE_LINKING);
+
+  return bndval;
+}
+
 /* 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..1bafe99 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 tree chkp_insert_retbnd_call (tree bndval, tree retval,
+				     gimple_stmt_iterator *gsi);
 
 #endif /* GCC_TREE_CHKP_H */

Re: [CHKP] Support returned bounds in thunks expand

[Jan Hubicka]

> > > 
> > > The problem with instrumented call is that instrumented function
> > > returns two values and call lhs gets only the first one. Thus we
> > > generate bndret call to get the second one to build own return with
> > > two values.
> > 
> > I see, patch is OK then (preferably merging as much as possible with ipa-split)
> > 
> > Honza
> 
> Here is a refactored version with common code moved to tree-chkp.c.  Bootstrapped and tested on x86_64-unknown-linux-gnu.  Does it look OK?
> 
> Thanks,
> Ilya
> --
> gcc/
> 
> 2015-04-07  Ilya Enkovich  <ilya.enkovich@intel.com>
> 
> 	* tree-chkp.h (chkp_insert_retbnd_call): New.
> 	* tree-chkp.c (chkp_insert_retbnd_call): New.
> 	* ipa-split.c (insert_bndret_call_after): Remove.
> 	(split_function): Use chkp_insert_retbnd_call.
> 	* cgraphunit.c (cgraph_node::expand_thunk): Build returned
> 	bounds for instrumented functions.
> 
> gcc/testsuite/
> 
> 2015-04-07  Ilya Enkovich  <ilya.enkovich@intel.com>
> 
> 	* gcc/testsuite/gcc.target/i386/thunk-retbnd.c: New.

OK, thanks!
> @@ -1697,6 +1698,17 @@ cgraph_node::expand_thunk (bool output_asm_thunks, bool force_gimple_thunk)
>        gsi_insert_after (&bsi, call, GSI_NEW_STMT);
>        if (!alias_is_noreturn)
>  	{
> +	  if (instrumentation_clone
> +	      && !DECL_BY_REFERENCE (resdecl)
> +	      && restmp
> +	      && BOUNDED_P (restmp))
> +	    {
> +	      resbnd = chkp_insert_retbnd_call (NULL, restmp, &bsi);
> +	      create_edge (get_create (gimple_call_fndecl (gsi_stmt (bsi))),
> +			   as_a <gcall *> (gsi_stmt (bsi)),
> +			   callees->count, callees->frequency);
> +	    }

Is there any reasons the rtbnd builtin call is not gimple_call_internal_p?
That way we would not need to worry about representing this in callgraph.

Honza

Re: [CHKP] Support returned bounds in thunks expand

[Ilya Enkovich]

2015-04-07 23:33 GMT+03:00 Jan Hubicka <hubicka@ucw.cz>:
>> > >
>> > > The problem with instrumented call is that instrumented function
>> > > returns two values and call lhs gets only the first one. Thus we
>> > > generate bndret call to get the second one to build own return with
>> > > two values.
>> >
>> > I see, patch is OK then (preferably merging as much as possible with ipa-split)
>> >
>> > Honza
>>
>> Here is a refactored version with common code moved to tree-chkp.c.  Bootstrapped and tested on x86_64-unknown-linux-gnu.  Does it look OK?
>>
>> Thanks,
>> Ilya
>> --
>> gcc/
>>
>> 2015-04-07  Ilya Enkovich  <ilya.enkovich@intel.com>
>>
>>       * tree-chkp.h (chkp_insert_retbnd_call): New.
>>       * tree-chkp.c (chkp_insert_retbnd_call): New.
>>       * ipa-split.c (insert_bndret_call_after): Remove.
>>       (split_function): Use chkp_insert_retbnd_call.
>>       * cgraphunit.c (cgraph_node::expand_thunk): Build returned
>>       bounds for instrumented functions.
>>
>> gcc/testsuite/
>>
>> 2015-04-07  Ilya Enkovich  <ilya.enkovich@intel.com>
>>
>>       * gcc/testsuite/gcc.target/i386/thunk-retbnd.c: New.
>
> OK, thanks!
>> @@ -1697,6 +1698,17 @@ cgraph_node::expand_thunk (bool output_asm_thunks, bool force_gimple_thunk)
>>        gsi_insert_after (&bsi, call, GSI_NEW_STMT);
>>        if (!alias_is_noreturn)
>>       {
>> +       if (instrumentation_clone
>> +           && !DECL_BY_REFERENCE (resdecl)
>> +           && restmp
>> +           && BOUNDED_P (restmp))
>> +         {
>> +           resbnd = chkp_insert_retbnd_call (NULL, restmp, &bsi);
>> +           create_edge (get_create (gimple_call_fndecl (gsi_stmt (bsi))),
>> +                        as_a <gcall *> (gsi_stmt (bsi)),
>> +                        callees->count, callees->frequency);
>> +         }
>
> Is there any reasons the rtbnd builtin call is not gimple_call_internal_p?
> That way we would not need to worry about representing this in callgraph.

Function called to get returned bounds (similar to many other
instrumentation functions) is target dependent. Internal function
usage would require significant redesign.

Thanks,
Ilya

>
> Honza