Message ID | 448b0744f89d5c6ba5d6a5bacda154fcb74b7bca.camel@tugraz.at |
---|---|
State | New |
Headers | show |
Series | [C,v2] Fix ICE with -g and -std=c23 related to incomplete types [PR114361] | expand |
(new email for Joseph) Am Sonntag, dem 14.04.2024 um 14:30 +0200 schrieb Martin Uecker: > I had to revert the old patch because it broke LTO which lead > to PR114574. We now set TYPE_STRUCTURAL_EQUALITY and properly > update TYPE_CANONICAL for such types and also for pointers > to such types via a new function c_update_type_canonical > (thanks to Jakob). > > > Bootstrapped and regession tested on x86_64. > > > > Fix ICE with -g and -std=c23 related to incomplete types [PR114361] > > We did not update TYPE_CANONICAL for incomplete variants when > completing a structure. We now set TYPE_STRUCTURAL_EQUALITY for > incomplete structure and union types and then update TYPE_CANONICAL > later. See PR114574 for discussion. > > > 2024-04-12 Martin Uecker <uecker@tugraz.at> > Jakub Jelinek <jakub@redhat.com> > > PR lto/114574 > PR c/114361 > gcc/ > * ipa-free-lang-data.cc (fld_incomplete_type_of): Allow > either of the types in the assert to have TYPE_STRUCTURAL_EQUALITY_P. > gcc/c/ > * c-decl.cc (shadow_tag_warned): For flag_isoc23 and code not > ENUMERAL_TYPE use SET_TYPE_STRUCTURAL_EQUALITY. > (parser_xref_tag): Likewise. > (start_struct): For flag_isoc23 use SET_TYPE_STRUCTURAL_EQUALITY. > (c_update_type_canonical): New function. > (finish_struct): Put NULL as second == operand rather than first. > Assert TYPE_STRUCTURAL_EQUALITY_P. Call c_update_type_canonical. > * c-typeck.cc (composite_type_internal): Use > SET_TYPE_STRUCTURAL_EQUALITY. Formatting fix. > gcc/testsuite/ > * gcc.dg/pr114574-1.c: New test. > * gcc.dg/pr114574-2.c: New test. > * gcc.dg/pr114361.c: New test. > * gcc.dg/c23-tag-incomplete-1.c: New test. > * gcc.dg/c23-tag-incomplete-2.c: New test. > --- > gcc/c/c-decl.cc | 47 ++++++++++++++++++++- > gcc/c/c-typeck.cc | 4 +- > gcc/ipa-free-lang-data.cc | 4 +- > gcc/testsuite/gcc.dg/c23-tag-incomplete-1.c | 14 ++++++ > gcc/testsuite/gcc.dg/c23-tag-incomplete-2.c | 13 ++++++ > gcc/testsuite/gcc.dg/pr114361.c | 11 +++++ > gcc/testsuite/gcc.dg/pr114574-1.c | 10 +++++ > gcc/testsuite/gcc.dg/pr114574-2.c | 10 +++++ > 8 files changed, 110 insertions(+), 3 deletions(-) > create mode 100644 gcc/testsuite/gcc.dg/c23-tag-incomplete-1.c > create mode 100644 gcc/testsuite/gcc.dg/c23-tag-incomplete-2.c > create mode 100644 gcc/testsuite/gcc.dg/pr114361.c > create mode 100644 gcc/testsuite/gcc.dg/pr114574-1.c > create mode 100644 gcc/testsuite/gcc.dg/pr114574-2.c > > diff --git a/gcc/c/c-decl.cc b/gcc/c/c-decl.cc > index 345090dae38..54917297b6a 100644 > --- a/gcc/c/c-decl.cc > +++ b/gcc/c/c-decl.cc > @@ -5051,6 +5051,8 @@ shadow_tag_warned (const struct c_declspecs *declspecs, int warned) > if (t == NULL_TREE) > { > t = make_node (code); > + if (flag_isoc23 && code != ENUMERAL_TYPE) > + SET_TYPE_STRUCTURAL_EQUALITY (t); > pushtag (input_location, name, t); > } > } > @@ -8809,6 +8811,8 @@ parser_xref_tag (location_t loc, enum tree_code code, tree name, > the forward-reference will be altered into a real type. */ > > ref = make_node (code); > + if (flag_isoc23 && code != ENUMERAL_TYPE) > + SET_TYPE_STRUCTURAL_EQUALITY (ref); > if (code == ENUMERAL_TYPE) > { > /* Give the type a default layout like unsigned int > @@ -8910,6 +8914,8 @@ start_struct (location_t loc, enum tree_code code, tree name, > if (ref == NULL_TREE || TREE_CODE (ref) != code) > { > ref = make_node (code); > + if (flag_isoc23) > + SET_TYPE_STRUCTURAL_EQUALITY (ref); > pushtag (loc, name, ref); > } > > @@ -9347,6 +9353,43 @@ is_flexible_array_member_p (bool is_last_field, > return false; > } > > +/* Recompute TYPE_CANONICAL for qualified versions of the type and > + related pointer types after an aggregate type has been finalized. > + Will not update array types, pointers to array types, function > + types and other derived types created while the type was still > + incomplete, those will remain TYPE_STRUCTURAL_EQUALITY_P. */ > + > +static void > +c_update_type_canonical (tree t) > +{ > + for (tree x = TYPE_MAIN_VARIANT (t); x; x = TYPE_NEXT_VARIANT (x)) > + { > + if (x != t > + && TYPE_STRUCTURAL_EQUALITY_P (x) > + && check_qualified_type (x, t, TYPE_QUALS (x))) > + { > + if (TYPE_CANONICAL (t) != t) > + TYPE_CANONICAL (x) > + = build_qualified_type (TYPE_CANONICAL (t), TYPE_QUALS (x)); > + else > + TYPE_CANONICAL (x) = x; > + } > + else if (x != t) > + continue; > + for (tree p = TYPE_POINTER_TO (x); p; p = TYPE_NEXT_PTR_TO (p)) > + { > + if (!TYPE_STRUCTURAL_EQUALITY_P (p)) > + continue; > + if (TYPE_CANONICAL (x) != x || TYPE_REF_CAN_ALIAS_ALL (p)) > + TYPE_CANONICAL (p) > + = build_pointer_type_for_mode (TYPE_CANONICAL (x), TYPE_MODE (p), > + false); > + else > + TYPE_CANONICAL (p) = p; > + c_update_type_canonical (p); > + } > + } > +} > > /* Fill in the fields of a RECORD_TYPE or UNION_TYPE node, T. > LOC is the location of the RECORD_TYPE or UNION_TYPE's definition. > @@ -9695,11 +9738,12 @@ finish_struct (location_t loc, tree t, tree fieldlist, tree attributes, > /* Set type canonical based on equivalence class. */ > if (flag_isoc23) > { > - if (NULL == c_struct_htab) > + if (c_struct_htab == NULL) > c_struct_htab = hash_table<c_struct_hasher>::create_ggc (61); > > hashval_t hash = c_struct_hasher::hash (t); > > + gcc_checking_assert (TYPE_STRUCTURAL_EQUALITY_P (t)); > tree *e = c_struct_htab->find_slot_with_hash (t, hash, INSERT); > if (*e) > TYPE_CANONICAL (t) = *e; > @@ -9708,6 +9752,7 @@ finish_struct (location_t loc, tree t, tree fieldlist, tree attributes, > TYPE_CANONICAL (t) = t; > *e = t; > } > + c_update_type_canonical (t); > } > > tree incomplete_vars = C_TYPE_INCOMPLETE_VARS (TYPE_MAIN_VARIANT (t)); > diff --git a/gcc/c/c-typeck.cc b/gcc/c/c-typeck.cc > index ddeab1e2a8a..4567b114734 100644 > --- a/gcc/c/c-typeck.cc > +++ b/gcc/c/c-typeck.cc > @@ -532,6 +532,7 @@ composite_type_internal (tree t1, tree t2, struct composite_cache* cache) > /* Otherwise, create a new type node and link it into the cache. */ > > tree n = make_node (code1); > + SET_TYPE_STRUCTURAL_EQUALITY (n); > TYPE_NAME (n) = TYPE_NAME (t1); > > struct composite_cache cache2 = { t1, t2, n, cache }; > @@ -590,7 +591,8 @@ composite_type_internal (tree t1, tree t2, struct composite_cache* cache) > TYPE_STUB_DECL (n) = pushdecl (build_decl (input_location, TYPE_DECL, > NULL_TREE, n)); > > - n = finish_struct(input_location, n, fields, attributes, NULL, &expr); > + n = finish_struct (input_location, n, fields, attributes, NULL, > + &expr); > > n = qualify_type (n, t1); > > diff --git a/gcc/ipa-free-lang-data.cc b/gcc/ipa-free-lang-data.cc > index 16ed55e2e5a..6f75444e513 100644 > --- a/gcc/ipa-free-lang-data.cc > +++ b/gcc/ipa-free-lang-data.cc > @@ -255,7 +255,9 @@ fld_incomplete_type_of (tree t, class free_lang_data_d *fld) > first = build_reference_type_for_mode (t2, TYPE_MODE (t), > TYPE_REF_CAN_ALIAS_ALL (t)); > gcc_assert (TYPE_CANONICAL (t2) != t2 > - && TYPE_CANONICAL (t2) == TYPE_CANONICAL (TREE_TYPE (t))); > + && (TYPE_CANONICAL (t2) == TYPE_CANONICAL (TREE_TYPE (t)) > + || TYPE_STRUCTURAL_EQUALITY_P (t2) > + || TYPE_STRUCTURAL_EQUALITY_P (TREE_TYPE (t)))); > if (!fld->pset.add (first)) > add_tree_to_fld_list (first, fld); > return fld_type_variant (first, t, fld); > diff --git a/gcc/testsuite/gcc.dg/c23-tag-incomplete-1.c b/gcc/testsuite/gcc.dg/c23-tag-incomplete-1.c > new file mode 100644 > index 00000000000..82d652569e9 > --- /dev/null > +++ b/gcc/testsuite/gcc.dg/c23-tag-incomplete-1.c > @@ -0,0 +1,14 @@ > +/* { dg-do compile } > + * { dg-options "-std=c23 -g" } */ > + > +struct a; > +typedef struct a b; > + > +void g() { > + struct a { b* x; }; > +} > + > +struct a { b* x; }; > + > + > + > diff --git a/gcc/testsuite/gcc.dg/c23-tag-incomplete-2.c b/gcc/testsuite/gcc.dg/c23-tag-incomplete-2.c > new file mode 100644 > index 00000000000..bc47a04ece5 > --- /dev/null > +++ b/gcc/testsuite/gcc.dg/c23-tag-incomplete-2.c > @@ -0,0 +1,13 @@ > +/* { dg-do compile } > + * { dg-options "-std=c23 -g" } */ > + > +struct a; > +typedef struct a b; > + > +void f() { > + extern struct a { b* x; } t; > +} > + > +extern struct a { b* x; } t; > + > + > diff --git a/gcc/testsuite/gcc.dg/pr114361.c b/gcc/testsuite/gcc.dg/pr114361.c > new file mode 100644 > index 00000000000..0f3feb53566 > --- /dev/null > +++ b/gcc/testsuite/gcc.dg/pr114361.c > @@ -0,0 +1,11 @@ > +/* PR c/114361 */ > +/* { dg-do compile } */ > +/* { dg-options "-std=gnu23 -g" } */ > + > +void f() > +{ > + typedef struct foo bar; > + typedef __typeof( ({ (struct foo { bar *x; }){ }; }) ) wuz; > + struct foo { wuz *x; }; > +} > + > diff --git a/gcc/testsuite/gcc.dg/pr114574-1.c b/gcc/testsuite/gcc.dg/pr114574-1.c > new file mode 100644 > index 00000000000..060dcdbe73e > --- /dev/null > +++ b/gcc/testsuite/gcc.dg/pr114574-1.c > @@ -0,0 +1,10 @@ > +/* PR lto/114574 > + * { dg-do compile } > + * { dg-options "-flto" } */ > + > +const struct S * x; > +struct S {}; > +void f(const struct S **); > + > + > + > diff --git a/gcc/testsuite/gcc.dg/pr114574-2.c b/gcc/testsuite/gcc.dg/pr114574-2.c > new file mode 100644 > index 00000000000..723291e2211 > --- /dev/null > +++ b/gcc/testsuite/gcc.dg/pr114574-2.c > @@ -0,0 +1,10 @@ > +/* PR lto/114574 > + * { dg-do compile } > + * { dg-options "-flto -std=c23" } */ > + > +const struct S * x; > +struct S {}; > +void f(const struct S **); > + > + > +
On Sun, 14 Apr 2024, Martin Uecker wrote: > > I had to revert the old patch because it broke LTO which lead > to PR114574. We now set TYPE_STRUCTURAL_EQUALITY and properly > update TYPE_CANONICAL for such types and also for pointers > to such types via a new function c_update_type_canonical > (thanks to Jakob). > > > Bootstrapped and regession tested on x86_64. > > > > Fix ICE with -g and -std=c23 related to incomplete types [PR114361] > > We did not update TYPE_CANONICAL for incomplete variants when > completing a structure. We now set TYPE_STRUCTURAL_EQUALITY for > incomplete structure and union types and then update TYPE_CANONICAL > later. See PR114574 for discussion. > > > 2024-04-12 Martin Uecker <uecker@tugraz.at> > Jakub Jelinek <jakub@redhat.com> > > PR lto/114574 > PR c/114361 > gcc/ > * ipa-free-lang-data.cc (fld_incomplete_type_of): Allow > either of the types in the assert to have TYPE_STRUCTURAL_EQUALITY_P. > gcc/c/ > * c-decl.cc (shadow_tag_warned): For flag_isoc23 and code not > ENUMERAL_TYPE use SET_TYPE_STRUCTURAL_EQUALITY. > (parser_xref_tag): Likewise. > (start_struct): For flag_isoc23 use SET_TYPE_STRUCTURAL_EQUALITY. > (c_update_type_canonical): New function. > (finish_struct): Put NULL as second == operand rather than first. > Assert TYPE_STRUCTURAL_EQUALITY_P. Call c_update_type_canonical. > * c-typeck.cc (composite_type_internal): Use > SET_TYPE_STRUCTURAL_EQUALITY. Formatting fix. > gcc/testsuite/ > * gcc.dg/pr114574-1.c: New test. > * gcc.dg/pr114574-2.c: New test. > * gcc.dg/pr114361.c: New test. > * gcc.dg/c23-tag-incomplete-1.c: New test. > * gcc.dg/c23-tag-incomplete-2.c: New test. > --- > gcc/c/c-decl.cc | 47 ++++++++++++++++++++- > gcc/c/c-typeck.cc | 4 +- > gcc/ipa-free-lang-data.cc | 4 +- > gcc/testsuite/gcc.dg/c23-tag-incomplete-1.c | 14 ++++++ > gcc/testsuite/gcc.dg/c23-tag-incomplete-2.c | 13 ++++++ > gcc/testsuite/gcc.dg/pr114361.c | 11 +++++ > gcc/testsuite/gcc.dg/pr114574-1.c | 10 +++++ > gcc/testsuite/gcc.dg/pr114574-2.c | 10 +++++ > 8 files changed, 110 insertions(+), 3 deletions(-) > create mode 100644 gcc/testsuite/gcc.dg/c23-tag-incomplete-1.c > create mode 100644 gcc/testsuite/gcc.dg/c23-tag-incomplete-2.c > create mode 100644 gcc/testsuite/gcc.dg/pr114361.c > create mode 100644 gcc/testsuite/gcc.dg/pr114574-1.c > create mode 100644 gcc/testsuite/gcc.dg/pr114574-2.c > > diff --git a/gcc/c/c-decl.cc b/gcc/c/c-decl.cc > index 345090dae38..54917297b6a 100644 > --- a/gcc/c/c-decl.cc > +++ b/gcc/c/c-decl.cc > @@ -5051,6 +5051,8 @@ shadow_tag_warned (const struct c_declspecs *declspecs, int warned) > if (t == NULL_TREE) > { > t = make_node (code); > + if (flag_isoc23 && code != ENUMERAL_TYPE) > + SET_TYPE_STRUCTURAL_EQUALITY (t); > pushtag (input_location, name, t); > } > } > @@ -8809,6 +8811,8 @@ parser_xref_tag (location_t loc, enum tree_code code, tree name, > the forward-reference will be altered into a real type. */ > > ref = make_node (code); > + if (flag_isoc23 && code != ENUMERAL_TYPE) > + SET_TYPE_STRUCTURAL_EQUALITY (ref); > if (code == ENUMERAL_TYPE) > { > /* Give the type a default layout like unsigned int > @@ -8910,6 +8914,8 @@ start_struct (location_t loc, enum tree_code code, tree name, > if (ref == NULL_TREE || TREE_CODE (ref) != code) > { > ref = make_node (code); > + if (flag_isoc23) > + SET_TYPE_STRUCTURAL_EQUALITY (ref); > pushtag (loc, name, ref); > } > > @@ -9347,6 +9353,43 @@ is_flexible_array_member_p (bool is_last_field, > return false; > } > > +/* Recompute TYPE_CANONICAL for qualified versions of the type and > + related pointer types after an aggregate type has been finalized. > + Will not update array types, pointers to array types, function > + types and other derived types created while the type was still > + incomplete, those will remain TYPE_STRUCTURAL_EQUALITY_P. */ > + > +static void > +c_update_type_canonical (tree t) > +{ > + for (tree x = TYPE_MAIN_VARIANT (t); x; x = TYPE_NEXT_VARIANT (x)) > + { > + if (x != t > + && TYPE_STRUCTURAL_EQUALITY_P (x) > + && check_qualified_type (x, t, TYPE_QUALS (x))) > + { > + if (TYPE_CANONICAL (t) != t) > + TYPE_CANONICAL (x) > + = build_qualified_type (TYPE_CANONICAL (t), TYPE_QUALS (x)); > + else > + TYPE_CANONICAL (x) = x; > + } > + else if (x != t) > + continue; > + for (tree p = TYPE_POINTER_TO (x); p; p = TYPE_NEXT_PTR_TO (p)) > + { > + if (!TYPE_STRUCTURAL_EQUALITY_P (p)) > + continue; > + if (TYPE_CANONICAL (x) != x || TYPE_REF_CAN_ALIAS_ALL (p)) > + TYPE_CANONICAL (p) > + = build_pointer_type_for_mode (TYPE_CANONICAL (x), TYPE_MODE (p), > + false); > + else > + TYPE_CANONICAL (p) = p; > + c_update_type_canonical (p); > + } > + } > +} > > /* Fill in the fields of a RECORD_TYPE or UNION_TYPE node, T. > LOC is the location of the RECORD_TYPE or UNION_TYPE's definition. > @@ -9695,11 +9738,12 @@ finish_struct (location_t loc, tree t, tree fieldlist, tree attributes, > /* Set type canonical based on equivalence class. */ > if (flag_isoc23) > { > - if (NULL == c_struct_htab) > + if (c_struct_htab == NULL) > c_struct_htab = hash_table<c_struct_hasher>::create_ggc (61); > > hashval_t hash = c_struct_hasher::hash (t); > > + gcc_checking_assert (TYPE_STRUCTURAL_EQUALITY_P (t)); > tree *e = c_struct_htab->find_slot_with_hash (t, hash, INSERT); > if (*e) > TYPE_CANONICAL (t) = *e; > @@ -9708,6 +9752,7 @@ finish_struct (location_t loc, tree t, tree fieldlist, tree attributes, > TYPE_CANONICAL (t) = t; > *e = t; > } > + c_update_type_canonical (t); > } > > tree incomplete_vars = C_TYPE_INCOMPLETE_VARS (TYPE_MAIN_VARIANT (t)); > diff --git a/gcc/c/c-typeck.cc b/gcc/c/c-typeck.cc > index ddeab1e2a8a..4567b114734 100644 > --- a/gcc/c/c-typeck.cc > +++ b/gcc/c/c-typeck.cc > @@ -532,6 +532,7 @@ composite_type_internal (tree t1, tree t2, struct composite_cache* cache) > /* Otherwise, create a new type node and link it into the cache. */ > > tree n = make_node (code1); > + SET_TYPE_STRUCTURAL_EQUALITY (n); > TYPE_NAME (n) = TYPE_NAME (t1); > > struct composite_cache cache2 = { t1, t2, n, cache }; > @@ -590,7 +591,8 @@ composite_type_internal (tree t1, tree t2, struct composite_cache* cache) > TYPE_STUB_DECL (n) = pushdecl (build_decl (input_location, TYPE_DECL, > NULL_TREE, n)); > > - n = finish_struct(input_location, n, fields, attributes, NULL, &expr); > + n = finish_struct (input_location, n, fields, attributes, NULL, > + &expr); > > n = qualify_type (n, t1); > > diff --git a/gcc/ipa-free-lang-data.cc b/gcc/ipa-free-lang-data.cc > index 16ed55e2e5a..6f75444e513 100644 > --- a/gcc/ipa-free-lang-data.cc > +++ b/gcc/ipa-free-lang-data.cc > @@ -255,7 +255,9 @@ fld_incomplete_type_of (tree t, class free_lang_data_d *fld) > first = build_reference_type_for_mode (t2, TYPE_MODE (t), > TYPE_REF_CAN_ALIAS_ALL (t)); > gcc_assert (TYPE_CANONICAL (t2) != t2 > - && TYPE_CANONICAL (t2) == TYPE_CANONICAL (TREE_TYPE (t))); > + && (TYPE_CANONICAL (t2) == TYPE_CANONICAL (TREE_TYPE (t)) > + || TYPE_STRUCTURAL_EQUALITY_P (t2) > + || TYPE_STRUCTURAL_EQUALITY_P (TREE_TYPE (t)))); I think we want the same canonical types for t2 and TREE_TYPE (t), why doesn't fld_incomplete_type_of make it so? Richard. > if (!fld->pset.add (first)) > add_tree_to_fld_list (first, fld); > return fld_type_variant (first, t, fld); > diff --git a/gcc/testsuite/gcc.dg/c23-tag-incomplete-1.c b/gcc/testsuite/gcc.dg/c23-tag-incomplete-1.c > new file mode 100644 > index 00000000000..82d652569e9 > --- /dev/null > +++ b/gcc/testsuite/gcc.dg/c23-tag-incomplete-1.c > @@ -0,0 +1,14 @@ > +/* { dg-do compile } > + * { dg-options "-std=c23 -g" } */ > + > +struct a; > +typedef struct a b; > + > +void g() { > + struct a { b* x; }; > +} > + > +struct a { b* x; }; > + > + > + > diff --git a/gcc/testsuite/gcc.dg/c23-tag-incomplete-2.c b/gcc/testsuite/gcc.dg/c23-tag-incomplete-2.c > new file mode 100644 > index 00000000000..bc47a04ece5 > --- /dev/null > +++ b/gcc/testsuite/gcc.dg/c23-tag-incomplete-2.c > @@ -0,0 +1,13 @@ > +/* { dg-do compile } > + * { dg-options "-std=c23 -g" } */ > + > +struct a; > +typedef struct a b; > + > +void f() { > + extern struct a { b* x; } t; > +} > + > +extern struct a { b* x; } t; > + > + > diff --git a/gcc/testsuite/gcc.dg/pr114361.c b/gcc/testsuite/gcc.dg/pr114361.c > new file mode 100644 > index 00000000000..0f3feb53566 > --- /dev/null > +++ b/gcc/testsuite/gcc.dg/pr114361.c > @@ -0,0 +1,11 @@ > +/* PR c/114361 */ > +/* { dg-do compile } */ > +/* { dg-options "-std=gnu23 -g" } */ > + > +void f() > +{ > + typedef struct foo bar; > + typedef __typeof( ({ (struct foo { bar *x; }){ }; }) ) wuz; > + struct foo { wuz *x; }; > +} > + > diff --git a/gcc/testsuite/gcc.dg/pr114574-1.c b/gcc/testsuite/gcc.dg/pr114574-1.c > new file mode 100644 > index 00000000000..060dcdbe73e > --- /dev/null > +++ b/gcc/testsuite/gcc.dg/pr114574-1.c > @@ -0,0 +1,10 @@ > +/* PR lto/114574 > + * { dg-do compile } > + * { dg-options "-flto" } */ > + > +const struct S * x; > +struct S {}; > +void f(const struct S **); > + > + > + > diff --git a/gcc/testsuite/gcc.dg/pr114574-2.c b/gcc/testsuite/gcc.dg/pr114574-2.c > new file mode 100644 > index 00000000000..723291e2211 > --- /dev/null > +++ b/gcc/testsuite/gcc.dg/pr114574-2.c > @@ -0,0 +1,10 @@ > +/* PR lto/114574 > + * { dg-do compile } > + * { dg-options "-flto -std=c23" } */ > + > +const struct S * x; > +struct S {}; > +void f(const struct S **); > + > + > + >
On Sun, Apr 14, 2024 at 02:30:27PM +0200, Martin Uecker wrote: > 2024-04-12 Martin Uecker <uecker@tugraz.at> > Jakub Jelinek <jakub@redhat.com> > > PR lto/114574 > PR c/114361 > gcc/ > * ipa-free-lang-data.cc (fld_incomplete_type_of): Allow > either of the types in the assert to have TYPE_STRUCTURAL_EQUALITY_P. > gcc/c/ > * c-decl.cc (shadow_tag_warned): For flag_isoc23 and code not > ENUMERAL_TYPE use SET_TYPE_STRUCTURAL_EQUALITY. > (parser_xref_tag): Likewise. > (start_struct): For flag_isoc23 use SET_TYPE_STRUCTURAL_EQUALITY. > (c_update_type_canonical): New function. > (finish_struct): Put NULL as second == operand rather than first. > Assert TYPE_STRUCTURAL_EQUALITY_P. Call c_update_type_canonical. > * c-typeck.cc (composite_type_internal): Use > SET_TYPE_STRUCTURAL_EQUALITY. Formatting fix. > gcc/testsuite/ > * gcc.dg/pr114574-1.c: New test. > * gcc.dg/pr114574-2.c: New test. > * gcc.dg/pr114361.c: New test. > * gcc.dg/c23-tag-incomplete-1.c: New test. > * gcc.dg/c23-tag-incomplete-2.c: New test. Just a nit I haven't seen before (not a review, can't do that when I've participated on the patch): > --- /dev/null > +++ b/gcc/testsuite/gcc.dg/c23-tag-incomplete-1.c > @@ -0,0 +1,14 @@ > +/* { dg-do compile } > + * { dg-options "-std=c23 -g" } */ > + > +struct a; > +typedef struct a b; > + > +void g() { > + struct a { b* x; }; > +} > + > +struct a { b* x; }; > + > + > + Please avoid the trailing whitespace in the testcases. They should end with a single newline, not 3-4 newlines. Jakub
On Mon, Apr 15, 2024 at 08:55:56AM +0200, Richard Biener wrote: > > diff --git a/gcc/ipa-free-lang-data.cc b/gcc/ipa-free-lang-data.cc > > index 16ed55e2e5a..6f75444e513 100644 > > --- a/gcc/ipa-free-lang-data.cc > > +++ b/gcc/ipa-free-lang-data.cc > > @@ -255,7 +255,9 @@ fld_incomplete_type_of (tree t, class free_lang_data_d *fld) > > first = build_reference_type_for_mode (t2, TYPE_MODE (t), > > TYPE_REF_CAN_ALIAS_ALL (t)); > > gcc_assert (TYPE_CANONICAL (t2) != t2 > > - && TYPE_CANONICAL (t2) == TYPE_CANONICAL (TREE_TYPE (t))); > > + && (TYPE_CANONICAL (t2) == TYPE_CANONICAL (TREE_TYPE (t)) > > + || TYPE_STRUCTURAL_EQUALITY_P (t2) > > + || TYPE_STRUCTURAL_EQUALITY_P (TREE_TYPE (t)))); > > I think we want the same canonical types for t2 and TREE_TYPE (t), why > doesn't fld_incomplete_type_of make it so? I had this spot instrumented to log the different cases (before adding the code to fix up also pointer types in c_update_type_canonical) and the only thing that triggered was that the 2 TYPE_CANONICALs weren't equal if TYPE_STRUCTURAL_EQUALITY_P (TREE_TYPE (t)), the other was just in case. gcc.c-torture/compile/20021205-1.c gcc.c-torture/compile/20040214-2.c gcc.c-torture/compile/20060109-1.c gcc.c-torture/compile/pr113623.c gcc.c-torture/compile/pr46866.c gcc.c-torture/compile/pta-1.c gcc.c-torture/execute/pr33870-1.c gcc.c-torture/execute/pr33870.c gcc.dg/torture/pr57478.c tests were affected in make check-gcc. I thought it would be a clear consequence of the choice we've discussed on IRC, that build_pointer_type_for_mode and other tree.cc functions which lookup/create derived types don't try to fill in TYPE_CANONICAL for types derived from something which initially had TYPE_STRUCTURAL_EQUALITY_P but later changed to non-TYPE_STRUCTURAL_EQUALITY_P. The patch updates it solely for qualified types/related pointer types, but doesn't do that for array types, pointer to array types, function types, ... So, I think the assertion could still trigger if we have something like -O2 -flto -std=c23 struct S; typedef struct S *T; typedef T U[10]; typedef U *V; V foo (int x) { return 0; } struct S { int s; }; (but doesn't, dunno what I'm missing; though here certainly V and U have TYPE_STRUCTURAL_EQUALITY_P, even T has because it is a typedef, not something actually normally returned by build_pointer_type). Jakub
On Mon, Apr 15, 2024 at 09:38:29AM +0200, Jakub Jelinek wrote: > I had this spot instrumented to log the different cases (before adding the > code to fix up also pointer types in c_update_type_canonical) and the only thing > that triggered was that the 2 TYPE_CANONICALs weren't equal if > TYPE_STRUCTURAL_EQUALITY_P (TREE_TYPE (t)), the other was just in case. > gcc.c-torture/compile/20021205-1.c > gcc.c-torture/compile/20040214-2.c > gcc.c-torture/compile/20060109-1.c > gcc.c-torture/compile/pr113623.c > gcc.c-torture/compile/pr46866.c > gcc.c-torture/compile/pta-1.c > gcc.c-torture/execute/pr33870-1.c > gcc.c-torture/execute/pr33870.c > gcc.dg/torture/pr57478.c > tests were affected in make check-gcc. > I thought it would be a clear consequence of the choice we've discussed on > IRC, that build_pointer_type_for_mode and other tree.cc functions which > lookup/create derived types don't try to fill in TYPE_CANONICAL for > types derived from something which initially had TYPE_STRUCTURAL_EQUALITY_P > but later changed to non-TYPE_STRUCTURAL_EQUALITY_P. The patch updates > it solely for qualified types/related pointer types, but doesn't do that > for array types, pointer to array types, function types, ... > So, I think the assertion could still trigger if we have something like > -O2 -flto -std=c23 > struct S; > typedef struct S *T; > typedef T U[10]; > typedef U *V; > V foo (int x) { return 0; } > struct S { int s; }; > (but doesn't, dunno what I'm missing; though here certainly V and U have > TYPE_STRUCTURAL_EQUALITY_P, even T has because it is a typedef, not > something actually normally returned by build_pointer_type). Though, haven't managed to reproduce it with -O2 -flto -std=c23 struct S; typedef struct S **V[10]; V **foo (int x) { return 0; } struct S { int s; }; either. So, maybe let's drop the ipa-free-lang-data.cc part? Seems fld_incomplete_type_of uses fld_type_variant which should copy over TYPE_CANONICAL. Jakub
On Mon, 15 Apr 2024, Jakub Jelinek wrote: > On Mon, Apr 15, 2024 at 09:38:29AM +0200, Jakub Jelinek wrote: > > I had this spot instrumented to log the different cases (before adding the > > code to fix up also pointer types in c_update_type_canonical) and the only thing > > that triggered was that the 2 TYPE_CANONICALs weren't equal if > > TYPE_STRUCTURAL_EQUALITY_P (TREE_TYPE (t)), the other was just in case. > > gcc.c-torture/compile/20021205-1.c > > gcc.c-torture/compile/20040214-2.c > > gcc.c-torture/compile/20060109-1.c > > gcc.c-torture/compile/pr113623.c > > gcc.c-torture/compile/pr46866.c > > gcc.c-torture/compile/pta-1.c > > gcc.c-torture/execute/pr33870-1.c > > gcc.c-torture/execute/pr33870.c > > gcc.dg/torture/pr57478.c > > tests were affected in make check-gcc. > > I thought it would be a clear consequence of the choice we've discussed on > > IRC, that build_pointer_type_for_mode and other tree.cc functions which > > lookup/create derived types don't try to fill in TYPE_CANONICAL for > > types derived from something which initially had TYPE_STRUCTURAL_EQUALITY_P > > but later changed to non-TYPE_STRUCTURAL_EQUALITY_P. The patch updates > > it solely for qualified types/related pointer types, but doesn't do that > > for array types, pointer to array types, function types, ... > > So, I think the assertion could still trigger if we have something like > > -O2 -flto -std=c23 > > struct S; > > typedef struct S *T; > > typedef T U[10]; > > typedef U *V; > > V foo (int x) { return 0; } > > struct S { int s; }; > > (but doesn't, dunno what I'm missing; though here certainly V and U have > > TYPE_STRUCTURAL_EQUALITY_P, even T has because it is a typedef, not > > something actually normally returned by build_pointer_type). > > Though, haven't managed to reproduce it with -O2 -flto -std=c23 > struct S; > typedef struct S **V[10]; > V **foo (int x) { return 0; } > struct S { int s; }; > either. > So, maybe let's drop the ipa-free-lang-data.cc part? > Seems fld_incomplete_type_of uses fld_type_variant which should > copy over TYPE_CANONICAL. If you have a testcase that still triggers it would be nice to see it. Richard.
On Mon, Apr 15, 2024 at 10:02:25AM +0200, Richard Biener wrote: > > Though, haven't managed to reproduce it with -O2 -flto -std=c23 > > struct S; > > typedef struct S **V[10]; > > V **foo (int x) { return 0; } > > struct S { int s; }; > > either. > > So, maybe let's drop the ipa-free-lang-data.cc part? > > Seems fld_incomplete_type_of uses fld_type_variant which should > > copy over TYPE_CANONICAL. > > If you have a testcase that still triggers it would be nice to see it. I don't, that is why I'm now suggesting to just drop that hunk. Jakub
diff --git a/gcc/c/c-decl.cc b/gcc/c/c-decl.cc index 345090dae38..54917297b6a 100644 --- a/gcc/c/c-decl.cc +++ b/gcc/c/c-decl.cc @@ -5051,6 +5051,8 @@ shadow_tag_warned (const struct c_declspecs *declspecs, int warned) if (t == NULL_TREE) { t = make_node (code); + if (flag_isoc23 && code != ENUMERAL_TYPE) + SET_TYPE_STRUCTURAL_EQUALITY (t); pushtag (input_location, name, t); } } @@ -8809,6 +8811,8 @@ parser_xref_tag (location_t loc, enum tree_code code, tree name, the forward-reference will be altered into a real type. */ ref = make_node (code); + if (flag_isoc23 && code != ENUMERAL_TYPE) + SET_TYPE_STRUCTURAL_EQUALITY (ref); if (code == ENUMERAL_TYPE) { /* Give the type a default layout like unsigned int @@ -8910,6 +8914,8 @@ start_struct (location_t loc, enum tree_code code, tree name, if (ref == NULL_TREE || TREE_CODE (ref) != code) { ref = make_node (code); + if (flag_isoc23) + SET_TYPE_STRUCTURAL_EQUALITY (ref); pushtag (loc, name, ref); } @@ -9347,6 +9353,43 @@ is_flexible_array_member_p (bool is_last_field, return false; } +/* Recompute TYPE_CANONICAL for qualified versions of the type and + related pointer types after an aggregate type has been finalized. + Will not update array types, pointers to array types, function + types and other derived types created while the type was still + incomplete, those will remain TYPE_STRUCTURAL_EQUALITY_P. */ + +static void +c_update_type_canonical (tree t) +{ + for (tree x = TYPE_MAIN_VARIANT (t); x; x = TYPE_NEXT_VARIANT (x)) + { + if (x != t + && TYPE_STRUCTURAL_EQUALITY_P (x) + && check_qualified_type (x, t, TYPE_QUALS (x))) + { + if (TYPE_CANONICAL (t) != t) + TYPE_CANONICAL (x) + = build_qualified_type (TYPE_CANONICAL (t), TYPE_QUALS (x)); + else + TYPE_CANONICAL (x) = x; + } + else if (x != t) + continue; + for (tree p = TYPE_POINTER_TO (x); p; p = TYPE_NEXT_PTR_TO (p)) + { + if (!TYPE_STRUCTURAL_EQUALITY_P (p)) + continue; + if (TYPE_CANONICAL (x) != x || TYPE_REF_CAN_ALIAS_ALL (p)) + TYPE_CANONICAL (p) + = build_pointer_type_for_mode (TYPE_CANONICAL (x), TYPE_MODE (p), + false); + else + TYPE_CANONICAL (p) = p; + c_update_type_canonical (p); + } + } +} /* Fill in the fields of a RECORD_TYPE or UNION_TYPE node, T. LOC is the location of the RECORD_TYPE or UNION_TYPE's definition. @@ -9695,11 +9738,12 @@ finish_struct (location_t loc, tree t, tree fieldlist, tree attributes, /* Set type canonical based on equivalence class. */ if (flag_isoc23) { - if (NULL == c_struct_htab) + if (c_struct_htab == NULL) c_struct_htab = hash_table<c_struct_hasher>::create_ggc (61); hashval_t hash = c_struct_hasher::hash (t); + gcc_checking_assert (TYPE_STRUCTURAL_EQUALITY_P (t)); tree *e = c_struct_htab->find_slot_with_hash (t, hash, INSERT); if (*e) TYPE_CANONICAL (t) = *e; @@ -9708,6 +9752,7 @@ finish_struct (location_t loc, tree t, tree fieldlist, tree attributes, TYPE_CANONICAL (t) = t; *e = t; } + c_update_type_canonical (t); } tree incomplete_vars = C_TYPE_INCOMPLETE_VARS (TYPE_MAIN_VARIANT (t)); diff --git a/gcc/c/c-typeck.cc b/gcc/c/c-typeck.cc index ddeab1e2a8a..4567b114734 100644 --- a/gcc/c/c-typeck.cc +++ b/gcc/c/c-typeck.cc @@ -532,6 +532,7 @@ composite_type_internal (tree t1, tree t2, struct composite_cache* cache) /* Otherwise, create a new type node and link it into the cache. */ tree n = make_node (code1); + SET_TYPE_STRUCTURAL_EQUALITY (n); TYPE_NAME (n) = TYPE_NAME (t1); struct composite_cache cache2 = { t1, t2, n, cache }; @@ -590,7 +591,8 @@ composite_type_internal (tree t1, tree t2, struct composite_cache* cache) TYPE_STUB_DECL (n) = pushdecl (build_decl (input_location, TYPE_DECL, NULL_TREE, n)); - n = finish_struct(input_location, n, fields, attributes, NULL, &expr); + n = finish_struct (input_location, n, fields, attributes, NULL, + &expr); n = qualify_type (n, t1); diff --git a/gcc/ipa-free-lang-data.cc b/gcc/ipa-free-lang-data.cc index 16ed55e2e5a..6f75444e513 100644 --- a/gcc/ipa-free-lang-data.cc +++ b/gcc/ipa-free-lang-data.cc @@ -255,7 +255,9 @@ fld_incomplete_type_of (tree t, class free_lang_data_d *fld) first = build_reference_type_for_mode (t2, TYPE_MODE (t), TYPE_REF_CAN_ALIAS_ALL (t)); gcc_assert (TYPE_CANONICAL (t2) != t2 - && TYPE_CANONICAL (t2) == TYPE_CANONICAL (TREE_TYPE (t))); + && (TYPE_CANONICAL (t2) == TYPE_CANONICAL (TREE_TYPE (t)) + || TYPE_STRUCTURAL_EQUALITY_P (t2) + || TYPE_STRUCTURAL_EQUALITY_P (TREE_TYPE (t)))); if (!fld->pset.add (first)) add_tree_to_fld_list (first, fld); return fld_type_variant (first, t, fld); diff --git a/gcc/testsuite/gcc.dg/c23-tag-incomplete-1.c b/gcc/testsuite/gcc.dg/c23-tag-incomplete-1.c new file mode 100644 index 00000000000..82d652569e9 --- /dev/null +++ b/gcc/testsuite/gcc.dg/c23-tag-incomplete-1.c @@ -0,0 +1,14 @@ +/* { dg-do compile } + * { dg-options "-std=c23 -g" } */ + +struct a; +typedef struct a b; + +void g() { + struct a { b* x; }; +} + +struct a { b* x; }; + + + diff --git a/gcc/testsuite/gcc.dg/c23-tag-incomplete-2.c b/gcc/testsuite/gcc.dg/c23-tag-incomplete-2.c new file mode 100644 index 00000000000..bc47a04ece5 --- /dev/null +++ b/gcc/testsuite/gcc.dg/c23-tag-incomplete-2.c @@ -0,0 +1,13 @@ +/* { dg-do compile } + * { dg-options "-std=c23 -g" } */ + +struct a; +typedef struct a b; + +void f() { + extern struct a { b* x; } t; +} + +extern struct a { b* x; } t; + + diff --git a/gcc/testsuite/gcc.dg/pr114361.c b/gcc/testsuite/gcc.dg/pr114361.c new file mode 100644 index 00000000000..0f3feb53566 --- /dev/null +++ b/gcc/testsuite/gcc.dg/pr114361.c @@ -0,0 +1,11 @@ +/* PR c/114361 */ +/* { dg-do compile } */ +/* { dg-options "-std=gnu23 -g" } */ + +void f() +{ + typedef struct foo bar; + typedef __typeof( ({ (struct foo { bar *x; }){ }; }) ) wuz; + struct foo { wuz *x; }; +} + diff --git a/gcc/testsuite/gcc.dg/pr114574-1.c b/gcc/testsuite/gcc.dg/pr114574-1.c new file mode 100644 index 00000000000..060dcdbe73e --- /dev/null +++ b/gcc/testsuite/gcc.dg/pr114574-1.c @@ -0,0 +1,10 @@ +/* PR lto/114574 + * { dg-do compile } + * { dg-options "-flto" } */ + +const struct S * x; +struct S {}; +void f(const struct S **); + + + diff --git a/gcc/testsuite/gcc.dg/pr114574-2.c b/gcc/testsuite/gcc.dg/pr114574-2.c new file mode 100644 index 00000000000..723291e2211 --- /dev/null +++ b/gcc/testsuite/gcc.dg/pr114574-2.c @@ -0,0 +1,10 @@ +/* PR lto/114574 + * { dg-do compile } + * { dg-options "-flto -std=c23" } */ + +const struct S * x; +struct S {}; +void f(const struct S **); + + +