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

login
register
mail settings
Submitter Ilya Enkovich
Date Oct. 31, 2013, 9:24 a.m.
Message ID <20131031092436.GF54327@msticlxl57.ims.intel.com>
Download mbox | patch
Permalink /patch/287440/
State New
Headers show

Comments

Ilya Enkovich - Oct. 31, 2013, 9:24 a.m.
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.
Richard Guenther - Nov. 4, 2013, 1:35 p.m.
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)))
>         ;
Ilya Enkovich - Nov. 5, 2013, 1:30 p.m.
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)))
>>         ;

Patch

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)))
 	;