2014-11-14 9:49 GMT+03:00 Jeff Law <l...@redhat.com>:
> On 11/06/14 05:10, Ilya Enkovich wrote:
>>
>> Hi,
>>
>> This patch enables instrumentation of chosen builtin calls.
>>
>> Thanks,
>> Ilya
>> --
>> 2014-11-06 Ilya Enkovich <ilya.enkov...@intel.com>
>>
>> * ipa-chkp.c (chkp_versioning): Clone builtin functions.
>> (chkp_instrument_normal_builtin): New.
>> (chkp_add_bounds_to_call_stmt): Support builtin functions.
>> (chkp_replace_function_pointer): Likewise.
>>
>>
>>
>> diff --git a/gcc/tree-chkp.c b/gcc/tree-chkp.c
>> index df7d425..9e2efdb 100644
>> --- a/gcc/tree-chkp.c
>> +++ b/gcc/tree-chkp.c
>> @@ -1586,6 +1586,50 @@ chkp_find_bound_slots (const_tree type, bitmap res)
>> chkp_find_bound_slots_1 (type, res, 0);
>> }
>>
>> +/* Return 1 if call to FNDECL should be instrumented
>> + and 0 otherwise. */
>> +
>> +static bool
>> +chkp_instrument_normal_builtin (tree fndecl)
>> +{
>> + switch (DECL_FUNCTION_CODE (fndecl))
>> + {
>> + case BUILT_IN_STRLEN:
>> + case BUILT_IN_STRCPY:
>> + case BUILT_IN_STRNCPY:
>> + case BUILT_IN_STPCPY:
>> + case BUILT_IN_STPNCPY:
>> + case BUILT_IN_STRCAT:
>> + case BUILT_IN_STRNCAT:
>> + case BUILT_IN_MEMCPY:
>> + case BUILT_IN_MEMPCPY:
>> + case BUILT_IN_MEMSET:
>> + case BUILT_IN_MEMMOVE:
>> + case BUILT_IN_BZERO:
>> + case BUILT_IN_STRCMP:
>> + case BUILT_IN_STRNCMP:
>> + case BUILT_IN_BCMP:
>> + case BUILT_IN_MEMCMP:
>> + case BUILT_IN_MEMCPY_CHK:
>> + case BUILT_IN_MEMPCPY_CHK:
>> + case BUILT_IN_MEMMOVE_CHK:
>> + case BUILT_IN_MEMSET_CHK:
>> + case BUILT_IN_STRCPY_CHK:
>> + case BUILT_IN_STRNCPY_CHK:
>> + case BUILT_IN_STPCPY_CHK:
>> + case BUILT_IN_STPNCPY_CHK:
>> + case BUILT_IN_STRCAT_CHK:
>> + case BUILT_IN_STRNCAT_CHK:
>> + case BUILT_IN_MALLOC:
>> + case BUILT_IN_CALLOC:
>> + case BUILT_IN_REALLOC:
>> + return 1;
>> +
>> + default:
>> + return 0;
>> + }
>> +}
>
> OK, this gates creation of the additional builtin and ensures we don't try
> to create an instrumention clone for anything outside the list above.
>
>
>> @@ -1686,11 +1730,18 @@ chkp_add_bounds_to_call_stmt (gimple_stmt_iterator
>> *gsi)
>> if (!flag_chkp_instrument_calls)
>> return;
>>
>> - /* Avoid instrumented builtin functions for now. Due to IPA
>> - it also means we have to avoid instrumentation of indirect
>> - calls. */
>> - if (fndecl && DECL_BUILT_IN_CLASS (fndecl) != NOT_BUILT_IN)
>> - return;
>> + /* We instrument only some subset of builtins. We also instrument
>> + builtin calls to be inlined. */
>> + if (fndecl
>> + && DECL_BUILT_IN_CLASS (fndecl) == BUILT_IN_NORMAL
>> + && !chkp_instrument_normal_builtin (fndecl))
>> + {
>> + struct cgraph_node *clone = chkp_maybe_create_clone (fndecl);
>> + if (!clone
>> + || !gimple_has_body_p (clone->decl)
>> + || !lookup_attribute ("always_inline", DECL_ATTRIBUTES
>> (fndecl)))
>> + return;
>> + }
>
> Is that outer conditional right? If we have a fndecl and it's a normal
> builtin, but it's _not_ one of hte builtins we're instrumenting, then call
> chkp_maybe_create_clone?
Some builtin functions (especially their *_chk version) are defined as
always_inline functions in headers. In this case we handle them as
regular functions (clone and instrument) because they will be inlined
anyway. Seems gimple_has_body_p should be applied to fndecl and moved
into outer if-statement along with attribute check. Thus unneeded
clones would be avoided.
Thanks,
Ilya
>
>
> Will reserve OK/Not OK decision until after you respond to that issue.
>
> jeff
