This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PATCH, MPX, 2/X] Pointers Checker [10/25] Calls copy and verification
- From: Ilya Enkovich <enkovich dot gnu at gmail dot com>
- To: Richard Biener <richard dot guenther at gmail dot com>
- Cc: GCC Patches <gcc-patches at gcc dot gnu dot org>
- Date: Tue, 5 Nov 2013 17:30:23 +0400
- Subject: Re: [PATCH, MPX, 2/X] Pointers Checker [10/25] Calls copy and verification
- Authentication-results: sourceware.org; auth=none
- References: <20131031092436 dot GF54327 at msticlxl57 dot ims dot intel dot com> <CAFiYyc0uNbo8xPRQYy6mt+tAe0qH8VZs0=FcGSH2cPr+EmYw0w at mail dot gmail dot com>
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)))
>> ;