Re: [PATCH, MPX, 2/X] Pointers Checker [2/25] Builtins

[Jeff Law <law at redhat dot com>]

On 10/21/13 05:49, Ilya Enkovich wrote:
> Hi,
>
> This patch introduces built-in functions used by Pointers Checker and flag to enable Pointers Checker. Builtins available for user are expanded in expand_builtin. All other builtins are not allowed in expand until generic version of Pointers Cheker is implemented.
>
> Bootstrapped and tested on linux-x86_64.
>
> Thanks,
> Ilya
> --
>
> gcc/
>
> 2013-10-04  Ilya Enkovich  <ilya.enkovich@intel.com>
>
> 	* builtin-types.def (BT_FN_VOID_CONST_PTR): New.
> 	(BT_FN_PTR_CONST_PTR): New.
> 	(BT_FN_CONST_PTR_CONST_PTR): New.
> 	(BT_FN_PTR_CONST_PTR_SIZE): New.
> 	(BT_FN_PTR_CONST_PTR_CONST_PTR): New.
> 	(BT_FN_VOID_PTRPTR_CONST_PTR): New.
> 	(BT_FN_VOID_CONST_PTR_SIZE): New.
> 	(BT_FN_PTR_CONST_PTR_CONST_PTR_SIZE): New.
> 	* chkp-builtins.def: New.
> 	* builtins.def: include chkp-builtins.def.
> 	(DEF_CHKP_BUILTIN): New.
> 	* builtins.c (expand_builtin): Support BUILT_IN_CHKP_INIT_PTR_BOUNDS,
> 	BUILT_IN_CHKP_NULL_PTR_BOUNDS, BUILT_IN_CHKP_COPY_PTR_BOUNDS,
> 	BUILT_IN_CHKP_CHECK_PTR_LBOUNDS, BUILT_IN_CHKP_CHECK_PTR_UBOUNDS,
> 	BUILT_IN_CHKP_CHECK_PTR_BOUNDS, BUILT_IN_CHKP_SET_PTR_BOUNDS,
> 	BUILT_IN_CHKP_NARROW_PTR_BOUNDS, BUILT_IN_CHKP_STORE_PTR_BOUNDS,
> 	BUILT_IN_CHKP_GET_PTR_LBOUND, BUILT_IN_CHKP_GET_PTR_UBOUND,
> 	BUILT_IN_CHKP_BNDMK, BUILT_IN_CHKP_BNDSTX, BUILT_IN_CHKP_BNDCL,
> 	BUILT_IN_CHKP_BNDCU, BUILT_IN_CHKP_BNDLDX, BUILT_IN_CHKP_BNDRET,
> 	BUILT_IN_CHKP_INTERSECT, BUILT_IN_CHKP_ARG_BND, BUILT_IN_CHKP_NARROW,
> 	BUILT_IN_CHKP_EXTRACT_LOWER, BUILT_IN_CHKP_EXTRACT_UPPER.
> 	* common.opt (fcheck-pointers): New.
> 	* toplev.c (process_options): Check Pointers Checker is supported.
> 	* doc/extend.texi: Document Pointers Checker built-in functions.
Just a few minor comments.

> --- a/gcc/common.opt
> +++ b/gcc/common.opt
> @@ -874,6 +874,11 @@ fbounds-check
>   Common Report Var(flag_bounds_check)
>   Generate code to check bounds before indexing arrays
>
> +fcheck-pointers
> +Common Report Var(flag_check_pointers)
> +Add pointers checker instrumentation.  fchkp-* flags are used to
> +control instrumentation.  Currently available for C, C++ and ObjC.
> +
I'd probably use "pointer bounds checking" rather than "pointers 
checker".  It's a nit, but most folks have heard the term "pointer 
bounds checking", but few probabaly use "pointers checker".

I think you make several references to "pointers checker" that are 
probably best reworded slightly to use "pointer bounds checker"


> diff --git a/gcc/toplev.c b/gcc/toplev.c
> index feba051..285b36d 100644
> --- a/gcc/toplev.c
> +++ b/gcc/toplev.c
> @@ -1290,6 +1290,18 @@ process_options (void)
>     if (flag_mudflap && flag_lto)
>       sorry ("mudflap cannot be used together with link-time optimization");
>
> +  if (flag_check_pointers)
> +    {
> +      if (flag_lto)
> +	sorry ("Pointers checker is not yet fully supported for link-time optimization");
What was the final resolution of this?  Like jsm, this seems to me to be 
papering over a problem elsewhere.

I'll pre-approve this patch with the terminology change and the flag_lto 
hack removed.

jeff