| Message ID | 2d5f1526-c124-03b4-63fd-a476da5c35f1@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | don't trim empty string initializers for pointers (PR 90947) | expand |
On 6/21/19 8:05 PM, Martin Sebor wrote: > The solution we implemented in GCC 9 to get the mangling of > non-type template arguments of class types containing array > members consistent regardless of the form of their > initialization introduced a couple of bugs. One of these > is the subject of this patch. The bug results in stripping > trailing initializers for array elements that involve the empty > string such as in here: > > void f (void) > { > const char* a[1][1] = { "" }; > if (!a[0][0]) > __builtin_abort (); > } > > The problem is caused by relying on initializer_zerop() that > returns true for the empty string regardless of whether it's > used to initialize an array or a pointer (the function doesn't > know what the initializer is being used for). Why doesn't the existing POINTER_TYPE_P check handle this? It's clearly intended to. Jason
On 6/22/19 9:37 PM, Jason Merrill wrote: > On 6/21/19 8:05 PM, Martin Sebor wrote: >> The solution we implemented in GCC 9 to get the mangling of >> non-type template arguments of class types containing array >> members consistent regardless of the form of their >> initialization introduced a couple of bugs. One of these >> is the subject of this patch. The bug results in stripping >> trailing initializers for array elements that involve the empty >> string such as in here: >> >> void f (void) >> { >> const char* a[1][1] = { "" }; >> if (!a[0][0]) >> __builtin_abort (); >> } >> >> The problem is caused by relying on initializer_zerop() that >> returns true for the empty string regardless of whether it's >> used to initialize an array or a pointer (the function doesn't >> know what the initializer is being used for). > > Why doesn't the existing POINTER_TYPE_P check handle this? It's clearly > intended to. It does but only only for initializers of "leaf" elements/members. The function processes initializers of enclosing elements or members recursively. When initializer_zerop returns true for one of those, it doesn't differentiate between an empty string that's being used to initialize a leaf array or a leaf pointer. For the example above, the first time through, elt_type is char* and elt_init is the empty string, or the array char[1], and so the initializer is retained. The second time through (after the leaf recursive call returns), elt_init is { "" } (i.e., an array of one empty string), elt_type is also an array, and initializer_zerop(elt_init) returns true, so the initializer is dropped. I'm actually surprised this initializer_zerop ambiguity between arrays and strings doesn't come up elsewhere. That's also why I added the new function to tree.c. Btw., it belatedly occurred to me that the new function doesn't correctly handled unnamed bit-fields so I've corrected it to handle those as well. Attached is the revised patch with this change and a test to exercise it. Martin PS I found DECL_UNNAMED_BIT_FIELD in c-family/c-common.h. I used (DECL_BIT_FIELD (fld) && !DECL_NAME (fld)) because the former macro isn't available in tree.c. Does that mean that that would make the new function not completely correct in some other languages?
On 6/23/19 5:51 PM, Martin Sebor wrote: > On 6/22/19 9:37 PM, Jason Merrill wrote: >> On 6/21/19 8:05 PM, Martin Sebor wrote: >>> The solution we implemented in GCC 9 to get the mangling of >>> non-type template arguments of class types containing array >>> members consistent regardless of the form of their >>> initialization introduced a couple of bugs. One of these >>> is the subject of this patch. The bug results in stripping >>> trailing initializers for array elements that involve the empty >>> string such as in here: >>> >>> void f (void) >>> { >>> const char* a[1][1] = { "" }; >>> if (!a[0][0]) >>> __builtin_abort (); >>> } >>> >>> The problem is caused by relying on initializer_zerop() that >>> returns true for the empty string regardless of whether it's >>> used to initialize an array or a pointer (the function doesn't >>> know what the initializer is being used for). >> >> Why doesn't the existing POINTER_TYPE_P check handle this? It's >> clearly intended to. > > It does but only only for initializers of "leaf" elements/members. > The function processes initializers of enclosing elements or members > recursively. When initializer_zerop returns true for one of those, > it doesn't differentiate between an empty string that's being used > to initialize a leaf array or a leaf pointer. > > For the example above, the first time through, elt_type is char* > and elt_init is the empty string, or the array char[1], and so > the initializer is retained. > > The second time through (after the leaf recursive call returns), > elt_init is { "" } (i.e., an array of one empty string), elt_type > is also an array, and initializer_zerop(elt_init) returns true, so > the initializer is dropped. > > I'm actually surprised this initializer_zerop ambiguity between > arrays and strings doesn't come up elsewhere. That's also why > I added the new function to tree.c. Makes sense. > Btw., it belatedly occurred to me that the new function doesn't > correctly handled unnamed bit-fields so I've corrected it to > handle those as well. Attached is the revised patch with this > change and a test to exercise it. > + || (DECL_BIT_FIELD (fld) && !!DECL_NAME (fld))) Hmm, this looks like it would skip named bit-fields rather than unnamed. > PS I found DECL_UNNAMED_BIT_FIELD in c-family/c-common.h. > I used (DECL_BIT_FIELD (fld) && !DECL_NAME (fld)) because > the former macro isn't available in tree.c. Does that mean > that that would make the new function not completely correct > in some other languages? I don't know if other languages treat unnamed bit-fields as initializable. > +/* Analogous to initializer_zerop but examines the type for which > + the initializer is being used but unlike it, considers empty > + strings to be zero initializers for arrays and non-zero for > + pointers. */ This comment could use more editing. Jason
On 6/27/19 3:25 PM, Jason Merrill wrote: > On 6/23/19 5:51 PM, Martin Sebor wrote: >> On 6/22/19 9:37 PM, Jason Merrill wrote: >>> On 6/21/19 8:05 PM, Martin Sebor wrote: >>>> The solution we implemented in GCC 9 to get the mangling of >>>> non-type template arguments of class types containing array >>>> members consistent regardless of the form of their >>>> initialization introduced a couple of bugs. One of these >>>> is the subject of this patch. The bug results in stripping >>>> trailing initializers for array elements that involve the empty >>>> string such as in here: >>>> >>>> void f (void) >>>> { >>>> const char* a[1][1] = { "" }; >>>> if (!a[0][0]) >>>> __builtin_abort (); >>>> } >>>> >>>> The problem is caused by relying on initializer_zerop() that >>>> returns true for the empty string regardless of whether it's >>>> used to initialize an array or a pointer (the function doesn't >>>> know what the initializer is being used for). >>> >>> Why doesn't the existing POINTER_TYPE_P check handle this? It's >>> clearly intended to. >> >> It does but only only for initializers of "leaf" elements/members. >> The function processes initializers of enclosing elements or members >> recursively. When initializer_zerop returns true for one of those, >> it doesn't differentiate between an empty string that's being used >> to initialize a leaf array or a leaf pointer. >> >> For the example above, the first time through, elt_type is char* >> and elt_init is the empty string, or the array char[1], and so >> the initializer is retained. >> >> The second time through (after the leaf recursive call returns), >> elt_init is { "" } (i.e., an array of one empty string), elt_type >> is also an array, and initializer_zerop(elt_init) returns true, so >> the initializer is dropped. >> >> I'm actually surprised this initializer_zerop ambiguity between >> arrays and strings doesn't come up elsewhere. That's also why >> I added the new function to tree.c. > > Makes sense. > >> Btw., it belatedly occurred to me that the new function doesn't >> correctly handled unnamed bit-fields so I've corrected it to >> handle those as well. Attached is the revised patch with this >> change and a test to exercise it. > >> + || (DECL_BIT_FIELD (fld) && !!DECL_NAME (fld))) > > Hmm, this looks like it would skip named bit-fields rather than unnamed. It was a typo that I fixed by hand separately in my tree and in the patch but missed Emacs' question when saving the patch and wound up sending the unchanged version. >> PS I found DECL_UNNAMED_BIT_FIELD in c-family/c-common.h. >> I used (DECL_BIT_FIELD (fld) && !DECL_NAME (fld)) because >> the former macro isn't available in tree.c. Does that mean >> that that would make the new function not completely correct >> in some other languages? > > I don't know if other languages treat unnamed bit-fields as initializable. > >> +/* Analogous to initializer_zerop but examines the type for which >> + the initializer is being used but unlike it, considers empty >> + strings to be zero initializers for arrays and non-zero for >> + pointers. */ > > This comment could use more editing. I've broken up the long sentence into two so it reads a little better. If you were objecting to something else please let me know. Other than that and the typo the patch is the same. Martin
On 8/1/19 4:22 PM, Martin Sebor wrote: > On 6/27/19 3:25 PM, Jason Merrill wrote: >> On 6/23/19 5:51 PM, Martin Sebor wrote: >>> On 6/22/19 9:37 PM, Jason Merrill wrote: >>>> On 6/21/19 8:05 PM, Martin Sebor wrote: >>>>> The solution we implemented in GCC 9 to get the mangling of >>>>> non-type template arguments of class types containing array >>>>> members consistent regardless of the form of their >>>>> initialization introduced a couple of bugs. One of these >>>>> is the subject of this patch. The bug results in stripping >>>>> trailing initializers for array elements that involve the empty >>>>> string such as in here: >>>>> >>>>> void f (void) >>>>> { >>>>> const char* a[1][1] = { "" }; >>>>> if (!a[0][0]) >>>>> __builtin_abort (); >>>>> } >>>>> >>>>> The problem is caused by relying on initializer_zerop() that >>>>> returns true for the empty string regardless of whether it's >>>>> used to initialize an array or a pointer (the function doesn't >>>>> know what the initializer is being used for). >>>> >>>> Why doesn't the existing POINTER_TYPE_P check handle this? It's >>>> clearly intended to. >>> >>> It does but only only for initializers of "leaf" elements/members. >>> The function processes initializers of enclosing elements or members >>> recursively. When initializer_zerop returns true for one of those, >>> it doesn't differentiate between an empty string that's being used >>> to initialize a leaf array or a leaf pointer. >>> >>> For the example above, the first time through, elt_type is char* >>> and elt_init is the empty string, or the array char[1], and so >>> the initializer is retained. >>> >>> The second time through (after the leaf recursive call returns), >>> elt_init is { "" } (i.e., an array of one empty string), elt_type >>> is also an array, and initializer_zerop(elt_init) returns true, so >>> the initializer is dropped. >>> >>> I'm actually surprised this initializer_zerop ambiguity between >>> arrays and strings doesn't come up elsewhere. That's also why >>> I added the new function to tree.c. >> >> Makes sense. >> >>> Btw., it belatedly occurred to me that the new function doesn't >>> correctly handled unnamed bit-fields so I've corrected it to >>> handle those as well. Attached is the revised patch with this >>> change and a test to exercise it. >> >>> + || (DECL_BIT_FIELD (fld) && !!DECL_NAME (fld))) >> >> Hmm, this looks like it would skip named bit-fields rather than unnamed. > > It was a typo that I fixed by hand separately in my tree and in > the patch but missed Emacs' question when saving the patch and > wound up sending the unchanged version. > >>> PS I found DECL_UNNAMED_BIT_FIELD in c-family/c-common.h. >>> I used (DECL_BIT_FIELD (fld) && !DECL_NAME (fld)) because >>> the former macro isn't available in tree.c. Does that mean >>> that that would make the new function not completely correct >>> in some other languages? >> >> I don't know if other languages treat unnamed bit-fields as >> initializable. >> >>> +/* Analogous to initializer_zerop but examines the type for which >>> + the initializer is being used but unlike it, considers empty >>> + strings to be zero initializers for arrays and non-zero for >>> + pointers. */ >> >> This comment could use more editing. > > I've broken up the long sentence into two so it reads a little > better. If you were objecting to something else please let me > know. > > Other than that and the typo the patch is the same. OK, thanks. Jason
On 8/1/19 3:09 PM, Jason Merrill wrote: > On 8/1/19 4:22 PM, Martin Sebor wrote: >> On 6/27/19 3:25 PM, Jason Merrill wrote: >>> On 6/23/19 5:51 PM, Martin Sebor wrote: >>>> On 6/22/19 9:37 PM, Jason Merrill wrote: >>>>> On 6/21/19 8:05 PM, Martin Sebor wrote: >>>>>> The solution we implemented in GCC 9 to get the mangling of >>>>>> non-type template arguments of class types containing array >>>>>> members consistent regardless of the form of their >>>>>> initialization introduced a couple of bugs. One of these >>>>>> is the subject of this patch. The bug results in stripping >>>>>> trailing initializers for array elements that involve the empty >>>>>> string such as in here: >>>>>> >>>>>> void f (void) >>>>>> { >>>>>> const char* a[1][1] = { "" }; >>>>>> if (!a[0][0]) >>>>>> __builtin_abort (); >>>>>> } >>>>>> >>>>>> The problem is caused by relying on initializer_zerop() that >>>>>> returns true for the empty string regardless of whether it's >>>>>> used to initialize an array or a pointer (the function doesn't >>>>>> know what the initializer is being used for). >>>>> >>>>> Why doesn't the existing POINTER_TYPE_P check handle this? It's >>>>> clearly intended to. >>>> >>>> It does but only only for initializers of "leaf" elements/members. >>>> The function processes initializers of enclosing elements or members >>>> recursively. When initializer_zerop returns true for one of those, >>>> it doesn't differentiate between an empty string that's being used >>>> to initialize a leaf array or a leaf pointer. >>>> >>>> For the example above, the first time through, elt_type is char* >>>> and elt_init is the empty string, or the array char[1], and so >>>> the initializer is retained. >>>> >>>> The second time through (after the leaf recursive call returns), >>>> elt_init is { "" } (i.e., an array of one empty string), elt_type >>>> is also an array, and initializer_zerop(elt_init) returns true, so >>>> the initializer is dropped. >>>> >>>> I'm actually surprised this initializer_zerop ambiguity between >>>> arrays and strings doesn't come up elsewhere. That's also why >>>> I added the new function to tree.c. >>> >>> Makes sense. >>> >>>> Btw., it belatedly occurred to me that the new function doesn't >>>> correctly handled unnamed bit-fields so I've corrected it to >>>> handle those as well. Attached is the revised patch with this >>>> change and a test to exercise it. >>> >>>> + || (DECL_BIT_FIELD (fld) && !!DECL_NAME (fld))) >>> >>> Hmm, this looks like it would skip named bit-fields rather than unnamed. >> >> It was a typo that I fixed by hand separately in my tree and in >> the patch but missed Emacs' question when saving the patch and >> wound up sending the unchanged version. >> >>>> PS I found DECL_UNNAMED_BIT_FIELD in c-family/c-common.h. >>>> I used (DECL_BIT_FIELD (fld) && !DECL_NAME (fld)) because >>>> the former macro isn't available in tree.c. Does that mean >>>> that that would make the new function not completely correct >>>> in some other languages? >>> >>> I don't know if other languages treat unnamed bit-fields as >>> initializable. >>> >>>> +/* Analogous to initializer_zerop but examines the type for which >>>> + the initializer is being used but unlike it, considers empty >>>> + strings to be zero initializers for arrays and non-zero for >>>> + pointers. */ >>> >>> This comment could use more editing. >> >> I've broken up the long sentence into two so it reads a little >> better. If you were objecting to something else please let me >> know. >> >> Other than that and the typo the patch is the same. > > OK, thanks. Just to be sure before I commit it: is this OK meant as an approval of the patch (as opposed to just an acknowledgment of the changes)? Thanks Martin
On Thu, Aug 1, 2019, 6:01 PM Martin Sebor <msebor@gmail.com> wrote: > On 8/1/19 3:09 PM, Jason Merrill wrote: > > On 8/1/19 4:22 PM, Martin Sebor wrote: > >> On 6/27/19 3:25 PM, Jason Merrill wrote: > >>> On 6/23/19 5:51 PM, Martin Sebor wrote: > >>>> On 6/22/19 9:37 PM, Jason Merrill wrote: > >>>>> On 6/21/19 8:05 PM, Martin Sebor wrote: > >>>>>> The solution we implemented in GCC 9 to get the mangling of > >>>>>> non-type template arguments of class types containing array > >>>>>> members consistent regardless of the form of their > >>>>>> initialization introduced a couple of bugs. One of these > >>>>>> is the subject of this patch. The bug results in stripping > >>>>>> trailing initializers for array elements that involve the empty > >>>>>> string such as in here: > >>>>>> > >>>>>> void f (void) > >>>>>> { > >>>>>> const char* a[1][1] = { "" }; > >>>>>> if (!a[0][0]) > >>>>>> __builtin_abort (); > >>>>>> } > >>>>>> > >>>>>> The problem is caused by relying on initializer_zerop() that > >>>>>> returns true for the empty string regardless of whether it's > >>>>>> used to initialize an array or a pointer (the function doesn't > >>>>>> know what the initializer is being used for). > >>>>> > >>>>> Why doesn't the existing POINTER_TYPE_P check handle this? It's > >>>>> clearly intended to. > >>>> > >>>> It does but only only for initializers of "leaf" elements/members. > >>>> The function processes initializers of enclosing elements or members > >>>> recursively. When initializer_zerop returns true for one of those, > >>>> it doesn't differentiate between an empty string that's being used > >>>> to initialize a leaf array or a leaf pointer. > >>>> > >>>> For the example above, the first time through, elt_type is char* > >>>> and elt_init is the empty string, or the array char[1], and so > >>>> the initializer is retained. > >>>> > >>>> The second time through (after the leaf recursive call returns), > >>>> elt_init is { "" } (i.e., an array of one empty string), elt_type > >>>> is also an array, and initializer_zerop(elt_init) returns true, so > >>>> the initializer is dropped. > >>>> > >>>> I'm actually surprised this initializer_zerop ambiguity between > >>>> arrays and strings doesn't come up elsewhere. That's also why > >>>> I added the new function to tree.c. > >>> > >>> Makes sense. > >>> > >>>> Btw., it belatedly occurred to me that the new function doesn't > >>>> correctly handled unnamed bit-fields so I've corrected it to > >>>> handle those as well. Attached is the revised patch with this > >>>> change and a test to exercise it. > >>> > >>>> + || (DECL_BIT_FIELD (fld) && !!DECL_NAME (fld))) > >>> > >>> Hmm, this looks like it would skip named bit-fields rather than > unnamed. > >> > >> It was a typo that I fixed by hand separately in my tree and in > >> the patch but missed Emacs' question when saving the patch and > >> wound up sending the unchanged version. > >> > >>>> PS I found DECL_UNNAMED_BIT_FIELD in c-family/c-common.h. > >>>> I used (DECL_BIT_FIELD (fld) && !DECL_NAME (fld)) because > >>>> the former macro isn't available in tree.c. Does that mean > >>>> that that would make the new function not completely correct > >>>> in some other languages? > >>> > >>> I don't know if other languages treat unnamed bit-fields as > >>> initializable. > >>> > >>>> +/* Analogous to initializer_zerop but examines the type for which > >>>> + the initializer is being used but unlike it, considers empty > >>>> + strings to be zero initializers for arrays and non-zero for > >>>> + pointers. */ > >>> > >>> This comment could use more editing. > >> > >> I've broken up the long sentence into two so it reads a little > >> better. If you were objecting to something else please let me > >> know. > >> > >> Other than that and the typo the patch is the same. > > > > OK, thanks. > > Just to be sure before I commit it: is this OK meant as an approval > of the patch (as opposed to just an acknowledgment of the changes)? > Yes. Jason
PR c++/90947 - Simple lookup table of array of strings is miscompiled gcc/cp/ChangeLog: PR c++/90947 * decl.c (reshape_init_array_1): Avoid truncating initializer lists containing string literals. gcc/testsuite/ChangeLog: PR c++/90947 * c-c++-common/array-1.c: New test. * g++.dg/abi/mangle73.C: New test. * g++.dg/cpp2a/nontype-class18.C: New test. gcc/ChangeLog: PR c++/90947 * tree.c (type_initializer_zero_p): Define. * tree.h (type_initializer_zero_p): New function. diff --git a/gcc/cp/decl.c b/gcc/cp/decl.c index 98b54d542a0..95893631992 100644 --- a/gcc/cp/decl.c +++ b/gcc/cp/decl.c @@ -5863,8 +5863,9 @@ reshape_init_array_1 (tree elt_type, tree max_index, reshape_iter *d, /* Pointers initialized to strings must be treated as non-zero even if the string is empty. */ tree init_type = TREE_TYPE (elt_init); - if ((POINTER_TYPE_P (elt_type) != POINTER_TYPE_P (init_type)) - || !initializer_zerop (elt_init)) + if ((POINTER_TYPE_P (elt_type) != POINTER_TYPE_P (init_type))) + last_nonzero = index; + else if (!type_initializer_zero_p (elt_type, elt_init)) last_nonzero = index; /* This can happen with an invalid initializer (c++/54501). */ diff --git a/gcc/testsuite/c-c++-common/array-1.c b/gcc/testsuite/c-c++-common/array-1.c new file mode 100644 index 00000000000..5de9ade4d43 --- /dev/null +++ b/gcc/testsuite/c-c++-common/array-1.c @@ -0,0 +1,247 @@ +// PR c++/90947 - Simple lookup table of array of strings is miscompiled +// { dg-do compile } +// { dg-options "-O1 -fdump-tree-optimized" } + +#define assert(expr) ((expr) ? (void)0 : __builtin_abort ()) + +void pr90947 (void) +{ + int vecsize = 4; + int index = 0; + static const char *a[4][4] = + { + { ".x", ".y", ".z", ".w" }, + { ".xy", ".yz", ".zw", 0 }, + { ".xyz", ".yzw", 0, 0 }, + { "", 0, 0, 0 }, + }; + + assert (vecsize >= 1 && vecsize <= 4); + assert (index >= 0 && index < 4); + assert (a[vecsize - 1][index]); +} + +void f_a1_1 (void) +{ + { + const char* a[1][1] = { { 0 } }; + assert (0 == a[0][0]); + } + { + const char* a[1][1] = { { "" } }; + assert ('\0' == *a[0][0]); + } +} + +void f_a2_1 (void) +{ + { + const char* a[2][1] = { { "" }, { "" } }; + assert ('\0' == *a[0][0] && '\0' == *a[1][0]); + } + { + const char* a[2][1] = { { 0 }, { "" } }; + assert (0 == a[0][0] && '\0' == *a[1][0]); + } + { + const char* a[2][1] = { { }, { "" } }; + assert (0 == a[0][0] && '\0' == *a[1][0]); + } +} + +void f_a2_2 (void) +{ + { + const char* a[2][2] = { { "", "" }, { "", "" } }; + assert ('\0' == *a[0][0] && '\0' == *a[0][1]); + assert ('\0' == *a[1][0] && '\0' == *a[1][1]); + } + { + const char* a[2][2] = { { "", "" }, { "", 0 } }; + assert ('\0' == *a[0][0] && '\0' == *a[0][1]); + assert ('\0' == *a[1][0] && 0 == a[1][1]); + } + { + const char* a[2][2] = { { "", "" }, { "" } }; + assert ('\0' == *a[0][0] && '\0' == *a[0][1]); + assert ('\0' == *a[1][0] && 0 == a[1][1]); + } + { + const char* a[2][2] = { { "", "" }, { 0, "" } }; + assert ('\0' == *a[0][0] && '\0' == *a[0][1]); + assert (0 == a[1][0] && '\0' == *a[1][1]); + } + { + const char* a[2][2] = { { "", 0 }, { 0, "" } }; + assert ('\0' == *a[0][0] && 0 == a[0][1]); + assert (0 == a[1][0] && '\0' == *a[1][1]); + } + { + const char* a[2][2] = { { 0, 0 }, { 0, "" } }; + assert (0 == a[0][0] && 0 == a[0][1]); + assert (0 == a[1][0] && '\0' == *a[1][1]); + } + { + const char* a[2][2] = { { 0 }, { 0, "" } }; + assert (0 == a[0][0] && 0 == a[0][1]); + assert (0 == a[1][0] && '\0' == *a[1][1]); + } + { + const char* a[2][2] = { { }, { 0, "" } }; + assert (0 == a[0][0] && 0 == a[0][1]); + assert (0 == a[1][0] && '\0' == *a[1][1]); + } +} + +void f_a2_2_2 (void) +{ + { + const char* a[2][2][2] = + { { { "", "" }, { "", "" } }, { { "", "" }, { "", "" } } }; + + assert ('\0' == *a[0][0][0] && '\0' == *a[0][0][1]); + assert ('\0' == *a[0][1][0] && '\0' == *a[0][1][1]); + assert ('\0' == *a[1][0][0] && '\0' == *a[1][0][1]); + assert ('\0' == *a[1][1][0] && '\0' == *a[1][1][1]); + } + + { + const char* a[2][2][2] = + { { { "", "" }, { "", "" } }, { { "", "" }, { 0, "" } } }; + + assert ('\0' == *a[0][0][0] && '\0' == *a[0][0][1]); + assert ('\0' == *a[0][1][0] && '\0' == *a[0][1][1]); + assert ('\0' == *a[1][0][0] && '\0' == *a[1][0][1]); + assert (0 == a[1][1][0] && '\0' == *a[1][1][1]); + } + + { + const char* a[2][2][2] = + { { { "", "" }, { "", "" } }, { { 0, 0 }, { 0, "" } } }; + + assert ('\0' == *a[0][0][0] && '\0' == *a[0][0][1]); + assert ('\0' == *a[0][1][0] && '\0' == *a[0][1][1]); + assert (0 == a[1][0][0] && 0 == a[1][0][1]); + assert (0 == a[1][1][0] && '\0' == *a[1][1][1]); + } + + { + const char* a[2][2][2] = + { { { "", "" }, { 0, 0 } }, { { 0, 0 }, { 0, "" } } }; + + assert ('\0' == *a[0][0][0] && '\0' == *a[0][0][1]); + assert (0 == a[0][1][0] && 0 == a[0][1][1]); + assert (0 == a[1][0][0] && 0 == a[1][0][1]); + assert (0 == a[1][1][0] && '\0' == *a[1][1][1]); + } + + { + const char* a[2][2][2] = + { { { 0, 0 }, { 0, 0 } }, { { 0, 0 }, { 0, "" } } }; + + assert (0 == a[0][0][0] && 0 == a[0][0][1]); + assert (0 == a[0][1][0] && 0 == a[0][1][1]); + assert (0 == a[1][0][0] && 0 == a[1][0][1]); + assert (0 == a[1][1][0] && '\0' == *a[1][1][1]); + } + + { + const char* a[2][2][2] = + { { { }, { } }, { { }, { 0, "" } } }; + + assert (0 == a[0][0][0] && 0 == a[0][0][1]); + assert (0 == a[0][1][0] && 0 == a[0][1][1]); + assert (0 == a[1][0][0] && 0 == a[1][0][1]); + assert (0 == a[1][1][0] && '\0' == *a[1][1][1]); + } +} + +void f_sa2_2_2 (void) +{ + struct S { const char a[2], *s, c; }; + + { + const struct S a[2][2][2] = { + { }, + { + { { }, { "", "" } }, + { } + } + }; + + assert ('\0' == *a[0][0][0].a && 0 == a[0][0][0].s && 0 == a[0][0][0].c); + assert ('\0' == *a[0][0][1].a && 0 == a[0][0][1].s && 0 == a[0][0][1].c); + assert ('\0' == *a[0][1][0].a && 0 == a[0][1][0].s && 0 == a[0][1][0].c); + assert ('\0' == *a[0][1][1].a && 0 == a[0][1][1].s && 0 == a[0][1][1].c); + + assert ('\0' == *a[1][0][0].a && 0 == a[1][0][0].s && 0 == a[1][0][0].c); + assert ('\0' == *a[1][0][1].a && '\0' == *a[1][0][1].s && 0 == a[1][0][1].c); + assert ('\0' == *a[1][1][0].a && 0 == a[1][1][0].s && 0 == a[1][1][0].c); + assert ('\0' == *a[1][1][1].a && 0 == a[1][1][1].s && 0 == a[1][1][1].c); + } + + { + const struct S a[2][2][2] = { + { }, + { + { { } }, + { { "", "" } } + } + }; + + assert ('\0' == *a[0][0][0].a && 0 == a[0][0][0].s); + assert ('\0' == *a[0][0][1].a && 0 == a[0][0][1].s); + assert ('\0' == *a[0][1][0].a && 0 == a[0][1][0].s); + assert ('\0' == *a[0][1][1].a && 0 == a[0][1][1].s); + + assert ('\0' == *a[1][0][0].a && 0 == a[1][0][0].s); + assert ('\0' == *a[1][0][1].a && 0 == a[1][0][1].s); + assert ('\0' == *a[1][1][0].a && '\0' == *a[1][1][0].s); + assert ('\0' == *a[1][1][1].a && 0 == a[1][1][1].s); + } + + { + const struct S a[2][2][2] = { + { }, + { + { { }, { } }, + { { }, { "", "", 0 } } + } + }; + + assert ('\0' == *a[0][0][0].a && 0 == a[0][0][0].s); + assert ('\0' == *a[0][0][1].a && 0 == a[0][0][1].s); + assert ('\0' == *a[0][1][0].a && 0 == a[0][1][0].s); + assert ('\0' == *a[0][1][1].a && 0 == a[0][1][1].s); + + assert ('\0' == *a[1][0][0].a && 0 == a[1][0][0].s); + assert ('\0' == *a[1][0][1].a && 0 == a[1][0][1].s); + assert ('\0' == *a[1][1][0].a && 0 == a[1][1][0].s); + assert ('\0' == *a[1][1][1].a && '\0' == *a[1][1][1].s); + } + + { + const struct S a[2][2][2] = { + { + { { { 0 }, 0, 0 }, { { 0 } , 0, 0 } }, + { { { 0 }, 0, 0 }, { { 0 } , 0, 0 } }, + }, + { + { { { 0 }, 0, 0 }, { { 0 } , 0, 0 } }, + { { }, { "", "", 0 } } + } + }; + + assert ('\0' == *a[0][0][0].a && 0 == a[0][0][0].s); + assert ('\0' == *a[0][0][1].a && 0 == a[0][0][1].s); + assert ('\0' == *a[0][1][0].a && 0 == a[0][1][0].s); + assert ('\0' == *a[0][1][1].a && 0 == a[0][1][1].s); + + assert ('\0' == *a[1][0][0].a && 0 == a[1][0][0].s); + assert ('\0' == *a[1][0][1].a && 0 == a[1][0][1].s); + assert ('\0' == *a[1][1][0].a && 0 == a[1][1][0].s); + assert ('\0' == *a[1][1][1].a && '\0' == *a[1][1][1].s); + } +} + +// { dg-final { scan-tree-dump-not "abort" "optimized" } } diff --git a/gcc/testsuite/g++.dg/abi/mangle73.C b/gcc/testsuite/g++.dg/abi/mangle73.C new file mode 100644 index 00000000000..2a5322a37c3 --- /dev/null +++ b/gcc/testsuite/g++.dg/abi/mangle73.C @@ -0,0 +1,96 @@ +// { dg-do compile { target c++2a } } + +struct A +{ + char a[2][2]; +}; + +template <A> struct B { }; + +typedef B<A{ { { 0, 0 }, { 0, 0 } } }> AZZZZ; +typedef B<A{ { { 0, 0 }, { 0 } } }> AZZZ_; +typedef B<A{ { { 0, 0 } } }> AZZ__; +typedef B<A{ { { 0 } } }> AZ___; +typedef B<A{ { { } } }> A____; + +typedef B<A{ { { "" }, { "" } } }> AS_S_; +typedef B<A{ { { "" }, { 0, 0 } } }> AS_ZZ; +typedef B<A{ { { "" }, { 0 } } }> AS_Z_; +typedef B<A{ { { "" } } }> AS___; + + +// Verify that the types mangle the same. +void a_zzzz (AZZZZ) { } +// { dg-final { scan-assembler "_Z6a_zzzz1BIXtl1AEEE" } } + +void a_zzz_ (AZZZ_) { } +// { dg-final { scan-assembler "_Z6a_zzz_1BIXtl1AEEE" } } + +void a_zz__ (AZZ__) { } +// { dg-final { scan-assembler "_Z6a_zz__1BIXtl1AEEE" } } + +void a_z___ (AZ___) { } +// { dg-final { scan-assembler "_Z6a_z___1BIXtl1AEEE" } } + +void a_____ (A____) { } +// { dg-final { scan-assembler "_Z6a_____1BIXtl1AEEE" } } + +void a_s_s_ (AS_S_) { } +// { dg-final { scan-assembler "_Z6a_s_s_1BIXtl1AEEE" } } + +void a_s_zz (AS_ZZ) { } +// { dg-final { scan-assembler "_Z6a_s_zz1BIXtl1AEEE" } } + +void a_s_z_ (AS_Z_) { } +// { dg-final { scan-assembler "_Z6a_s_z_1BIXtl1AEEE" } } + +void a_s___ (AS___) { } +// { dg-final { scan-assembler "_Z6a_s___1BIXtl1AEEE" } } + + +struct C +{ + struct { const char a[2][2], *p; } a[2]; +}; + +template <C> struct D { }; + +typedef D<C{{{{{ 0, 0 }, { 0, 0 }}, 0 }, {{{ 0, 0 }, { 0, 0 }}, 0 }}}> DZZZZZZZZZZ; +typedef D<C{{{{{ 0, 0 }, { 0, 0 }}, 0 }, {{{ 0, 0 }, { 0, 0 }}}}}> DZZZZZZZZZ_; +typedef D<C{{{{{ 0, 0 }, { 0, 0 }}, 0 }, {{{ 0, 0 }, { 0 }}}}}> DZZZZZZZZ__; +typedef D<C{{{{{ 0, 0 }, { 0, 0 }}, 0 }, {{{ 0, 0 } }}}}> DZZZZZZZ___; +typedef D<C{{{{{ 0, 0 }, { 0, 0 }}, 0 }, {{{ 0 } }}}}> DZZZZZZ____; +typedef D<C{{{{{ 0, 0 }, { 0, 0 }}, 0 }}}> DZZZZZ_____; +typedef D<C{{{{{ 0, 0 }, { 0, 0 }}}}}> DZZZZ______; +typedef D<C{{{{{ 0, 0 }, { 0 }}}}}> DZZZ_______; +typedef D<C{{{{{ 0, 0 }}}}}> DZZ________; +typedef D<C{{{{{ 0 }}}}}> DZ_________; +typedef D<C{ }> D__________; + +typedef D<C{{{{{ "" }, { "" }}, 0 }, {{{ "" }, { "" }}, 0 }}}> DS_S_ZS_S_Z; + +void d_zzzzzzzzzz (DZZZZZZZZZZ) { } +// { dg-final { scan-assembler "_Z12d_zzzzzzzzzz1DIXtl1CEEE" } } +void d_zzzzzzzzz_ (DZZZZZZZZZ_) { } +// { dg-final { scan-assembler "_Z12d_zzzzzzzzz_1DIXtl1CEEE" } } +void d_zzzzzzzz__ (DZZZZZZZZ__) { } +// { dg-final { scan-assembler "_Z12d_zzzzzzzz__1DIXtl1CEEE" } } +void d_zzzzzzz___ (DZZZZZZZ___) { } +// { dg-final { scan-assembler "_Z12d_zzzzzzz___1DIXtl1CEEE" } } +void d_zzzzzz____ (DZZZZZZ____) { } +// { dg-final { scan-assembler "_Z12d_zzzzzz____1DIXtl1CEEE" } } +void d_zzzzz_____ (DZZZZZ_____) { } +// { dg-final { scan-assembler "_Z12d_zzzzz_____1DIXtl1CEEE" } } +void d_zzzz______ (DZZZZ______) { } +// { dg-final { scan-assembler "_Z12d_zzzz______1DIXtl1CEEE" } } +void d_zzz_______ (DZZZ_______) { } +// { dg-final { scan-assembler "_Z12d_zzz_______1DIXtl1CEEE" } } +void d_zz________ (DZZ________) { } +// { dg-final { scan-assembler "_Z12d_zz________1DIXtl1CEEE" } } +void d_z_________ (DZ_________) { } +// { dg-final { scan-assembler "_Z12d_z_________1DIXtl1CEEE" } } +void d___________ (D__________) { } +// { dg-final { scan-assembler "_Z12d___________1DIXtl1CEEE" } } + +void d_s_s_zs_s_z (DS_S_ZS_S_Z) { } +// { dg-final { scan-assembler "_Z12d_s_s_zs_s_z1DIXtl1CEEE" } } diff --git a/gcc/testsuite/g++.dg/cpp2a/nontype-class18.C b/gcc/testsuite/g++.dg/cpp2a/nontype-class18.C new file mode 100644 index 00000000000..ab9e80fd335 --- /dev/null +++ b/gcc/testsuite/g++.dg/cpp2a/nontype-class18.C @@ -0,0 +1,102 @@ +// PR c++/90947 - Simple lookup table of array of strings is miscompiled +// Test to verify that the same specializations on non-type template +// parameters of class types are in fact treated as the same. Unlike +// nontype-class15.C which involves only one-dimensional arrays this +// test involves arrays of arrays and arrays of structs. +// { dg-do compile { target c++2a } } + +struct AA3 +{ + const char a[2][2][2]; +}; + +template <AA3> struct BAA3 { }; + +// Redeclare the same variable using different initialization forms +// of the same constant to verify that they are in fact all recognized +// as the same. +extern BAA3<AA3{{{ "", "" }, { "", "" }}}> baa3; +extern BAA3<AA3{{{ "", "" }, { "", { 0, 0 } }}}> baa3; +extern BAA3<AA3{{{ "", "" }, { "", { 0 } }}}> baa3; +extern BAA3<AA3{{{ "", "" }, { "", {} }}}> baa3; +extern BAA3<AA3{{{ "", "" }, { "" }}}> baa3; +extern BAA3<AA3{{{ "", "" }, { { 0, 0 } }}}> baa3; +extern BAA3<AA3{{{ "", "" }, { { 0 } }}}> baa3; +extern BAA3<AA3{{{ "", "" }, { {} }}}> baa3; +extern BAA3<AA3{{{ "", "" }, { }}}> baa3; +extern BAA3<AA3{{{ "", "" }}}> baa3; +extern BAA3<AA3{{{ "", { 0, 0 } }}}> baa3; +extern BAA3<AA3{{{ "", { 0 } }}}> baa3; +extern BAA3<AA3{{{ "", {} }}}> baa3; +extern BAA3<AA3{{{ "" }}}> baa3; +extern BAA3<AA3{{{ { 0, 0 } }}}> baa3; +extern BAA3<AA3{{{ { 0 } }}}> baa3; +extern BAA3<AA3{{{ {} }}}> baa3; +extern BAA3<AA3{{{ }}}> baa3; +extern BAA3<AA3{{ }}> baa3; +extern BAA3<AA3{ }> baa3; + +extern BAA3<AA3{{{ "", "" }, { "", "1" }}}> baa3_1; +extern BAA3<AA3{{{ "", "" }, { "", { '1', 0 } }}}> baa3_1; +extern BAA3<AA3{{{ "", "" }, { "", { '1' } }}}> baa3_1; + +extern BAA3<AA3{{{ "", "" }, { "1", {} }}}> baa3_2; +extern BAA3<AA3{{{ "", "" }, { "1" }}}> baa3_2; +extern BAA3<AA3{{{ "", "" }, { { '1', 0 } }}}> baa3_2; +extern BAA3<AA3{{{ "", "" }, { { '1' } }}}> baa3_2; + +extern BAA3<AA3{{{ "", "1" }}}> baa3_3; +extern BAA3<AA3{{{ "", { '1', 0 } }}}> baa3_3; +extern BAA3<AA3{{{ "", { '1' } }}}> baa3_3; + +extern BAA3<AA3{{{ "1" }}}> baa3_4; +extern BAA3<AA3{{{ { '1', 0 } }}}> baa3_4; +extern BAA3<AA3{{{ { '1' } }}}> baa3_4; + +struct AS2 +{ + struct S { const char a[2], *p; } a[2]; +}; + +template <AS2> struct BAS2 { }; + +extern BAS2<AS2{{{ "", 0 }, { "", 0 }}}> bas2; +extern BAS2<AS2{{{ "", 0 }, { {}, 0 }}}> bas2; +extern BAS2<AS2{{{ "", 0 }, { "" }}}> bas2; +extern BAS2<AS2{{{ "", 0 }, { {} }}}> bas2; +extern BAS2<AS2{{{ "", 0 }, { }}}> bas2; +extern BAS2<AS2{{{ "", 0 }}}> bas2; +extern BAS2<AS2{{{ {}, 0 }}}> bas2; +extern BAS2<AS2{{{ "" }}}> bas2; +extern BAS2<AS2{{{ {} }}}> bas2; +extern BAS2<AS2{{{ }}}> bas2; +extern BAS2<AS2{{ }}> bas2; +extern BAS2<AS2{ }> bas2; + +struct AS2_2 +{ + struct S { const char a[2], *p; } a[2][2]; +}; + +template <AS2_2> struct BAS2_2 { }; + +extern BAS2_2<AS2_2{{{{ "", 0 }, { "", 0 } }, { { "", 0 }, { "", 0 }}}}> b2_2; +extern BAS2_2<AS2_2{{{{ "", 0 }, { "", 0 } }, { { "", 0 }, { "" }}}}> b2_2; +extern BAS2_2<AS2_2{{{{ "", 0 }, { "", 0 } }, { { "", 0 }, { {} }}}}> b2_2; +extern BAS2_2<AS2_2{{{{ "", 0 }, { "", 0 } }, { { "", 0 }, { }}}}> b2_2; +extern BAS2_2<AS2_2{{{{ "", 0 }, { "", 0 } }, { { "", 0 } }}}> b2_2; +extern BAS2_2<AS2_2{{{{ "", 0 }, { "", 0 } }, { { "" } }}}> b2_2; +extern BAS2_2<AS2_2{{{{ "", 0 }, { "", 0 } }, { { {} } }}}> b2_2; +extern BAS2_2<AS2_2{{{{ "", 0 }, { "", 0 } }, { { }}}}> b2_2; +extern BAS2_2<AS2_2{{{{ "", 0 }, { "", 0 } }, { }}}> b2_2; +extern BAS2_2<AS2_2{{{{ "", 0 }, { "", 0 }}}}> b2_2; +extern BAS2_2<AS2_2{{{{ "", 0 }, { "" }}}}> b2_2; +extern BAS2_2<AS2_2{{{{ "", 0 }, { {} }}}}> b2_2; +extern BAS2_2<AS2_2{{{{ "", 0 }, { }}}}> b2_2; +extern BAS2_2<AS2_2{{{{ "", 0 }}}}> b2_2; +extern BAS2_2<AS2_2{{{{ "" }}}}> b2_2; +extern BAS2_2<AS2_2{{{{ {} }}}}> b2_2; +extern BAS2_2<AS2_2{{{{ }}}}> b2_2; +extern BAS2_2<AS2_2{{{ }}}> b2_2; +extern BAS2_2<AS2_2{{ }}> b2_2; +extern BAS2_2<AS2_2{ }> b2_2; diff --git a/gcc/tree.c b/gcc/tree.c index f65025f1089..ecc60b11a2c 100644 --- a/gcc/tree.c +++ b/gcc/tree.c @@ -11357,6 +11357,67 @@ initializer_each_zero_or_onep (const_tree expr) } } +/* Given an initializer INIT for a TYPE, return true if INIT is zero + so that it can be replaced by value initialization. This function + distinguishes betwen empty strings as initializers for arrays and + for pointers (which make it return false). */ + +bool +type_initializer_zero_p (tree type, tree init) +{ + STRIP_NOPS (init); + + if (POINTER_TYPE_P (type)) + return TREE_CODE (init) != STRING_CST && initializer_zerop (init); + + if (TREE_CODE (init) != CONSTRUCTOR) + return initializer_zerop (init); + + if (TREE_CODE (type) == ARRAY_TYPE) + { + tree elt_type = TREE_TYPE (type); + elt_type = TYPE_MAIN_VARIANT (elt_type); + if (elt_type == char_type_node) + return initializer_zerop (init); + + tree elt_init; + unsigned HOST_WIDE_INT i; + FOR_EACH_CONSTRUCTOR_VALUE (CONSTRUCTOR_ELTS (init), i, elt_init) + if (!type_initializer_zero_p (elt_type, elt_init)) + return false; + return true; + } + + if (TREE_CODE (type) != RECORD_TYPE) + return initializer_zerop (init); + + tree fld = TYPE_FIELDS (type); + + tree fld_init; + unsigned HOST_WIDE_INT i; + FOR_EACH_CONSTRUCTOR_VALUE (CONSTRUCTOR_ELTS (init), i, fld_init) + { + while (TREE_CODE (fld) != FIELD_DECL + || DECL_ARTIFICIAL (fld)) + { + fld = DECL_CHAIN (fld); + if (!fld) + return true; + continue; + } + + tree fldtype = TREE_TYPE (fld); + if (!type_initializer_zero_p (fldtype, fld_init)) + return false; + + fld = DECL_CHAIN (fld); + if (!fld) + break; + } + + return true; +} + /* Check if vector VEC consists of all the equal elements and that the number of elements corresponds to the type of VEC. The function returns first element of the vector diff --git a/gcc/tree.h b/gcc/tree.h index 23ac9b1ff5e..1082b6dab96 100644 --- a/gcc/tree.h +++ b/gcc/tree.h @@ -4558,6 +4558,12 @@ extern tree first_field (const_tree); extern bool initializer_zerop (const_tree, bool * = NULL); extern bool initializer_each_zero_or_onep (const_tree); +/* Analogous to initializer_zerop but examines the type for which + the initializer is being used but unlike it, considers empty + strings to be zero initializers for arrays and non-zero for + pointers. */ +extern bool type_initializer_zero_p (tree, tree); + extern wide_int vector_cst_int_elt (const_tree, unsigned int); extern tree vector_cst_elt (const_tree, unsigned int);
The solution we implemented in GCC 9 to get the mangling of non-type template arguments of class types containing array members consistent regardless of the form of their initialization introduced a couple of bugs. One of these is the subject of this patch. The bug results in stripping trailing initializers for array elements that involve the empty string such as in here: void f (void) { const char* a[1][1] = { "" }; if (!a[0][0]) __builtin_abort (); } The problem is caused by relying on initializer_zerop() that returns true for the empty string regardless of whether it's used to initialize an array or a pointer (the function doesn't know what the initializer is being used for). To handle this correctly the attached patch introduces a new function, type_initializer_zero_p, that takes in addition to an initializer also the type it is being used to initialize. The function then traverses both arguments and returns true only if each string is being used to initialize an array and false if any of them is being used to initialize a pointer. Since the function is generic the patch adds it to tree.c rather than somewhere in the C++ front-end. Tested on x86_64-linux. Martin