| Message ID | 223aa096afbdbb177d4ad5245696d439ad4cf87f.camel@tugraz.at |
|---|---|
| State | New |
| Headers | show |
| Series | [1/4] c23: tag compatibility rules for struct and unions | expand |
On Thu, 16 Nov 2023, Martin Uecker wrote: > Tell the backend which types are equivalent by setting > TYPE_CANONICAL to one struct in the set of equivalent > structs. Structs are considered equivalent by ignoring > all sizes of arrays nested in types below field level. Is TYPE_CANONICAL *only* used for alias analysis? It's not obvious to me that setting TYPE_CANONICAL to a type that's definitely not equivalent for other purposes is necessarily safe. I also think more rationale is needed for ignoring sizes like this. Is it intended for e.g. making structs with flexible array members alias-compatible with similar structs with a fixed-size array? > @@ -1250,6 +1266,9 @@ comptypes_internal (const_tree type1, const_tree type2, > > if ((d1 == NULL_TREE) != (d2 == NULL_TREE)) > data->different_types_p = true; > + /* ignore size mismatches */ > + if (data->equiv) > + return 1; Should start comment with capital letter, end with '.'. > diff --git a/gcc/testsuite/gcc.dg/c23-tag-2.c b/gcc/testsuite/gcc.dg/c23-tag-2.c > index 5dd4a21e9df..e28c2b5eea2 100644 > --- a/gcc/testsuite/gcc.dg/c23-tag-2.c > +++ b/gcc/testsuite/gcc.dg/c23-tag-2.c > @@ -1,5 +1,5 @@ > -/* { dg-do compile { target { ! "*-*-*" } } } > - * { dg-options "-std=c23" } > +/* { dg-do compile } > + * { dg-options "-std=c2x" } > */ > > // compatibility of structs in assignment > diff --git a/gcc/testsuite/gcc.dg/c23-tag-5.c b/gcc/testsuite/gcc.dg/c23-tag-5.c > index ff40d07aef1..95a04bf9b0e 100644 > --- a/gcc/testsuite/gcc.dg/c23-tag-5.c > +++ b/gcc/testsuite/gcc.dg/c23-tag-5.c > @@ -1,5 +1,6 @@ > -/* { dg-do run { target { ! "*-*-*" } } } > - * { dg-options "-std=c23" } > +/* > + * { dg-do run } > + * { dg-options "-std=c2x" } These tests should not be changed to use -std=c2x. > diff --git a/gcc/testsuite/gcc.dg/c23-tag-alias-2.c b/gcc/testsuite/gcc.dg/c23-tag-alias-2.c > new file mode 100644 > index 00000000000..555c30a8501 > --- /dev/null > +++ b/gcc/testsuite/gcc.dg/c23-tag-alias-2.c > @@ -0,0 +1,73 @@ > +/* > + * { dg-do run } > + * { dg-options "-std=c23 -O2" } > + */ > + > + > +struct foo { int x; }; > + > +int test_foo1(struct foo* a, void* b) > +{ > + a->x = 1; > + > + struct foo { int x; int y; }* p = b; > + p->x = 2; > + > + return a->x; > +} > +int main() > +{ > + struct foo y; > + > + if (1 != test_foo1(&y, &y)) > + __builtin_abort(); This test appears to be testing various invalid cases - testing that the compiler does not consider aliasing to occur in those cases (even though in fact there is aliasing). If that's the intent of this test, it definitely needs commenting. The test would also need to (be a gnu23-* test and) use appropriate attributes to disable interprocedural analysis, since it would be entirely valid for the compiler in this test to inline test_foo1, see that p->x in fact points to the same location as a->x despite the incompatible types, and have the function return 2. The same applies to c23-tag-alias-4.c and c23-tag-alias-5.c. > diff --git a/gcc/testsuite/gcc.dg/c23-tag-alias-5.c b/gcc/testsuite/gcc.dg/c23-tag-alias-5.c > new file mode 100644 > index 00000000000..4e956720143 > --- /dev/null > +++ b/gcc/testsuite/gcc.dg/c23-tag-alias-5.c > @@ -0,0 +1,30 @@ > +/* { dg-do run } > + * { dg-options "-std=c23 -O2" } > + */ > + > +// not sure this is wise, but this was already like thi sbefore "this before"
Thanks Joseph, I will sent an updated series tomorrow. Richard, maybe you could look at what I wrote below about my use of TYPE_CANONICAL ? Does this make sense? Am Donnerstag, dem 23.11.2023 um 23:47 +0000 schrieb Joseph Myers: > On Thu, 16 Nov 2023, Martin Uecker wrote: > > > Tell the backend which types are equivalent by setting > > TYPE_CANONICAL to one struct in the set of equivalent > > structs. Structs are considered equivalent by ignoring > > all sizes of arrays nested in types below field level. > > Is TYPE_CANONICAL *only* used for alias analysis? It's not obvious to me > that setting TYPE_CANONICAL to a type that's definitely not equivalent for > other purposes is necessarily safe. My understand is that it is used for aliasing analysis and also checking of conversions. TYPE_CANONICAL must be consistent with the idea the middle-end has about type conversions. But as long as we do not give the same TYPE_CANONICAL to types the middle-end thinks must be incompatible using its own type checking machinery, it should be safe even for types the C standard thinks must be incompatible for some reason. > I also think more rationale is needed for ignoring sizes like this. Is it > intended for e.g. making structs with flexible array members > alias-compatible with similar structs with a fixed-size array? The main reason are pointers to arrays: struct foo { int (*x)[]; } struct foo { int (*x)[2]; }; struct foo { int (*x)[1]; }; So at least when putting it in terms of equivalence classes, one has no choice than making those types equivalent. So all those would get the same TYPE_CANONICAL. The middle-end does not care about the different pointer types (in useless_type_conversion_p or gimple_canonical_types_compatible_p). Martin > > > @@ -1250,6 +1266,9 @@ comptypes_internal (const_tree type1, const_tree type2, > > > > if ((d1 == NULL_TREE) != (d2 == NULL_TREE)) > > data->different_types_p = true; > > + /* ignore size mismatches */ > > + if (data->equiv) > > + return 1; > > Should start comment with capital letter, end with '.'. > > > diff --git a/gcc/testsuite/gcc.dg/c23-tag-2.c b/gcc/testsuite/gcc.dg/c23-tag-2.c > > index 5dd4a21e9df..e28c2b5eea2 100644 > > --- a/gcc/testsuite/gcc.dg/c23-tag-2.c > > +++ b/gcc/testsuite/gcc.dg/c23-tag-2.c > > @@ -1,5 +1,5 @@ > > -/* { dg-do compile { target { ! "*-*-*" } } } > > - * { dg-options "-std=c23" } > > +/* { dg-do compile } > > + * { dg-options "-std=c2x" } > > */ > > > > // compatibility of structs in assignment > > diff --git a/gcc/testsuite/gcc.dg/c23-tag-5.c b/gcc/testsuite/gcc.dg/c23-tag-5.c > > index ff40d07aef1..95a04bf9b0e 100644 > > --- a/gcc/testsuite/gcc.dg/c23-tag-5.c > > +++ b/gcc/testsuite/gcc.dg/c23-tag-5.c > > @@ -1,5 +1,6 @@ > > -/* { dg-do run { target { ! "*-*-*" } } } > > - * { dg-options "-std=c23" } > > +/* > > + * { dg-do run } > > + * { dg-options "-std=c2x" } > > These tests should not be changed to use -std=c2x. > > > diff --git a/gcc/testsuite/gcc.dg/c23-tag-alias-2.c b/gcc/testsuite/gcc.dg/c23-tag-alias-2.c > > new file mode 100644 > > index 00000000000..555c30a8501 > > --- /dev/null > > +++ b/gcc/testsuite/gcc.dg/c23-tag-alias-2.c > > @@ -0,0 +1,73 @@ > > +/* > > + * { dg-do run } > > + * { dg-options "-std=c23 -O2" } > > + */ > > + > > + > > +struct foo { int x; }; > > + > > +int test_foo1(struct foo* a, void* b) > > +{ > > + a->x = 1; > > + > > + struct foo { int x; int y; }* p = b; > > + p->x = 2; > > + > > + return a->x; > > +} > > > +int main() > > +{ > > + struct foo y; > > + > > + if (1 != test_foo1(&y, &y)) > > + __builtin_abort(); > > This test appears to be testing various invalid cases - testing that the > compiler does not consider aliasing to occur in those cases (even though > in fact there is aliasing). > > If that's the intent of this test, it definitely needs commenting. The > test would also need to (be a gnu23-* test and) use appropriate attributes > to disable interprocedural analysis, since it would be entirely valid for > the compiler in this test to inline test_foo1, see that p->x in fact > points to the same location as a->x despite the incompatible types, and > have the function return 2. > > The same applies to c23-tag-alias-4.c and c23-tag-alias-5.c. > > > diff --git a/gcc/testsuite/gcc.dg/c23-tag-alias-5.c b/gcc/testsuite/gcc.dg/c23-tag-alias-5.c > > new file mode 100644 > > index 00000000000..4e956720143 > > --- /dev/null > > +++ b/gcc/testsuite/gcc.dg/c23-tag-alias-5.c > > @@ -0,0 +1,30 @@ > > +/* { dg-do run } > > + * { dg-options "-std=c23 -O2" } > > + */ > > + > > +// not sure this is wise, but this was already like thi sbefore > > "this before" >
On Sun, 26 Nov 2023, Martin Uecker wrote: > > Thanks Joseph, I will sent an updated series tomorrow. > > Richard, maybe you could look at what I wrote below > about my use of TYPE_CANONICAL ? Does this make sense? > > > Am Donnerstag, dem 23.11.2023 um 23:47 +0000 schrieb Joseph Myers: > > On Thu, 16 Nov 2023, Martin Uecker wrote: > > > > > Tell the backend which types are equivalent by setting > > > TYPE_CANONICAL to one struct in the set of equivalent > > > structs. Structs are considered equivalent by ignoring > > > all sizes of arrays nested in types below field level. > > > > Is TYPE_CANONICAL *only* used for alias analysis? It's not obvious to me > > that setting TYPE_CANONICAL to a type that's definitely not equivalent for > > other purposes is necessarily safe. > > My understand is that it is used for aliasing analysis and also > checking of conversions. TYPE_CANONICAL must be consistent with > the idea the middle-end has about type conversions. But as long > as we do not give the same TYPE_CANONICAL to types the middle-end > thinks must be incompatible using its own type checking machinery, > it should be safe even for types the C standard thinks must be > incompatible for some reason. TYPE_CANONICAL is used to record structurally equivalent types that should be in a common class for TBAA. GIMPLE allows direct assignment of structurally equivalent types in aggregate copies and with LTO we are forming conservatively larger such groups to make cross-language TBAA possible. That means when the language standards says two types are compatible (and for example valid to assign to each other) then they have to have the same canonical type (or the frontend has to insert conversions). It's also the only means besides the type variant chain to assign the same alias set to a class of types when LTO is in effect (the exception is when the langhook assigns alias-set zero which is honored). > > I also think more rationale is needed for ignoring sizes like this. Is it > > intended for e.g. making structs with flexible array members > > alias-compatible with similar structs with a fixed-size array? > > The main reason are pointers to arrays: > > struct foo { int (*x)[]; } > struct foo { int (*x)[2]; }; > struct foo { int (*x)[1]; }; > > So at least when putting it in terms of equivalence classes, > one has no choice than making those types equivalent. So > all those would get the same TYPE_CANONICAL. The middle-end > does not care about the different pointer types (in > useless_type_conversion_p or > gimple_canonical_types_compatible_p). Indeed LTO would put those into the same equivalence class. Richard. > > Martin > > > > > > > > > @@ -1250,6 +1266,9 @@ comptypes_internal (const_tree type1, const_tree type2, > > > > > > if ((d1 == NULL_TREE) != (d2 == NULL_TREE)) > > > data->different_types_p = true; > > > + /* ignore size mismatches */ > > > + if (data->equiv) > > > + return 1; > > > > Should start comment with capital letter, end with '.'. > > > > > diff --git a/gcc/testsuite/gcc.dg/c23-tag-2.c b/gcc/testsuite/gcc.dg/c23-tag-2.c > > > index 5dd4a21e9df..e28c2b5eea2 100644 > > > --- a/gcc/testsuite/gcc.dg/c23-tag-2.c > > > +++ b/gcc/testsuite/gcc.dg/c23-tag-2.c > > > @@ -1,5 +1,5 @@ > > > -/* { dg-do compile { target { ! "*-*-*" } } } > > > - * { dg-options "-std=c23" } > > > +/* { dg-do compile } > > > + * { dg-options "-std=c2x" } > > > */ > > > > > > // compatibility of structs in assignment > > > diff --git a/gcc/testsuite/gcc.dg/c23-tag-5.c b/gcc/testsuite/gcc.dg/c23-tag-5.c > > > index ff40d07aef1..95a04bf9b0e 100644 > > > --- a/gcc/testsuite/gcc.dg/c23-tag-5.c > > > +++ b/gcc/testsuite/gcc.dg/c23-tag-5.c > > > @@ -1,5 +1,6 @@ > > > -/* { dg-do run { target { ! "*-*-*" } } } > > > - * { dg-options "-std=c23" } > > > +/* > > > + * { dg-do run } > > > + * { dg-options "-std=c2x" } > > > > These tests should not be changed to use -std=c2x. > > > > > diff --git a/gcc/testsuite/gcc.dg/c23-tag-alias-2.c b/gcc/testsuite/gcc.dg/c23-tag-alias-2.c > > > new file mode 100644 > > > index 00000000000..555c30a8501 > > > --- /dev/null > > > +++ b/gcc/testsuite/gcc.dg/c23-tag-alias-2.c > > > @@ -0,0 +1,73 @@ > > > +/* > > > + * { dg-do run } > > > + * { dg-options "-std=c23 -O2" } > > > + */ > > > + > > > + > > > +struct foo { int x; }; > > > + > > > +int test_foo1(struct foo* a, void* b) > > > +{ > > > + a->x = 1; > > > + > > > + struct foo { int x; int y; }* p = b; > > > + p->x = 2; > > > + > > > + return a->x; > > > +} > > > > > +int main() > > > +{ > > > + struct foo y; > > > + > > > + if (1 != test_foo1(&y, &y)) > > > + __builtin_abort(); > > > > This test appears to be testing various invalid cases - testing that the > > compiler does not consider aliasing to occur in those cases (even though > > in fact there is aliasing). > > > > If that's the intent of this test, it definitely needs commenting. The > > test would also need to (be a gnu23-* test and) use appropriate attributes > > to disable interprocedural analysis, since it would be entirely valid for > > the compiler in this test to inline test_foo1, see that p->x in fact > > points to the same location as a->x despite the incompatible types, and > > have the function return 2. > > > > The same applies to c23-tag-alias-4.c and c23-tag-alias-5.c. > > > > > diff --git a/gcc/testsuite/gcc.dg/c23-tag-alias-5.c b/gcc/testsuite/gcc.dg/c23-tag-alias-5.c > > > new file mode 100644 > > > index 00000000000..4e956720143 > > > --- /dev/null > > > +++ b/gcc/testsuite/gcc.dg/c23-tag-alias-5.c > > > @@ -0,0 +1,30 @@ > > > +/* { dg-do run } > > > + * { dg-options "-std=c23 -O2" } > > > + */ > > > + > > > +// not sure this is wise, but this was already like thi sbefore > > > > "this before" > > > >
On Sun, 26 Nov 2023, Martin Uecker wrote: > My understand is that it is used for aliasing analysis and also > checking of conversions. TYPE_CANONICAL must be consistent with > the idea the middle-end has about type conversions. But as long > as we do not give the same TYPE_CANONICAL to types the middle-end > thinks must be incompatible using its own type checking machinery, > it should be safe even for types the C standard thinks must be > incompatible for some reason. And presumably also for types that definitely can't be assigned because they have incompatible layout through the use of different array sizes - since the front end won't generate such assignments, it would never matter whether the middle end considers them valid without conversion or not? > > I also think more rationale is needed for ignoring sizes like this. Is it > > intended for e.g. making structs with flexible array members > > alias-compatible with similar structs with a fixed-size array? > > The main reason are pointers to arrays: > > struct foo { int (*x)[]; } > struct foo { int (*x)[2]; }; > struct foo { int (*x)[1]; }; Thanks for the explanation. I guess the cases involving flexible array members actually show up a bug in the standard rather than any kind of issue with this patch - the standard allows one structure ending with a flexible array member, and another ending with a fixed-size array, to be compatible (and in different translation units allowed that even before C23), but there is also clear text in the standard showing it's not intended to require the layout to be consistent (the fixed-size and flexible arrays might have different offsets), and what you'd actually get with an assignment or conditional expression mixing such structures certainly isn't clearly specified. Maybe the right resolution for that issue with the standard would be to make that particular case incompatible, but it would need to be raised as an issue after C23 is out.
Am Dienstag, dem 28.11.2023 um 01:00 +0000 schrieb Joseph Myers: > On Sun, 26 Nov 2023, Martin Uecker wrote: > > > My understand is that it is used for aliasing analysis and also > > checking of conversions. TYPE_CANONICAL must be consistent with > > the idea the middle-end has about type conversions. But as long > > as we do not give the same TYPE_CANONICAL to types the middle-end > > thinks must be incompatible using its own type checking machinery, > > it should be safe even for types the C standard thinks must be > > incompatible for some reason. > > And presumably also for types that definitely can't be assigned because > they have incompatible layout through the use of different array sizes - > since the front end won't generate such assignments, it would never matter > whether the middle end considers them valid without conversion or not? Yes, for checking assignments we use the stricter language semantics so we should never generate assignments for structs with different fields offsets or sizes. (I will check this again). > > > I also think more rationale is needed for ignoring sizes like this. Is it > > > intended for e.g. making structs with flexible array members > > > alias-compatible with similar structs with a fixed-size array? > > > > The main reason are pointers to arrays: > > > > struct foo { int (*x)[]; } > > struct foo { int (*x)[2]; }; > > struct foo { int (*x)[1]; }; > > Thanks for the explanation. > > I guess the cases involving flexible array members actually show up a bug > in the standard rather than any kind of issue with this patch - the > standard allows one structure ending with a flexible array member, and > another ending with a fixed-size array, to be compatible (and in different > translation units allowed that even before C23), but there is also clear > text in the standard showing it's not intended to require the layout to be > consistent (the fixed-size and flexible arrays might have different > offsets), and what you'd actually get with an assignment or conditional > expression mixing such structures certainly isn't clearly specified. I agree, unfortunately it seems not well specified and we somehow missed that this now becomes more important. > Maybe the right resolution for that issue with the standard would be to > make that particular case incompatible, but it would need to be raised as > an issue after C23 is out. I think FAM may need further consideration from the standard point of view for also other reasons. My other issues I have now run into are structs with variably size which I think do not work properly with LTO already today: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=112716 Martin >
On Tue, 28 Nov 2023, Joseph Myers wrote: > On Sun, 26 Nov 2023, Martin Uecker wrote: > > > My understand is that it is used for aliasing analysis and also > > checking of conversions. TYPE_CANONICAL must be consistent with > > the idea the middle-end has about type conversions. But as long > > as we do not give the same TYPE_CANONICAL to types the middle-end > > thinks must be incompatible using its own type checking machinery, > > it should be safe even for types the C standard thinks must be > > incompatible for some reason. > > And presumably also for types that definitely can't be assigned because > they have incompatible layout through the use of different array sizes - > since the front end won't generate such assignments, it would never matter > whether the middle end considers them valid without conversion or not? I guess so. The middle-end considers an aggregate copy to have the size of the LHS, so any mismatch there (in size) is the frontends fault and will likely result in code generation with problems. > > > I also think more rationale is needed for ignoring sizes like this. Is it > > > intended for e.g. making structs with flexible array members > > > alias-compatible with similar structs with a fixed-size array? > > > > The main reason are pointers to arrays: > > > > struct foo { int (*x)[]; } > > struct foo { int (*x)[2]; }; > > struct foo { int (*x)[1]; }; > > Thanks for the explanation. > > I guess the cases involving flexible array members actually show up a bug > in the standard rather than any kind of issue with this patch - the > standard allows one structure ending with a flexible array member, and > another ending with a fixed-size array, to be compatible (and in different > translation units allowed that even before C23), but there is also clear > text in the standard showing it's not intended to require the layout to be > consistent (the fixed-size and flexible arrays might have different > offsets), and what you'd actually get with an assignment or conditional > expression mixing such structures certainly isn't clearly specified. > Maybe the right resolution for that issue with the standard would be to > make that particular case incompatible, but it would need to be raised as > an issue after C23 is out. I think from a middle-end perspective it's desirable that accessing either of the foo with size 2 and 1 through a pointer of the type of the foo with unknown size is permissible from a TBAA view. But it's not clear to me that accessing the foo with array size 2 via an lvalue of the type of foo with array size 1 is required to be supported (from a TBAA view). Richard.
Am Dienstag, dem 28.11.2023 um 10:47 +0000 schrieb Richard Biener: > On Tue, 28 Nov 2023, Joseph Myers wrote: > > > On Sun, 26 Nov 2023, Martin Uecker wrote: > > > > > > > I also think more rationale is needed for ignoring sizes like this. Is it > > > > intended for e.g. making structs with flexible array members > > > > alias-compatible with similar structs with a fixed-size array? > > > > > > The main reason are pointers to arrays: > > > > > > struct foo { int (*x)[]; } > > > struct foo { int (*x)[2]; }; > > > struct foo { int (*x)[1]; }; > > > > Thanks for the explanation. > > > > I guess the cases involving flexible array members actually show up a bug > > in the standard rather than any kind of issue with this patch - the > > standard allows one structure ending with a flexible array member, and > > another ending with a fixed-size array, to be compatible (and in different > > translation units allowed that even before C23), but there is also clear > > text in the standard showing it's not intended to require the layout to be > > consistent (the fixed-size and flexible arrays might have different > > offsets), and what you'd actually get with an assignment or conditional > > expression mixing such structures certainly isn't clearly specified. > > Maybe the right resolution for that issue with the standard would be to > > make that particular case incompatible, but it would need to be raised as > > an issue after C23 is out. > > I think from a middle-end perspective it's desirable that accessing > either of the foo with size 2 and 1 through a pointer of the type > of the foo with unknown size is permissible from a TBAA view. But > it's not clear to me that accessing the foo with array size 2 via > an lvalue of the type of foo with array size 1 is required to be > supported (from a TBAA view). This is the same from the C language side. In principle, accesses to an object with effective type foo [2] using an lvalue of type foo [1] or vice versa are undefined behavior. So a model based on equivalence classes loses some information. Martin
diff --git a/gcc/c/c-decl.cc b/gcc/c/c-decl.cc index e5d48c3fa56..d0a405087c3 100644 --- a/gcc/c/c-decl.cc +++ b/gcc/c/c-decl.cc @@ -634,6 +634,36 @@ public: auto_vec<tree> typedefs_seen; }; + +/* Hash table for structs and unions. */ +struct c_struct_hasher : ggc_ptr_hash<tree_node> +{ + static hashval_t hash (tree t); + static bool equal (tree, tree); +}; + +/* Hash an RECORD OR UNION. */ +hashval_t +c_struct_hasher::hash (tree type) +{ + inchash::hash hstate; + + hstate.add_int (TREE_CODE (type)); + hstate.add_object (TYPE_NAME (type)); + + return hstate.end (); +} + +/* Compare two RECORD or UNION types. */ +bool +c_struct_hasher::equal (tree t1, tree t2) +{ + return comptypes_equiv_p (t1, t2); +} + +/* All tagged typed so that TYPE_CANONICAL can be set correctly. */ +static GTY (()) hash_table<c_struct_hasher> *c_struct_htab; + /* Information for the struct or union currently being parsed, or NULL if not parsing a struct or union. */ static class c_struct_parse_info *struct_parse_info; @@ -9646,6 +9676,24 @@ finish_struct (location_t loc, tree t, tree fieldlist, tree attributes, C_TYPE_BEING_DEFINED (t) = 0; + /* Set type canonical based on equivalence class. */ + if (flag_isoc23) + { + if (NULL == c_struct_htab) + c_struct_htab = hash_table<c_struct_hasher>::create_ggc (61); + + hashval_t hash = c_struct_hasher::hash (t); + + tree *e = c_struct_htab->find_slot_with_hash (t, hash, INSERT); + if (*e) + TYPE_CANONICAL (t) = *e; + else + { + TYPE_CANONICAL (t) = t; + *e = t; + } + } + tree incomplete_vars = C_TYPE_INCOMPLETE_VARS (TYPE_MAIN_VARIANT (t)); for (x = TYPE_MAIN_VARIANT (t); x; x = TYPE_NEXT_VARIANT (x)) { diff --git a/gcc/c/c-objc-common.cc b/gcc/c/c-objc-common.cc index c8f49aa2370..738afbad770 100644 --- a/gcc/c/c-objc-common.cc +++ b/gcc/c/c-objc-common.cc @@ -389,6 +389,11 @@ c_get_alias_set (tree t) if (TREE_CODE (t) == ENUMERAL_TYPE) return get_alias_set (ENUM_UNDERLYING_TYPE (t)); + /* Structs with variable size can alias different incompatible + structs. Let them alias anything. */ + if (RECORD_OR_UNION_TYPE_P (t) && C_TYPE_VARIABLE_SIZE (t)) + return 0; + return c_common_get_alias_set (t); } diff --git a/gcc/c/c-tree.h b/gcc/c/c-tree.h index a5dd9a37944..ece5b6a5d26 100644 --- a/gcc/c/c-tree.h +++ b/gcc/c/c-tree.h @@ -758,6 +758,7 @@ extern tree require_complete_type (location_t, tree); extern bool same_translation_unit_p (const_tree, const_tree); extern int comptypes (tree, tree); extern bool comptypes_same_p (tree, tree); +extern int comptypes_equiv_p (tree, tree); extern int comptypes_check_different_types (tree, tree, bool *); extern int comptypes_check_enum_int (tree, tree, bool *); extern bool c_mark_addressable (tree, bool = false); diff --git a/gcc/c/c-typeck.cc b/gcc/c/c-typeck.cc index 8116c9b3e68..262b04c582f 100644 --- a/gcc/c/c-typeck.cc +++ b/gcc/c/c-typeck.cc @@ -1063,6 +1063,7 @@ struct comptypes_data { bool different_types_p; bool warning_needed; bool anon_field; + bool equiv; const struct tagged_tu_seen_cache* cache; }; @@ -1123,6 +1124,21 @@ comptypes_check_different_types (tree type1, tree type2, return ret ? (data.warning_needed ? 2 : 1) : 0; } + + +/* Like comptypes, but if it returns nonzero for struct and union + types considered equivalent for aliasing purposes. */ + +int +comptypes_equiv_p (tree type1, tree type2) +{ + struct comptypes_data data = { }; + data.equiv = true; + bool ret = comptypes_internal (type1, type2, &data); + + return ret; +} + /* Return true if TYPE1 and TYPE2 are compatible types for assignment or various other operations. If they are compatible but a warning may @@ -1250,6 +1266,9 @@ comptypes_internal (const_tree type1, const_tree type2, if ((d1 == NULL_TREE) != (d2 == NULL_TREE)) data->different_types_p = true; + /* ignore size mismatches */ + if (data->equiv) + return 1; /* Sizes must match unless one is missing or variable. */ if (d1 == NULL_TREE || d2 == NULL_TREE || d1 == d2) return true; @@ -1467,6 +1486,9 @@ tagged_types_tu_compatible_p (const_tree t1, const_tree t2, if (list_length (TYPE_FIELDS (t1)) != list_length (TYPE_FIELDS (t2))) return false; + if (data->equiv && (C_TYPE_VARIABLE_SIZE (t1) || C_TYPE_VARIABLE_SIZE (t2))) + return 0; + for (s1 = TYPE_FIELDS (t1), s2 = TYPE_FIELDS (t2); s1 && s2; s1 = DECL_CHAIN (s1), s2 = DECL_CHAIN (s2)) @@ -1486,6 +1508,15 @@ tagged_types_tu_compatible_p (const_tree t1, const_tree t2, && simple_cst_equal (DECL_FIELD_BIT_OFFSET (s1), DECL_FIELD_BIT_OFFSET (s2)) != 1) return false; + + tree st1 = TYPE_SIZE (TREE_TYPE (s1)); + tree st2 = TYPE_SIZE (TREE_TYPE (s2)); + + if (data->equiv + && st1 && TREE_CODE (st1) == INTEGER_CST + && st2 && TREE_CODE (st2) == INTEGER_CST + && !tree_int_cst_equal (st1, st2)) + return 0; } return true; diff --git a/gcc/testsuite/gcc.dg/c23-tag-2.c b/gcc/testsuite/gcc.dg/c23-tag-2.c index 5dd4a21e9df..e28c2b5eea2 100644 --- a/gcc/testsuite/gcc.dg/c23-tag-2.c +++ b/gcc/testsuite/gcc.dg/c23-tag-2.c @@ -1,5 +1,5 @@ -/* { dg-do compile { target { ! "*-*-*" } } } - * { dg-options "-std=c23" } +/* { dg-do compile } + * { dg-options "-std=c2x" } */ // compatibility of structs in assignment diff --git a/gcc/testsuite/gcc.dg/c23-tag-5.c b/gcc/testsuite/gcc.dg/c23-tag-5.c index ff40d07aef1..95a04bf9b0e 100644 --- a/gcc/testsuite/gcc.dg/c23-tag-5.c +++ b/gcc/testsuite/gcc.dg/c23-tag-5.c @@ -1,5 +1,6 @@ -/* { dg-do run { target { ! "*-*-*" } } } - * { dg-options "-std=c23" } +/* + * { dg-do run } + * { dg-options "-std=c2x" } */ // nesting and parameters diff --git a/gcc/testsuite/gcc.dg/c23-tag-alias-1.c b/gcc/testsuite/gcc.dg/c23-tag-alias-1.c new file mode 100644 index 00000000000..6704ba9c8b4 --- /dev/null +++ b/gcc/testsuite/gcc.dg/c23-tag-alias-1.c @@ -0,0 +1,48 @@ +/* + * { dg-do run } + * { dg-options "-std=c23 -O2" } + */ + + +struct foo { int x; }; + +int test_foo(struct foo* a, void* b) +{ + a->x = 1; + + struct foo { int x; }* p = b; + p->x = 2; + + return a->x; +} + + +enum bar { A = 1, B = 3 }; + +int test_bar(enum bar* a, void* b) +{ + *a = A; + + enum bar { A = 1, B = 3 }* p = b; + *p = B; + + return *a; +} + + +int main() +{ + struct foo y; + + if (2 != test_foo(&y, &y)) + __builtin_abort(); + + enum bar z; + + if (A == test_bar(&z, &z)) + __builtin_abort(); + + return 0; +} + + diff --git a/gcc/testsuite/gcc.dg/c23-tag-alias-2.c b/gcc/testsuite/gcc.dg/c23-tag-alias-2.c new file mode 100644 index 00000000000..555c30a8501 --- /dev/null +++ b/gcc/testsuite/gcc.dg/c23-tag-alias-2.c @@ -0,0 +1,73 @@ +/* + * { dg-do run } + * { dg-options "-std=c23 -O2" } + */ + + +struct foo { int x; }; + +int test_foo1(struct foo* a, void* b) +{ + a->x = 1; + + struct foo { int x; int y; }* p = b; + p->x = 2; + + return a->x; +} + +int test_foo2(struct foo* a, void* b) +{ + a->x = 1; + + struct fox { int x; }* p = b; + p->x = 2; + + return a->x; +} + +enum bar { A = 1, B = 3, C = 5, D = 9 }; + +int test_bar1(enum bar* a, void* b) +{ + *a = A; + + enum bar { A = 1, B = 3, C = 6, D = 9 }* p = b; + *p = B; + + return *a; +} + +int test_bar2(enum bar* a, void* b) +{ + *a = A; + + enum baX { A = 1, B = 3, C = 5, D = 9 }* p = b; + *p = B; + + return *a; +} + + +int main() +{ + struct foo y; + + if (1 != test_foo1(&y, &y)) + __builtin_abort(); + + if (1 != test_foo2(&y, &y)) + __builtin_abort(); + + enum bar z; + + if (A == test_bar1(&z, &z)) + __builtin_abort(); + + if (A == test_bar2(&z, &z)) + __builtin_abort(); + + return 0; +} + + diff --git a/gcc/testsuite/gcc.dg/c23-tag-alias-3.c b/gcc/testsuite/gcc.dg/c23-tag-alias-3.c new file mode 100644 index 00000000000..122e8806af8 --- /dev/null +++ b/gcc/testsuite/gcc.dg/c23-tag-alias-3.c @@ -0,0 +1,48 @@ +/* + * { dg-do run } + * { dg-options "-std=c23 -flto -O2" } + */ + + +struct foo { int x; }; + +int test_foo(struct foo* a, void* b) +{ + a->x = 1; + + struct foo { int x; }* p = b; + p->x = 2; + + return a->x; +} + + +enum bar { A = 1, B = 3 }; + +int test_bar(enum bar* a, void* b) +{ + *a = A; + + enum bar { A = 1, B = 3 }* p = b; + *p = B; + + return *a; +} + + +int main() +{ + struct foo y; + + if (2 != test_foo(&y, &y)) + __builtin_abort(); + + enum bar z; + + if (A == test_bar(&z, &z)) + __builtin_abort(); + + return 0; +} + + diff --git a/gcc/testsuite/gcc.dg/c23-tag-alias-4.c b/gcc/testsuite/gcc.dg/c23-tag-alias-4.c new file mode 100644 index 00000000000..d86a174f4a2 --- /dev/null +++ b/gcc/testsuite/gcc.dg/c23-tag-alias-4.c @@ -0,0 +1,73 @@ +/* + * { dg-do run } + * { dg-options "-std=c23 -flto -O2" } + */ + + +struct foo { int x; }; + +int test_foo1(struct foo* a, void* b) +{ + a->x = 1; + + struct foo { int x; int y; }* p = b; + p->x = 2; + + return a->x; +} + +int test_foo2(struct foo* a, void* b) +{ + a->x = 1; + + struct fox { int x; }* p = b; + p->x = 2; + + return a->x; +} + +enum bar { A = 1, B = 3, C = 5, D = 9 }; + +int test_bar1(enum bar* a, void* b) +{ + *a = A; + + enum bar { A = 1, B = 3, C = 6, D = 9 }* p = b; + *p = B; + + return *a; +} + +int test_bar2(enum bar* a, void* b) +{ + *a = A; + + enum baX { A = 1, B = 3, C = 5, D = 9 }* p = b; + *p = B; + + return *a; +} + + +int main() +{ + struct foo y; + + if (1 != test_foo1(&y, &y)) + __builtin_abort(); + + if (1 != test_foo2(&y, &y)) + __builtin_abort(); + + enum bar z; + + if (A == test_bar1(&z, &z)) + __builtin_abort(); + + if (A == test_bar2(&z, &z)) + __builtin_abort(); + + return 0; +} + + diff --git a/gcc/testsuite/gcc.dg/c23-tag-alias-5.c b/gcc/testsuite/gcc.dg/c23-tag-alias-5.c new file mode 100644 index 00000000000..4e956720143 --- /dev/null +++ b/gcc/testsuite/gcc.dg/c23-tag-alias-5.c @@ -0,0 +1,30 @@ +/* { dg-do run } + * { dg-options "-std=c23 -O2" } + */ + +// not sure this is wise, but this was already like thi sbefore + +typedef struct { int x; } foo_t; + +int test_foo(foo_t* a, void* b) +{ + a->x = 1; + + struct { int x; }* p = b; + p->x = 2; + + return a->x; +} + + +int main() +{ + foo_t y; + + if (1 != test_foo(&y, &y)) + __builtin_abort(); + + return 0; +} + + diff --git a/gcc/testsuite/gcc.dg/c23-tag-alias-6.c b/gcc/testsuite/gcc.dg/c23-tag-alias-6.c new file mode 100644 index 00000000000..3f3e5f01473 --- /dev/null +++ b/gcc/testsuite/gcc.dg/c23-tag-alias-6.c @@ -0,0 +1,77 @@ +/* + * { dg-do run } + * { dg-options "-std=c23 -O2" } + */ + + +/* We check that we tolerate differences for + * optimization. + */ + + +struct bar0 { int x; int f[3]; int y; }; + +int test_bar0(struct bar0* a, void* b) +{ + a->x = 1; + + struct bar0 { int x; int f[4]; int y; }* p = b; + p->x = 2; + + return a->x; +} + + + +struct bar1 { int x; int (*f)[3]; }; + +int test_bar1(struct bar1* a, void* b) +{ + a->x = 1; + + struct bar1 { int x; int (*f)[3]; }* p = b; + p->x = 2; + + return a->x; +} + + +struct bar2 { int x; int (*f)[3]; }; + +int test_bar2(struct bar2* a, void* b) +{ + a->x = 1; + + struct bar2 { int x; int (*f)[4]; }* p = b; + p->x = 2; + + return a->x; +} + + + +int main() +{ + // control + + struct bar0 z0; + + if (1 != test_bar0(&z0, &z0)) + __builtin_abort(); + + // this could be different + struct bar1 z1; + + if (2 != test_bar1(&z1, &z1)) + __builtin_abort(); + + struct bar2 z2; + + if (2 != test_bar2(&z2, &z2)) + __builtin_abort(); + + + return 0; +} + + diff --git a/gcc/testsuite/gcc.dg/c23-tag-alias-7.c b/gcc/testsuite/gcc.dg/c23-tag-alias-7.c new file mode 100644 index 00000000000..74cf5f212b3 --- /dev/null +++ b/gcc/testsuite/gcc.dg/c23-tag-alias-7.c @@ -0,0 +1,86 @@ +/* + * { dg-do run } + * { dg-options "-std=c23 -O2" } + */ + + + +struct bar { int x; int f[]; }; + +int test_bar1(struct bar* a, void* b) +{ + a->x = 1; + + struct bar { int x; int f[]; }* p = b; + struct bar* q = a; + p->x = 2; + + return a->x; +} + + +int test_bar3(struct bar* a, void* b) +{ + a->x = 1; + + struct bar { int x; int f[1]; }* p = b; + struct bar* q = a; /* { dg-warning "incompatible" } */ + p->x = 2; + + return a->x; +} + + +int test_bar4(struct bar* a, void* b) +{ + a->x = 1; + + int n = 3; + struct bar { int x; int f[n]; }* p = b; + struct bar* q = a; + p->x = 2; + + return a->x; +} + + +struct foo { int x; int f[3]; }; + + +int test_foo1(struct foo* a, void* b) +{ + a->x = 1; + + int n = 3; + struct foo { int x; int f[n]; }* p = b; + struct foo* q = a; + p->x = 2; + + return a->x; +} + + + +int main() +{ + struct bar z; + + if (2 != test_bar1(&z, &z)) + __builtin_abort(); + +#if 0 + if (1 != test_bar3(&z, &z)) + __builtin_abort(); +#endif + if (2 != test_bar4(&z, &z)) + __builtin_abort(); + + struct foo y; + + if (2 != test_foo1(&y, &y)) + __builtin_abort(); + + return 0; +} + + diff --git a/gcc/testsuite/gcc.dg/c23-tag-alias-8.c b/gcc/testsuite/gcc.dg/c23-tag-alias-8.c new file mode 100644 index 00000000000..7c2ced4de51 --- /dev/null +++ b/gcc/testsuite/gcc.dg/c23-tag-alias-8.c @@ -0,0 +1,90 @@ +/* + * { dg-do run } + * { dg-options "-std=c23 -O2" } + */ + + +/* We check that we tolerate differences for + * optimization. + */ + +struct bar1 { int x; enum A1 { X1 = 1 } f; }; + +int test_bar1(struct bar1* a, void* b) +{ + a->x = 1; + + struct bar1 { int x; enum A1 { X1 = 2 } f; }* p = b; + p->x = 2; + + return a->x; +} + + +struct bar2 { int x; enum A2 { X2 = 1 } f; }; + +int test_bar2(struct bar2* a, void* b) +{ + a->x = 1; + + struct bar2 { int x; enum B2 { X2 = 1 } f; }* p = b; + p->x = 2; + + return a->x; +} + + + +struct bar3 { int x; enum A3 { X3 = 1 } f; }; + +int test_bar3(struct bar3* a, void* b) +{ + a->x = 1; + + struct bar3 { int x; enum A3 { Y3 = 1 } f; }* p = b; + p->x = 2; + + return a->x; +} + + +struct bar4 { int x; enum { Z4 = 1 } f; }; + +int test_bar4(struct bar4* a, void* b) +{ + a->x = 1; + + struct bar4 { int x; enum { Z4 = 1 } f; }* p = b; + p->x = 2; + + return a->x; +} + + + +int main() +{ + struct bar1 z1; + + if (1 != test_bar1(&z1, &z1)) + __builtin_abort(); + + struct bar2 z2; + + if (1 != test_bar2(&z2, &z2)) + __builtin_abort(); + + struct bar3 z3; + + if (1 != test_bar3(&z3, &z3)) + __builtin_abort(); + + struct bar4 z4; + + if (1 != test_bar4(&z4, &z4)) + __builtin_abort(); + + return 0; +} + + diff --git a/gcc/testsuite/gcc.dg/gnu23-tag-alias-1.c b/gcc/testsuite/gcc.dg/gnu23-tag-alias-1.c new file mode 100644 index 00000000000..9bccc651afb --- /dev/null +++ b/gcc/testsuite/gcc.dg/gnu23-tag-alias-1.c @@ -0,0 +1,33 @@ +/* + * { dg-do run } + * { dg-options "-std=gnu23 -O2" } + */ + + + +struct bar { int x; int f[]; }; + +int test_bar2(struct bar* a, void* b) +{ + a->x = 1; + + struct bar { int x; int f[0]; }* p = b; + struct bar* q = a; + p->x = 2; + + return a->x; +} + + + +int main() +{ + struct bar z; + + if (2 != test_bar2(&z, &z)) + __builtin_abort(); + + return 0; +} + +