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

[Ilya Enkovich]

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.

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)