| Message ID | 20180125210937.GA85544@kam.mff.cuni.cz |
|---|---|
| State | New |
| Headers | show |
| Series | Silence false positive on LTO type merging waring | expand |
On Thu, 25 Jan 2018, Jan Hubicka wrote: > Hi, > the testcase triggers invalid warning on type mismatch because array > of pointers to complete type has different alias set from array of pointers > to incomplete type. This is valid, because incoplete pointer has alias set > of void_ptr which alias all pointers and arrays alias with their members. But isn't that a problem? Not if the pointer to incomlete type have alias set zero but IIRC they don't, right? Richard. > I already silenced the wanring for pointers but I forgot about arrays. > > Bootstrapped/regtested x86_64-linux, OK? > > * gcc.dg/lto/pr83954.h: New testcase. > * gcc.dg/lto/pr83954_0.c: New testcase. > * gcc.dg/lto/pr83954_1.c: New testcase. > * lto-symtab.c (warn_type_compatibility_p): Silence false positive > for type match warning on arrays of pointers. > > Index: testsuite/gcc.dg/lto/pr83954.h > =================================================================== > --- testsuite/gcc.dg/lto/pr83954.h (revision 0) > +++ testsuite/gcc.dg/lto/pr83954.h (working copy) > @@ -0,0 +1,3 @@ > +struct foo; > +extern struct foo *FOO_PTR_ARR[1]; > + > Index: testsuite/gcc.dg/lto/pr83954_0.c > =================================================================== > --- testsuite/gcc.dg/lto/pr83954_0.c (revision 0) > +++ testsuite/gcc.dg/lto/pr83954_0.c (working copy) > @@ -0,0 +1,8 @@ > +/* { dg-lto-do link } */ > +#include "pr83954.h" > + > +int main() { > + // just to prevent symbol removal > + FOO_PTR_ARR[1] = 0; > + return 0; > +} > Index: testsuite/gcc.dg/lto/pr83954_1.c > =================================================================== > --- testsuite/gcc.dg/lto/pr83954_1.c (revision 0) > +++ testsuite/gcc.dg/lto/pr83954_1.c (working copy) > @@ -0,0 +1,7 @@ > +#include "pr83954.h" > + > +struct foo { > + int x; > +}; > +struct foo *FOO_PTR_ARR[1] = { 0 }; > + > Index: lto/lto-symtab.c > =================================================================== > --- lto/lto-symtab.c (revision 257009) > +++ lto/lto-symtab.c (working copy) > @@ -284,11 +284,22 @@ warn_type_compatibility_p (tree prevaili > alias_set_type set1 = get_alias_set (type); > alias_set_type set2 = get_alias_set (prevailing_type); > > - if (set1 && set2 && set1 != set2 > - && (!POINTER_TYPE_P (type) || !POINTER_TYPE_P (prevailing_type) > + if (set1 && set2 && set1 != set2) > + { > + tree t1 = type, t2 = prevailing_type; > + > + /* Alias sets of arrays are the same as alias sets of the inner > + types. */ > + while (TREE_CODE (t1) == ARRAY_TYPE && TREE_CODE (t2) == ARRAY_TYPE) > + { > + t1 = TREE_TYPE (t1); > + t2 = TREE_TYPE (t2); > + } > + if ((!POINTER_TYPE_P (t1) || !POINTER_TYPE_P (t2)) > || (set1 != TYPE_ALIAS_SET (ptr_type_node) > - && set2 != TYPE_ALIAS_SET (ptr_type_node)))) > - lev |= 5; > + && set2 != TYPE_ALIAS_SET (ptr_type_node))) > + lev |= 5; > + } > } > > return lev; > >
> On Thu, 25 Jan 2018, Jan Hubicka wrote: > > > Hi, > > the testcase triggers invalid warning on type mismatch because array > > of pointers to complete type has different alias set from array of pointers > > to incomplete type. This is valid, because incoplete pointer has alias set > > of void_ptr which alias all pointers and arrays alias with their members. > > But isn't that a problem? Not if the pointer to incomlete type have > alias set zero but IIRC they don't, right? pointers to incomplete type are same as alias set of ptr_type_node (1 in this case) and pointer to complete types have different alias set but that set has is_pointer flag. When checking for conflict we make 1 to alias with everything that has is_pointer flag in it, so it will return true for these two array types. Honza > > Richard. > > > I already silenced the wanring for pointers but I forgot about arrays. > > > > Bootstrapped/regtested x86_64-linux, OK? > > > > * gcc.dg/lto/pr83954.h: New testcase. > > * gcc.dg/lto/pr83954_0.c: New testcase. > > * gcc.dg/lto/pr83954_1.c: New testcase. > > * lto-symtab.c (warn_type_compatibility_p): Silence false positive > > for type match warning on arrays of pointers. > > > > Index: testsuite/gcc.dg/lto/pr83954.h > > =================================================================== > > --- testsuite/gcc.dg/lto/pr83954.h (revision 0) > > +++ testsuite/gcc.dg/lto/pr83954.h (working copy) > > @@ -0,0 +1,3 @@ > > +struct foo; > > +extern struct foo *FOO_PTR_ARR[1]; > > + > > Index: testsuite/gcc.dg/lto/pr83954_0.c > > =================================================================== > > --- testsuite/gcc.dg/lto/pr83954_0.c (revision 0) > > +++ testsuite/gcc.dg/lto/pr83954_0.c (working copy) > > @@ -0,0 +1,8 @@ > > +/* { dg-lto-do link } */ > > +#include "pr83954.h" > > + > > +int main() { > > + // just to prevent symbol removal > > + FOO_PTR_ARR[1] = 0; > > + return 0; > > +} > > Index: testsuite/gcc.dg/lto/pr83954_1.c > > =================================================================== > > --- testsuite/gcc.dg/lto/pr83954_1.c (revision 0) > > +++ testsuite/gcc.dg/lto/pr83954_1.c (working copy) > > @@ -0,0 +1,7 @@ > > +#include "pr83954.h" > > + > > +struct foo { > > + int x; > > +}; > > +struct foo *FOO_PTR_ARR[1] = { 0 }; > > + > > Index: lto/lto-symtab.c > > =================================================================== > > --- lto/lto-symtab.c (revision 257009) > > +++ lto/lto-symtab.c (working copy) > > @@ -284,11 +284,22 @@ warn_type_compatibility_p (tree prevaili > > alias_set_type set1 = get_alias_set (type); > > alias_set_type set2 = get_alias_set (prevailing_type); > > > > - if (set1 && set2 && set1 != set2 > > - && (!POINTER_TYPE_P (type) || !POINTER_TYPE_P (prevailing_type) > > + if (set1 && set2 && set1 != set2) > > + { > > + tree t1 = type, t2 = prevailing_type; > > + > > + /* Alias sets of arrays are the same as alias sets of the inner > > + types. */ > > + while (TREE_CODE (t1) == ARRAY_TYPE && TREE_CODE (t2) == ARRAY_TYPE) > > + { > > + t1 = TREE_TYPE (t1); > > + t2 = TREE_TYPE (t2); > > + } > > + if ((!POINTER_TYPE_P (t1) || !POINTER_TYPE_P (t2)) > > || (set1 != TYPE_ALIAS_SET (ptr_type_node) > > - && set2 != TYPE_ALIAS_SET (ptr_type_node)))) > > - lev |= 5; > > + && set2 != TYPE_ALIAS_SET (ptr_type_node))) > > + lev |= 5; > > + } > > } > > > > return lev; > > > > > > -- > Richard Biener <rguenther@suse.de> > SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nuernberg)
On Fri, 26 Jan 2018, Jan Hubicka wrote: > > On Thu, 25 Jan 2018, Jan Hubicka wrote: > > > > > Hi, > > > the testcase triggers invalid warning on type mismatch because array > > > of pointers to complete type has different alias set from array of pointers > > > to incomplete type. This is valid, because incoplete pointer has alias set > > > of void_ptr which alias all pointers and arrays alias with their members. > > > > But isn't that a problem? Not if the pointer to incomlete type have > > alias set zero but IIRC they don't, right? > > pointers to incomplete type are same as alias set of ptr_type_node (1 in this case) > and pointer to complete types have different alias set but that set has > is_pointer flag. When checking for conflict we make 1 to alias with everything that > has is_pointer flag in it, so it will return true for these two array types. Ah, ok. The patch is ok then. I suppose in theory complex pointers or vector pointers would have the same issue if we'd ever generate those (GIMPLE at least doens't like them ;)). Richard. > Honza > > > > Richard. > > > > > I already silenced the wanring for pointers but I forgot about arrays. > > > > > > Bootstrapped/regtested x86_64-linux, OK? > > > > > > * gcc.dg/lto/pr83954.h: New testcase. > > > * gcc.dg/lto/pr83954_0.c: New testcase. > > > * gcc.dg/lto/pr83954_1.c: New testcase. > > > * lto-symtab.c (warn_type_compatibility_p): Silence false positive > > > for type match warning on arrays of pointers. > > > > > > Index: testsuite/gcc.dg/lto/pr83954.h > > > =================================================================== > > > --- testsuite/gcc.dg/lto/pr83954.h (revision 0) > > > +++ testsuite/gcc.dg/lto/pr83954.h (working copy) > > > @@ -0,0 +1,3 @@ > > > +struct foo; > > > +extern struct foo *FOO_PTR_ARR[1]; > > > + > > > Index: testsuite/gcc.dg/lto/pr83954_0.c > > > =================================================================== > > > --- testsuite/gcc.dg/lto/pr83954_0.c (revision 0) > > > +++ testsuite/gcc.dg/lto/pr83954_0.c (working copy) > > > @@ -0,0 +1,8 @@ > > > +/* { dg-lto-do link } */ > > > +#include "pr83954.h" > > > + > > > +int main() { > > > + // just to prevent symbol removal > > > + FOO_PTR_ARR[1] = 0; > > > + return 0; > > > +} > > > Index: testsuite/gcc.dg/lto/pr83954_1.c > > > =================================================================== > > > --- testsuite/gcc.dg/lto/pr83954_1.c (revision 0) > > > +++ testsuite/gcc.dg/lto/pr83954_1.c (working copy) > > > @@ -0,0 +1,7 @@ > > > +#include "pr83954.h" > > > + > > > +struct foo { > > > + int x; > > > +}; > > > +struct foo *FOO_PTR_ARR[1] = { 0 }; > > > + > > > Index: lto/lto-symtab.c > > > =================================================================== > > > --- lto/lto-symtab.c (revision 257009) > > > +++ lto/lto-symtab.c (working copy) > > > @@ -284,11 +284,22 @@ warn_type_compatibility_p (tree prevaili > > > alias_set_type set1 = get_alias_set (type); > > > alias_set_type set2 = get_alias_set (prevailing_type); > > > > > > - if (set1 && set2 && set1 != set2 > > > - && (!POINTER_TYPE_P (type) || !POINTER_TYPE_P (prevailing_type) > > > + if (set1 && set2 && set1 != set2) > > > + { > > > + tree t1 = type, t2 = prevailing_type; > > > + > > > + /* Alias sets of arrays are the same as alias sets of the inner > > > + types. */ > > > + while (TREE_CODE (t1) == ARRAY_TYPE && TREE_CODE (t2) == ARRAY_TYPE) > > > + { > > > + t1 = TREE_TYPE (t1); > > > + t2 = TREE_TYPE (t2); > > > + } > > > + if ((!POINTER_TYPE_P (t1) || !POINTER_TYPE_P (t2)) > > > || (set1 != TYPE_ALIAS_SET (ptr_type_node) > > > - && set2 != TYPE_ALIAS_SET (ptr_type_node)))) > > > - lev |= 5; > > > + && set2 != TYPE_ALIAS_SET (ptr_type_node))) > > > + lev |= 5; > > > + } > > > } > > > > > > return lev; > > > > > > > > > > -- > > Richard Biener <rguenther@suse.de> > > SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nuernberg) > >
> On Fri, 26 Jan 2018, Jan Hubicka wrote: > > > > On Thu, 25 Jan 2018, Jan Hubicka wrote: > > > > > > > Hi, > > > > the testcase triggers invalid warning on type mismatch because array > > > > of pointers to complete type has different alias set from array of pointers > > > > to incomplete type. This is valid, because incoplete pointer has alias set > > > > of void_ptr which alias all pointers and arrays alias with their members. > > > > > > But isn't that a problem? Not if the pointer to incomlete type have > > > alias set zero but IIRC they don't, right? > > > > pointers to incomplete type are same as alias set of ptr_type_node (1 in this case) > > and pointer to complete types have different alias set but that set has > > is_pointer flag. When checking for conflict we make 1 to alias with everything that > > has is_pointer flag in it, so it will return true for these two array types. > > Ah, ok. The patch is ok then. I suppose in theory complex pointers > or vector pointers would have the same issue if we'd ever generate > those (GIMPLE at least doens't like them ;)). Yep, VECTOR_TYPE is only remaining case where get_alias_set recurse, but I would wait for those to be actually used. We will not only generate them but produce static variables of them for waring to triger. Honza
Index: testsuite/gcc.dg/lto/pr83954.h =================================================================== --- testsuite/gcc.dg/lto/pr83954.h (revision 0) +++ testsuite/gcc.dg/lto/pr83954.h (working copy) @@ -0,0 +1,3 @@ +struct foo; +extern struct foo *FOO_PTR_ARR[1]; + Index: testsuite/gcc.dg/lto/pr83954_0.c =================================================================== --- testsuite/gcc.dg/lto/pr83954_0.c (revision 0) +++ testsuite/gcc.dg/lto/pr83954_0.c (working copy) @@ -0,0 +1,8 @@ +/* { dg-lto-do link } */ +#include "pr83954.h" + +int main() { + // just to prevent symbol removal + FOO_PTR_ARR[1] = 0; + return 0; +} Index: testsuite/gcc.dg/lto/pr83954_1.c =================================================================== --- testsuite/gcc.dg/lto/pr83954_1.c (revision 0) +++ testsuite/gcc.dg/lto/pr83954_1.c (working copy) @@ -0,0 +1,7 @@ +#include "pr83954.h" + +struct foo { + int x; +}; +struct foo *FOO_PTR_ARR[1] = { 0 }; + Index: lto/lto-symtab.c =================================================================== --- lto/lto-symtab.c (revision 257009) +++ lto/lto-symtab.c (working copy) @@ -284,11 +284,22 @@ warn_type_compatibility_p (tree prevaili alias_set_type set1 = get_alias_set (type); alias_set_type set2 = get_alias_set (prevailing_type); - if (set1 && set2 && set1 != set2 - && (!POINTER_TYPE_P (type) || !POINTER_TYPE_P (prevailing_type) + if (set1 && set2 && set1 != set2) + { + tree t1 = type, t2 = prevailing_type; + + /* Alias sets of arrays are the same as alias sets of the inner + types. */ + while (TREE_CODE (t1) == ARRAY_TYPE && TREE_CODE (t2) == ARRAY_TYPE) + { + t1 = TREE_TYPE (t1); + t2 = TREE_TYPE (t2); + } + if ((!POINTER_TYPE_P (t1) || !POINTER_TYPE_P (t2)) || (set1 != TYPE_ALIAS_SET (ptr_type_node) - && set2 != TYPE_ALIAS_SET (ptr_type_node)))) - lev |= 5; + && set2 != TYPE_ALIAS_SET (ptr_type_node))) + lev |= 5; + } } return lev;