| Message ID | 20140416123320.GE4040@msticlxl57.ims.intel.com |
|---|---|
| State | New |
| Headers | show |
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
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
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)
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)
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)
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)
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)
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
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)