[PATCH, MPX, 2/X] Pointers Checker [10/25] Calls copy and verification

[PATCH, MPX, 2/X] Pointers Checker [10/25] Calls copy and verification

[Ilya Enkovich]

Hi,

Here is a patch to support of instrumented code in calls verifiers and calls copy with skipped args.

Thanks,
Ilya
--

gcc/

2013-10-29  Ilya Enkovich  <ilya.enkovich@intel.com>

	* cgraph.c (gimple_check_call_args): Handle bound args.
	* gimple.c (gimple_call_copy_skip_args): Likewise.
	(validate_call): Likewise.


diff --git a/gcc/cgraph.c b/gcc/cgraph.c
index 52d9ab0..9d7ae85 100644
--- a/gcc/cgraph.c
+++ b/gcc/cgraph.c
@@ -3030,40 +3030,54 @@ gimple_check_call_args (gimple stmt, tree fndecl, bool args_count_match)
     {
       for (i = 0, p = DECL_ARGUMENTS (fndecl);
 	   i < nargs;
-	   i++, p = DECL_CHAIN (p))
+	   i++)
 	{
-	  tree arg;
+	  tree arg = gimple_call_arg (stmt, i);
+
+	  /* Skip bound args inserted by Pointer Bounds Checker.  */
+	  if (POINTER_BOUNDS_P (arg))
+	    continue;
+
 	  /* We cannot distinguish a varargs function from the case
 	     of excess parameters, still deferring the inlining decision
 	     to the callee is possible.  */
 	  if (!p)
 	    break;
-	  arg = gimple_call_arg (stmt, i);
+
 	  if (p == error_mark_node
 	      || arg == error_mark_node
 	      || (!types_compatible_p (DECL_ARG_TYPE (p), TREE_TYPE (arg))
 		  && !fold_convertible_p (DECL_ARG_TYPE (p), arg)))
             return false;
+
+	  p = DECL_CHAIN (p);
 	}
       if (args_count_match && p)
 	return false;
     }
   else if (parms)
     {
-      for (i = 0, p = parms; i < nargs; i++, p = TREE_CHAIN (p))
+      for (i = 0, p = parms; i < nargs; i++)
 	{
-	  tree arg;
+	  tree arg = gimple_call_arg (stmt, i);
+
+	  /* Skip bound args inserted by Pointer Bounds Checker.  */
+	  if (POINTER_BOUNDS_P (arg))
+	    continue;
+
 	  /* If this is a varargs function defer inlining decision
 	     to callee.  */
 	  if (!p)
 	    break;
-	  arg = gimple_call_arg (stmt, i);
+
 	  if (TREE_VALUE (p) == error_mark_node
 	      || arg == error_mark_node
 	      || TREE_CODE (TREE_VALUE (p)) == VOID_TYPE
 	      || (!types_compatible_p (TREE_VALUE (p), TREE_TYPE (arg))
 		  && !fold_convertible_p (TREE_VALUE (p), arg)))
             return false;
+
+	  p = TREE_CHAIN (p);
 	}
     }
   else
diff --git a/gcc/gimple.c b/gcc/gimple.c
index 20f6010..dc85bf8 100644
--- a/gcc/gimple.c
+++ b/gcc/gimple.c
@@ -3048,15 +3048,20 @@ canonicalize_cond_expr_cond (tree t)
 gimple
 gimple_call_copy_skip_args (gimple stmt, bitmap args_to_skip)
 {
-  int i;
+  int i, bit;
   int nargs = gimple_call_num_args (stmt);
   vec<tree> vargs;
   vargs.create (nargs);
   gimple new_stmt;
 
-  for (i = 0; i < nargs; i++)
-    if (!bitmap_bit_p (args_to_skip, i))
-      vargs.quick_push (gimple_call_arg (stmt, i));
+  for (i = 0, bit = 0; i < nargs; i++, bit++)
+      if (POINTER_BOUNDS_P (gimple_call_arg (stmt, i)))
+	{
+	  if (!bitmap_bit_p (args_to_skip, --bit))
+	    vargs.quick_push (gimple_call_arg (stmt, i));
+	}
+      else if (!bitmap_bit_p (args_to_skip, bit))
+	  vargs.quick_push (gimple_call_arg (stmt, i));
 
   if (gimple_call_internal_p (stmt))
     new_stmt = gimple_build_call_internal_vec (gimple_call_internal_fn (stmt),
@@ -3702,6 +3707,9 @@ validate_call (gimple stmt, tree fndecl)
       if (!targs)
 	return true;
       tree arg = gimple_call_arg (stmt, i);
+      /* Skip bounds.  */
+      if (flag_check_pointer_bounds && POINTER_BOUNDS_P (arg))
+	continue;
       if (INTEGRAL_TYPE_P (TREE_TYPE (arg))
 	  && INTEGRAL_TYPE_P (TREE_VALUE (targs)))
 	;

Re: [PATCH, MPX, 2/X] Pointers Checker [10/25] Calls copy and verification

[Richard Biener]

On Thu, Oct 31, 2013 at 10:24 AM, Ilya Enkovich <enkovich.gnu@gmail.com> wrote:
> Hi,
>
> Here is a patch to support of instrumented code in calls verifiers and calls copy with skipped args.
>
> Thanks,
> Ilya
> --
>
> gcc/
>
> 2013-10-29  Ilya Enkovich  <ilya.enkovich@intel.com>
>
>         * cgraph.c (gimple_check_call_args): Handle bound args.
>         * gimple.c (gimple_call_copy_skip_args): Likewise.
>         (validate_call): Likewise.
>
>
> diff --git a/gcc/cgraph.c b/gcc/cgraph.c
> index 52d9ab0..9d7ae85 100644
> --- a/gcc/cgraph.c
> +++ b/gcc/cgraph.c
> @@ -3030,40 +3030,54 @@ gimple_check_call_args (gimple stmt, tree fndecl, bool args_count_match)
>      {
>        for (i = 0, p = DECL_ARGUMENTS (fndecl);
>            i < nargs;
> -          i++, p = DECL_CHAIN (p))
> +          i++)
>         {
> -         tree arg;
> +         tree arg = gimple_call_arg (stmt, i);
> +
> +         /* Skip bound args inserted by Pointer Bounds Checker.  */
> +         if (POINTER_BOUNDS_P (arg))
> +           continue;
> +
>           /* We cannot distinguish a varargs function from the case
>              of excess parameters, still deferring the inlining decision
>              to the callee is possible.  */
>           if (!p)
>             break;
> -         arg = gimple_call_arg (stmt, i);
> +
>           if (p == error_mark_node
>               || arg == error_mark_node
>               || (!types_compatible_p (DECL_ARG_TYPE (p), TREE_TYPE (arg))
>                   && !fold_convertible_p (DECL_ARG_TYPE (p), arg)))
>              return false;
> +
> +         p = DECL_CHAIN (p);
>         }
>        if (args_count_match && p)
>         return false;
>      }
>    else if (parms)
>      {
> -      for (i = 0, p = parms; i < nargs; i++, p = TREE_CHAIN (p))
> +      for (i = 0, p = parms; i < nargs; i++)
>         {
> -         tree arg;
> +         tree arg = gimple_call_arg (stmt, i);
> +
> +         /* Skip bound args inserted by Pointer Bounds Checker.  */
> +         if (POINTER_BOUNDS_P (arg))
> +           continue;
> +
>           /* If this is a varargs function defer inlining decision
>              to callee.  */
>           if (!p)
>             break;
> -         arg = gimple_call_arg (stmt, i);
> +
>           if (TREE_VALUE (p) == error_mark_node
>               || arg == error_mark_node
>               || TREE_CODE (TREE_VALUE (p)) == VOID_TYPE
>               || (!types_compatible_p (TREE_VALUE (p), TREE_TYPE (arg))
>                   && !fold_convertible_p (TREE_VALUE (p), arg)))
>              return false;
> +
> +         p = TREE_CHAIN (p);
>         }
>      }
>    else
> diff --git a/gcc/gimple.c b/gcc/gimple.c
> index 20f6010..dc85bf8 100644
> --- a/gcc/gimple.c
> +++ b/gcc/gimple.c
> @@ -3048,15 +3048,20 @@ canonicalize_cond_expr_cond (tree t)
>  gimple
>  gimple_call_copy_skip_args (gimple stmt, bitmap args_to_skip)
>  {
> -  int i;
> +  int i, bit;
>    int nargs = gimple_call_num_args (stmt);
>    vec<tree> vargs;
>    vargs.create (nargs);
>    gimple new_stmt;
>
> -  for (i = 0; i < nargs; i++)
> -    if (!bitmap_bit_p (args_to_skip, i))
> -      vargs.quick_push (gimple_call_arg (stmt, i));
> +  for (i = 0, bit = 0; i < nargs; i++, bit++)
> +      if (POINTER_BOUNDS_P (gimple_call_arg (stmt, i)))
> +       {
> +         if (!bitmap_bit_p (args_to_skip, --bit))
> +           vargs.quick_push (gimple_call_arg (stmt, i));
> +       }
> +      else if (!bitmap_bit_p (args_to_skip, bit))
> +         vargs.quick_push (gimple_call_arg (stmt, i));

The new code is completely confusing.  You need to update
comments with what bits args_to_skip refers to and what happens
with pointer bounds.  I suppose the bitmap also contains
bound parameters as they are in calls (but not in fndecls -- I think
this discrepancy still is a way to desaster).

>    if (gimple_call_internal_p (stmt))
>      new_stmt = gimple_build_call_internal_vec (gimple_call_internal_fn (stmt),
> @@ -3702,6 +3707,9 @@ validate_call (gimple stmt, tree fndecl)
>        if (!targs)
>         return true;
>        tree arg = gimple_call_arg (stmt, i);
> +      /* Skip bounds.  */
> +      if (flag_check_pointer_bounds && POINTER_BOUNDS_P (arg))
> +       continue;

Why check flag_check_pointer_bounds here but not elsewhere?

>        if (INTEGRAL_TYPE_P (TREE_TYPE (arg))
>           && INTEGRAL_TYPE_P (TREE_VALUE (targs)))
>         ;

Re: [PATCH, MPX, 2/X] Pointers Checker [10/25] Calls copy and verification

[Ilya Enkovich]

2013/11/4 Richard Biener <richard.guenther@gmail.com>:
> On Thu, Oct 31, 2013 at 10:24 AM, Ilya Enkovich <enkovich.gnu@gmail.com> wrote:
>> Hi,
>>
>> Here is a patch to support of instrumented code in calls verifiers and calls copy with skipped args.
>>
>> Thanks,
>> Ilya
>> --
>>
>> gcc/
>>
>> 2013-10-29  Ilya Enkovich  <ilya.enkovich@intel.com>
>>
>>         * cgraph.c (gimple_check_call_args): Handle bound args.
>>         * gimple.c (gimple_call_copy_skip_args): Likewise.
>>         (validate_call): Likewise.
>>
>>
>> diff --git a/gcc/cgraph.c b/gcc/cgraph.c
>> index 52d9ab0..9d7ae85 100644
>> --- a/gcc/cgraph.c
>> +++ b/gcc/cgraph.c
>> @@ -3030,40 +3030,54 @@ gimple_check_call_args (gimple stmt, tree fndecl, bool args_count_match)
>>      {
>>        for (i = 0, p = DECL_ARGUMENTS (fndecl);
>>            i < nargs;
>> -          i++, p = DECL_CHAIN (p))
>> +          i++)
>>         {
>> -         tree arg;
>> +         tree arg = gimple_call_arg (stmt, i);
>> +
>> +         /* Skip bound args inserted by Pointer Bounds Checker.  */
>> +         if (POINTER_BOUNDS_P (arg))
>> +           continue;
>> +
>>           /* We cannot distinguish a varargs function from the case
>>              of excess parameters, still deferring the inlining decision
>>              to the callee is possible.  */
>>           if (!p)
>>             break;
>> -         arg = gimple_call_arg (stmt, i);
>> +
>>           if (p == error_mark_node
>>               || arg == error_mark_node
>>               || (!types_compatible_p (DECL_ARG_TYPE (p), TREE_TYPE (arg))
>>                   && !fold_convertible_p (DECL_ARG_TYPE (p), arg)))
>>              return false;
>> +
>> +         p = DECL_CHAIN (p);
>>         }
>>        if (args_count_match && p)
>>         return false;
>>      }
>>    else if (parms)
>>      {
>> -      for (i = 0, p = parms; i < nargs; i++, p = TREE_CHAIN (p))
>> +      for (i = 0, p = parms; i < nargs; i++)
>>         {
>> -         tree arg;
>> +         tree arg = gimple_call_arg (stmt, i);
>> +
>> +         /* Skip bound args inserted by Pointer Bounds Checker.  */
>> +         if (POINTER_BOUNDS_P (arg))
>> +           continue;
>> +
>>           /* If this is a varargs function defer inlining decision
>>              to callee.  */
>>           if (!p)
>>             break;
>> -         arg = gimple_call_arg (stmt, i);
>> +
>>           if (TREE_VALUE (p) == error_mark_node
>>               || arg == error_mark_node
>>               || TREE_CODE (TREE_VALUE (p)) == VOID_TYPE
>>               || (!types_compatible_p (TREE_VALUE (p), TREE_TYPE (arg))
>>                   && !fold_convertible_p (TREE_VALUE (p), arg)))
>>              return false;
>> +
>> +         p = TREE_CHAIN (p);
>>         }
>>      }
>>    else
>> diff --git a/gcc/gimple.c b/gcc/gimple.c
>> index 20f6010..dc85bf8 100644
>> --- a/gcc/gimple.c
>> +++ b/gcc/gimple.c
>> @@ -3048,15 +3048,20 @@ canonicalize_cond_expr_cond (tree t)
>>  gimple
>>  gimple_call_copy_skip_args (gimple stmt, bitmap args_to_skip)
>>  {
>> -  int i;
>> +  int i, bit;
>>    int nargs = gimple_call_num_args (stmt);
>>    vec<tree> vargs;
>>    vargs.create (nargs);
>>    gimple new_stmt;
>>
>> -  for (i = 0; i < nargs; i++)
>> -    if (!bitmap_bit_p (args_to_skip, i))
>> -      vargs.quick_push (gimple_call_arg (stmt, i));
>> +  for (i = 0, bit = 0; i < nargs; i++, bit++)
>> +      if (POINTER_BOUNDS_P (gimple_call_arg (stmt, i)))
>> +       {
>> +         if (!bitmap_bit_p (args_to_skip, --bit))
>> +           vargs.quick_push (gimple_call_arg (stmt, i));
>> +       }
>> +      else if (!bitmap_bit_p (args_to_skip, bit))
>> +         vargs.quick_push (gimple_call_arg (stmt, i));
>
> The new code is completely confusing.  You need to update
> comments with what bits args_to_skip refers to and what happens
> with pointer bounds.  I suppose the bitmap also contains
> bound parameters as they are in calls (but not in fndecls -- I think
> this discrepancy still is a way to desaster).

args_to_skip here does not hold bits for bounds. It's content is equal
for both instrumented and original call.  Will make it explicit in
comments.

>
>>    if (gimple_call_internal_p (stmt))
>>      new_stmt = gimple_build_call_internal_vec (gimple_call_internal_fn (stmt),
>> @@ -3702,6 +3707,9 @@ validate_call (gimple stmt, tree fndecl)
>>        if (!targs)
>>         return true;
>>        tree arg = gimple_call_arg (stmt, i);
>> +      /* Skip bounds.  */
>> +      if (flag_check_pointer_bounds && POINTER_BOUNDS_P (arg))
>> +       continue;
>
> Why check flag_check_pointer_bounds here but not elsewhere?

No reason actually. I'll remove flag check here.

>
>>        if (INTEGRAL_TYPE_P (TREE_TYPE (arg))
>>           && INTEGRAL_TYPE_P (TREE_VALUE (targs)))
>>         ;

Re: [PATCH, MPX, 2/X] Pointers Checker [10/25] Calls copy and verification

[Ilya Enkovich]

Hi,

Here is an updated patch version.

Thanks,
Ilya
--
diff --git a/gcc/cgraph.c b/gcc/cgraph.c
index 52d9ab0..9d7ae85 100644
--- a/gcc/cgraph.c
+++ b/gcc/cgraph.c
@@ -3030,40 +3030,54 @@ gimple_check_call_args (gimple stmt, tree fndecl, bool args_count_match)
     {
       for (i = 0, p = DECL_ARGUMENTS (fndecl);
 	   i < nargs;
-	   i++, p = DECL_CHAIN (p))
+	   i++)
 	{
-	  tree arg;
+	  tree arg = gimple_call_arg (stmt, i);
+
+	  /* Skip bound args inserted by Pointer Bounds Checker.  */
+	  if (POINTER_BOUNDS_P (arg))
+	    continue;
+
 	  /* We cannot distinguish a varargs function from the case
 	     of excess parameters, still deferring the inlining decision
 	     to the callee is possible.  */
 	  if (!p)
 	    break;
-	  arg = gimple_call_arg (stmt, i);
+
 	  if (p == error_mark_node
 	      || arg == error_mark_node
 	      || (!types_compatible_p (DECL_ARG_TYPE (p), TREE_TYPE (arg))
 		  && !fold_convertible_p (DECL_ARG_TYPE (p), arg)))
             return false;
+
+	  p = DECL_CHAIN (p);
 	}
       if (args_count_match && p)
 	return false;
     }
   else if (parms)
     {
-      for (i = 0, p = parms; i < nargs; i++, p = TREE_CHAIN (p))
+      for (i = 0, p = parms; i < nargs; i++)
 	{
-	  tree arg;
+	  tree arg = gimple_call_arg (stmt, i);
+
+	  /* Skip bound args inserted by Pointer Bounds Checker.  */
+	  if (POINTER_BOUNDS_P (arg))
+	    continue;
+
 	  /* If this is a varargs function defer inlining decision
 	     to callee.  */
 	  if (!p)
 	    break;
-	  arg = gimple_call_arg (stmt, i);
+
 	  if (TREE_VALUE (p) == error_mark_node
 	      || arg == error_mark_node
 	      || TREE_CODE (TREE_VALUE (p)) == VOID_TYPE
 	      || (!types_compatible_p (TREE_VALUE (p), TREE_TYPE (arg))
 		  && !fold_convertible_p (TREE_VALUE (p), arg)))
             return false;
+
+	  p = TREE_CHAIN (p);
 	}
     }
   else
diff --git a/gcc/gimple.c b/gcc/gimple.c
index 20f6010..0fc704b 100644
--- a/gcc/gimple.c
+++ b/gcc/gimple.c
@@ -3043,20 +3043,27 @@ canonicalize_cond_expr_cond (tree t)
 }
 
 /* Build a GIMPLE_CALL identical to STMT but skipping the arguments in
-   the positions marked by the set ARGS_TO_SKIP.  */
+   the positions marked by the set ARGS_TO_SKIP.  ARGS_TO_SKIP does not
+   take into account bounds arguments.  Bounds passed for skipped args
+   are also skipped.  */
 
 gimple
 gimple_call_copy_skip_args (gimple stmt, bitmap args_to_skip)
 {
-  int i;
+  int i, bit;
   int nargs = gimple_call_num_args (stmt);
   vec<tree> vargs;
   vargs.create (nargs);
   gimple new_stmt;
 
-  for (i = 0; i < nargs; i++)
-    if (!bitmap_bit_p (args_to_skip, i))
-      vargs.quick_push (gimple_call_arg (stmt, i));
+  for (i = 0, bit = 0; i < nargs; i++, bit++)
+      if (POINTER_BOUNDS_P (gimple_call_arg (stmt, i)))
+	{
+	  if (!bitmap_bit_p (args_to_skip, --bit))
+	    vargs.quick_push (gimple_call_arg (stmt, i));
+	}
+      else if (!bitmap_bit_p (args_to_skip, bit))
+	  vargs.quick_push (gimple_call_arg (stmt, i));
 
   if (gimple_call_internal_p (stmt))
     new_stmt = gimple_build_call_internal_vec (gimple_call_internal_fn (stmt),
@@ -3702,6 +3709,9 @@ validate_call (gimple stmt, tree fndecl)
       if (!targs)
 	return true;
       tree arg = gimple_call_arg (stmt, i);
+      /* Skip bounds.  */
+      if (POINTER_BOUNDS_P (arg))
+	continue;
       if (INTEGRAL_TYPE_P (TREE_TYPE (arg))
 	  && INTEGRAL_TYPE_P (TREE_VALUE (targs)))
 	;

Re: [PATCH, MPX, 2/X] Pointers Checker [10/25] Calls copy and verification

[Jeff Law]

On 11/07/13 04:50, Ilya Enkovich wrote:
> Hi,
>
> Here is an updated patch version.
I think this needs to hold until we have a consensus on what the 
parameter passing looks like for bounded pointers.

Jeff

Re: [PATCH, MPX, 2/X] Pointers Checker [10/25] Calls copy and verification

[Richard Biener]

On Thu, Nov 7, 2013 at 7:55 PM, Jeff Law <law@redhat.com> wrote:
> On 11/07/13 04:50, Ilya Enkovich wrote:
>>
>> Hi,
>>
>> Here is an updated patch version.
>
> I think this needs to hold until we have a consensus on what the parameter
> passing looks like for bounded pointers.

I still think the best thing to do on GIMPLE is

arg_2 = __builtin_ia32_bnd_arg (arg_1(D));
foo (arg_2);

that is, make the parameter an implicit pair of {value, bound} where
the bound is determined by the value going through a bound association
builtin.  No extra explicit argument to the calls so arguments match
the fndecl and fntype.  All the complexity is defered to the expander
which can trivially lookup bound arguments via the SSA def (I suppose
it does that anyway now for getting at the explicit bound argument now).

As far as I can see (well, think), all currently passed bound arguments
are the return value of such builtin already.

Richard.



> Jeff

Re: [PATCH, MPX, 2/X] Pointers Checker [10/25] Calls copy and verification

[Ilya Enkovich]

2013/11/8 Richard Biener <richard.guenther@gmail.com>:
> On Thu, Nov 7, 2013 at 7:55 PM, Jeff Law <law@redhat.com> wrote:
>> On 11/07/13 04:50, Ilya Enkovich wrote:
>>>
>>> Hi,
>>>
>>> Here is an updated patch version.
>>
>> I think this needs to hold until we have a consensus on what the parameter
>> passing looks like for bounded pointers.
>
> I still think the best thing to do on GIMPLE is
>
> arg_2 = __builtin_ia32_bnd_arg (arg_1(D));
> foo (arg_2);
>
> that is, make the parameter an implicit pair of {value, bound} where
> the bound is determined by the value going through a bound association
> builtin.  No extra explicit argument to the calls so arguments match
> the fndecl and fntype.  All the complexity is defered to the expander
> which can trivially lookup bound arguments via the SSA def (I suppose
> it does that anyway now for getting at the explicit bound argument now).
>
> As far as I can see (well, think), all currently passed bound arguments
> are the return value of such builtin already.

All bounds are result of different builtin calls. Small example:

int *global_p;
void foo (int *p)
{
  int buf[10];
  bar (p, buf, global_p);
}


It is translated into:

  __bound_tmp.1_7 = __builtin_ia32_bndmk (&buf, 40);
  __bound_tmp.1_6 = __builtin_ia32_arg_bnd (p_3(D)(ab));
  global_p.0_2 = global_p;
  __bound_tmp.1_8 = __builtin_ia32_bndldx (&global_p, global_p.0_2);
  bar (p_3(D)(ab), __bound_tmp.1_6, &buf, __bound_tmp.1_7,
global_p.0_2, __bound_tmp.1_8);

Bounds binding via calls as you suggest may be done as following:

  __bound_tmp.1_7 = __builtin_ia32_bndmk (&buf, 40);
  __bound_tmp.1_6 = __builtin_ia32_arg_bnd (p_3(D)(ab));
  global_p.0_2 = global_p;
  __bound_tmp.1_8 = __builtin_ia32_bndldx (&global_p, global_p.0_2);
  _9 = __builtin_bind_bounds (p_3(D)(ab), __bound_tmp.1_6);
  _10 = __builtin_bind_bounds (&buf, __bound_tmp.1_7);
  _11 = __builtin_bind_bounds (global_p.0_2, __bound_tmp.1_8);
  bar (_9, _10, _11);

Is it close to what you propose?

Ilya
>
> Richard.
>
>
>
>> Jeff

Re: [PATCH, MPX, 2/X] Pointers Checker [10/25] Calls copy and verification

[Jakub Jelinek]

On Fri, Nov 08, 2013 at 10:43:26AM +0100, Richard Biener wrote:
> >> Here is an updated patch version.
> >
> > I think this needs to hold until we have a consensus on what the parameter
> > passing looks like for bounded pointers.
> 
> I still think the best thing to do on GIMPLE is
> 
> arg_2 = __builtin_ia32_bnd_arg (arg_1(D));
> foo (arg_2);

Well, in that case it would likely have to be an internal builtin fn,
because it needs to be a pass through for random argument types, so
return long for long argument, or double for double argument, etc.
Or is that only done for pointer arguments?  Then it could be
like __builtin_assume_aligned, which takes void * and returns void *.

	Jakub

Re: [PATCH, MPX, 2/X] Pointers Checker [10/25] Calls copy and verification

[Richard Biener]

On Fri, Nov 8, 2013 at 11:03 AM, Ilya Enkovich <enkovich.gnu@gmail.com> wrote:
> 2013/11/8 Richard Biener <richard.guenther@gmail.com>:
>> On Thu, Nov 7, 2013 at 7:55 PM, Jeff Law <law@redhat.com> wrote:
>>> On 11/07/13 04:50, Ilya Enkovich wrote:
>>>>
>>>> Hi,
>>>>
>>>> Here is an updated patch version.
>>>
>>> I think this needs to hold until we have a consensus on what the parameter
>>> passing looks like for bounded pointers.
>>
>> I still think the best thing to do on GIMPLE is
>>
>> arg_2 = __builtin_ia32_bnd_arg (arg_1(D));
>> foo (arg_2);
>>
>> that is, make the parameter an implicit pair of {value, bound} where
>> the bound is determined by the value going through a bound association
>> builtin.  No extra explicit argument to the calls so arguments match
>> the fndecl and fntype.  All the complexity is defered to the expander
>> which can trivially lookup bound arguments via the SSA def (I suppose
>> it does that anyway now for getting at the explicit bound argument now).
>>
>> As far as I can see (well, think), all currently passed bound arguments
>> are the return value of such builtin already.
>
> All bounds are result of different builtin calls. Small example:
>
> int *global_p;
> void foo (int *p)
> {
>   int buf[10];
>   bar (p, buf, global_p);
> }
>
>
> It is translated into:
>
>   __bound_tmp.1_7 = __builtin_ia32_bndmk (&buf, 40);
>   __bound_tmp.1_6 = __builtin_ia32_arg_bnd (p_3(D)(ab));
>   global_p.0_2 = global_p;
>   __bound_tmp.1_8 = __builtin_ia32_bndldx (&global_p, global_p.0_2);
>   bar (p_3(D)(ab), __bound_tmp.1_6, &buf, __bound_tmp.1_7,
> global_p.0_2, __bound_tmp.1_8);
>
> Bounds binding via calls as you suggest may be done as following:
>
>   __bound_tmp.1_7 = __builtin_ia32_bndmk (&buf, 40);
>   __bound_tmp.1_6 = __builtin_ia32_arg_bnd (p_3(D)(ab));
>   global_p.0_2 = global_p;
>   __bound_tmp.1_8 = __builtin_ia32_bndldx (&global_p, global_p.0_2);
>   _9 = __builtin_bind_bounds (p_3(D)(ab), __bound_tmp.1_6);
>   _10 = __builtin_bind_bounds (&buf, __bound_tmp.1_7);
>   _11 = __builtin_bind_bounds (global_p.0_2, __bound_tmp.1_8);
>   bar (_9, _10, _11);
>
> Is it close to what you propose?

Yes.

Richard.

> Ilya
>>
>> Richard.
>>
>>
>>
>>> Jeff

Re: [PATCH, MPX, 2/X] Pointers Checker [10/25] Calls copy and verification

[Ilya Enkovich]

2013/11/11 Richard Biener <richard.guenther@gmail.com>:
> On Fri, Nov 8, 2013 at 11:03 AM, Ilya Enkovich <enkovich.gnu@gmail.com> wrote:
>> 2013/11/8 Richard Biener <richard.guenther@gmail.com>:
>>> On Thu, Nov 7, 2013 at 7:55 PM, Jeff Law <law@redhat.com> wrote:
>>>> On 11/07/13 04:50, Ilya Enkovich wrote:
>>>>>
>>>>> Hi,
>>>>>
>>>>> Here is an updated patch version.
>>>>
>>>> I think this needs to hold until we have a consensus on what the parameter
>>>> passing looks like for bounded pointers.
>>>
>>> I still think the best thing to do on GIMPLE is
>>>
>>> arg_2 = __builtin_ia32_bnd_arg (arg_1(D));
>>> foo (arg_2);
>>>
>>> that is, make the parameter an implicit pair of {value, bound} where
>>> the bound is determined by the value going through a bound association
>>> builtin.  No extra explicit argument to the calls so arguments match
>>> the fndecl and fntype.  All the complexity is defered to the expander
>>> which can trivially lookup bound arguments via the SSA def (I suppose
>>> it does that anyway now for getting at the explicit bound argument now).
>>>
>>> As far as I can see (well, think), all currently passed bound arguments
>>> are the return value of such builtin already.
>>
>> All bounds are result of different builtin calls. Small example:
>>
>> int *global_p;
>> void foo (int *p)
>> {
>>   int buf[10];
>>   bar (p, buf, global_p);
>> }
>>
>>
>> It is translated into:
>>
>>   __bound_tmp.1_7 = __builtin_ia32_bndmk (&buf, 40);
>>   __bound_tmp.1_6 = __builtin_ia32_arg_bnd (p_3(D)(ab));
>>   global_p.0_2 = global_p;
>>   __bound_tmp.1_8 = __builtin_ia32_bndldx (&global_p, global_p.0_2);
>>   bar (p_3(D)(ab), __bound_tmp.1_6, &buf, __bound_tmp.1_7,
>> global_p.0_2, __bound_tmp.1_8);
>>
>> Bounds binding via calls as you suggest may be done as following:
>>
>>   __bound_tmp.1_7 = __builtin_ia32_bndmk (&buf, 40);
>>   __bound_tmp.1_6 = __builtin_ia32_arg_bnd (p_3(D)(ab));
>>   global_p.0_2 = global_p;
>>   __bound_tmp.1_8 = __builtin_ia32_bndldx (&global_p, global_p.0_2);
>>   _9 = __builtin_bind_bounds (p_3(D)(ab), __bound_tmp.1_6);
>>   _10 = __builtin_bind_bounds (&buf, __bound_tmp.1_7);
>>   _11 = __builtin_bind_bounds (global_p.0_2, __bound_tmp.1_8);
>>   bar (_9, _10, _11);
>>
>> Is it close to what you propose?
>
> Yes.

Is there a way to bind bounds with structures in a similar way?  For
SSA names I may easily find definition and check if it is a binding
builtin call. But for structures it is not possible.  The way I see it
to mark all such args as addressable and load required bounds on
expand pass.

Ilya
>
> Richard.
>
>> Ilya
>>>
>>> Richard.
>>>
>>>
>>>
>>>> Jeff

Re: [PATCH, MPX, 2/X] Pointers Checker [10/25] Calls copy and verification

[Richard Biener]

On Mon, Nov 11, 2013 at 3:00 PM, Ilya Enkovich <enkovich.gnu@gmail.com> wrote:
> 2013/11/11 Richard Biener <richard.guenther@gmail.com>:
>> On Fri, Nov 8, 2013 at 11:03 AM, Ilya Enkovich <enkovich.gnu@gmail.com> wrote:
>>> 2013/11/8 Richard Biener <richard.guenther@gmail.com>:
>>>> On Thu, Nov 7, 2013 at 7:55 PM, Jeff Law <law@redhat.com> wrote:
>>>>> On 11/07/13 04:50, Ilya Enkovich wrote:
>>>>>>
>>>>>> Hi,
>>>>>>
>>>>>> Here is an updated patch version.
>>>>>
>>>>> I think this needs to hold until we have a consensus on what the parameter
>>>>> passing looks like for bounded pointers.
>>>>
>>>> I still think the best thing to do on GIMPLE is
>>>>
>>>> arg_2 = __builtin_ia32_bnd_arg (arg_1(D));
>>>> foo (arg_2);
>>>>
>>>> that is, make the parameter an implicit pair of {value, bound} where
>>>> the bound is determined by the value going through a bound association
>>>> builtin.  No extra explicit argument to the calls so arguments match
>>>> the fndecl and fntype.  All the complexity is defered to the expander
>>>> which can trivially lookup bound arguments via the SSA def (I suppose
>>>> it does that anyway now for getting at the explicit bound argument now).
>>>>
>>>> As far as I can see (well, think), all currently passed bound arguments
>>>> are the return value of such builtin already.
>>>
>>> All bounds are result of different builtin calls. Small example:
>>>
>>> int *global_p;
>>> void foo (int *p)
>>> {
>>>   int buf[10];
>>>   bar (p, buf, global_p);
>>> }
>>>
>>>
>>> It is translated into:
>>>
>>>   __bound_tmp.1_7 = __builtin_ia32_bndmk (&buf, 40);
>>>   __bound_tmp.1_6 = __builtin_ia32_arg_bnd (p_3(D)(ab));
>>>   global_p.0_2 = global_p;
>>>   __bound_tmp.1_8 = __builtin_ia32_bndldx (&global_p, global_p.0_2);
>>>   bar (p_3(D)(ab), __bound_tmp.1_6, &buf, __bound_tmp.1_7,
>>> global_p.0_2, __bound_tmp.1_8);
>>>
>>> Bounds binding via calls as you suggest may be done as following:
>>>
>>>   __bound_tmp.1_7 = __builtin_ia32_bndmk (&buf, 40);
>>>   __bound_tmp.1_6 = __builtin_ia32_arg_bnd (p_3(D)(ab));
>>>   global_p.0_2 = global_p;
>>>   __bound_tmp.1_8 = __builtin_ia32_bndldx (&global_p, global_p.0_2);
>>>   _9 = __builtin_bind_bounds (p_3(D)(ab), __bound_tmp.1_6);
>>>   _10 = __builtin_bind_bounds (&buf, __bound_tmp.1_7);
>>>   _11 = __builtin_bind_bounds (global_p.0_2, __bound_tmp.1_8);
>>>   bar (_9, _10, _11);
>>>
>>> Is it close to what you propose?
>>
>> Yes.
>
> Is there a way to bind bounds with structures in a similar way?

Not to have them easy to lookup in the SSA web.  A long time ago
I proposed to make SSA aggregates possible, so you could do

 tem_2 = __internal_bind_bounds (aggr(D), __bound_tmp.1_3,
__bound_tmp.1_4, ...);
 bar (tem_2);

(originally the SSA aggregates were supposed to make copy-propgagation
possible using the SSA copy propagator, and of course I needed it for
the middle-end array work)

Not sure if that will give other people the creeps (expand would
never expand the "load" from tem_2 but instead handle aggr as
parameter).  A slight complication is due to alias analysis
which would be need to taught that bar performs a load of aggr.

Richard.

>  For
> SSA names I may easily find definition and check if it is a binding
> builtin call. But for structures it is not possible.  The way I see it
> to mark all such args as addressable and load required bounds on
> expand pass.
>
> Ilya
>>
>> Richard.
>>
>>> Ilya
>>>>
>>>> Richard.
>>>>
>>>>
>>>>
>>>>> Jeff

Re: [PATCH, MPX, 2/X] Pointers Checker [10/25] Calls copy and verification

[Ilya Enkovich]

2013/11/11 Richard Biener <richard.guenther@gmail.com>:
> On Mon, Nov 11, 2013 at 3:00 PM, Ilya Enkovich <enkovich.gnu@gmail.com> wrote:
>> 2013/11/11 Richard Biener <richard.guenther@gmail.com>:
>>> On Fri, Nov 8, 2013 at 11:03 AM, Ilya Enkovich <enkovich.gnu@gmail.com> wrote:
>>>> 2013/11/8 Richard Biener <richard.guenther@gmail.com>:
>>>>> On Thu, Nov 7, 2013 at 7:55 PM, Jeff Law <law@redhat.com> wrote:
>>>>>> On 11/07/13 04:50, Ilya Enkovich wrote:
>>>>>>>
>>>>>>> Hi,
>>>>>>>
>>>>>>> Here is an updated patch version.
>>>>>>
>>>>>> I think this needs to hold until we have a consensus on what the parameter
>>>>>> passing looks like for bounded pointers.
>>>>>
>>>>> I still think the best thing to do on GIMPLE is
>>>>>
>>>>> arg_2 = __builtin_ia32_bnd_arg (arg_1(D));
>>>>> foo (arg_2);
>>>>>
>>>>> that is, make the parameter an implicit pair of {value, bound} where
>>>>> the bound is determined by the value going through a bound association
>>>>> builtin.  No extra explicit argument to the calls so arguments match
>>>>> the fndecl and fntype.  All the complexity is defered to the expander
>>>>> which can trivially lookup bound arguments via the SSA def (I suppose
>>>>> it does that anyway now for getting at the explicit bound argument now).
>>>>>
>>>>> As far as I can see (well, think), all currently passed bound arguments
>>>>> are the return value of such builtin already.
>>>>
>>>> All bounds are result of different builtin calls. Small example:
>>>>
>>>> int *global_p;
>>>> void foo (int *p)
>>>> {
>>>>   int buf[10];
>>>>   bar (p, buf, global_p);
>>>> }
>>>>
>>>>
>>>> It is translated into:
>>>>
>>>>   __bound_tmp.1_7 = __builtin_ia32_bndmk (&buf, 40);
>>>>   __bound_tmp.1_6 = __builtin_ia32_arg_bnd (p_3(D)(ab));
>>>>   global_p.0_2 = global_p;
>>>>   __bound_tmp.1_8 = __builtin_ia32_bndldx (&global_p, global_p.0_2);
>>>>   bar (p_3(D)(ab), __bound_tmp.1_6, &buf, __bound_tmp.1_7,
>>>> global_p.0_2, __bound_tmp.1_8);
>>>>
>>>> Bounds binding via calls as you suggest may be done as following:
>>>>
>>>>   __bound_tmp.1_7 = __builtin_ia32_bndmk (&buf, 40);
>>>>   __bound_tmp.1_6 = __builtin_ia32_arg_bnd (p_3(D)(ab));
>>>>   global_p.0_2 = global_p;
>>>>   __bound_tmp.1_8 = __builtin_ia32_bndldx (&global_p, global_p.0_2);
>>>>   _9 = __builtin_bind_bounds (p_3(D)(ab), __bound_tmp.1_6);
>>>>   _10 = __builtin_bind_bounds (&buf, __bound_tmp.1_7);
>>>>   _11 = __builtin_bind_bounds (global_p.0_2, __bound_tmp.1_8);
>>>>   bar (_9, _10, _11);
>>>>
>>>> Is it close to what you propose?
>>>
>>> Yes.
>>
>> Is there a way to bind bounds with structures in a similar way?
>
> Not to have them easy to lookup in the SSA web.  A long time ago
> I proposed to make SSA aggregates possible, so you could do
>
>  tem_2 = __internal_bind_bounds (aggr(D), __bound_tmp.1_3,
> __bound_tmp.1_4, ...);
>  bar (tem_2);
>
> (originally the SSA aggregates were supposed to make copy-propgagation
> possible using the SSA copy propagator, and of course I needed it for
> the middle-end array work)
>
> Not sure if that will give other people the creeps (expand would
> never expand the "load" from tem_2 but instead handle aggr as
> parameter).  A slight complication is due to alias analysis
> which would be need to taught that bar performs a load of aggr.

It would require bounds loading for aggr before __internal_bind_bounds
anyway.  So, why not to do it in expand?  I just need to mark calls
with a flag (which you've proposed few times already) to let expand
know when it should load bounds.  Having SSA aggregates would be nice
but I suspect it has much higher impact then loading bounds in expand.
 I want to try simpler variant first.

Thanks,
Ilya

>
> Richard.
>
>>  For
>> SSA names I may easily find definition and check if it is a binding
>> builtin call. But for structures it is not possible.  The way I see it
>> to mark all such args as addressable and load required bounds on
>> expand pass.
>>
>> Ilya
>>>
>>> Richard.
>>>
>>>> Ilya
>>>>>
>>>>> Richard.
>>>>>
>>>>>
>>>>>
>>>>>> Jeff

Re: [PATCH, MPX, 2/X] Pointers Checker [10/25] Calls copy and verification

[Richard Biener]

On Mon, Nov 11, 2013 at 3:45 PM, Ilya Enkovich <enkovich.gnu@gmail.com> wrote:
> 2013/11/11 Richard Biener <richard.guenther@gmail.com>:
>> On Mon, Nov 11, 2013 at 3:00 PM, Ilya Enkovich <enkovich.gnu@gmail.com> wrote:
>>> 2013/11/11 Richard Biener <richard.guenther@gmail.com>:
>>>> On Fri, Nov 8, 2013 at 11:03 AM, Ilya Enkovich <enkovich.gnu@gmail.com> wrote:
>>>>> 2013/11/8 Richard Biener <richard.guenther@gmail.com>:
>>>>>> On Thu, Nov 7, 2013 at 7:55 PM, Jeff Law <law@redhat.com> wrote:
>>>>>>> On 11/07/13 04:50, Ilya Enkovich wrote:
>>>>>>>>
>>>>>>>> Hi,
>>>>>>>>
>>>>>>>> Here is an updated patch version.
>>>>>>>
>>>>>>> I think this needs to hold until we have a consensus on what the parameter
>>>>>>> passing looks like for bounded pointers.
>>>>>>
>>>>>> I still think the best thing to do on GIMPLE is
>>>>>>
>>>>>> arg_2 = __builtin_ia32_bnd_arg (arg_1(D));
>>>>>> foo (arg_2);
>>>>>>
>>>>>> that is, make the parameter an implicit pair of {value, bound} where
>>>>>> the bound is determined by the value going through a bound association
>>>>>> builtin.  No extra explicit argument to the calls so arguments match
>>>>>> the fndecl and fntype.  All the complexity is defered to the expander
>>>>>> which can trivially lookup bound arguments via the SSA def (I suppose
>>>>>> it does that anyway now for getting at the explicit bound argument now).
>>>>>>
>>>>>> As far as I can see (well, think), all currently passed bound arguments
>>>>>> are the return value of such builtin already.
>>>>>
>>>>> All bounds are result of different builtin calls. Small example:
>>>>>
>>>>> int *global_p;
>>>>> void foo (int *p)
>>>>> {
>>>>>   int buf[10];
>>>>>   bar (p, buf, global_p);
>>>>> }
>>>>>
>>>>>
>>>>> It is translated into:
>>>>>
>>>>>   __bound_tmp.1_7 = __builtin_ia32_bndmk (&buf, 40);
>>>>>   __bound_tmp.1_6 = __builtin_ia32_arg_bnd (p_3(D)(ab));
>>>>>   global_p.0_2 = global_p;
>>>>>   __bound_tmp.1_8 = __builtin_ia32_bndldx (&global_p, global_p.0_2);
>>>>>   bar (p_3(D)(ab), __bound_tmp.1_6, &buf, __bound_tmp.1_7,
>>>>> global_p.0_2, __bound_tmp.1_8);
>>>>>
>>>>> Bounds binding via calls as you suggest may be done as following:
>>>>>
>>>>>   __bound_tmp.1_7 = __builtin_ia32_bndmk (&buf, 40);
>>>>>   __bound_tmp.1_6 = __builtin_ia32_arg_bnd (p_3(D)(ab));
>>>>>   global_p.0_2 = global_p;
>>>>>   __bound_tmp.1_8 = __builtin_ia32_bndldx (&global_p, global_p.0_2);
>>>>>   _9 = __builtin_bind_bounds (p_3(D)(ab), __bound_tmp.1_6);
>>>>>   _10 = __builtin_bind_bounds (&buf, __bound_tmp.1_7);
>>>>>   _11 = __builtin_bind_bounds (global_p.0_2, __bound_tmp.1_8);
>>>>>   bar (_9, _10, _11);
>>>>>
>>>>> Is it close to what you propose?
>>>>
>>>> Yes.
>>>
>>> Is there a way to bind bounds with structures in a similar way?
>>
>> Not to have them easy to lookup in the SSA web.  A long time ago
>> I proposed to make SSA aggregates possible, so you could do
>>
>>  tem_2 = __internal_bind_bounds (aggr(D), __bound_tmp.1_3,
>> __bound_tmp.1_4, ...);
>>  bar (tem_2);
>>
>> (originally the SSA aggregates were supposed to make copy-propgagation
>> possible using the SSA copy propagator, and of course I needed it for
>> the middle-end array work)
>>
>> Not sure if that will give other people the creeps (expand would
>> never expand the "load" from tem_2 but instead handle aggr as
>> parameter).  A slight complication is due to alias analysis
>> which would be need to taught that bar performs a load of aggr.
>
> It would require bounds loading for aggr before __internal_bind_bounds
> anyway.  So, why not to do it in expand?

Ah, ok I wasn't aware of that.

>  I just need to mark calls
> with a flag (which you've proposed few times already) to let expand
> know when it should load bounds.  Having SSA aggregates would be nice
> but I suspect it has much higher impact then loading bounds in expand.
>  I want to try simpler variant first.

Good.

Richard.

> Thanks,
> Ilya
>
>>
>> Richard.
>>
>>>  For
>>> SSA names I may easily find definition and check if it is a binding
>>> builtin call. But for structures it is not possible.  The way I see it
>>> to mark all such args as addressable and load required bounds on
>>> expand pass.
>>>
>>> Ilya
>>>>
>>>> Richard.
>>>>
>>>>> Ilya
>>>>>>
>>>>>> Richard.
>>>>>>
>>>>>>
>>>>>>
>>>>>>> Jeff

Re: [PATCH, MPX, 2/X] Pointers Checker [10/25] Calls copy and verification

[Ilya Enkovich]

2013/11/7 Jeff Law <law@redhat.com>:
> On 11/07/13 04:50, Ilya Enkovich wrote:
>>
>> Hi,
>>
>> Here is an updated patch version.
>
> I think this needs to hold until we have a consensus on what the parameter
> passing looks like for bounded pointers.

With the new parameters passing model this patch is not needed.

Thanks,
Ilya

>
> Jeff