[Bug lto/114574] New: [14 regression] ICE when building curl with LTO (internal compiler error: in fld_incomplete_type_of, at ipa-free-lang-data.cc:257)

[Bug lto/114574] New: [14 regression] ICE when building curl with LTO (internal compiler error: in fld_incomplete_type_of, at ipa-free-lang-data.cc:257)

[sjames at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=114574

            Bug ID: 114574
           Summary: [14 regression] ICE when building curl with LTO
                    (internal compiler error: in fld_incomplete_type_of,
                    at ipa-free-lang-data.cc:257)
           Product: gcc
           Version: 14.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: lto
          Assignee: unassigned at gcc dot gnu.org
          Reporter: sjames at gcc dot gnu.org
  Target Milestone: ---

Created attachment 57861
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=57861&action=edit
libcurl_la-curl_ntlm_core.i.xz

Tonnes of these failures. Just picked curl at random.

```
$ x86_64-pc-linux-gnu-gcc -m32 -mfpmath=sse -DHAVE_CONFIG_H
-I/var/tmp/portage/net-misc/curl-8.7.1-r1/work/curl-8.7.1/include -I../lib
-I/var/tmp/portage/net-misc/curl-8.7.1-r1/work/curl-8.7.1/lib
-DBUILDING_LIBCURL -DCURL_HIDDEN_SYMBOLS -fvisibility=hidden -O3 -pipe
-march=native -fdiagnostics-color=always -flto -fno-vect-cost-model
-fpermissive -Werror-implicit-function-declaration -c
/var/tmp/portage/net-misc/curl-8.7.1-r1/work/curl-8.7.1/lib/curl_ntlm_core.c 
-fPIC -DPIC -o .libs/libcurl_la-curl_ntlm_core.o
during IPA pass: *free_lang_data
/var/tmp/portage/net-misc/curl-8.7.1-r1/work/curl-8.7.1/lib/curl_ntlm_core.c:665:1:
internal compiler error: in fld_incomplete_type_of, at
ipa-free-lang-data.cc:257
  665 | }
      | ^
0x55f1b9e2de0f fld_incomplete_type_of
       
/usr/src/debug/sys-devel/gcc-14.0.9999/gcc-14.0.9999/gcc/ipa-free-lang-data.cc:257
0x55f1bb41a2ad fld_simplified_type
       
/usr/src/debug/sys-devel/gcc-14.0.9999/gcc-14.0.9999/gcc/ipa-free-lang-data.cc:344
0x55f1bb41a2ad free_lang_data_in_type
       
/usr/src/debug/sys-devel/gcc-14.0.9999/gcc-14.0.9999/gcc/ipa-free-lang-data.cc:439
0x55f1bbaa5ad0 free_lang_data_in_cgraph
       
/usr/src/debug/sys-devel/gcc-14.0.9999/gcc-14.0.9999/gcc/ipa-free-lang-data.cc:1072
0x55f1bbaa5ad0 free_lang_data
       
/usr/src/debug/sys-devel/gcc-14.0.9999/gcc-14.0.9999/gcc/ipa-free-lang-data.cc:1109
0x55f1bbaa5ad0 execute
       
/usr/src/debug/sys-devel/gcc-14.0.9999/gcc-14.0.9999/gcc/ipa-free-lang-data.cc:1176
Please submit a full bug report, with preprocessed source (by using
-freport-bug).
Please include the complete backtrace with any bug report.
See <https://bugs.gentoo.org/> for instructions.
```

'gcc -c libcurl_la-curl_ntlm_core.i -O2 -flto' is enough to reproduce. I last
built curl fine on 30th March, apparently.

```
Using built-in specs.
COLLECT_GCC=gcc
COLLECT_LTO_WRAPPER=/usr/libexec/gcc/x86_64-pc-linux-gnu/14/lto-wrapper
Target: x86_64-pc-linux-gnu
Configured with:
/var/tmp/portage/sys-devel/gcc-14.0.9999/work/gcc-14.0.9999/configure
--host=x86_64-pc-linux-gnu --build=x86_64-pc-linux-gnu --prefix=/usr
--bindir=/usr/x86_64-pc-linux-gnu/gcc-bin/14
--includedir=/usr/lib/gcc/x86_64-pc-linux-gnu/14/include
--datadir=/usr/share/gcc-data/x86_64-pc-linux-gnu/14
--mandir=/usr/share/gcc-data/x86_64-pc-linux-gnu/14/man
--infodir=/usr/share/gcc-data/x86_64-pc-linux-gnu/14/info
--with-gxx-include-dir=/usr/lib/gcc/x86_64-pc-linux-gnu/14/include/g++-v14
--disable-silent-rules --disable-dependency-tracking
--with-python-dir=/share/gcc-data/x86_64-pc-linux-gnu/14/python
--enable-languages=c,c++,fortran --enable-obsolete --enable-secureplt
--disable-werror --with-system-zlib --enable-nls --without-included-gettext
--disable-libunwind-exceptions --enable-checking=yes,extra,rtl,df
--with-bugurl=https://bugs.gentoo.org/ --with-pkgversion='Gentoo 14.0.9999 p,
commit 7bbfb01a32b73842f8908de028703510a0e12057' --with-gcc-major-version-only
--enable-libstdcxx-time --enable-lto --disable-libstdcxx-pch --enable-shared
--enable-threads=posix --enable-__cxa_atexit --enable-clocale=gnu
--enable-multilib --with-multilib-list=m32,m64 --disable-fixed-point
--enable-targets=all --enable-libgomp --disable-libssp --disable-libada
--disable-cet --disable-systemtap --disable-valgrind-annotations
--disable-vtable-verify --disable-libvtv --with-zstd --without-isl
--enable-default-pie --enable-host-pie --disable-host-bind-now
--enable-default-ssp --disable-fixincludes --with-build-config='bootstrap-O3
bootstrap-lto'
Thread model: posix
Supported LTO compression algorithms: zlib zstd
gcc version 14.0.1 20240403 (experimental)
8455d6f6cd43b7b143ab9ee19437452fceba9cc9 (Gentoo 14.0.9999 p, commit
7bbfb01a32b73842f8908de028703510a0e12057)
```

[Bug lto/114574] [14 regression] ICE when building curl with LTO (fld_incomplete_type_of, at ipa-free-lang-data.cc:257)

[sjames at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=114574

--- Comment #1 from Sam James <sjames at gcc dot gnu.org> ---
reducing

[Bug lto/114574] [14 regression] ICE when building curl with LTO (fld_incomplete_type_of, at ipa-free-lang-data.cc:257)

[sjames at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=114574

--- Comment #2 from Sam James <sjames at gcc dot gnu.org> ---
Reduced:
```
struct X509_algor_st sk_X509_ALGOR_copyfunc(const struct X509_algor_st *);
struct X509_algor_st {
} PKCS8_pkey_get0(const struct X509_algor_st **) {
}
```

[Bug lto/114574] [14 regression] ICE when building curl with LTO (fld_incomplete_type_of, at ipa-free-lang-data.cc:257) since r14-9763

[jakub at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=114574

Jakub Jelinek <jakub at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Priority|P3                          |P1
            Summary|[14 regression] ICE when    |[14 regression] ICE when
                   |building curl with LTO      |building curl with LTO
                   |(fld_incomplete_type_of, at |(fld_incomplete_type_of, at
                   |ipa-free-lang-data.cc:257)  |ipa-free-lang-data.cc:257)
                   |                            |since r14-9763
   Last reconfirmed|                            |2024-04-03
             Status|UNCONFIRMED                 |NEW
   Target Milestone|---                         |14.0
                 CC|                            |jakub at gcc dot gnu.org,
                   |                            |uecker at gcc dot gnu.org
     Ever confirmed|0                           |1

--- Comment #3 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Started with r14-9763-g871bb5ad2dd56343d80b6a6d269e85efdc9999e5

[Bug lto/114574] [14 regression] ICE when building curl with LTO (fld_incomplete_type_of, at ipa-free-lang-data.cc:257) since r14-9763

[jakub at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=114574

--- Comment #4 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Slightly cleaned up testcase:
struct S foo (const struct S *);
struct S {};
struct S bar (const struct S **) {}

[Bug lto/114574] [14 regression] ICE when building curl with LTO (fld_incomplete_type_of, at ipa-free-lang-data.cc:257) since r14-9763

[jakub at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=114574

--- Comment #5 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Oops, return stmt missing:
struct S foo (const struct S *);
struct S {};
struct S bar (const struct S **) { return (struct S) {}; }

[Bug lto/114574] [14 regression] ICE when building curl with LTO (fld_incomplete_type_of, at ipa-free-lang-data.cc:257) since r14-9763

[uecker at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=114574

--- Comment #6 from uecker at gcc dot gnu.org ---
Hm, this is enough:

const struct S * x;
struct S {};
void f(const struct S **);

The TYPE_CANONICAL of the pointer type depends on TYPE_CANONICAL of the target.
So it seems if I set it for completed types the new pointer get a new
TYPE_CANONICAL while old pointers to the incomplete struct are not updated
which then somehow triggers the assertion  (and is wrong).

[Bug lto/114574] [14 regression] ICE when building curl with LTO (fld_incomplete_type_of, at ipa-free-lang-data.cc:257) since r14-9763

[uecker at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=114574

--- Comment #7 from uecker at gcc dot gnu.org ---
What is strange is that not updating TYPE_CANONICAL also seems wrong even
before C23, because then it seems we should be able to get pointers with
different TYPE_CANONICAL which are compatible (to the incomplete and to the
completed struct). But apparently this never did cause problems...

[Bug lto/114574] [14 regression] ICE when building curl with LTO (fld_incomplete_type_of, at ipa-free-lang-data.cc:257) since r14-9763

[uecker at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=114574

--- Comment #8 from uecker at gcc dot gnu.org ---

If we do not care too much about TYPE_CANONICAL being correct in this case
anyway, we could turn off the test and add a condition flag_isoc23 as a
precaution in the FE to not risk any other regressions for pre C23.  Otherwise
we could revert the fix to PR114361 but this also does not seem ideal.


diff --git a/gcc/c/c-decl.cc b/gcc/c/c-decl.cc
index f2083b9d96f..efdbe1a3bde 100644
--- a/gcc/c/c-decl.cc
+++ b/gcc/c/c-decl.cc
@@ -9722,7 +9722,8 @@ finish_struct (location_t loc, tree t, tree fieldlist,
tree attributes,
       C_TYPE_VARIABLE_SIZE (x) = C_TYPE_VARIABLE_SIZE (t);
       C_TYPE_VARIABLY_MODIFIED (x) = C_TYPE_VARIABLY_MODIFIED (t);
       C_TYPE_INCOMPLETE_VARS (x) = NULL_TREE;
-      TYPE_CANONICAL (x) = TYPE_CANONICAL (t);
+      if (flag_isoc23)
+       TYPE_CANONICAL (x) = TYPE_CANONICAL (t);
     }

   /* Update type location to the one of the definition, instead of e.g.
diff --git a/gcc/ipa-free-lang-data.cc b/gcc/ipa-free-lang-data.cc
index 16ed55e2e5a..0b75f05fab3 100644
--- a/gcc/ipa-free-lang-data.cc
+++ b/gcc/ipa-free-lang-data.cc
@@ -254,8 +254,11 @@ fld_incomplete_type_of (tree t, class free_lang_data_d
*fld)
          else
            first = build_reference_type_for_mode (t2, TYPE_MODE (t),
                                                   TYPE_REF_CAN_ALIAS_ALL (t));
+         // FIXME: temporarily decativated because of PR114493
+#if 0
          gcc_assert (TYPE_CANONICAL (t2) != t2
                      && TYPE_CANONICAL (t2) == TYPE_CANONICAL (TREE_TYPE
(t)));
+#endif
          if (!fld->pset.add (first))
            add_tree_to_fld_list (first, fld);
          return fld_type_variant (first, t, fld);

[Bug lto/114574] [14 regression] ICE when building curl with LTO (fld_incomplete_type_of, at ipa-free-lang-data.cc:257) since r14-9763

[jakub at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=114574

Jakub Jelinek <jakub at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |hubicka at gcc dot gnu.org,
                   |                            |jsm28 at gcc dot gnu.org

--- Comment #9 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Note, build_qualified_type sets TYPE_CANONICAL of qualified types to either the
returned type or build_qualified_type of the TYPE_CANONICAL of the base type.
So, updating TYPE_CANONICAL of the variants to TYPE_CANONICAL of the base type
looks always wrong, it would need to recompute the TYPE_CANONICAL using
build_qualified_type instead.  But, similarly to build_qualified_type,
build_pointer_type, build_array_type etc. also similarly set TYPE_CANONICAL to
something based on the TYPE_CANONICAL.

So, if you really need to update TYPE_CANONICAL after type is completed, I'd at
least do it only if the new TYPE_CANONICAL is actually different from the base
type, otherwise don't update anything, and if you do update, one needs to also
recompute TYPE_CANONICAL on all the pointer types (those can be found through
for (t = TYPE_POINTER_TO (to_type); t; t = TYPE_NEXT_PTR_TO (t))),
but one would need to do that recursively to also update the **, ***, **** etc.
pointers), but also all created ARRAY_TYPEs (the shared ones can be found in
type_hash_table but non-shared can't), FUNCTION_TYPEs etc.

Which makes me wonder if for flag_isoc23 it wouldn't be better to
SET_TYPE_STRUCTURAL_EQUALITY on incomplete structure/union types and therefore
also on any POINTER_TYPE, ARRAY_TYPE, FUNCTION_TYPE, ... derived from that, and
only set
TYPE_CANONICAL when the aggregate is completed.  Yes, it will be slower to
compare
some of the types because one won't be able to use TYPE_CANONICAL, but given
the above it seems really hard to recompute TYPE_CANONICAL on everything that
could have been derived from TYPE_CANONICAL (type) = type of the incomplete
type when it will be later changed.

[Bug lto/114574] [14 regression] ICE when building curl with LTO (fld_incomplete_type_of, at ipa-free-lang-data.cc:257) since r14-9763

[uecker at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=114574

--- Comment #10 from uecker at gcc dot gnu.org ---

Yes, this makes sense. I will try this. (I tried this approach already but I
did not get this work, probably because something I missed). 

Otherwise one could try setting TYPE_CANONICAL only based on the tag, which
would then never need to change, but this would lose a lot of information. 

What consequences does it have to have the wrong TYPE_CANONICAL on pointers? I
am asking because the current behavior also seems wrong because now
TYPE_CANONICAL for complete and incomplete types may turn out to be different.
Or am I missing something?

[Bug lto/114574] [14 regression] ICE when building curl with LTO (fld_incomplete_type_of, at ipa-free-lang-data.cc:257) since r14-9763

[rguenth at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=114574

Richard Biener <rguenth at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Keywords|                            |lto

--- Comment #11 from Richard Biener <rguenth at gcc dot gnu.org> ---
Wrong TYPE_CANONICAL leads to wrong TBAA.  And yes "fixing" TYPE_CANONICAL
after the fact means possibly merging two equivalence classes which means you
have to
fix all members of both classes to have the corresponding TYPE_CANONICAL.  Plus
the complication of derived types (vector, pointer, and others) which have
constraints based on their contained type equivalence class.

[Bug lto/114574] [14 regression] ICE when building curl with LTO (fld_incomplete_type_of, at ipa-free-lang-data.cc:257) since r14-9763

[jakub at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=114574

--- Comment #12 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Not an expert on TYPE_CANONICAL, but my understanding is that non-NULL
TYPE_CANONICAL is just an optimization to speed up type comparisons (but it
seems c-typeck.cc doesn't actually use that, so it is mainly middle-end then).
See how e.g. cp/typeck.cc (comptypes) will do structural_comptypes if at least
one TYPE_CANONICAL is NULL (aka TYPE_STRUCTURAL_EQUALITY_P), and otherwise at
least without checking will rely on TYPE_CANONICAL comparison and that is it.
So, having TYPE_CANONICAL NULL should just be slower, but not wrong (as long as
comptypes etc. works correctly).  Having TYPE_CANONICAL non-NULL and not
actually matching could cause type comparisons to think types are the same when
they aren't or vice versa.

Now, when you set TYPE_CANONICAL on completion of the flag_isoc23 aggregates,
you could surely also update TYPE_CANONICAL of the variant types using
build_qualified_type and perhaps pointers too, just to handle the common case. 
But unlike the case where the TYPE_CANONICAL of the aggregate TYPE_MAIN_VARIANT
would be initially different, you wouldn't need to chase down everything,
including TYPE_CANONICAL on FUNCTION_TYPE which have pointers to array type of
array type of pointer of pointer of qualified pointer to the aggregate etc.,
just the common things.
And setting TYPE_CANONICAL just based on the tag would be also wrong, if one
can define struct S { int s; }; in one scope and struct S { int a, b; }; in
another scope and those types are supposed to be different.

[Bug lto/114574] [14 regression] ICE when building curl with LTO (fld_incomplete_type_of, at ipa-free-lang-data.cc:257) since r14-9763

[muecker at gwdg dot de]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=114574

Martin Uecker <muecker at gwdg dot de> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |muecker at gwdg dot de

--- Comment #13 from Martin Uecker <muecker at gwdg dot de> ---


(In reply to Richard Biener from comment #11)
> Wrong TYPE_CANONICAL leads to wrong TBAA.  And yes "fixing" TYPE_CANONICAL
> after the fact means possibly merging two equivalence classes which means
> you have to
> fix all members of both classes to have the corresponding TYPE_CANONICAL. 
> Plus
> the complication of derived types (vector, pointer, and others) which have
> constraints based on their contained type equivalence class.


Is upgrading incomplete types from structural comparison to TYPE_CANONICAL
meant to be allowed?

[Bug lto/114574] [14 regression] ICE when building curl with LTO (fld_incomplete_type_of, at ipa-free-lang-data.cc:257) since r14-9763

[muecker at gwdg dot de]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=114574

--- Comment #14 from Martin Uecker <muecker at gwdg dot de> ---
(In reply to Jakub Jelinek from comment #12)
> Not an expert on TYPE_CANONICAL, but my understanding is that non-NULL
> TYPE_CANONICAL is just an optimization to speed up type comparisons (but it
> seems c-typeck.cc doesn't actually use that, so it is mainly middle-end
> then).
> See how e.g. cp/typeck.cc (comptypes) will do structural_comptypes if at
> least one TYPE_CANONICAL is NULL (aka TYPE_STRUCTURAL_EQUALITY_P), and
> otherwise at least without checking will rely on TYPE_CANONICAL comparison
> and that is it.
> So, having TYPE_CANONICAL NULL should just be slower, but not wrong (as long
> as comptypes etc. works correctly).  Having TYPE_CANONICAL non-NULL and not
> actually matching could cause type comparisons to think types are the same
> when they aren't or vice versa.
> 
> Now, when you set TYPE_CANONICAL on completion of the flag_isoc23
> aggregates, you could surely also update TYPE_CANONICAL of the variant types
> using build_qualified_type and perhaps pointers too, just to handle the
> common case.  But unlike the case where the TYPE_CANONICAL of the aggregate
> TYPE_MAIN_VARIANT would be initially different, you wouldn't need to chase
> down everything, including TYPE_CANONICAL on FUNCTION_TYPE which have
> pointers to array type of array type of pointer of pointer of qualified
> pointer to the aggregate etc., just the common things.
> And setting TYPE_CANONICAL just based on the tag would be also wrong, if one
> can define struct S { int s; }; in one scope and struct S { int a, b; }; in
> another scope and those types are supposed to be different.


According to my understanding, setting based on tag would form larger
equivalence classes, so should give correct code but suboptimal aliasing
analysis.

There are two general issues I see here.

1.) The middle end idea of type compatibility is not necessarily the same as
the language semantics. In particular type compatibility in C is not based on
mutual equivalence, so any mechanism based on forming equivalence classes using
TYPE_CANONICAL can only be an approximation (which to be correct, must be less
precise than the language semantics). 

2.) If compatibility relationship changes for types (as it the case when
completing types, then we either have to use an equivalence class that does not
need to change (e.g. based on tags) or only use structural equivalency, or
upgrade structural equivalency to the  equivalency class for the completed type
if this is allowed.

[Bug lto/114574] [14 regression] ICE when building curl with LTO (fld_incomplete_type_of, at ipa-free-lang-data.cc:257) since r14-9763

[muecker at gwdg dot de]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=114574

--- Comment #15 from Martin Uecker <muecker at gwdg dot de> ---
Created attachment 57874
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=57874&action=edit
patch


Tentative patch for the fix that makes the incomplete types have structural
equivalence at the beginning and then sets TYPE_CANONICAL when they are
completed.  There is some fallout when one removes the flag_isoc23 condition
which I need to look into...  

Assuming this approach is ok, then I think this change should be done also for
pre-C23 in the future.

[Bug lto/114574] [14 regression] ICE when building curl with LTO (fld_incomplete_type_of, at ipa-free-lang-data.cc:257) since r14-9763

[muecker at gwdg dot de]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=114574

Martin Uecker <muecker at gwdg dot de> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #57874|0                           |1
        is obsolete|                            |

--- Comment #16 from Martin Uecker <muecker at gwdg dot de> ---
Created attachment 57885
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=57885&action=edit
patch


New version. This should work with and without the flag_isoc23 guards.

[Bug lto/114574] [14 regression] ICE when building curl with LTO (fld_incomplete_type_of, at ipa-free-lang-data.cc:257) since r14-9763

[jakub at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=114574

--- Comment #17 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Some comments:
+  else
+    {
+       TYPE_CANONICAL (t) = t;
+    }
The formatting here is wrong, TYPE_CANONICAL is indented too much, but we also
don't put a single statement between {}s, so just
  else
    TYPE_CANONICAL (t) = t;

NULL_TREE == TYPE_CANONICAL
!TYPE_CANONICAL

We don't use constant == non-constant style, but non-constant == constant, but
here
both of the above should just be
TYPE_STRUCTURAL_EQUALITY_P

+         TYPE_CANONICAL (x) = TYPE_CANONICAL (t);
As has been discussed, this is wrong, it should have been
TYPE_CANONICAL (x) = build_qualified_type (TYPE_CANONICAL (t), TYPE_QUALS (x));
or so.

The build_pointer_type change looks really weird.
I'd say if you want to be prepared for to_type formerly
TYPE_STRUCTURAL_EQUALITY_P types and now no longer so, we should just change
  for (t = TYPE_POINTER_TO (to_type); t; t = TYPE_NEXT_PTR_TO (t))
    if (TYPE_MODE (t) == mode && TYPE_REF_CAN_ALIAS_ALL (t) == can_alias_all)
      return t;
loop to check for this case before return t; (i.e. { if
(!TYPE_STRUCTURAL_EQUALITY_P (to_type) && !in_lto_p &&
TYPE_STRUCTURAL_EQUALITY_P (t)) TYPE_CANONICAL (t) =
build_pointer_type_for_mode (TYPE_CANONICAL (to_type), mode, false); return t;
}
or so, and do something similar also for array types and the like.
And maybe instead of tweaking asserts in ipa-free-lang-data also update
TYPE_CANONICAL on the derived types if the base type doesn't have
TYPE_STRUCTURAL_EQUALITY_P.

[Bug lto/114574] [14 regression] ICE when building curl with LTO (fld_incomplete_type_of, at ipa-free-lang-data.cc:257) since r14-9763

[muecker at gwdg dot de]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=114574

--- Comment #18 from Martin Uecker <muecker at gwdg dot de> ---

> 
> +	  TYPE_CANONICAL (x) = TYPE_CANONICAL (t);
> As has been discussed, this is wrong, it should have been
> TYPE_CANONICAL (x) = build_qualified_type (TYPE_CANONICAL (t), TYPE_QUALS
> (x));
> or so.

Thanks

> 
> The build_pointer_type change looks really weird.
> I'd say if you want to be prepared for to_type formerly
> TYPE_STRUCTURAL_EQUALITY_P types and now no longer so, we should just change
>   for (t = TYPE_POINTER_TO (to_type); t; t = TYPE_NEXT_PTR_TO (t))
>     if (TYPE_MODE (t) == mode && TYPE_REF_CAN_ALIAS_ALL (t) == can_alias_all)
>       return t;
> loop to check for this case before return t; (i.e. { if
> (!TYPE_STRUCTURAL_EQUALITY_P (to_type) && !in_lto_p &&
> TYPE_STRUCTURAL_EQUALITY_P (t)) TYPE_CANONICAL (t) =
> build_pointer_type_for_mode (TYPE_CANONICAL (to_type), mode, false); return
> t; }
> or so, and do something similar also for array types and the like.
> And maybe instead of tweaking asserts in ipa-free-lang-data also update
> TYPE_CANONICAL on the derived types if the base type doesn't have
> TYPE_STRUCTURAL_EQUALITY_P.

I am just looking at a version that would have changed the condition to:

  if (TYPE_MODE (t) == mode && TYPE_REF_CAN_ALIAS_ALL (t) == can_alias_all
     && TYPE_STRUCTURAL_EQUALITY_P (t) == TYPE_STRUCTURAL_EQUALITY_P (to_type))

Martin

[Bug lto/114574] [14 regression] ICE when building curl with LTO (fld_incomplete_type_of, at ipa-free-lang-data.cc:257) since r14-9763

[jakub at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=114574

--- Comment #19 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
(In reply to Martin Uecker from comment #18)
> I am just looking at a version that would have changed the condition to:
> 
>   if (TYPE_MODE (t) == mode && TYPE_REF_CAN_ALIAS_ALL (t) == can_alias_all
>      && TYPE_STRUCTURAL_EQUALITY_P (t) == TYPE_STRUCTURAL_EQUALITY_P
> (to_type))

That would just keep perhaps lots of pointer types duplicated and no longer
usable (so why keep them in the cache then?).
I mean, if it is fine to update the base type from TYPE_STRUCTURAL_EQUALITY_P
to some
TYPE_CANONICAL, why it wouldn't be ok to update the pointers as well?
Also, for in_lto_p pointer types are always TYPE_STRUCTURAL_EQUALITY_P even
when
to_type is not.

[Bug lto/114574] [14 regression] ICE when building curl with LTO (fld_incomplete_type_of, at ipa-free-lang-data.cc:257) since r14-9763

[muecker at gwdg dot de]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=114574

--- Comment #20 from Martin Uecker <muecker at gwdg dot de> ---
(In reply to Martin Uecker from comment #18)
> > 
> > +	  TYPE_CANONICAL (x) = TYPE_CANONICAL (t);
> > As has been discussed, this is wrong, it should have been
> > TYPE_CANONICAL (x) = build_qualified_type (TYPE_CANONICAL (t), TYPE_QUALS
> > (x));
> > or so.
> 
> Thanks

Actually, I think we need to also add the lookup in the hash table to
c_build_qualified_type to find the right TYPE_CANONICAL.

[Bug lto/114574] [14 regression] ICE when building curl with LTO (fld_incomplete_type_of, at ipa-free-lang-data.cc:257) since r14-9763

[jakub at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=114574

--- Comment #21 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Why?  That is already set by
        TYPE_CANONICAL (t) = *e;
      else
        {
          TYPE_CANONICAL (t) = t;

[Bug lto/114574] [14 regression] ICE when building curl with LTO (fld_incomplete_type_of, at ipa-free-lang-data.cc:257) since r14-9763

[jakub at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=114574

--- Comment #22 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
BTW, wonder if it wouldn't be desirable to revert the PR114361 patch until this
is sorted out, or at least limit the effects of that patch to flag_isoc23 and
using multiple types with the same tag and same content.
Like guard
      TYPE_CANONICAL (x) = TYPE_CANONICAL (t);
with if (flag_isoc23 && TYPE_CANONICAL (t) != t)
Because otherwise the trunk is just completely broken for all of LTO with C of
any version.

[Bug lto/114574] [14 regression] ICE when building curl with LTO (fld_incomplete_type_of, at ipa-free-lang-data.cc:257) since r14-9763

[cvs-commit at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=114574

--- Comment #23 from GCC Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Martin Uecker <uecker@gcc.gnu.org>:

https://gcc.gnu.org/g:8057f9aa1f7e70490064de796d7a8d42d446caf8

commit r14-9805-g8057f9aa1f7e70490064de796d7a8d42d446caf8
Author: Martin Uecker <uecker@tugraz.at>
Date:   Fri Apr 5 12:14:56 2024 +0200

    Revert "Fix ICE with -g and -std=c23 related to incomplete types
[PR114361]"

    This reverts commit 871bb5ad2dd56343d80b6a6d269e85efdc9999e5  because it
    breaks LTO and needs a bit more work. See PR 114574.

[Bug lto/114574] [14 regression] ICE when building curl with LTO (fld_incomplete_type_of, at ipa-free-lang-data.cc:257) since r14-9763

[xry111 at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=114574

Xi Ruoyao <xry111 at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |xry111 at gcc dot gnu.org
           Priority|P1                          |P3

--- Comment #24 from Xi Ruoyao <xry111 at gcc dot gnu.org> ---
So no longer a P1.

[Bug lto/114574] [14 regression] ICE when building curl with LTO (fld_incomplete_type_of, at ipa-free-lang-data.cc:257) since r14-9763

[law at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=114574

Jeffrey A. Law <law at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |law at gcc dot gnu.org
           Priority|P3                          |P2

[Bug lto/114574] [14 regression] ICE when building curl with LTO (fld_incomplete_type_of, at ipa-free-lang-data.cc:257) since r14-9763

[jakub at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=114574

--- Comment #25 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Created attachment 57935
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=57935&action=edit
gcc14-pr114574.patch

What about this patch then?  So far just make check-gcc -j32 checked (though in
a version that did it for !flag_isoc23 as well).
I've talked to Richi on IRC and he prefers not to tweak build_qualified_type,
build_pointer_type_for_mode, build_array_type_1, ... etc. to compute
TYPE_CANONICAL if the base type changed from structural identity to non-NULL
TYPE_CANONICAL, so the patch just updates the most common things (qualified
types and pointers, recursively) and will leave function types, array types and
other derived types created before the type is completed with structural
identity forever.

[Bug lto/114574] [14 regression] ICE when building curl with LTO (fld_incomplete_type_of, at ipa-free-lang-data.cc:257) since r14-9763

[uecker at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=114574

--- Comment #26 from uecker at gcc dot gnu.org ---

Note that not updating the types seems wrong also pre C23. PR114493 could be an
example of this:

typedef struct a a;
int c;
int f(a **);
struct __attribute__((__may_alias__)) a {};

[Bug lto/114574] [14 regression] ICE when building curl with LTO (fld_incomplete_type_of, at ipa-free-lang-data.cc:257) since r14-9763

[jakub at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=114574

--- Comment #27 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
(In reply to uecker from comment #26)
> Note that not updating the types seems wrong also pre C23. PR114493 could be
> an example of this:
> 
> typedef struct a a;
> int c;
> int f(a **);
> struct __attribute__((__may_alias__)) a {};

That should be the same type, not different, no?

[Bug lto/114574] [14 regression] ICE when building curl with LTO (fld_incomplete_type_of, at ipa-free-lang-data.cc:257) since r14-9763

[uecker at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=114574

--- Comment #28 from uecker at gcc dot gnu.org ---

I do not fully understand yet what happens for may_alias, but it if we later
complete the struct with the may_alias attribute it seems we would also need to
update the previously created pointer so that it has TYPE_REF_CAN_ALIAS_ALL. 
Setting TYPE_STRUCTURAL_EQUALITY for incomplete structs also for pre-C23 seems
to fix the assertion failure, but I am not sure if this fixes the underlying
bug.

[Bug lto/114574] [14 regression] ICE when building curl with LTO (fld_incomplete_type_of, at ipa-free-lang-data.cc:257) since r14-9763

[jakub at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=114574

--- Comment #29 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
(In reply to uecker from comment #28)
> I do not fully understand yet what happens for may_alias, but it if we later
> complete the struct with the may_alias attribute it seems we would also need
> to update the previously created pointer so that it has
> TYPE_REF_CAN_ALIAS_ALL.  Setting TYPE_STRUCTURAL_EQUALITY for incomplete
> structs also for pre-C23 seems to fix the assertion failure, but I am not
> sure if this fixes the underlying bug.

Certainly that may_alias case isn't specific to just C, C++ behaves the same,
and I'd just say don't do that, you can always put the may_alias attribute on
the forward declaration of the struct if all pointers to it are supposed to
alias.

[Bug lto/114574] [14 regression] ICE when building curl with LTO (fld_incomplete_type_of, at ipa-free-lang-data.cc:257) since r14-9763

[muecker at gwdg dot de]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=114574

--- Comment #30 from Martin Uecker <muecker at gwdg dot de> ---
Am Donnerstag, dem 18.04.2024 um 11:57 +0000 schrieb jakub at gcc dot gnu.org:
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=114574
> 
> --- Comment #29 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
> (In reply to uecker from comment #28)
> > I do not fully understand yet what happens for may_alias, but it if we later
> > complete the struct with the may_alias attribute it seems we would also need
> > to update the previously created pointer so that it has
> > TYPE_REF_CAN_ALIAS_ALL.  Setting TYPE_STRUCTURAL_EQUALITY for incomplete
> > structs also for pre-C23 seems to fix the assertion failure, but I am not
> > sure if this fixes the underlying bug.
> 
> Certainly that may_alias case isn't specific to just C, C++ behaves the same,
> and I'd just say don't do that, you can always put the may_alias attribute on
> the forward declaration of the struct if all pointers to it are supposed to
> alias.

The problem I was referring to is the ICE, not that we get from
assertions in the middle end, not that some pointers constructed
from the incomplete type without the attribute may not have this 
property, cf. PR114493

For C++, the example does not ICE. I haven't looked at
what the C++ FE does differently, I simply observed that setting
TYPE_STRUCTURAL_EQUALITY as the patch does for C23 also seems to
fix (?) this ICE (but only for C23 if guarded by flag_iso_c23).

I haven't had time to look more into this. 

Martin

[Bug lto/114574] [14 regression] ICE when building curl with LTO (fld_incomplete_type_of, at ipa-free-lang-data.cc:257) since r14-9763

[jakub at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=114574

--- Comment #31 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
I've tried to construct a wrong-code testcase, but so far haven't been
successful.
In e.g.
struct S { int s; } s;
struct T { long l; } t;

const struct S *
foo (const struct S **p, const struct T **q)
{
  *p = &s;
  *q = &t;
  return *p;
}
fre1 optimizes the return *p to return &s;
So I've tried
[[gnu::always_inline]] static inline const void *
foo (void *x, bool y)
{
  const struct S **p = (const struct S **) x;
  struct S { int s; };
  static struct S s;
  if (y)
    *p = &s;
  return *p;
}

[[gnu::always_inline]] static inline void
bar (void *x)
{
  const struct S **q = (const struct S **) x;
  struct S { int s; };
  static struct S s;
  *q = &s;
}

const void *
baz (void *x, void *y)
{
  foo (x, true);
  bar (y);
  return foo (x, false);
}
where without the posted patch I think the TYPE_CANONICAL on const struct S*
should be wrong in the bar function, but strangely we don't optimize it.

[Bug lto/114574] [14 regression] ICE when building curl with LTO (fld_incomplete_type_of, at ipa-free-lang-data.cc:257) since r14-9763

[jakub at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=114574

--- Comment #32 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Ah, but get_alias_set has quite special handling of pointers, it finds the
ultimately pointed type and uses TYPE_MAIN_VARIANT for it, so it actually
ignores TYPE_CANONICAL of the pointer type itself for TYPE_ALIAS_SET decisions.

[Bug lto/114574] [14 regression] ICE when building curl with LTO (fld_incomplete_type_of, at ipa-free-lang-data.cc:257) since r14-9763

[pinskia at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=114574

--- Comment #33 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
(In reply to Jakub Jelinek from comment #32)
> Ah, but get_alias_set has quite special handling of pointers, it finds the
> ultimately pointed type and uses TYPE_MAIN_VARIANT for it, so it actually
> ignores TYPE_CANONICAL of the pointer type itself for TYPE_ALIAS_SET
> decisions.

Yes because you need to ignore all the qualifiers on the pointed to types.
Otherwise you end up with `const int*const*` and `int**` have not the same
aliasing set.

[Bug lto/114574] [14 regression] ICE when building curl with LTO (fld_incomplete_type_of, at ipa-free-lang-data.cc:257) since r14-9763

[cvs-commit at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=114574

--- Comment #34 from GCC Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Jakub Jelinek <jakub@gcc.gnu.org>:

https://gcc.gnu.org/g:a39983bf58d3097c472252f6989d19b60909dd9a

commit r14-10045-ga39983bf58d3097c472252f6989d19b60909dd9a
Author: Jakub Jelinek <jakub@redhat.com>
Date:   Sat Apr 20 00:05:21 2024 +0200

    c: 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 for flag_isoc23
TYPE_STRUCTURAL_EQUALITY_P
    for incomplete structure and union types and then update TYPE_CANONICAL
    later, though update it only for the variants and derived pointer types
    which can be easily discovered.  Other derived types created while
    the type was still incomplete will remain TYPE_STRUCTURAL_EQUALITY_P.
    See PR114574 for discussion.

    2024-04-20  Martin Uecker  <uecker@tugraz.at>
                Jakub Jelinek  <jakub@redhat.com>

            PR lto/114574
            PR c/114361
    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.

[Bug lto/114574] [14 regression] ICE when building curl with LTO (fld_incomplete_type_of, at ipa-free-lang-data.cc:257) since r14-9763

[jakub at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=114574

Jakub Jelinek <jakub at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
         Resolution|---                         |FIXED
             Status|NEW                         |RESOLVED

--- Comment #35 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Fixed.