Ping
2014-06-06 13:32 GMT+04:00 Ilya Enkovich <enkovich....@gmail.com>:
> 2014-06-03 12:46 GMT+04:00 Richard Biener <richard.guent...@gmail.com>:
>> On Mon, Jun 2, 2014 at 4:51 PM, Ilya Enkovich <enkovich....@gmail.com> wrote:
>>> Hi,
>>>
>>> This patch adds support for normal builtin function calls (target ones are
>>> not instrumented). The basic idea of the patch is to make call expr copy
>>> with no bounds and expand it instead. If expr is going to be emitted as a
>>> function call then original instrumented expr takes place. Handle string
>>> function separately because they are of high interest for the checker.
>>
>> It seems to be this should be user-controllable (always expand builtins with
>> bounds as calls, or not), rather than deciding what is of interest yourself.
>
> User has no idea what can be transformed into a builtin call. He may
> pragmas in his source code to control instrumentation. Compiler just
> tries to cover as much code as it can but reasonable compromise with
> performance should be made.
>
>>
>> From a high-level perspective the expansion path doing inline expansion
>> should be separated from that expanding as a call (or expanding as a
>> different call). I don't like that building of yet another GENERIC call expr
>> in this code ... it goes very much backwards of the idea that we should
>> expand from the GIMPLE representation. You are making such transition
>> even harder.
>>
>> Can't you do that frobbing from cfgexpand.c:expand_call_stmt instead
>> (please!)?
>
> I need both instrumented and not instrumented versions of call in
> expand_builtin, so I do not see how it may be handled in
> expand_call_stmt.
>
> Ilya
>>
>> Thanks,
>> Richard.
>>
>>> Bootstrapped and tested on linux-x86_64.
>>>
>>> Thanks,
>>> Ilya
>>> --
>>> gcc/
>>>
>>> 2014-06-02 Ilya Enkovich <ilya.enkov...@intel.com>
>>>
>>> * builtins.c: Include rtl-chkp.h, tree-chkp.h.
>>> (expand_builtin_mempcpy_args): Add orig exp as argument.
>>> Support BUILT_IN_CHKP_MEMPCPY_NOBND_NOCHK.
>>> (expand_builtin_mempcpy): Adjust expand_builtin_mempcpy_args call.
>>> (expand_builtin_stpcpy): Likewise.
>>> (expand_builtin_memset_args): Support
>>> BUILT_IN_CHKP_MEMSET_NOBND_NOCHK.
>>> (std_expand_builtin_va_start): Initialize bounds for va_list.
>>> (expand_builtin): Support instrumented calls.
>>>
>>>
>>> diff --git a/gcc/builtins.c b/gcc/builtins.c
>>> index 7357124..c0140e1 100644
>>> --- a/gcc/builtins.c
>>> +++ b/gcc/builtins.c
>>> @@ -59,6 +59,8 @@ along with GCC; see the file COPYING3. If not see
>>> #include "builtins.h"
>>> #include "ubsan.h"
>>> #include "cilk.h"
>>> +#include "tree-chkp.h"
>>> +#include "rtl-chkp.h"
>>>
>>>
>>> static tree do_mpc_arg1 (tree, tree, int (*)(mpc_ptr, mpc_srcptr,
>>> mpc_rnd_t));
>>> @@ -124,7 +126,7 @@ static rtx builtin_memcpy_read_str (void *,
>>> HOST_WIDE_INT, enum machine_mode);
>>> static rtx expand_builtin_memcpy (tree, rtx);
>>> static rtx expand_builtin_mempcpy (tree, rtx, enum machine_mode);
>>> static rtx expand_builtin_mempcpy_args (tree, tree, tree, rtx,
>>> - enum machine_mode, int);
>>> + enum machine_mode, int, tree);
>>> static rtx expand_builtin_strcpy (tree, rtx);
>>> static rtx expand_builtin_strcpy_args (tree, tree, rtx);
>>> static rtx expand_builtin_stpcpy (tree, rtx, enum machine_mode);
>>> @@ -3284,7 +3286,8 @@ expand_builtin_mempcpy (tree exp, rtx target, enum
>>> machine_mode mode)
>>> tree src = CALL_EXPR_ARG (exp, 1);
>>> tree len = CALL_EXPR_ARG (exp, 2);
>>> return expand_builtin_mempcpy_args (dest, src, len,
>>> - target, mode, /*endp=*/ 1);
>>> + target, mode, /*endp=*/ 1,
>>> + exp);
>>> }
>>> }
>>>
>>> @@ -3296,10 +3299,23 @@ expand_builtin_mempcpy (tree exp, rtx target, enum
>>> machine_mode mode)
>>>
>>> static rtx
>>> expand_builtin_mempcpy_args (tree dest, tree src, tree len,
>>> - rtx target, enum machine_mode mode, int endp)
>>> + rtx target, enum machine_mode mode, int endp,
>>> + tree orig_exp)
>>> {
>>> + tree fndecl = get_callee_fndecl (orig_exp);
>>> +
>>> /* If return value is ignored, transform mempcpy into memcpy. */
>>> - if (target == const0_rtx && builtin_decl_implicit_p (BUILT_IN_MEMCPY))
>>> + if (target == const0_rtx
>>> + && DECL_FUNCTION_CODE (fndecl) == BUILT_IN_CHKP_MEMPCPY_NOBND_NOCHK
>>> + && builtin_decl_implicit_p (BUILT_IN_CHKP_MEMCPY_NOBND_NOCHK))
>>> + {
>>> + tree fn = builtin_decl_implicit (BUILT_IN_CHKP_MEMCPY_NOBND_NOCHK);
>>> + tree result = build_call_nofold_loc (UNKNOWN_LOCATION, fn, 3,
>>> + dest, src, len);
>>> + return expand_expr (result, target, mode, EXPAND_NORMAL);
>>> + }
>>> + else if (target == const0_rtx
>>> + && builtin_decl_implicit_p (BUILT_IN_MEMCPY))
>>> {
>>> tree fn = builtin_decl_implicit (BUILT_IN_MEMCPY);
>>> tree result = build_call_nofold_loc (UNKNOWN_LOCATION, fn, 3,
>>> @@ -3484,7 +3500,8 @@ expand_builtin_stpcpy (tree exp, rtx target, enum
>>> machine_mode mode)
>>>
>>> lenp1 = size_binop_loc (loc, PLUS_EXPR, len, ssize_int (1));
>>> ret = expand_builtin_mempcpy_args (dst, src, lenp1,
>>> - target, mode, /*endp=*/2);
>>> + target, mode, /*endp=*/2,
>>> + exp);
>>>
>>> if (ret)
>>> return ret;
>>> @@ -3778,7 +3795,8 @@ expand_builtin_memset_args (tree dest, tree val, tree
>>> len,
>>> do_libcall:
>>> fndecl = get_callee_fndecl (orig_exp);
>>> fcode = DECL_FUNCTION_CODE (fndecl);
>>> - if (fcode == BUILT_IN_MEMSET)
>>> + if (fcode == BUILT_IN_MEMSET
>>> + || fcode == BUILT_IN_CHKP_MEMSET_NOBND_NOCHK)
>>> fn = build_call_nofold_loc (EXPR_LOCATION (orig_exp), fndecl, 3,
>>> dest, val, len);
>>> else if (fcode == BUILT_IN_BZERO)
>>> @@ -4330,6 +4348,13 @@ std_expand_builtin_va_start (tree valist, rtx
>>> nextarg)
>>> {
>>> rtx va_r = expand_expr (valist, NULL_RTX, VOIDmode, EXPAND_WRITE);
>>> convert_move (va_r, nextarg, 0);
>>> +
>>> + /* We do not have any valid bounds for the pointer, so
>>> + just store zero bounds for it. */
>>> + if (chkp_function_instrumented_p (current_function_decl))
>>> + chkp_expand_bounds_reset_for_mem (valist,
>>> + make_tree (TREE_TYPE (valist),
>>> + nextarg));
>>> }
>>>
>>> /* Expand EXP, a call to __builtin_va_start. */
>>> @@ -5793,6 +5818,7 @@ expand_builtin (tree exp, rtx target, rtx subtarget,
>>> enum machine_mode mode,
>>> enum built_in_function fcode = DECL_FUNCTION_CODE (fndecl);
>>> enum machine_mode target_mode = TYPE_MODE (TREE_TYPE (exp));
>>> int flags;
>>> + tree orig_exp = exp;
>>>
>>> if (DECL_BUILT_IN_CLASS (fndecl) == BUILT_IN_MD)
>>> return targetm.expand_builtin (exp, target, subtarget, mode, ignore);
>>> @@ -5856,6 +5882,41 @@ expand_builtin (tree exp, rtx target, rtx subtarget,
>>> enum machine_mode mode,
>>> }
>>> }
>>>
>>> + /* Currently none of builtin expand functions works with bounds.
>>> + To avoid modification of all expanders we just make a new call
>>> + expression without bound args. The original expression is used
>>> + in case we expand builtin as a call. */
>>> + if (CALL_WITH_BOUNDS_P (exp))
>>> + {
>>> + int new_arg_no = 0;
>>> + tree new_call;
>>> + tree arg;
>>> + call_expr_arg_iterator iter;
>>> + tree *new_args = XALLOCAVEC (tree, call_expr_nargs (exp));
>>> +
>>> + FOR_EACH_CALL_EXPR_ARG (arg, iter, exp)
>>> + if (!POINTER_BOUNDS_P (arg))
>>> + new_args[new_arg_no++] = arg;
>>> +
>>> + if (new_arg_no > 0)
>>> + {
>>> + new_call = build_call_array (TREE_TYPE (exp), fndecl, new_arg_no,
>>> new_args);
>>> + CALL_EXPR_STATIC_CHAIN (new_call) = CALL_EXPR_STATIC_CHAIN (exp);
>>> + CALL_EXPR_FN (new_call) = CALL_EXPR_FN (exp);
>>> + TREE_SIDE_EFFECTS (new_call) = TREE_SIDE_EFFECTS (exp);
>>> + TREE_NOTHROW (new_call) = TREE_NOTHROW (exp);
>>> + CALL_EXPR_TAILCALL (new_call) = CALL_EXPR_TAILCALL (exp);
>>> + CALL_EXPR_RETURN_SLOT_OPT (new_call) = CALL_EXPR_RETURN_SLOT_OPT
>>> (exp);
>>> + CALL_ALLOCA_FOR_VAR_P (new_call) = CALL_ALLOCA_FOR_VAR_P (exp);
>>> + CALL_FROM_THUNK_P (new_call) = CALL_FROM_THUNK_P (exp);
>>> + CALL_EXPR_VA_ARG_PACK (new_call) = CALL_EXPR_VA_ARG_PACK (exp);
>>> + SET_EXPR_LOCATION (new_call, EXPR_LOCATION (exp));
>>> + TREE_SET_BLOCK (new_call, TREE_BLOCK (exp));
>>> +
>>> + exp = new_call;
>>> + }
>>> + }
>>> +
>>> switch (fcode)
>>> {
>>> CASE_FLT_FN (BUILT_IN_FABS):
>>> @@ -6146,60 +6207,116 @@ expand_builtin (tree exp, rtx target, rtx
>>> subtarget, enum machine_mode mode,
>>> break;
>>>
>>> case BUILT_IN_STRLEN:
>>> + if (CALL_WITH_BOUNDS_P (orig_exp))
>>> + break;
>>> target = expand_builtin_strlen (exp, target, target_mode);
>>> if (target)
>>> return target;
>>> break;
>>>
>>> case BUILT_IN_STRCPY:
>>> + if (CALL_WITH_BOUNDS_P (orig_exp))
>>> + break;
>>> target = expand_builtin_strcpy (exp, target);
>>> if (target)
>>> return target;
>>> break;
>>>
>>> case BUILT_IN_STRNCPY:
>>> + if (CALL_WITH_BOUNDS_P (orig_exp))
>>> + break;
>>> target = expand_builtin_strncpy (exp, target);
>>> if (target)
>>> return target;
>>> break;
>>>
>>> case BUILT_IN_STPCPY:
>>> + if (CALL_WITH_BOUNDS_P (orig_exp))
>>> + break;
>>> target = expand_builtin_stpcpy (exp, target, mode);
>>> if (target)
>>> return target;
>>> break;
>>>
>>> case BUILT_IN_MEMCPY:
>>> + case BUILT_IN_CHKP_MEMCPY_NOBND_NOCHK:
>>> + if (CALL_WITH_BOUNDS_P (orig_exp)
>>> + && fcode == BUILT_IN_MEMCPY)
>>> + break;
>>> target = expand_builtin_memcpy (exp, target);
>>> if (target)
>>> - return target;
>>> + {
>>> + /* We need to set returned bounds for instrumented
>>> + calls. */
>>> + if (CALL_WITH_BOUNDS_P (orig_exp))
>>> + {
>>> + rtx bnd = force_reg (BNDmode,
>>> + expand_normal (CALL_EXPR_ARG (orig_exp,
>>> 1)));
>>> + target = chkp_join_splitted_slot (target, bnd);
>>> + }
>>> + return target;
>>> + }
>>> break;
>>>
>>> case BUILT_IN_MEMPCPY:
>>> + case BUILT_IN_CHKP_MEMPCPY_NOBND_NOCHK:
>>> + if (CALL_WITH_BOUNDS_P (orig_exp)
>>> + && fcode == BUILT_IN_MEMPCPY)
>>> + break;
>>> target = expand_builtin_mempcpy (exp, target, mode);
>>> if (target)
>>> - return target;
>>> + {
>>> + /* We need to set returned bounds for instrumented
>>> + calls. */
>>> + if (CALL_WITH_BOUNDS_P (orig_exp))
>>> + {
>>> + rtx bnd = force_reg (BNDmode,
>>> + expand_normal (CALL_EXPR_ARG (orig_exp,
>>> 1)));
>>> + target = chkp_join_splitted_slot (target, bnd);
>>> + }
>>> + return target;
>>> + }
>>> break;
>>>
>>> case BUILT_IN_MEMSET:
>>> + case BUILT_IN_CHKP_MEMSET_NOBND_NOCHK:
>>> + if (CALL_WITH_BOUNDS_P (orig_exp)
>>> + && fcode == BUILT_IN_MEMSET)
>>> + break;
>>> target = expand_builtin_memset (exp, target, mode);
>>> if (target)
>>> - return target;
>>> + {
>>> + /* We need to set returned bounds for instrumented
>>> + calls. */
>>> + if (CALL_WITH_BOUNDS_P (orig_exp))
>>> + {
>>> + rtx bnd = force_reg (BNDmode,
>>> + expand_normal (CALL_EXPR_ARG (orig_exp,
>>> 1)));
>>> + target = chkp_join_splitted_slot (target, bnd);
>>> + }
>>> + return target;
>>> + }
>>> break;
>>>
>>> case BUILT_IN_BZERO:
>>> + if (CALL_WITH_BOUNDS_P (orig_exp))
>>> + break;
>>> target = expand_builtin_bzero (exp);
>>> if (target)
>>> return target;
>>> break;
>>>
>>> case BUILT_IN_STRCMP:
>>> + if (CALL_WITH_BOUNDS_P (orig_exp))
>>> + break;
>>> target = expand_builtin_strcmp (exp, target);
>>> if (target)
>>> return target;
>>> break;
>>>
>>> case BUILT_IN_STRNCMP:
>>> + if (CALL_WITH_BOUNDS_P (orig_exp))
>>> + break;
>>> target = expand_builtin_strncmp (exp, target, mode);
>>> if (target)
>>> return target;
>>> @@ -6207,6 +6324,8 @@ expand_builtin (tree exp, rtx target, rtx subtarget,
>>> enum machine_mode mode,
>>>
>>> case BUILT_IN_BCMP:
>>> case BUILT_IN_MEMCMP:
>>> + if (CALL_WITH_BOUNDS_P (orig_exp))
>>> + break;
>>> target = expand_builtin_memcmp (exp, target, mode);
>>> if (target)
>>> return target;
>>> @@ -6588,14 +6707,17 @@ expand_builtin (tree exp, rtx target, rtx
>>> subtarget, enum machine_mode mode,
>>>
>>> /* If this is turned into an external library call, the weak
>>> parameter
>>> must be dropped to match the expected parameter list. */
>>> - nargs = call_expr_nargs (exp);
>>> + nargs = call_expr_nargs (orig_exp);
>>> vec_alloc (vec, nargs - 1);
>>> - for (z = 0; z < 3; z++)
>>> - vec->quick_push (CALL_EXPR_ARG (exp, z));
>>> - /* Skip the boolean weak parameter. */
>>> - for (z = 4; z < 6; z++)
>>> - vec->quick_push (CALL_EXPR_ARG (exp, z));
>>> - exp = build_call_vec (TREE_TYPE (exp), CALL_EXPR_FN (exp), vec);
>>> + for (z = 0; z < nargs; z++)
>>> + /* Skip the boolean weak parameter. */
>>> + if ((!CALL_WITH_BOUNDS_P (orig_exp) && z == 3)
>>> + || (CALL_WITH_BOUNDS_P (orig_exp) && z == 5))
>>> + continue;
>>> + else
>>> + vec->quick_push (CALL_EXPR_ARG (orig_exp, z));
>>> + orig_exp = build_call_vec (TREE_TYPE (orig_exp),
>>> + CALL_EXPR_FN (orig_exp), vec);
>>> break;
>>> }
>>>
>>> @@ -6819,6 +6941,8 @@ expand_builtin (tree exp, rtx target, rtx subtarget,
>>> enum machine_mode mode,
>>> case BUILT_IN_MEMPCPY_CHK:
>>> case BUILT_IN_MEMMOVE_CHK:
>>> case BUILT_IN_MEMSET_CHK:
>>> + if (CALL_WITH_BOUNDS_P (orig_exp))
>>> + break;
>>> target = expand_builtin_memory_chk (exp, target, mode, fcode);
>>> if (target)
>>> return target;
>>> @@ -6873,7 +6997,7 @@ expand_builtin (tree exp, rtx target, rtx subtarget,
>>> enum machine_mode mode,
>>> case BUILT_IN_CHKP_GET_PTR_UBOUND:
>>> /* We allow user CHKP builtins if Pointer Bounds
>>> Checker is off. */
>>> - if (!flag_check_pointer_bounds)
>>> + if (!chkp_function_instrumented_p (current_function_decl))
>>> {
>>> if (fcode == BUILT_IN_CHKP_SET_PTR_BOUNDS
>>> || fcode == BUILT_IN_CHKP_NARROW_PTR_BOUNDS
>>> @@ -6911,7 +7035,7 @@ expand_builtin (tree exp, rtx target, rtx subtarget,
>>> enum machine_mode mode,
>>>
>>> /* The switch statement above can drop through to cause the function
>>> to be called normally. */
>>> - return expand_call (exp, target, ignore);
>>> + return expand_call (orig_exp, target, ignore);
>>> }
>>>
>>> /* Determine whether a tree node represents a call to a built-in
