Re: [PATCH, i386, Pointer Bounds Checker 18/x] Expand instrumented builtin function calls
Since we still don't have a final decision about how instrumented builtin functions would look like, I replace this patch with a new one which assumes we don't instrument them for now. It only has an additional init for va_start and also makes sure there is no instrumented builtin calls. All instrumented builtins related codes will go into a separate series. Thanks, Ilya -- 2014-10-03 Ilya Enkovich ilya.enkov...@intel.com * builtins.c: Include tree-chkp.h and rtl-chkp.h. (std_expand_builtin_va_start): Init bounds for va_list. (expand_builtin): Do not allow instrumented calls. diff --git a/gcc/builtins.c b/gcc/builtins.c index 78ac91f..f0bb55a 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)); @@ -4319,6 +4321,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. */ @@ -5824,6 +5833,8 @@ expand_builtin (tree exp, rtx target, rtx subtarget, enum machine_mode mode, } } + gcc_assert (!CALL_WITH_BOUNDS_P (exp)); + switch (fcode) { CASE_FLT_FN (BUILT_IN_FABS): @@ -6841,7 +6852,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
Re: [PATCH, i386, Pointer Bounds Checker 18/x] Expand instrumented builtin function calls
On 10/03/14 02:47, Ilya Enkovich wrote: Since we still don't have a final decision about how instrumented builtin functions would look like, I replace this patch with a new one which assumes we don't instrument them for now. It only has an additional init for va_start and also makes sure there is no instrumented builtin calls. All instrumented builtins related codes will go into a separate series. Thanks, Ilya -- 2014-10-03 Ilya Enkovich ilya.enkov...@intel.com * builtins.c: Include tree-chkp.h and rtl-chkp.h. (std_expand_builtin_va_start): Init bounds for va_list. (expand_builtin): Do not allow instrumented calls. OK. Obviously we'll want to return to the builtins at some point in the relatively new future ;-) jeff
Re: [PATCH, i386, Pointer Bounds Checker 18/x] Expand instrumented builtin function calls
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
Re: [PATCH, i386, Pointer Bounds Checker 18/x] Expand instrumented builtin function calls
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,
Re: [PATCH, i386, Pointer Bounds Checker 18/x] Expand instrumented builtin function calls
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. 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!)? 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 =
[PATCH, i386, Pointer Bounds Checker 18/x] Expand instrumented builtin function calls
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. 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), +