Re: [C PATCH] Fix ICE related to incomplete structures in C23 [PR114930,PR115502].
On Wed, Jun 19, 2024 at 07:32:28PM +0200, Jakub Jelinek wrote:
> Ok, I've tried that, but that doesn't work, it ICEs on the
> pr114574-2.c testcase.
The following works on quick testing of dg.exp=pr11[45]*.c
but haven't bootstrapped/regtested it yet.
2024-06-19 Jakub Jelinek
Martin Uecker
PR c/114930
PR c/115502
gcc/c/
* c-decl.cc (c_update_type_canonical): Assert t is main variant
with 0 TYPE_QUALS. Simplify and don't use check_qualified_type.
Deal with the case where build_qualified_type returns
TYPE_STRUCTURAL_EQUALITY_P type.
gcc/testsuite/
* gcc.dg/pr114574-1.c: Require lto effective target.
* gcc.dg/pr114574-2.c: Likewise.
* gcc.dg/pr114930.c: New test.
* gcc.dg/pr115502.c: New test.
--- gcc/c/c-decl.cc.jj 2024-06-07 12:17:09.582969919 +0200
+++ gcc/c/c-decl.cc 2024-06-19 19:59:24.955836263 +0200
@@ -9367,18 +9367,44 @@ is_flexible_array_member_p (bool is_last
static void
c_update_type_canonical (tree t)
{
- for (tree x = TYPE_MAIN_VARIANT (t); x; x = TYPE_NEXT_VARIANT (x))
+ gcc_checking_assert (TYPE_MAIN_VARIANT (t) == t && !TYPE_QUALS (t));
+ for (tree x = t, l = NULL_TREE; x; l = x, x = TYPE_NEXT_VARIANT (x))
{
if (x != t && TYPE_STRUCTURAL_EQUALITY_P (x))
{
- if (TYPE_QUALS (x) == TYPE_QUALS (t))
+ if (!TYPE_QUALS (x))
TYPE_CANONICAL (x) = TYPE_CANONICAL (t);
- else if (TYPE_CANONICAL (t) != t
- || check_qualified_type (x, t, TYPE_QUALS (x)))
- TYPE_CANONICAL (x)
- = build_qualified_type (TYPE_CANONICAL (t), TYPE_QUALS (x));
else
- TYPE_CANONICAL (x) = x;
+ {
+ tree
+ c = build_qualified_type (TYPE_CANONICAL (t), TYPE_QUALS (x));
+ if (TYPE_STRUCTURAL_EQUALITY_P (c))
+ {
+ gcc_checking_assert (TYPE_CANONICAL (t) == t);
+ if (c == x)
+ TYPE_CANONICAL (x) = x;
+ else
+ {
+ /* build_qualified_type for this function unhelpfully
+moved c from some later spot in TYPE_MAIN_VARIANT (t)
+chain to right after t (or created it there). Move
+it right before x and process c and then x. */
+ gcc_checking_assert (TYPE_NEXT_VARIANT (t) == c);
+ if (l == t)
+ x = t;
+ else
+ {
+ TYPE_NEXT_VARIANT (t) = TYPE_NEXT_VARIANT (c);
+ TYPE_NEXT_VARIANT (l) = c;
+ TYPE_NEXT_VARIANT (c) = x;
+ x = l;
+ }
+ continue;
+ }
+ }
+ else
+ TYPE_CANONICAL (x) = TYPE_CANONICAL (c);
+ }
}
else if (x != t)
continue;
--- gcc/testsuite/gcc.dg/pr114574-1.c.jj2024-04-20 00:05:07.273690453
+0200
+++ gcc/testsuite/gcc.dg/pr114574-1.c 2024-06-19 20:00:33.015984692 +0200
@@ -1,6 +1,6 @@
-/* PR lto/114574
- * { dg-do compile }
- * { dg-options "-flto" } */
+/* PR lto/114574 */
+/* { dg-do compile { target lto } } */
+/* { dg-options "-flto" } */
const struct S * x;
struct S {};
--- gcc/testsuite/gcc.dg/pr114574-2.c.jj2024-04-20 00:05:07.274690440
+0200
+++ gcc/testsuite/gcc.dg/pr114574-2.c 2024-06-19 20:00:55.084704278 +0200
@@ -1,6 +1,6 @@
-/* PR lto/114574
- * { dg-do compile }
- * { dg-options "-flto -std=c23" } */
+/* PR lto/114574 */
+/* { dg-do compile { target lto } } */
+/* { dg-options "-flto -std=c23" } */
const struct S * x;
struct S {};
--- gcc/testsuite/gcc.dg/pr114930.c.jj 2024-06-18 21:27:53.782729543 +0200
+++ gcc/testsuite/gcc.dg/pr114930.c 2024-06-18 21:27:53.782729543 +0200
@@ -0,0 +1,9 @@
+/* PR c/114930 */
+/* { dg-do compile { target lto } } */
+/* { dg-options "-std=c23 -flto" } */
+
+typedef struct WebPPicture WebPPicture;
+typedef int (*WebPProgressHook)(const WebPPicture *);
+WebPProgressHook progress_hook;
+struct WebPPicture {
+} WebPGetColorPalette(const struct WebPPicture *);
--- gcc/testsuite/gcc.dg/pr115502.c.jj 2024-06-18 21:27:53.793729408 +0200
+++ gcc/testsuite/gcc.dg/pr115502.c 2024-06-18 21:27:53.793729408 +0200
@@ -0,0 +1,9 @@
+/* PR c/115502 */
+/* { dg-do compile { target lto } } */
+/* { dg-options "-std=c23 -flto" } */
+
+typedef struct _OSet OSet;
+typedef OSet AvlTree;
+void vgPlain_OSetGen_Lookup(const OSet *);
+struct _OSet {};
+void vgPlain_OSetGen_Lookup(const AvlTree *);
Jakub
Re: [C PATCH] Fix ICE related to incomplete structures in C23 [PR114930,PR115502].
On Wed, Jun 19, 2024 at 09:26:00AM +0200, Martin Uecker wrote:
> Ok. Then should it, instead of
>
> TYPE_CANONICAL (x)
> = build_qualified_type (TYPE_CANONICAL (t), TYPE_QUALS (x));
>
> be
>
> tree c = build_qualified_type (TYPE_CANONICAL (t), TYPE_QUALS (x));
> TYPE_CANONICAL (x) = TREE_CANONICAL (c);
>
> in the patch below?
Ok, I've tried that, but that doesn't work, it ICEs on the
pr114574-2.c testcase.
What happens is that TYPE_CANONICAL (t) = t; is set by the caller
and the first loop first sees x == t, so only handles pointers, and then
moves to const struct S. As TYPE_CANONICAL (t) is t, build_qualified_type
looks for an existing qualified type (and, not just that, it also moves
the found qualified type to TYPE_NEXT_VARIANT (t) to speed it up next
time!!), in this case returns x, so it then effectively does nothing,
TYPE_CANONICAL (x) = TYPE_CANONICAL (x);
and leaves the type TYPE_STRUCTURAL_EQUALITY_P, which is not what we want.
Dunno if it in theory could also find some type later in the
TYPE_NEXT_VARIANT chain and move it earlier.
So, if the new patch is to be used, we need to add some extra handling
for these problematic cases.
One is if c == x (which can happen solely if TYPE_CANONICAL (t) == t),
that is easy to handle, in that case we should make it the canonical type
of itself, so TYPE_CANONICAL (x) = x; rather than TYPE_CANONICAL (x) =
TYPE_CANONICAL (c).
And then there is the theoretical case that c is some type from
the TYPE_MAIN_VARIANT chain which we haven't processed yet. And that
build_qualified_type moved it to the second position in the chain
even when we haven't processed that yet. For that, I think we need
to first process that c and only then restart handling of x.
So, either we could:
gcc_checking_assert (TYPE_MAIN_VARIANT (t) == t && !TYPE_QUALS (t));
for (tree x = t; x; x = TYPE_NEXT_VARIANT (x))
{
if (x != t && TYPE_STRUCTURAL_EQUALITY_P (x))
{
if (!TYPE_QUALS (x))
TYPE_CANONICAL (x) = TYPE_CANONICAL (t);
else
{
tree
c = build_qualified_type (TYPE_CANONICAL (t), TYPE_QUALS (x));
if (TYPE_STRUCTURAL_EQUALITY_P (c))
{
gcc_checking_assert (TYPE_CANONICAL (t) == t);
if (c == x)
TYPE_CANONICAL (x) = x;
else
{
/* build_qualified_type unhelpfully moved c from some
later spot in TYPE_MAIN_VARIANT (t) chain to right
after t. Restart processing the whole chain. */
gcc_checking_assert (TYPE_MAIN_VARIANT (t) == c);
x = t;
continue;
}
}
else
TYPE_CANONICAL (x) = TYPE_CANONICAL (c);
}
}
...
but that could walk perhaps very long chain over and over (sure, for the
already handled cases it would see TYPE_STRUCTUAL_EQUALITY_P (x) is no
longer the case, but still, I'm afraid it increases compile time complexity
for pathological cases too much.
Or perhaps undo what get_qualified_type's
/* Put the found variant at the head of the variant list so
frequently searched variants get found faster. The C++ FE
benefits greatly from this. */
tree t = *tp;
*tp = TYPE_NEXT_VARIANT (t);
TYPE_NEXT_VARIANT (t) = TYPE_NEXT_VARIANT (mv);
TYPE_NEXT_VARIANT (mv) = t;
return t;
optimization or build_variant_type_copy clearly as well (it chains new types
to TYPE_NEXT_VARIANT (mv) as well).
So perhaps instead we need to undo the move.
Here is what I've bootstrapped/regtested and what broke pr114574-2.c:
gcc/c/
* c-decl.cc (c_update_type_canonical): Assert t is main variant
with 0 TYPE_QUALS. Simplify and don't use check_qualified_type.
gcc/testsuite/
* gcc.dg/pr114930.c: New test.
* gcc.dg/pr115502.c: New test.
--- gcc/c/c-decl.cc.jj 2024-06-07 12:17:09.582969919 +0200
+++ gcc/c/c-decl.cc 2024-06-19 13:35:46.648956792 +0200
@@ -9367,18 +9367,19 @@ is_flexible_array_member_p (bool is_last
static void
c_update_type_canonical (tree t)
{
- for (tree x = TYPE_MAIN_VARIANT (t); x; x = TYPE_NEXT_VARIANT (x))
+ gcc_checking_assert (TYPE_MAIN_VARIANT (t) == t && !TYPE_QUALS (t));
+ for (tree x = t; x; x = TYPE_NEXT_VARIANT (x))
{
if (x != t && TYPE_STRUCTURAL_EQUALITY_P (x))
{
- if (TYPE_QUALS (x) == TYPE_QUALS (t))
+ if (!TYPE_QUALS (x))
TYPE_CANONICAL (x) = TYPE_CANONICAL (t);
- else if (TYPE_CANONICAL (t) != t
- || check_qualified_type (x, t, TYPE_QUALS (x)))
- TYPE_CANONICAL (x)
- = build_qualified_type (TYPE_CANONICAL (t), TYPE_QUALS (x));
else
- TYPE_CANONICAL (x) = x;
+ {
+ tree
+ c = build_qualified
Re: [C PATCH] Fix ICE related to incomplete structures in C23 [PR114930,PR115502].
On Wed, Jun 19, 2024 at 09:36:40AM +0200, Richard Biener wrote:
> > TYPE_CANONICAL (x)
> > = build_qualified_type (TYPE_CANONICAL (t), TYPE_QUALS (x));
>
> That looks indeed weird. What are the constraints on 't' for
> c_update_type_canonical? If it is TYPE_STRUCTURAL_EQUALITY_P
> the above will segfault.
>
> The docs of the function says
>
> /* Recompute TYPE_CANONICAL for variants of the type including qualified
>versions of the type and related pointer types after an aggregate type
>has been finalized.
>
> but it seems to also possibly update the main variants canonical type.
> But the main variants canonical type should be the canonical type of
> all other variants, so I would expect the function to determine
> the main variant, possibly update (why?) it's canonical type and
> then assign that to all variant types.
The function is supposed to be called on previously
TYPE_STRUCTURAL_EQUALITY_P main variant on which the FE set
TYPE_CANONICAL to non-NULL:
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;
else
{
TYPE_CANONICAL (t) = t;
*e = t;
}
c_update_type_canonical (t);
and the function should propagate that canonical type or canonical types
derived from that also to its variants (qualified or unqualified) and
pointers to those as long as the variants/pointers are still
TYPE_STRUCTURAL_EQUALITY_P before the change.
Jakub
Re: [C PATCH] Fix ICE related to incomplete structures in C23 [PR114930,PR115502].
On Wed, Jun 19, 2024 at 09:26:00AM +0200, Martin Uecker wrote:
> Ok. Then should it, instead of
>
> TYPE_CANONICAL (x)
> = build_qualified_type (TYPE_CANONICAL (t), TYPE_QUALS (x));
>
> be
>
> tree c = build_qualified_type (TYPE_CANONICAL (t), TYPE_QUALS (x));
> TYPE_CANONICAL (x) = TREE_CANONICAL (c);
>
> in the patch below?
I think you are right, that matches what build_qualified_type does:
else if (TYPE_CANONICAL (type) != type)
/* Build the underlying canonical type, since it is different
from TYPE. */
{
tree c = build_qualified_type (TYPE_CANONICAL (type), type_quals);
TYPE_CANONICAL (t) = TYPE_CANONICAL (c);
}
else
/* T is its own canonical type. */
TYPE_CANONICAL (t) = t;
Jakub
Re: [C PATCH] Fix ICE related to incomplete structures in C23 [PR114930,PR115502].
On Wed, 19 Jun 2024, Martin Uecker wrote: > Am Mittwoch, dem 19.06.2024 um 08:57 +0200 schrieb Richard Biener: > > On Wed, 19 Jun 2024, Martin Uecker wrote: > > > > > Am Mittwoch, dem 19.06.2024 um 08:04 +0200 schrieb Richard Biener: > > > > > > > > > Am 18.06.2024 um 20:18 schrieb Martin Uecker : > > > > > > > > > > Am Dienstag, dem 18.06.2024 um 17:27 +0200 schrieb Richard Biener: > > > > > > > > > > > > > > Am 18.06.2024 um 17:20 schrieb Martin Uecker : > > > > > > > > > > > > > > > > > > > > > As discussed this replaces the use of check_qualified_type with > > > > > > > a simple check for qualifiers as suggested by Jakub in > > > > > > > c_update_type_canonical. > > > > > > > > > > > > Note a canonical type should always be unqualified (for > > > > > > classical qualifiers, not address space or atomic qualification) > > > > > > > > > > The logic in build_qualified_type is the same as in this patch, > > > > > it constructs TYPE_CANONICAL with qualifiers. Or what am I > > > > > missing? > > > > > > > > Nothing if you chose to do TYPE_QUALS (canonical) by copy-pasting. > > > > > > I would rather like to undertand how this work. Is the following > > > correct? > > > > > > For a type T with TYPE_CANONICAL (T) == S, if we construct a qualified > > > type "const" T it would get a const qualified version Q of S as > > > canonical type. S has TYPE_CANONICAL (S) == S and Q has > > > TYPE_CANONICAL (Q) == Q. > > > > I think this is a misunderstanding - TYPE_CANONICAL of both T and Q > > should be the same (S). > > Ok. Then should it, instead of > > TYPE_CANONICAL (x) > = build_qualified_type (TYPE_CANONICAL (t), TYPE_QUALS (x)); That looks indeed weird. What are the constraints on 't' for c_update_type_canonical? If it is TYPE_STRUCTURAL_EQUALITY_P the above will segfault. The docs of the function says /* Recompute TYPE_CANONICAL for variants of the type including qualified versions of the type and related pointer types after an aggregate type has been finalized. but it seems to also possibly update the main variants canonical type. But the main variants canonical type should be the canonical type of all other variants, so I would expect the function to determine the main variant, possibly update (why?) it's canonical type and then assign that to all variant types. Richard. > be > > tree c = build_qualified_type (TYPE_CANONICAL (t), TYPE_QUALS (x)); > TYPE_CANONICAL (x) = TREE_CANONICAL (c); > > in the patch below? > > Martin > > > > > > But the middle end would then ignore this for regular qualifiers > > > and use the TYPE_CANONICAL of the main variant for computing > > > the aliasing set? > > > > The middle end uses the canonical type for alias set purposes. > > The indirection via TYPE_MAIN_VARIANT shouldn't be necessary. > > > > > Setting it differently for qualified types would still be important > > > so that derived types get their own different TYPE_CANONICAL > > > during construction. > > > > > > > > > The other thing I would like to confirm? The alias set is > > > initialized to -1 so set later in the middle end based on > > > TYPE_CANONICAL (if not set to zero before). This is never > > > done before the FE is finished or do we need to worry that > > > this may become inconsistent? > > > > It used to be that -Wstrict-aliasing calls get_alias_set so > > TYPE_ALIAS_SET can become set, I don't remember this being > > avoided so yes, you'd have to worry about this. Note > > TYPE_ALIAS_SET is only ever considered on the canonical type > > (but IIRC we have checking code looking at other type alias set). > > > > Richard. > > > > > Martin > > > > > > > Richard > > > > > > > > > Martin > > > > > > > > > > > > > > > > > Richard > > > > > > > > > > > > > Martin > > > > > > > > > > > > > > > > > > > > > Bootstrapped and regression tested on x86_64. > > > > > > > > > > > > > > > > > > > > > C23: Fix ICE related to incomplete structures > > > > > > > [PR114930,PR115502]. > > > > > > > > > > > > > > The fix for PR114574 needs to be further revised because > > > > > > > check_qualified_type > > > > > > > makes decision based on TYPE_NAME which can be incorrect for C > > > > > > > when there > > > > > > > are TYPE_DECLS involved. > > > > > > > > > > > > > > gcc/c/: > > > > > > > * c-decl.c (c_update_type_canonical): Do not use > > > > > > > check_qualified_type. > > > > > > > > > > > > > > gcc/testsuite/: > > > > > > > * gcc.dg/pr114930.c: New test. > > > > > > > * gcc.dg/pr115502.c: New test. > > > > > > > > > > > > > > diff --git a/gcc/c/c-decl.cc b/gcc/c/c-decl.cc > > > > > > > index 01326570e2b..610061a07f8 100644 > > > > > > > --- a/gcc/c/c-decl.cc > > > > > > > +++ b/gcc/c/c-decl.cc > > > > > > > @@ -9374,7 +9374,7 @@ c_update_type_canonical (tree t) > > > > > > > if (TYPE_QUALS (x) == TYPE_QUALS (t)) > > > > > > > TYPE_CANONICAL (x) = TYPE_CANONICAL (t); > > > > > > > else if (TYPE
Re: [C PATCH] Fix ICE related to incomplete structures in C23 [PR114930,PR115502].
Am Mittwoch, dem 19.06.2024 um 08:57 +0200 schrieb Richard Biener:
> On Wed, 19 Jun 2024, Martin Uecker wrote:
>
> > Am Mittwoch, dem 19.06.2024 um 08:04 +0200 schrieb Richard Biener:
> > >
> > > > Am 18.06.2024 um 20:18 schrieb Martin Uecker :
> > > >
> > > > Am Dienstag, dem 18.06.2024 um 17:27 +0200 schrieb Richard Biener:
> > > > >
> > > > > > > Am 18.06.2024 um 17:20 schrieb Martin Uecker :
> > > > > >
> > > > > >
> > > > > > As discussed this replaces the use of check_qualified_type with
> > > > > > a simple check for qualifiers as suggested by Jakub in
> > > > > > c_update_type_canonical.
> > > > >
> > > > > Note a canonical type should always be unqualified (for
> > > > > classical qualifiers, not address space or atomic qualification)
> > > >
> > > > The logic in build_qualified_type is the same as in this patch,
> > > > it constructs TYPE_CANONICAL with qualifiers. Or what am I
> > > > missing?
> > >
> > > Nothing if you chose to do TYPE_QUALS (canonical) by copy-pasting.
> >
> > I would rather like to undertand how this work. Is the following
> > correct?
> >
> > For a type T with TYPE_CANONICAL (T) == S, if we construct a qualified
> > type "const" T it would get a const qualified version Q of S as
> > canonical type. S has TYPE_CANONICAL (S) == S and Q has
> > TYPE_CANONICAL (Q) == Q.
>
> I think this is a misunderstanding - TYPE_CANONICAL of both T and Q
> should be the same (S).
Ok. Then should it, instead of
TYPE_CANONICAL (x)
= build_qualified_type (TYPE_CANONICAL (t), TYPE_QUALS (x));
be
tree c = build_qualified_type (TYPE_CANONICAL (t), TYPE_QUALS (x));
TYPE_CANONICAL (x) = TREE_CANONICAL (c);
in the patch below?
Martin
>
> > But the middle end would then ignore this for regular qualifiers
> > and use the TYPE_CANONICAL of the main variant for computing
> > the aliasing set?
>
> The middle end uses the canonical type for alias set purposes.
> The indirection via TYPE_MAIN_VARIANT shouldn't be necessary.
>
> > Setting it differently for qualified types would still be important
> > so that derived types get their own different TYPE_CANONICAL
> > during construction.
> >
> >
> > The other thing I would like to confirm? The alias set is
> > initialized to -1 so set later in the middle end based on
> > TYPE_CANONICAL (if not set to zero before). This is never
> > done before the FE is finished or do we need to worry that
> > this may become inconsistent?
>
> It used to be that -Wstrict-aliasing calls get_alias_set so
> TYPE_ALIAS_SET can become set, I don't remember this being
> avoided so yes, you'd have to worry about this. Note
> TYPE_ALIAS_SET is only ever considered on the canonical type
> (but IIRC we have checking code looking at other type alias set).
>
> Richard.
>
> > Martin
> >
> > > Richard
> > >
> > > > Martin
> > > >
> > > > >
> > > > > Richard
> > > > >
> > > > > > Martin
> > > > > >
> > > > > >
> > > > > > Bootstrapped and regression tested on x86_64.
> > > > > >
> > > > > >
> > > > > > C23: Fix ICE related to incomplete structures [PR114930,PR115502].
> > > > > >
> > > > > > The fix for PR114574 needs to be further revised because
> > > > > > check_qualified_type
> > > > > > makes decision based on TYPE_NAME which can be incorrect for C
> > > > > > when there
> > > > > > are TYPE_DECLS involved.
> > > > > >
> > > > > > gcc/c/:
> > > > > > * c-decl.c (c_update_type_canonical): Do not use
> > > > > > check_qualified_type.
> > > > > >
> > > > > > gcc/testsuite/:
> > > > > > * gcc.dg/pr114930.c: New test.
> > > > > > * gcc.dg/pr115502.c: New test.
> > > > > >
> > > > > > diff --git a/gcc/c/c-decl.cc b/gcc/c/c-decl.cc
> > > > > > index 01326570e2b..610061a07f8 100644
> > > > > > --- a/gcc/c/c-decl.cc
> > > > > > +++ b/gcc/c/c-decl.cc
> > > > > > @@ -9374,7 +9374,7 @@ c_update_type_canonical (tree t)
> > > > > > if (TYPE_QUALS (x) == TYPE_QUALS (t))
> > > > > > TYPE_CANONICAL (x) = TYPE_CANONICAL (t);
> > > > > > else if (TYPE_CANONICAL (t) != t
> > > > > > - || check_qualified_type (x, t, TYPE_QUALS (x)))
> > > > > > + || TYPE_QUALS (x) != TYPE_QUALS (TYPE_CANONICAL (t)))
> > > > > > TYPE_CANONICAL (x)
> > > > > > = build_qualified_type (TYPE_CANONICAL (t), TYPE_QUALS (x));
> > > > > > else
> > > > > > diff --git a/gcc/testsuite/gcc.dg/pr114930.c
> > > > > > b/gcc/testsuite/gcc.dg/pr114930.c
> > > > > > new file mode 100644
> > > > > > index 000..5e982fb8929
> > > > > > --- /dev/null
> > > > > > +++ b/gcc/testsuite/gcc.dg/pr114930.c
> > > > > > @@ -0,0 +1,9 @@
> > > > > > +/* { dg-do compile }
> > > > > > + * { dg-options "-std=c23 -flto" } */
> > > > > > +
> > > > > > +typedef struct WebPPicture WebPPicture;
> > > > > > +typedef int (*WebPProgressHook)(const WebPPicture *);
> > > > > > +WebPProgressHook progress_hook;
> > > > > > +struct WebPPicture {
> > > > > > +} WebPGetColorPalette(const struct WebPPicture *);
> >
Re: [C PATCH] Fix ICE related to incomplete structures in C23 [PR114930,PR115502].
On Wed, 19 Jun 2024, Jakub Jelinek wrote:
> On Wed, Jun 19, 2024 at 08:04:55AM +0200, Richard Biener wrote:
> > >> Note a canonical type should always be unqualified (for
> > >> classical qualifiers, not address space or atomic qualification)
> > >
> > > The logic in build_qualified_type is the same as in this patch,
> > > it constructs TYPE_CANONICAL with qualifiers. Or what am I
> > > missing?
> >
> > Nothing if you chose to do TYPE_QUALS (canonical) by copy-pasting.
>
> As I mentioned earlier, I'd prefer the following version and I have
> already tested it on the testcases (note, some changes to the tests,
> e.g. using lto effective target), just haven't bootstrapped/regtested it
> fully yet.
LGTM, but I'd defer to a FE maintainer for approval.
Richard.
> gcc/c/
> * c-decl.cc (c_update_type_canonical): Assert t is main variant
> with 0 TYPE_QUALS. Simplify and don't use check_qualified_type.
> gcc/testsuite/
> * gcc.dg/pr114930.c: New test.
> * gcc.dg/pr115502.c: New test.
>
> --- gcc/c/c-decl.cc.jj2024-06-07 12:17:09.582969919 +0200
> +++ gcc/c/c-decl.cc 2024-06-18 21:30:22.881904947 +0200
> @@ -9367,18 +9367,16 @@ is_flexible_array_member_p (bool is_last
> static void
> c_update_type_canonical (tree t)
> {
> - for (tree x = TYPE_MAIN_VARIANT (t); x; x = TYPE_NEXT_VARIANT (x))
> + gcc_checking_assert (TYPE_MAIN_VARIANT (t) == t && !TYPE_QUALS (t));
> + for (tree x = t; x; x = TYPE_NEXT_VARIANT (x))
> {
>if (x != t && TYPE_STRUCTURAL_EQUALITY_P (x))
> {
> - if (TYPE_QUALS (x) == TYPE_QUALS (t))
> + if (!TYPE_QUALS (x))
> TYPE_CANONICAL (x) = TYPE_CANONICAL (t);
> - else if (TYPE_CANONICAL (t) != t
> -|| check_qualified_type (x, t, TYPE_QUALS (x)))
> + else
> TYPE_CANONICAL (x)
> = build_qualified_type (TYPE_CANONICAL (t), TYPE_QUALS (x));
> - else
> - TYPE_CANONICAL (x) = x;
> }
>else if (x != t)
> continue;
> --- gcc/testsuite/gcc.dg/pr114930.c.jj2024-06-18 21:27:53.782729543
> +0200
> +++ gcc/testsuite/gcc.dg/pr114930.c 2024-06-18 21:27:53.782729543 +0200
> @@ -0,0 +1,9 @@
> +/* PR c/114930 */
> +/* { dg-do compile { target lto } } */
> +/* { dg-options "-std=c23 -flto" } */
> +
> +typedef struct WebPPicture WebPPicture;
> +typedef int (*WebPProgressHook)(const WebPPicture *);
> +WebPProgressHook progress_hook;
> +struct WebPPicture {
> +} WebPGetColorPalette(const struct WebPPicture *);
> --- gcc/testsuite/gcc.dg/pr115502.c.jj2024-06-18 21:27:53.793729408
> +0200
> +++ gcc/testsuite/gcc.dg/pr115502.c 2024-06-18 21:27:53.793729408 +0200
> @@ -0,0 +1,9 @@
> +/* PR c/115502 */
> +/* { dg-do compile { target lto } } */
> +/* { dg-options "-std=c23 -flto" } */
> +
> +typedef struct _OSet OSet;
> +typedef OSet AvlTree;
> +void vgPlain_OSetGen_Lookup(const OSet *);
> +struct _OSet {};
> +void vgPlain_OSetGen_Lookup(const AvlTree *);
>
> Jakub
>
>
--
Richard Biener
SUSE Software Solutions Germany GmbH,
Frankenstrasse 146, 90461 Nuernberg, Germany;
GF: Ivo Totev, Andrew McDonald, Werner Knoblich; (HRB 36809, AG Nuernberg)
Re: [C PATCH] Fix ICE related to incomplete structures in C23 [PR114930,PR115502].
On Wed, 19 Jun 2024, Martin Uecker wrote:
> Am Mittwoch, dem 19.06.2024 um 08:04 +0200 schrieb Richard Biener:
> >
> > > Am 18.06.2024 um 20:18 schrieb Martin Uecker :
> > >
> > > Am Dienstag, dem 18.06.2024 um 17:27 +0200 schrieb Richard Biener:
> > > >
> > > > > > Am 18.06.2024 um 17:20 schrieb Martin Uecker :
> > > > >
> > > > >
> > > > > As discussed this replaces the use of check_qualified_type with
> > > > > a simple check for qualifiers as suggested by Jakub in
> > > > > c_update_type_canonical.
> > > >
> > > > Note a canonical type should always be unqualified (for
> > > > classical qualifiers, not address space or atomic qualification)
> > >
> > > The logic in build_qualified_type is the same as in this patch,
> > > it constructs TYPE_CANONICAL with qualifiers. Or what am I
> > > missing?
> >
> > Nothing if you chose to do TYPE_QUALS (canonical) by copy-pasting.
>
> I would rather like to undertand how this work. Is the following
> correct?
>
> For a type T with TYPE_CANONICAL (T) == S, if we construct a qualified
> type "const" T it would get a const qualified version Q of S as
> canonical type. S has TYPE_CANONICAL (S) == S and Q has
> TYPE_CANONICAL (Q) == Q.
I think this is a misunderstanding - TYPE_CANONICAL of both T and Q
should be the same (S).
> But the middle end would then ignore this for regular qualifiers
> and use the TYPE_CANONICAL of the main variant for computing
> the aliasing set?
The middle end uses the canonical type for alias set purposes.
The indirection via TYPE_MAIN_VARIANT shouldn't be necessary.
> Setting it differently for qualified types would still be important
> so that derived types get their own different TYPE_CANONICAL
> during construction.
>
>
> The other thing I would like to confirm? The alias set is
> initialized to -1 so set later in the middle end based on
> TYPE_CANONICAL (if not set to zero before). This is never
> done before the FE is finished or do we need to worry that
> this may become inconsistent?
It used to be that -Wstrict-aliasing calls get_alias_set so
TYPE_ALIAS_SET can become set, I don't remember this being
avoided so yes, you'd have to worry about this. Note
TYPE_ALIAS_SET is only ever considered on the canonical type
(but IIRC we have checking code looking at other type alias set).
Richard.
> Martin
>
> > Richard
> >
> > > Martin
> > >
> > > >
> > > > Richard
> > > >
> > > > > Martin
> > > > >
> > > > >
> > > > > Bootstrapped and regression tested on x86_64.
> > > > >
> > > > >
> > > > > C23: Fix ICE related to incomplete structures [PR114930,PR115502].
> > > > >
> > > > > The fix for PR114574 needs to be further revised because
> > > > > check_qualified_type
> > > > > makes decision based on TYPE_NAME which can be incorrect for C when
> > > > > there
> > > > > are TYPE_DECLS involved.
> > > > >
> > > > > gcc/c/:
> > > > > * c-decl.c (c_update_type_canonical): Do not use
> > > > > check_qualified_type.
> > > > >
> > > > > gcc/testsuite/:
> > > > > * gcc.dg/pr114930.c: New test.
> > > > > * gcc.dg/pr115502.c: New test.
> > > > >
> > > > > diff --git a/gcc/c/c-decl.cc b/gcc/c/c-decl.cc
> > > > > index 01326570e2b..610061a07f8 100644
> > > > > --- a/gcc/c/c-decl.cc
> > > > > +++ b/gcc/c/c-decl.cc
> > > > > @@ -9374,7 +9374,7 @@ c_update_type_canonical (tree t)
> > > > > if (TYPE_QUALS (x) == TYPE_QUALS (t))
> > > > > TYPE_CANONICAL (x) = TYPE_CANONICAL (t);
> > > > > else if (TYPE_CANONICAL (t) != t
> > > > > - || check_qualified_type (x, t, TYPE_QUALS (x)))
> > > > > + || TYPE_QUALS (x) != TYPE_QUALS (TYPE_CANONICAL (t)))
> > > > > TYPE_CANONICAL (x)
> > > > > = build_qualified_type (TYPE_CANONICAL (t), TYPE_QUALS (x));
> > > > > else
> > > > > diff --git a/gcc/testsuite/gcc.dg/pr114930.c
> > > > > b/gcc/testsuite/gcc.dg/pr114930.c
> > > > > new file mode 100644
> > > > > index 000..5e982fb8929
> > > > > --- /dev/null
> > > > > +++ b/gcc/testsuite/gcc.dg/pr114930.c
> > > > > @@ -0,0 +1,9 @@
> > > > > +/* { dg-do compile }
> > > > > + * { dg-options "-std=c23 -flto" } */
> > > > > +
> > > > > +typedef struct WebPPicture WebPPicture;
> > > > > +typedef int (*WebPProgressHook)(const WebPPicture *);
> > > > > +WebPProgressHook progress_hook;
> > > > > +struct WebPPicture {
> > > > > +} WebPGetColorPalette(const struct WebPPicture *);
> > > > > +
> > > > > diff --git a/gcc/testsuite/gcc.dg/pr115502.c
> > > > > b/gcc/testsuite/gcc.dg/pr115502.c
> > > > > new file mode 100644
> > > > > index 000..02b52622c5a
> > > > > --- /dev/null
> > > > > +++ b/gcc/testsuite/gcc.dg/pr115502.c
> > > > > @@ -0,0 +1,9 @@
> > > > > +/* { dg-do compile }
> > > > > + * { dg-options "-std=c23 -flto" } */
> > > > > +
> > > > > +typedef struct _OSet OSet;
> > > > > +typedef OSet AvlTree;
> > > > > +void vgPlain_OSetGen_Lookup(const OSet *);
> > > > > +struct _OSet {};
> > > > > +void vgPlain_OSetGen_Lookup(const Avl
Re: [C PATCH] Fix ICE related to incomplete structures in C23 [PR114930,PR115502].
Am Mittwoch, dem 19.06.2024 um 08:29 +0200 schrieb Jakub Jelinek:
> On Wed, Jun 19, 2024 at 08:04:55AM +0200, Richard Biener wrote:
> > > > Note a canonical type should always be unqualified (for
> > > > classical qualifiers, not address space or atomic qualification)
> > >
> > > The logic in build_qualified_type is the same as in this patch,
> > > it constructs TYPE_CANONICAL with qualifiers. Or what am I
> > > missing?
> >
> > Nothing if you chose to do TYPE_QUALS (canonical) by copy-pasting.
>
> As I mentioned earlier, I'd prefer the following version and I have
> already tested it on the testcases (note, some changes to the tests,
> e.g. using lto effective target), just haven't bootstrapped/regtested it
> fully yet.
Ok, thanks, I wasn't sure about the conclusion of the discussion.
Martin
>
> gcc/c/
> * c-decl.cc (c_update_type_canonical): Assert t is main variant
> with 0 TYPE_QUALS. Simplify and don't use check_qualified_type.
> gcc/testsuite/
> * gcc.dg/pr114930.c: New test.
> * gcc.dg/pr115502.c: New test.
>
> --- gcc/c/c-decl.cc.jj2024-06-07 12:17:09.582969919 +0200
> +++ gcc/c/c-decl.cc 2024-06-18 21:30:22.881904947 +0200
> @@ -9367,18 +9367,16 @@ is_flexible_array_member_p (bool is_last
> static void
> c_update_type_canonical (tree t)
> {
> - for (tree x = TYPE_MAIN_VARIANT (t); x; x = TYPE_NEXT_VARIANT (x))
> + gcc_checking_assert (TYPE_MAIN_VARIANT (t) == t && !TYPE_QUALS (t));
> + for (tree x = t; x; x = TYPE_NEXT_VARIANT (x))
> {
>if (x != t && TYPE_STRUCTURAL_EQUALITY_P (x))
> {
> - if (TYPE_QUALS (x) == TYPE_QUALS (t))
> + if (!TYPE_QUALS (x))
> TYPE_CANONICAL (x) = TYPE_CANONICAL (t);
> - else if (TYPE_CANONICAL (t) != t
> -|| check_qualified_type (x, t, TYPE_QUALS (x)))
> + else
> TYPE_CANONICAL (x)
> = build_qualified_type (TYPE_CANONICAL (t), TYPE_QUALS (x));
> - else
> - TYPE_CANONICAL (x) = x;
> }
>else if (x != t)
> continue;
> --- gcc/testsuite/gcc.dg/pr114930.c.jj2024-06-18 21:27:53.782729543
> +0200
> +++ gcc/testsuite/gcc.dg/pr114930.c 2024-06-18 21:27:53.782729543 +0200
> @@ -0,0 +1,9 @@
> +/* PR c/114930 */
> +/* { dg-do compile { target lto } } */
> +/* { dg-options "-std=c23 -flto" } */
> +
> +typedef struct WebPPicture WebPPicture;
> +typedef int (*WebPProgressHook)(const WebPPicture *);
> +WebPProgressHook progress_hook;
> +struct WebPPicture {
> +} WebPGetColorPalette(const struct WebPPicture *);
> --- gcc/testsuite/gcc.dg/pr115502.c.jj2024-06-18 21:27:53.793729408
> +0200
> +++ gcc/testsuite/gcc.dg/pr115502.c 2024-06-18 21:27:53.793729408 +0200
> @@ -0,0 +1,9 @@
> +/* PR c/115502 */
> +/* { dg-do compile { target lto } } */
> +/* { dg-options "-std=c23 -flto" } */
> +
> +typedef struct _OSet OSet;
> +typedef OSet AvlTree;
> +void vgPlain_OSetGen_Lookup(const OSet *);
> +struct _OSet {};
> +void vgPlain_OSetGen_Lookup(const AvlTree *);
>
> Jakub
>
Re: [C PATCH] Fix ICE related to incomplete structures in C23 [PR114930,PR115502].
Am Mittwoch, dem 19.06.2024 um 08:04 +0200 schrieb Richard Biener:
>
> > Am 18.06.2024 um 20:18 schrieb Martin Uecker :
> >
> > Am Dienstag, dem 18.06.2024 um 17:27 +0200 schrieb Richard Biener:
> > >
> > > > > Am 18.06.2024 um 17:20 schrieb Martin Uecker :
> > > >
> > > >
> > > > As discussed this replaces the use of check_qualified_type with
> > > > a simple check for qualifiers as suggested by Jakub in
> > > > c_update_type_canonical.
> > >
> > > Note a canonical type should always be unqualified (for
> > > classical qualifiers, not address space or atomic qualification)
> >
> > The logic in build_qualified_type is the same as in this patch,
> > it constructs TYPE_CANONICAL with qualifiers. Or what am I
> > missing?
>
> Nothing if you chose to do TYPE_QUALS (canonical) by copy-pasting.
I would rather like to undertand how this work. Is the following
correct?
For a type T with TYPE_CANONICAL (T) == S, if we construct a qualified
type "const" T it would get a const qualified version Q of S as
canonical type. S has TYPE_CANONICAL (S) == S and Q has
TYPE_CANONICAL (Q) == Q.
But the middle end would then ignore this for regular qualifiers
and use the TYPE_CANONICAL of the main variant for computing
the aliasing set?
Setting it differently for qualified types would still be important
so that derived types get their own different TYPE_CANONICAL
during construction.
The other thing I would like to confirm? The alias set is
initialized to -1 so set later in the middle end based on
TYPE_CANONICAL (if not set to zero before). This is never
done before the FE is finished or do we need to worry that
this may become inconsistent?
Martin
> Richard
>
> > Martin
> >
> > >
> > > Richard
> > >
> > > > Martin
> > > >
> > > >
> > > > Bootstrapped and regression tested on x86_64.
> > > >
> > > >
> > > > C23: Fix ICE related to incomplete structures [PR114930,PR115502].
> > > >
> > > > The fix for PR114574 needs to be further revised because
> > > > check_qualified_type
> > > > makes decision based on TYPE_NAME which can be incorrect for C when
> > > > there
> > > > are TYPE_DECLS involved.
> > > >
> > > > gcc/c/:
> > > > * c-decl.c (c_update_type_canonical): Do not use
> > > > check_qualified_type.
> > > >
> > > > gcc/testsuite/:
> > > > * gcc.dg/pr114930.c: New test.
> > > > * gcc.dg/pr115502.c: New test.
> > > >
> > > > diff --git a/gcc/c/c-decl.cc b/gcc/c/c-decl.cc
> > > > index 01326570e2b..610061a07f8 100644
> > > > --- a/gcc/c/c-decl.cc
> > > > +++ b/gcc/c/c-decl.cc
> > > > @@ -9374,7 +9374,7 @@ c_update_type_canonical (tree t)
> > > > if (TYPE_QUALS (x) == TYPE_QUALS (t))
> > > > TYPE_CANONICAL (x) = TYPE_CANONICAL (t);
> > > > else if (TYPE_CANONICAL (t) != t
> > > > - || check_qualified_type (x, t, TYPE_QUALS (x)))
> > > > + || TYPE_QUALS (x) != TYPE_QUALS (TYPE_CANONICAL (t)))
> > > > TYPE_CANONICAL (x)
> > > > = build_qualified_type (TYPE_CANONICAL (t), TYPE_QUALS (x));
> > > > else
> > > > diff --git a/gcc/testsuite/gcc.dg/pr114930.c
> > > > b/gcc/testsuite/gcc.dg/pr114930.c
> > > > new file mode 100644
> > > > index 000..5e982fb8929
> > > > --- /dev/null
> > > > +++ b/gcc/testsuite/gcc.dg/pr114930.c
> > > > @@ -0,0 +1,9 @@
> > > > +/* { dg-do compile }
> > > > + * { dg-options "-std=c23 -flto" } */
> > > > +
> > > > +typedef struct WebPPicture WebPPicture;
> > > > +typedef int (*WebPProgressHook)(const WebPPicture *);
> > > > +WebPProgressHook progress_hook;
> > > > +struct WebPPicture {
> > > > +} WebPGetColorPalette(const struct WebPPicture *);
> > > > +
> > > > diff --git a/gcc/testsuite/gcc.dg/pr115502.c
> > > > b/gcc/testsuite/gcc.dg/pr115502.c
> > > > new file mode 100644
> > > > index 000..02b52622c5a
> > > > --- /dev/null
> > > > +++ b/gcc/testsuite/gcc.dg/pr115502.c
> > > > @@ -0,0 +1,9 @@
> > > > +/* { dg-do compile }
> > > > + * { dg-options "-std=c23 -flto" } */
> > > > +
> > > > +typedef struct _OSet OSet;
> > > > +typedef OSet AvlTree;
> > > > +void vgPlain_OSetGen_Lookup(const OSet *);
> > > > +struct _OSet {};
> > > > +void vgPlain_OSetGen_Lookup(const AvlTree *);
> > > > +
> > > >
> >
Re: [C PATCH] Fix ICE related to incomplete structures in C23 [PR114930,PR115502].
On Wed, Jun 19, 2024 at 08:04:55AM +0200, Richard Biener wrote:
> >> Note a canonical type should always be unqualified (for
> >> classical qualifiers, not address space or atomic qualification)
> >
> > The logic in build_qualified_type is the same as in this patch,
> > it constructs TYPE_CANONICAL with qualifiers. Or what am I
> > missing?
>
> Nothing if you chose to do TYPE_QUALS (canonical) by copy-pasting.
As I mentioned earlier, I'd prefer the following version and I have
already tested it on the testcases (note, some changes to the tests,
e.g. using lto effective target), just haven't bootstrapped/regtested it
fully yet.
gcc/c/
* c-decl.cc (c_update_type_canonical): Assert t is main variant
with 0 TYPE_QUALS. Simplify and don't use check_qualified_type.
gcc/testsuite/
* gcc.dg/pr114930.c: New test.
* gcc.dg/pr115502.c: New test.
--- gcc/c/c-decl.cc.jj 2024-06-07 12:17:09.582969919 +0200
+++ gcc/c/c-decl.cc 2024-06-18 21:30:22.881904947 +0200
@@ -9367,18 +9367,16 @@ is_flexible_array_member_p (bool is_last
static void
c_update_type_canonical (tree t)
{
- for (tree x = TYPE_MAIN_VARIANT (t); x; x = TYPE_NEXT_VARIANT (x))
+ gcc_checking_assert (TYPE_MAIN_VARIANT (t) == t && !TYPE_QUALS (t));
+ for (tree x = t; x; x = TYPE_NEXT_VARIANT (x))
{
if (x != t && TYPE_STRUCTURAL_EQUALITY_P (x))
{
- if (TYPE_QUALS (x) == TYPE_QUALS (t))
+ if (!TYPE_QUALS (x))
TYPE_CANONICAL (x) = TYPE_CANONICAL (t);
- else if (TYPE_CANONICAL (t) != t
- || check_qualified_type (x, t, TYPE_QUALS (x)))
+ else
TYPE_CANONICAL (x)
= build_qualified_type (TYPE_CANONICAL (t), TYPE_QUALS (x));
- else
- TYPE_CANONICAL (x) = x;
}
else if (x != t)
continue;
--- gcc/testsuite/gcc.dg/pr114930.c.jj 2024-06-18 21:27:53.782729543 +0200
+++ gcc/testsuite/gcc.dg/pr114930.c 2024-06-18 21:27:53.782729543 +0200
@@ -0,0 +1,9 @@
+/* PR c/114930 */
+/* { dg-do compile { target lto } } */
+/* { dg-options "-std=c23 -flto" } */
+
+typedef struct WebPPicture WebPPicture;
+typedef int (*WebPProgressHook)(const WebPPicture *);
+WebPProgressHook progress_hook;
+struct WebPPicture {
+} WebPGetColorPalette(const struct WebPPicture *);
--- gcc/testsuite/gcc.dg/pr115502.c.jj 2024-06-18 21:27:53.793729408 +0200
+++ gcc/testsuite/gcc.dg/pr115502.c 2024-06-18 21:27:53.793729408 +0200
@@ -0,0 +1,9 @@
+/* PR c/115502 */
+/* { dg-do compile { target lto } } */
+/* { dg-options "-std=c23 -flto" } */
+
+typedef struct _OSet OSet;
+typedef OSet AvlTree;
+void vgPlain_OSetGen_Lookup(const OSet *);
+struct _OSet {};
+void vgPlain_OSetGen_Lookup(const AvlTree *);
Jakub
Re: [C PATCH] Fix ICE related to incomplete structures in C23 [PR114930, PR115502].
> Am 18.06.2024 um 20:18 schrieb Martin Uecker :
>
> Am Dienstag, dem 18.06.2024 um 17:27 +0200 schrieb Richard Biener:
>>
Am 18.06.2024 um 17:20 schrieb Martin Uecker :
>>>
>>>
>>> As discussed this replaces the use of check_qualified_type with
>>> a simple check for qualifiers as suggested by Jakub in
>>> c_update_type_canonical.
>>
>> Note a canonical type should always be unqualified (for
>> classical qualifiers, not address space or atomic qualification)
>
> The logic in build_qualified_type is the same as in this patch,
> it constructs TYPE_CANONICAL with qualifiers. Or what am I
> missing?
Nothing if you chose to do TYPE_QUALS (canonical) by copy-pasting.
Richard
> Martin
>
>>
>> Richard
>>
>>> Martin
>>>
>>>
>>> Bootstrapped and regression tested on x86_64.
>>>
>>>
>>> C23: Fix ICE related to incomplete structures [PR114930,PR115502].
>>>
>>> The fix for PR114574 needs to be further revised because
>>> check_qualified_type
>>> makes decision based on TYPE_NAME which can be incorrect for C when there
>>> are TYPE_DECLS involved.
>>>
>>> gcc/c/:
>>> * c-decl.c (c_update_type_canonical): Do not use
>>> check_qualified_type.
>>>
>>> gcc/testsuite/:
>>> * gcc.dg/pr114930.c: New test.
>>> * gcc.dg/pr115502.c: New test.
>>>
>>> diff --git a/gcc/c/c-decl.cc b/gcc/c/c-decl.cc
>>> index 01326570e2b..610061a07f8 100644
>>> --- a/gcc/c/c-decl.cc
>>> +++ b/gcc/c/c-decl.cc
>>> @@ -9374,7 +9374,7 @@ c_update_type_canonical (tree t)
>>> if (TYPE_QUALS (x) == TYPE_QUALS (t))
>>> TYPE_CANONICAL (x) = TYPE_CANONICAL (t);
>>> else if (TYPE_CANONICAL (t) != t
>>> - || check_qualified_type (x, t, TYPE_QUALS (x)))
>>> + || TYPE_QUALS (x) != TYPE_QUALS (TYPE_CANONICAL (t)))
>>> TYPE_CANONICAL (x)
>>> = build_qualified_type (TYPE_CANONICAL (t), TYPE_QUALS (x));
>>> else
>>> diff --git a/gcc/testsuite/gcc.dg/pr114930.c
>>> b/gcc/testsuite/gcc.dg/pr114930.c
>>> new file mode 100644
>>> index 000..5e982fb8929
>>> --- /dev/null
>>> +++ b/gcc/testsuite/gcc.dg/pr114930.c
>>> @@ -0,0 +1,9 @@
>>> +/* { dg-do compile }
>>> + * { dg-options "-std=c23 -flto" } */
>>> +
>>> +typedef struct WebPPicture WebPPicture;
>>> +typedef int (*WebPProgressHook)(const WebPPicture *);
>>> +WebPProgressHook progress_hook;
>>> +struct WebPPicture {
>>> +} WebPGetColorPalette(const struct WebPPicture *);
>>> +
>>> diff --git a/gcc/testsuite/gcc.dg/pr115502.c
>>> b/gcc/testsuite/gcc.dg/pr115502.c
>>> new file mode 100644
>>> index 000..02b52622c5a
>>> --- /dev/null
>>> +++ b/gcc/testsuite/gcc.dg/pr115502.c
>>> @@ -0,0 +1,9 @@
>>> +/* { dg-do compile }
>>> + * { dg-options "-std=c23 -flto" } */
>>> +
>>> +typedef struct _OSet OSet;
>>> +typedef OSet AvlTree;
>>> +void vgPlain_OSetGen_Lookup(const OSet *);
>>> +struct _OSet {};
>>> +void vgPlain_OSetGen_Lookup(const AvlTree *);
>>> +
>>>
>
Re: [C PATCH] Fix ICE related to incomplete structures in C23 [PR114930,PR115502].
Am Dienstag, dem 18.06.2024 um 17:27 +0200 schrieb Richard Biener:
>
> > Am 18.06.2024 um 17:20 schrieb Martin Uecker :
> >
> >
> > As discussed this replaces the use of check_qualified_type with
> > a simple check for qualifiers as suggested by Jakub in
> > c_update_type_canonical.
>
> Note a canonical type should always be unqualified (for
> classical qualifiers, not address space or atomic qualification)
The logic in build_qualified_type is the same as in this patch,
it constructs TYPE_CANONICAL with qualifiers. Or what am I
missing?
Martin
>
> Richard
>
> > Martin
> >
> >
> > Bootstrapped and regression tested on x86_64.
> >
> >
> >C23: Fix ICE related to incomplete structures [PR114930,PR115502].
> >
> >The fix for PR114574 needs to be further revised because
> > check_qualified_type
> >makes decision based on TYPE_NAME which can be incorrect for C when there
> >are TYPE_DECLS involved.
> >
> >gcc/c/:
> >* c-decl.c (c_update_type_canonical): Do not use
> > check_qualified_type.
> >
> >gcc/testsuite/:
> >* gcc.dg/pr114930.c: New test.
> >* gcc.dg/pr115502.c: New test.
> >
> > diff --git a/gcc/c/c-decl.cc b/gcc/c/c-decl.cc
> > index 01326570e2b..610061a07f8 100644
> > --- a/gcc/c/c-decl.cc
> > +++ b/gcc/c/c-decl.cc
> > @@ -9374,7 +9374,7 @@ c_update_type_canonical (tree t)
> > if (TYPE_QUALS (x) == TYPE_QUALS (t))
> >TYPE_CANONICAL (x) = TYPE_CANONICAL (t);
> > else if (TYPE_CANONICAL (t) != t
> > - || check_qualified_type (x, t, TYPE_QUALS (x)))
> > + || TYPE_QUALS (x) != TYPE_QUALS (TYPE_CANONICAL (t)))
> >TYPE_CANONICAL (x)
> > = build_qualified_type (TYPE_CANONICAL (t), TYPE_QUALS (x));
> > else
> > diff --git a/gcc/testsuite/gcc.dg/pr114930.c
> > b/gcc/testsuite/gcc.dg/pr114930.c
> > new file mode 100644
> > index 000..5e982fb8929
> > --- /dev/null
> > +++ b/gcc/testsuite/gcc.dg/pr114930.c
> > @@ -0,0 +1,9 @@
> > +/* { dg-do compile }
> > + * { dg-options "-std=c23 -flto" } */
> > +
> > +typedef struct WebPPicture WebPPicture;
> > +typedef int (*WebPProgressHook)(const WebPPicture *);
> > +WebPProgressHook progress_hook;
> > +struct WebPPicture {
> > +} WebPGetColorPalette(const struct WebPPicture *);
> > +
> > diff --git a/gcc/testsuite/gcc.dg/pr115502.c
> > b/gcc/testsuite/gcc.dg/pr115502.c
> > new file mode 100644
> > index 000..02b52622c5a
> > --- /dev/null
> > +++ b/gcc/testsuite/gcc.dg/pr115502.c
> > @@ -0,0 +1,9 @@
> > +/* { dg-do compile }
> > + * { dg-options "-std=c23 -flto" } */
> > +
> > +typedef struct _OSet OSet;
> > +typedef OSet AvlTree;
> > +void vgPlain_OSetGen_Lookup(const OSet *);
> > +struct _OSet {};
> > +void vgPlain_OSetGen_Lookup(const AvlTree *);
> > +
> >
Re: [C PATCH] Fix ICE related to incomplete structures in C23 [PR114930, PR115502].
> Am 18.06.2024 um 17:20 schrieb Martin Uecker :
>
>
> As discussed this replaces the use of check_qualified_type with
> a simple check for qualifiers as suggested by Jakub in
> c_update_type_canonical.
Note a canonical type should always be unqualified (for classical qualifiers,
not address space or atomic qualification)
Richard
> Martin
>
>
> Bootstrapped and regression tested on x86_64.
>
>
>C23: Fix ICE related to incomplete structures [PR114930,PR115502].
>
>The fix for PR114574 needs to be further revised because
> check_qualified_type
>makes decision based on TYPE_NAME which can be incorrect for C when there
>are TYPE_DECLS involved.
>
>gcc/c/:
>* c-decl.c (c_update_type_canonical): Do not use
> check_qualified_type.
>
>gcc/testsuite/:
>* gcc.dg/pr114930.c: New test.
>* gcc.dg/pr115502.c: New test.
>
> diff --git a/gcc/c/c-decl.cc b/gcc/c/c-decl.cc
> index 01326570e2b..610061a07f8 100644
> --- a/gcc/c/c-decl.cc
> +++ b/gcc/c/c-decl.cc
> @@ -9374,7 +9374,7 @@ c_update_type_canonical (tree t)
> if (TYPE_QUALS (x) == TYPE_QUALS (t))
>TYPE_CANONICAL (x) = TYPE_CANONICAL (t);
> else if (TYPE_CANONICAL (t) != t
> - || check_qualified_type (x, t, TYPE_QUALS (x)))
> + || TYPE_QUALS (x) != TYPE_QUALS (TYPE_CANONICAL (t)))
>TYPE_CANONICAL (x)
> = build_qualified_type (TYPE_CANONICAL (t), TYPE_QUALS (x));
> else
> diff --git a/gcc/testsuite/gcc.dg/pr114930.c b/gcc/testsuite/gcc.dg/pr114930.c
> new file mode 100644
> index 000..5e982fb8929
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/pr114930.c
> @@ -0,0 +1,9 @@
> +/* { dg-do compile }
> + * { dg-options "-std=c23 -flto" } */
> +
> +typedef struct WebPPicture WebPPicture;
> +typedef int (*WebPProgressHook)(const WebPPicture *);
> +WebPProgressHook progress_hook;
> +struct WebPPicture {
> +} WebPGetColorPalette(const struct WebPPicture *);
> +
> diff --git a/gcc/testsuite/gcc.dg/pr115502.c b/gcc/testsuite/gcc.dg/pr115502.c
> new file mode 100644
> index 000..02b52622c5a
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/pr115502.c
> @@ -0,0 +1,9 @@
> +/* { dg-do compile }
> + * { dg-options "-std=c23 -flto" } */
> +
> +typedef struct _OSet OSet;
> +typedef OSet AvlTree;
> +void vgPlain_OSetGen_Lookup(const OSet *);
> +struct _OSet {};
> +void vgPlain_OSetGen_Lookup(const AvlTree *);
> +
>
[C PATCH] Fix ICE related to incomplete structures in C23 [PR114930,PR115502].
As discussed this replaces the use of check_qualified_type with
a simple check for qualifiers as suggested by Jakub in
c_update_type_canonical.
Martin
Bootstrapped and regression tested on x86_64.
C23: Fix ICE related to incomplete structures [PR114930,PR115502].
The fix for PR114574 needs to be further revised because
check_qualified_type
makes decision based on TYPE_NAME which can be incorrect for C when there
are TYPE_DECLS involved.
gcc/c/:
* c-decl.c (c_update_type_canonical): Do not use
check_qualified_type.
gcc/testsuite/:
* gcc.dg/pr114930.c: New test.
* gcc.dg/pr115502.c: New test.
diff --git a/gcc/c/c-decl.cc b/gcc/c/c-decl.cc
index 01326570e2b..610061a07f8 100644
--- a/gcc/c/c-decl.cc
+++ b/gcc/c/c-decl.cc
@@ -9374,7 +9374,7 @@ c_update_type_canonical (tree t)
if (TYPE_QUALS (x) == TYPE_QUALS (t))
TYPE_CANONICAL (x) = TYPE_CANONICAL (t);
else if (TYPE_CANONICAL (t) != t
- || check_qualified_type (x, t, TYPE_QUALS (x)))
+ || TYPE_QUALS (x) != TYPE_QUALS (TYPE_CANONICAL (t)))
TYPE_CANONICAL (x)
= build_qualified_type (TYPE_CANONICAL (t), TYPE_QUALS (x));
else
diff --git a/gcc/testsuite/gcc.dg/pr114930.c b/gcc/testsuite/gcc.dg/pr114930.c
new file mode 100644
index 000..5e982fb8929
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/pr114930.c
@@ -0,0 +1,9 @@
+/* { dg-do compile }
+ * { dg-options "-std=c23 -flto" } */
+
+typedef struct WebPPicture WebPPicture;
+typedef int (*WebPProgressHook)(const WebPPicture *);
+WebPProgressHook progress_hook;
+struct WebPPicture {
+} WebPGetColorPalette(const struct WebPPicture *);
+
diff --git a/gcc/testsuite/gcc.dg/pr115502.c b/gcc/testsuite/gcc.dg/pr115502.c
new file mode 100644
index 000..02b52622c5a
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/pr115502.c
@@ -0,0 +1,9 @@
+/* { dg-do compile }
+ * { dg-options "-std=c23 -flto" } */
+
+typedef struct _OSet OSet;
+typedef OSet AvlTree;
+void vgPlain_OSetGen_Lookup(const OSet *);
+struct _OSet {};
+void vgPlain_OSetGen_Lookup(const AvlTree *);
+
