[PATCH, Pointer Bounds Checker 6/x] New static constructor types

Jeff Law law@redhat.com
Mon Sep 15 21:16:00 GMT 2014


On 09/15/14 01:14, Ilya Enkovich wrote:
> Ping
>
> 2014-06-05 15:03 GMT+04:00 Ilya Enkovich <enkovich.gnu@gmail.com>:
>> 2014-06-04 17:35 GMT+04:00 Richard Biener <richard.guenther@gmail.com>:
>>> On Wed, Jun 4, 2014 at 3:13 PM, Ilya Enkovich <enkovich.gnu@gmail.com> wrote:
>>>> 2014-06-04 13:58 GMT+04:00 Richard Biener <richard.guenther@gmail.com>:
>>>>> On Wed, Apr 16, 2014 at 2:33 PM, Ilya Enkovich <enkovich.gnu@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.enkovich@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.
This patch is fine.  I know Richi didn't like it, but I don't see a 
really good way around this.

Jeff



More information about the Gcc-patches mailing list