https://gcc.gnu.org/g:37bbd2c1667c70387f5fa6b52f461d57a204229d

commit r14-10366-g37bbd2c1667c70387f5fa6b52f461d57a204229d
Author: Jakub Jelinek <ja...@redhat.com>
Date:   Tue Jun 25 08:35:56 2024 +0200

    c: Fix ICE related to incomplete structures in C23 [PR114930]
    
    Here is a version of the c_update_type_canonical fixes which passed
    bootstrap/regtest.
    The non-trivial part is the handling of the case when
    build_qualified_type (TYPE_CANONICAL (t), TYPE_QUALS (x))
    returns a type with NULL TYPE_CANONICAL.  That should happen only
    if TYPE_CANONICAL (t) == t, because otherwise c_update_type_canonical should
    have been already called on the other type.  c, the returned type, is 
usually x
    and in that case it should have TYPE_CANONICAL set to itself, or worst
    for whatever reason x is not the right canonical type (say it has attributes
    or whatever disqualifies it from check_qualified_type).  In that case
    either it finds some pre-existing type from the variant chain of t which
    is later in the chain and we haven't processed it yet (but then
    get_qualified_type moves it right after t in:
            /* 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 creates a fresh new type using build_variant_type_copy,
    which again places the new type right after t:
      /* Add the new type to the chain of variants of TYPE.  */
      TYPE_NEXT_VARIANT (t) = TYPE_NEXT_VARIANT (m);
      TYPE_NEXT_VARIANT (m) = t;
      TYPE_MAIN_VARIANT (t) = m;
    At this point we want to make c its own canonical type (i.e. TYPE_CANONICAL
    (c) = c;), but also need to process pointers to it and only then return back
    to processing x.  Processing the whole chain from c again could be costly,
    we could have hundreds of types in the chain already processed, and while
    the loop would just quickly skip them
      for (tree x = t, l = NULL_TREE; x; l = x, x = TYPE_NEXT_VARIANT (x))
        {
          if (x != t && TYPE_STRUCTURAL_EQUALITY_P (x))
    ...
          else if (x != t)
            continue;
    it feels costly.  So, this patch instead moves c from right after t
    to right before x in the chain (that shouldn't change anything, because
    clearly build_qualified_type didn't find any matches in the chain before
    x) and continues processing the c at that position, so should handle the
    x that encountered this in the next iteration.
    
    We could avoid some of the moving in the chain if we processed the chain
    twice, once deal only with x != t && TYPE_STRUCTURAL_EQUALITY_P (x)
    && TYPE_CANONICAL (t) == t && check_qualified_type (t, x, TYPE_QUALS (x))
    types (in that case set TYPE_CANONICAL (x) = x) and once the rest.  There
    is still the theoretical case where build_qualified_type would return
    a new type and in that case we are back to the moving the type around and
    needing to handle it though.
    
    2024-06-25  Jakub Jelinek  <ja...@redhat.com>
                Martin Uecker  <uec...@tugraz.at>
    
            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.
    
    (cherry picked from commit 777cc6a01d1cf783a36d0fa67ab20f0312f35d7a)

Diff:
---
 gcc/c/c-decl.cc                   | 38 +++++++++++++++++++++++++++++++-------
 gcc/testsuite/gcc.dg/pr114574-1.c |  6 +++---
 gcc/testsuite/gcc.dg/pr114574-2.c |  6 +++---
 gcc/testsuite/gcc.dg/pr114930.c   |  9 +++++++++
 gcc/testsuite/gcc.dg/pr115502.c   |  9 +++++++++
 5 files changed, 55 insertions(+), 13 deletions(-)

diff --git a/gcc/c/c-decl.cc b/gcc/c/c-decl.cc
index e63dab49589..0e52f217618 100644
--- a/gcc/c/c-decl.cc
+++ b/gcc/c/c-decl.cc
@@ -9363,18 +9363,42 @@ is_flexible_array_member_p (bool is_last_field,
 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)
+                       {
+                         TYPE_NEXT_VARIANT (t) = TYPE_NEXT_VARIANT (c);
+                         TYPE_NEXT_VARIANT (l) = c;
+                         TYPE_NEXT_VARIANT (c) = x;
+                       }
+                     TYPE_CANONICAL (c) = c;
+                     x = c;
+                   }
+               }
+             else
+               TYPE_CANONICAL (x) = TYPE_CANONICAL (c);
+           }
        }
       else if (x != t)
        continue;
diff --git a/gcc/testsuite/gcc.dg/pr114574-1.c 
b/gcc/testsuite/gcc.dg/pr114574-1.c
index e125b68a646..984996f59c6 100644
--- a/gcc/testsuite/gcc.dg/pr114574-1.c
+++ b/gcc/testsuite/gcc.dg/pr114574-1.c
@@ -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 {};
diff --git a/gcc/testsuite/gcc.dg/pr114574-2.c 
b/gcc/testsuite/gcc.dg/pr114574-2.c
index 0e709975e61..b73ef0956a4 100644
--- a/gcc/testsuite/gcc.dg/pr114574-2.c
+++ b/gcc/testsuite/gcc.dg/pr114574-2.c
@@ -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 {};
diff --git a/gcc/testsuite/gcc.dg/pr114930.c b/gcc/testsuite/gcc.dg/pr114930.c
new file mode 100644
index 00000000000..17075a14db8
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/pr114930.c
@@ -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 *);
diff --git a/gcc/testsuite/gcc.dg/pr115502.c b/gcc/testsuite/gcc.dg/pr115502.c
new file mode 100644
index 00000000000..84b1a235f12
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/pr115502.c
@@ -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 *);

Reply via email to