[C PATCH] Fix ICE related to incomplete structures in C23 [PR114930,PR115502].

[Martin Uecker]

Am Mittwoch, dem 19.06.2024 um 08:04 +0200 schrieb Richard Biener:
> 
> > Am 18.06.2024 um 20:18 schrieb Martin Uecker <uecker@tugraz.at>:
> > 
> > Am Dienstag, dem 18.06.2024 um 17:27 +0200 schrieb Richard Biener:
> > > 
> > > > > Am 18.06.2024 um 17:20 schrieb Martin Uecker <uecker@tugraz.at>:
> > > > 
> > > > 
> > > > 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 00000000000..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 00000000000..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 *);
> > > > +
> > > > 
> >