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

[Ilya Enkovich <enkovich dot gnu at gmail dot com>]

2013/10/25 Jeff Law <law@redhat.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
>

There are currently two known issues with LTO. The first one is ICE in
LTO streamer when it reads instrumented code. The second one is
unitialized flag_check_pointers when code is compiled by lto1 (do you
know why it may happen BTW?). It also causes ICE beacause instrumented
code is met when not expected. Of course, I'll fix these problems
anyway, but I was going to allow '-fcheck-pointers -flto' only when
checker testsuite has 100% pass rate with lto.

Ilya