Ping
2014-06-05 15:03 GMT+04:00 Ilya Enkovich <enkovich....@gmail.com>:
> 2014-06-04 17:35 GMT+04:00 Richard Biener <richard.guent...@gmail.com>:
>> On Wed, Jun 4, 2014 at 3:13 PM, Ilya Enkovich <enkovich....@gmail.com> wrote:
>>> 2014-06-04 13:58 GMT+04:00 Richard Biener <richard.guent...@gmail.com>:
>>>> On Wed, Apr 16, 2014 at 2:33 PM, Ilya Enkovich <enkovich....@gmail.com>
>>>> wrote:
>>>>> Hi,
>>>>>
>>>>> This patch add new static constructor types used by Pointer Bounds
>>>>> Checker. It was approved earlier for 4.9 and I'll assume patch is OK for
>>>>> trunk if no objections arise.
>>>>>
>>>>> Patch was bootstrapped and tested for linux-x86_64.
>>>>>
>>>>> Thanks,
>>>>> Ilya
>>>>> --
>>>>> gcc/
>>>>>
>>>>> 2014-04-16 Ilya Enkovich <ilya.enkov...@intel.com>
>>>>>
>>>>> * ipa.c (cgraph_build_static_cdtor_1): Support contructors
>>>>> with "chkp ctor" and "bnd_legacy" attributes.
>>>>> * gimplify.c (gimplify_init_constructor): Avoid infinite
>>>>> loop during gimplification of bounds initializer.
>>>>>
>>>>>
>>>>> diff --git a/gcc/gimplify.c b/gcc/gimplify.c
>>>>> index 7441784..67ab515 100644
>>>>> --- a/gcc/gimplify.c
>>>>> +++ b/gcc/gimplify.c
>>>>> @@ -3803,10 +3803,19 @@ gimplify_init_constructor (tree *expr_p,
>>>>> gimple_seq *pre_p, gimple_seq *post_p,
>>>>> individual element initialization. Also don't do this for
>>>>> small
>>>>> all-zero initializers (which aren't big enough to merit
>>>>> clearing), and don't try to make bitwise copies of
>>>>> - TREE_ADDRESSABLE types. */
>>>>> + TREE_ADDRESSABLE types.
>>>>> +
>>>>> + We cannot apply such transformation when compiling chkp static
>>>>> + initializer because creation of initializer image in the memory
>>>>> + will require static initialization of bounds for it. It should
>>>>> + result in another gimplification of similar initializer and we
>>>>> + may fall into infinite loop. */
>>>>> if (valid_const_initializer
>>>>> && !(cleared || num_nonzero_elements == 0)
>>>>> - && !TREE_ADDRESSABLE (type))
>>>>> + && !TREE_ADDRESSABLE (type)
>>>>> + && (!current_function_decl
>>>>> + || !lookup_attribute ("chkp ctor",
>>>>> + DECL_ATTRIBUTES
>>>>> (current_function_decl))))
>>>>
>>>> Simply make the type TREE_ADDRESSABLE?
>>>
>>> Wouldn't it be a hack to mark it addressable just to not hit this
>>> condition? It would also require to have an unshared copy of the type
>>> to not affect other statements with that type.
>>>
>>>>
>>>>> {
>>>>> HOST_WIDE_INT size = int_size_in_bytes (type);
>>>>> unsigned int align;
>>>>> diff --git a/gcc/ipa.c b/gcc/ipa.c
>>>>> index 26e9b03..5ab3aed 100644
>>>>> --- a/gcc/ipa.c
>>>>> +++ b/gcc/ipa.c
>>>>> @@ -1345,9 +1345,11 @@ make_pass_ipa_whole_program_visibility
>>>>> (gcc::context *ctxt)
>>>>> }
>>>>>
>>>>> /* Generate and emit a static constructor or destructor. WHICH must
>>>>> - be one of 'I' (for a constructor) or 'D' (for a destructor). BODY
>>>>> - is a STATEMENT_LIST containing GENERIC statements. PRIORITY is the
>>>>> - initialization priority for this constructor or destructor.
>>>>> + be one of 'I' (for a constructor), 'D' (for a destructor), 'P'
>>>>> + (for chp static vars constructor) or 'B' (for chkp static bounds
>>>>> + constructor). BODY is a STATEMENT_LIST containing GENERIC
>>>>> + statements. PRIORITY is the initialization priority for this
>>>>> + constructor or destructor.
>>>>>
>>>>> FINAL specify whether the externally visible name for collect2 should
>>>>> be produced. */
>>>>> @@ -1406,6 +1408,20 @@ cgraph_build_static_cdtor_1 (char which, tree
>>>>> body, int priority, bool final)
>>>>> DECL_STATIC_CONSTRUCTOR (decl) = 1;
>>>>> decl_init_priority_insert (decl, priority);
>>>>> break;
>>>>> + case 'P':
>>>>> + DECL_STATIC_CONSTRUCTOR (decl) = 1;
>>>>> + DECL_ATTRIBUTES (decl) = tree_cons (get_identifier ("chkp ctor"),
>>>>> + NULL,
>>>>> + NULL_TREE);
>>>>
>>>> Ick. Please try to avoid using attributes for this. Rather adjust the
>>>> caller of this function to set a flag in the cgraph node.
>>>
>>> It is too late because all early local passes are executed by that
>>> time and I need this attribute to be set before them.
>>
>> Ok, so where do you call this from? It should be possible to create
>> the function in GIMPLE form directly, avoiding the need to go through
>> gimplification.
>
> I create constructors at the end of unit compilation in
> chkp_finish_file. Constructor body in this case is MODIFY_EXPRs for
> all statically initialized pointers. "chkp ctor" attribute is used to
> instrument this constructor properly - all pointer modifications
> should be replaced with corresponding bounds initialization. Thus
> gimplification is not the only place where this attribute is used.
>
> Ilya
>
>>
>> Richard.
>>
>>> Ilya
>>>
>>>>
>>>> So I don't like this patch at all.
>>>>
>>>> Richard.
>>>>
>>>>> + decl_init_priority_insert (decl, priority);
>>>>> + break;
>>>>> + case 'B':
>>>>> + DECL_STATIC_CONSTRUCTOR (decl) = 1;
>>>>> + DECL_ATTRIBUTES (decl) = tree_cons (get_identifier ("bnd_legacy"),
>>>>> + NULL,
>>>>> + NULL_TREE);
>>>>> + decl_init_priority_insert (decl, priority);
>>>>> + break;
>>>>> case 'D':
>>>>> DECL_STATIC_DESTRUCTOR (decl) = 1;
>>>>> decl_fini_priority_insert (decl, priority);
>>>>> @@ -1423,9 +1439,11 @@ cgraph_build_static_cdtor_1 (char which, tree
>>>>> body, int priority, bool final)
>>>>> }
>>>>>
>>>>> /* Generate and emit a static constructor or destructor. WHICH must
>>>>> - be one of 'I' (for a constructor) or 'D' (for a destructor). BODY
>>>>> - is a STATEMENT_LIST containing GENERIC statements. PRIORITY is the
>>>>> - initialization priority for this constructor or destructor. */
>>>>> + be one of 'I' (for a constructor), 'D' (for a destructor), 'P'
>>>>> + (for chkp static vars constructor) or 'B' (for chkp static bounds
>>>>> + constructor). BODY is a STATEMENT_LIST containing GENERIC
>>>>> + statements. PRIORITY is the initialization priority for this
>>>>> + constructor or destructor. */
>>>>>
>>>>> void
>>>>> cgraph_build_static_cdtor (char which, tree body, int priority)
