Message ID | 20170427132422.GA1809@tucnak |
---|---|
State | New |
Headers | show |
On Thu, 27 Apr 2017, Jakub Jelinek wrote: > Hi! > > As mentioned in the PR, stor-layout.c (layout_type) can set > TYPE_TYPELESS_STORAGE flag on ARRAY_TYPE that didn't have it before, > which breaks ARRAY_TYPE hashing in build_array_type_1/type_hash_canon. > > We have 2 cases, one is ARRAY_TYPE with non-AGGREGATE_TYPE_P element type > (note: TYPE_TYPELESS_STORAGE macro is only allowed on AGGREGATE_TYPE_P > types), here we want to use that flag in hashing and treat differently > C++ FE created types with char/unsigned char/signed char/std::byte elements > from middle-end created arrays or other FE created arrays for LTO. > And the other case is ARRAY_TYPE with AGGREGATE_TYPE_P element type, > here the flag is always just inherited (copied) from the element type, > but it might not be finalized by the time first array of such a type > is created (added into hash table). The following patch fixes this > by only doing hashing and comparison of this flag for ARRAY_TYPEs with > non-AGGREGATE_TYPE_P elements, for other arrays it is expected that all > array types with that element type will have the flag set eventually, > but it might not be finalized immediately. > > Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk and 7.1? Ok. Thanks, Richard. > 2017-04-27 Jakub Jelinek <jakub@redhat.com> > > PR c++/80534 > * tree.c (type_cache_hasher::equal): Only compare > TYPE_TYPELESS_STORAGE flag on non-aggregate element types. > (build_array_type_1): Only hash TYPE_TYPELESS_STORAGE flag on > non-aggregate element types. > * tree.h (TYPE_TYPELESS_STORAGE): Fix comment typo, add more details > about the flag on ARRAY_TYPEs in the comment, formatting fix. > c-family/ > * c-common.c (complete_array_type): Only hash TYPE_TYPELESS_STORAGE > flag on non-aggregate element types. > testsuite/ > * g++.dg/other/pr80534-1.C: New test. > * g++.dg/other/pr80534-2.C: New test. > > --- gcc/tree.c.jj 2017-04-27 09:11:09.000000000 +0200 > +++ gcc/tree.c 2017-04-27 11:48:08.177168696 +0200 > @@ -7073,9 +7073,16 @@ type_cache_hasher::equal (type_hash *a, > break; > return 0; > case ARRAY_TYPE: > - return (TYPE_TYPELESS_STORAGE (a->type) > - == TYPE_TYPELESS_STORAGE (b->type) > - && TYPE_DOMAIN (a->type) == TYPE_DOMAIN (b->type)); > + /* Don't compare TYPE_TYPELESS_STORAGE flag on aggregates, > + where the flag should be inherited from the element type > + and can change after ARRAY_TYPEs are created; on non-aggregates > + compare it and hash it, scalars will never have that flag set > + and we need to differentiate between arrays created by different > + front-ends or middle-end created arrays. */ > + return (TYPE_DOMAIN (a->type) == TYPE_DOMAIN (b->type) > + && (AGGREGATE_TYPE_P (TREE_TYPE (a->type)) > + || (TYPE_TYPELESS_STORAGE (a->type) > + == TYPE_TYPELESS_STORAGE (b->type)))); > > case RECORD_TYPE: > case UNION_TYPE: > @@ -8386,7 +8393,8 @@ build_array_type_1 (tree elt_type, tree > hstate.add_object (TYPE_HASH (elt_type)); > if (index_type) > hstate.add_object (TYPE_HASH (index_type)); > - hstate.add_flag (typeless_storage); > + if (!AGGREGATE_TYPE_P (elt_type)) > + hstate.add_flag (TYPE_TYPELESS_STORAGE (t)); > t = type_hash_canon (hstate.end (), t); > } > > --- gcc/tree.h.jj 2017-04-27 09:11:09.000000000 +0200 > +++ gcc/tree.h 2017-04-27 12:19:32.886969545 +0200 > @@ -2037,10 +2037,16 @@ extern machine_mode element_mode (const_ > > /* For an ARRAY_TYPE, a RECORD_TYPE, a UNION_TYPE or a QUAL_UNION_TYPE > whether the array is typeless storage or the type contains a member > - with this flag set. Such types are excempt from type-based alias > - analysis. */ > + with this flag set. Such types are exempt from type-based alias > + analysis. For ARRAY_TYPEs with AGGREGATE_TYPE_P element types > + the flag should be inherited from the element type, can change > + when type is finalized and because of that should not be used in > + type hashing. For ARRAY_TYPEs with non-AGGREGATE_TYPE_P element types > + the flag should not be changed after the array is created and should > + be used in type hashing. */ > #define TYPE_TYPELESS_STORAGE(NODE) \ > - (TREE_CHECK4 (NODE, RECORD_TYPE, UNION_TYPE, QUAL_UNION_TYPE, ARRAY_TYPE)->type_common.typeless_storage) > + (TREE_CHECK4 (NODE, RECORD_TYPE, UNION_TYPE, QUAL_UNION_TYPE, \ > + ARRAY_TYPE)->type_common.typeless_storage) > > /* Indicated that objects of this type should be laid out in as > compact a way as possible. */ > --- gcc/c-family/c-common.c.jj 2017-04-25 15:51:08.000000000 +0200 > +++ gcc/c-family/c-common.c 2017-04-27 11:47:39.089543361 +0200 > @@ -6371,7 +6371,8 @@ complete_array_type (tree *ptype, tree i > inchash::hash hstate; > hstate.add_object (TYPE_HASH (unqual_elt)); > hstate.add_object (TYPE_HASH (TYPE_DOMAIN (main_type))); > - hstate.add_flag (TYPE_TYPELESS_STORAGE (main_type)); > + if (!AGGREGATE_TYPE_P (unqual_elt)) > + hstate.add_flag (TYPE_TYPELESS_STORAGE (main_type)); > main_type = type_hash_canon (hstate.end (), main_type); > > /* Fix the canonical type. */ > --- gcc/testsuite/g++.dg/other/pr80534-1.C.jj 2017-04-27 11:51:53.278269268 +0200 > +++ gcc/testsuite/g++.dg/other/pr80534-1.C 2017-04-27 11:53:03.733361767 +0200 > @@ -0,0 +1,21 @@ > +// PR c++/80534 > +// { dg-do compile { target c++11 } } > +// { dg-options "" } > + > +template <int> struct A { > + struct type { > + char __data[0]; > + }; > +}; > +template <typename _Tp, typename = _Tp> struct B; > +template <typename _Tp, typename _Dp> struct B<_Tp[], _Dp> { > + _Tp _M_t; > + using pointer = int; > + void m_fn1() {} > +}; > +struct C { > + using Storage = A<0>::type; > + using StorageUniquePointer = B<Storage[]>; > + void m_fn2() { storageUniquePointer_.m_fn1(); } > + StorageUniquePointer storageUniquePointer_; > +}; > --- gcc/testsuite/g++.dg/other/pr80534-2.C.jj 2017-04-27 11:51:58.419203049 +0200 > +++ gcc/testsuite/g++.dg/other/pr80534-2.C 2017-04-27 11:53:12.952243022 +0200 > @@ -0,0 +1,27 @@ > +// PR c++/80534 > +// { dg-do compile { target c++11 } } > +// { dg-options "" } > + > +template <int, int> struct aligned_storage { > + struct type { > + char __data[0]; > + }; > +}; > +struct A {}; > +template <typename _Tp, typename = _Tp> struct unique_ptr; > +template <typename _Tp, typename _Dp> struct unique_ptr<_Tp[], _Dp> { > + int _M_t; > + void get() { _M_t; } > +}; > +struct B { > + using Association = A; > + using Storage = aligned_storage<sizeof(Association), alignof(Association)>::type; > + using StorageUniquePointer = unique_ptr<Storage[]>; > + void getAssociationsBegin() { storageUniquePointer_.get(); } > + StorageUniquePointer storageUniquePointer_; > +}; > +struct C {}; > +using MainThreadStaticSignalsReceiver = C; > +aligned_storage<sizeof(MainThreadStaticSignalsReceiver), > + alignof(MainThreadStaticSignalsReceiver)>::type > + mainThreadStaticSignalsReceiverStorage; > > Jakub > >
--- gcc/tree.c.jj 2017-04-27 09:11:09.000000000 +0200 +++ gcc/tree.c 2017-04-27 11:48:08.177168696 +0200 @@ -7073,9 +7073,16 @@ type_cache_hasher::equal (type_hash *a, break; return 0; case ARRAY_TYPE: - return (TYPE_TYPELESS_STORAGE (a->type) - == TYPE_TYPELESS_STORAGE (b->type) - && TYPE_DOMAIN (a->type) == TYPE_DOMAIN (b->type)); + /* Don't compare TYPE_TYPELESS_STORAGE flag on aggregates, + where the flag should be inherited from the element type + and can change after ARRAY_TYPEs are created; on non-aggregates + compare it and hash it, scalars will never have that flag set + and we need to differentiate between arrays created by different + front-ends or middle-end created arrays. */ + return (TYPE_DOMAIN (a->type) == TYPE_DOMAIN (b->type) + && (AGGREGATE_TYPE_P (TREE_TYPE (a->type)) + || (TYPE_TYPELESS_STORAGE (a->type) + == TYPE_TYPELESS_STORAGE (b->type)))); case RECORD_TYPE: case UNION_TYPE: @@ -8386,7 +8393,8 @@ build_array_type_1 (tree elt_type, tree hstate.add_object (TYPE_HASH (elt_type)); if (index_type) hstate.add_object (TYPE_HASH (index_type)); - hstate.add_flag (typeless_storage); + if (!AGGREGATE_TYPE_P (elt_type)) + hstate.add_flag (TYPE_TYPELESS_STORAGE (t)); t = type_hash_canon (hstate.end (), t); } --- gcc/tree.h.jj 2017-04-27 09:11:09.000000000 +0200 +++ gcc/tree.h 2017-04-27 12:19:32.886969545 +0200 @@ -2037,10 +2037,16 @@ extern machine_mode element_mode (const_ /* For an ARRAY_TYPE, a RECORD_TYPE, a UNION_TYPE or a QUAL_UNION_TYPE whether the array is typeless storage or the type contains a member - with this flag set. Such types are excempt from type-based alias - analysis. */ + with this flag set. Such types are exempt from type-based alias + analysis. For ARRAY_TYPEs with AGGREGATE_TYPE_P element types + the flag should be inherited from the element type, can change + when type is finalized and because of that should not be used in + type hashing. For ARRAY_TYPEs with non-AGGREGATE_TYPE_P element types + the flag should not be changed after the array is created and should + be used in type hashing. */ #define TYPE_TYPELESS_STORAGE(NODE) \ - (TREE_CHECK4 (NODE, RECORD_TYPE, UNION_TYPE, QUAL_UNION_TYPE, ARRAY_TYPE)->type_common.typeless_storage) + (TREE_CHECK4 (NODE, RECORD_TYPE, UNION_TYPE, QUAL_UNION_TYPE, \ + ARRAY_TYPE)->type_common.typeless_storage) /* Indicated that objects of this type should be laid out in as compact a way as possible. */ --- gcc/c-family/c-common.c.jj 2017-04-25 15:51:08.000000000 +0200 +++ gcc/c-family/c-common.c 2017-04-27 11:47:39.089543361 +0200 @@ -6371,7 +6371,8 @@ complete_array_type (tree *ptype, tree i inchash::hash hstate; hstate.add_object (TYPE_HASH (unqual_elt)); hstate.add_object (TYPE_HASH (TYPE_DOMAIN (main_type))); - hstate.add_flag (TYPE_TYPELESS_STORAGE (main_type)); + if (!AGGREGATE_TYPE_P (unqual_elt)) + hstate.add_flag (TYPE_TYPELESS_STORAGE (main_type)); main_type = type_hash_canon (hstate.end (), main_type); /* Fix the canonical type. */ --- gcc/testsuite/g++.dg/other/pr80534-1.C.jj 2017-04-27 11:51:53.278269268 +0200 +++ gcc/testsuite/g++.dg/other/pr80534-1.C 2017-04-27 11:53:03.733361767 +0200 @@ -0,0 +1,21 @@ +// PR c++/80534 +// { dg-do compile { target c++11 } } +// { dg-options "" } + +template <int> struct A { + struct type { + char __data[0]; + }; +}; +template <typename _Tp, typename = _Tp> struct B; +template <typename _Tp, typename _Dp> struct B<_Tp[], _Dp> { + _Tp _M_t; + using pointer = int; + void m_fn1() {} +}; +struct C { + using Storage = A<0>::type; + using StorageUniquePointer = B<Storage[]>; + void m_fn2() { storageUniquePointer_.m_fn1(); } + StorageUniquePointer storageUniquePointer_; +}; --- gcc/testsuite/g++.dg/other/pr80534-2.C.jj 2017-04-27 11:51:58.419203049 +0200 +++ gcc/testsuite/g++.dg/other/pr80534-2.C 2017-04-27 11:53:12.952243022 +0200 @@ -0,0 +1,27 @@ +// PR c++/80534 +// { dg-do compile { target c++11 } } +// { dg-options "" } + +template <int, int> struct aligned_storage { + struct type { + char __data[0]; + }; +}; +struct A {}; +template <typename _Tp, typename = _Tp> struct unique_ptr; +template <typename _Tp, typename _Dp> struct unique_ptr<_Tp[], _Dp> { + int _M_t; + void get() { _M_t; } +}; +struct B { + using Association = A; + using Storage = aligned_storage<sizeof(Association), alignof(Association)>::type; + using StorageUniquePointer = unique_ptr<Storage[]>; + void getAssociationsBegin() { storageUniquePointer_.get(); } + StorageUniquePointer storageUniquePointer_; +}; +struct C {}; +using MainThreadStaticSignalsReceiver = C; +aligned_storage<sizeof(MainThreadStaticSignalsReceiver), + alignof(MainThreadStaticSignalsReceiver)>::type + mainThreadStaticSignalsReceiverStorage;