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

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

[Ilya Enkovich]

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))))
 	  {
 	    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);
+      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)

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

[Ilya Enkovich]

2014-04-16 16:33 GMT+04:00 Ilya Enkovich <enkovich.gnu@gmail.com>:
> 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))))
>           {
>             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);
> +      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)

Will install it in a couple of days.

Ilya

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

[Jeff Law]

On 04/16/14 06:33, Ilya Enkovich 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.
This is OK for the trunk.  Per Richi's request, please hold off 
committing until the entire series is approved.

Thanks,

jeff

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

[Richard Biener]

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?

>           {
>             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.

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)

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

[Ilya Enkovich]

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.

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)

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

[Richard Biener]

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.

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)

Re: [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)

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

[Ilya Enkovich]

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.
>
> 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)

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

[Jeff Law]

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