Bug 105297 - [12 Regression] new modules 'xtreme' test cases FAILs
Summary: [12 Regression] new modules 'xtreme' test cases FAILs
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: c++ (show other bugs)
Version: 12.0
: P3 normal
Target Milestone: 12.0
Assignee: Patrick Palka
URL:
Keywords: testsuite-fail
Depends on:
Blocks: c++-modules
  Show dependency treegraph
 
Reported: 2022-04-16 20:21 UTC by Iain Sandoe
Modified: 2022-11-28 20:17 UTC (History)
8 users (show)

See Also:
Host:
Target: *-darwin*, x86_64-linux-gnu
Build:
Known to work:
Known to fail:
Last reconfirmed: 2022-04-20 00:00:00


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Iain Sandoe 2022-04-16 20:21:12 UTC
between r12-8146 and r12-8179. for each std= tested

g++.dg/modules/xtreme-header-4_a.H -std=c++* (internal compiler error: in insert, at cp/module.cc:4800)

likewise
g++.dg/modules/xtreme-header_a.H
Comment 1 Jakub Jelinek 2022-04-20 13:53:58 UTC
I think it is some libstdc++-v3 header change that triggered it lately, but don't have setup where I can easily bisect that.
When I try to bisect with preprocessed xtreme-header-4_a.ii
and
-quiet xtreme-header-4_a.ii -quiet -Wno-long-long -std=c++17 -fmodule-header -o xtreme-header-4_a.s
options to cc1plus, the ICE started with
r11-6083-gb7dfc2074c78415d451eb34d1608016c80b1c41a
Comment 2 Jakub Jelinek 2022-04-20 13:59:00 UTC
Based on the ICE details (on __data member of anonymous struct inside of
__from_chars_alnum_to_val_table) I think this started with:
r12-8175-ga54137c88061c7495728fc6b8dfd0474e812b2cb
Comment 3 Jakub Jelinek 2022-04-20 14:23:47 UTC
Small testcase:
pr105297.h:

constexpr auto
foo ()
{
  struct S { unsigned char d[1u << __CHAR_BIT__] = {}; } t;
  return t;
}

template <int N>
unsigned char
bar (unsigned char x)
{
  static constexpr auto t = foo ();
  return t.d[x];
}

./cc1plus -o pr105297.{s,h} -fmodule-header
Comment 4 Jakub Jelinek 2022-04-20 14:29:06 UTC
If we want a quick work-around, I think:
--- libstdc++-v3/include/std/charconv.jj	2022-04-19 07:20:56.772166410 +0200
+++ libstdc++-v3/include/std/charconv	2022-04-20 16:27:47.971314921 +0200
@@ -407,6 +407,10 @@ namespace __detail
       return true;
     }
 
+  struct __from_chars_alnum_to_val_table_type {
+    unsigned char __data[1u << __CHAR_BIT__] = {};
+  };
+
   // Construct and return a lookup table that maps 0-9, A-Z and a-z to their
   // corresponding base-36 value and maps all other characters to 127.
   constexpr auto
@@ -420,7 +424,7 @@ namespace __detail
       = { 'A', 'B', 'C', 'D', 'E', 'F', 'G', 'H', 'I', 'J',
 	  'K', 'L', 'M', 'N', 'O', 'P', 'Q', 'R', 'S', 'T',
 	  'U', 'V', 'W', 'X', 'Y', 'Z' };
-    struct { unsigned char __data[1u << __CHAR_BIT__] = {}; } __table;
+    struct __from_chars_alnum_to_val_table_type __table;
     for (auto& __entry : __table.__data)
       __entry = 127;
     for (int __i = 0; __i < 10; ++__i)
or so would do it.
Comment 5 Hans-Peter Nilsson 2022-04-20 14:31:28 UTC
Consistent with Jakub's comment#2, also appearing for cris-elf (between r12-8173-ge580f81d22d611 and r12-8175-ga54137c88061c7), and apparently all other targets judging from today's gcc-testresults posts.
Comment 6 Jonathan Wakely 2022-04-20 16:01:45 UTC
Patrick, I suggest we go with Jakub's suggestion in comment 4, but PTAL.
Comment 7 Patrick Palka 2022-04-20 16:14:25 UTC
(In reply to Jonathan Wakely from comment #6)
> Patrick, I suggest we go with Jakub's suggestion in comment 4, but PTAL.

LGTM, it seems to be the simplest workaround.  I also tried replacing the NSDMI but that seems to prevent the ICE only for the reduced testcase in comment #3.
Comment 8 Patrick Palka 2022-04-20 16:18:02 UTC
I shall test/push that fix (unless Jakub beats me to it :))
Comment 9 Jakub Jelinek 2022-04-20 17:17:26 UTC
(In reply to Patrick Palka from comment #7)
> (In reply to Jonathan Wakely from comment #6)
> > Patrick, I suggest we go with Jakub's suggestion in comment 4, but PTAL.
> 
> LGTM, it seems to be the simplest workaround.  I also tried replacing the
> NSDMI but that seems to prevent the ICE only for the reduced testcase in
> comment #3.

Note, the original reduced testcase didn't have the " S" part in there,
i.e. used anonymous struct.  Is that what matters for the NSDMI?
I
Comment 10 Patrick Palka 2022-04-20 17:59:18 UTC
(In reply to Jakub Jelinek from comment #9)
> (In reply to Patrick Palka from comment #7)
> > (In reply to Jonathan Wakely from comment #6)
> > > Patrick, I suggest we go with Jakub's suggestion in comment 4, but PTAL.
> > 
> > LGTM, it seems to be the simplest workaround.  I also tried replacing the
> > NSDMI but that seems to prevent the ICE only for the reduced testcase in
> > comment #3.
> 
> Note, the original reduced testcase didn't have the " S" part in there,
> i.e. used anonymous struct.  Is that what matters for the NSDMI?

Interestingly that doesn't seem to make a difference.  What seems to matter is whether the constexpr function modifies the CONSTRUCTOR that it returns:

constexpr auto foo() {
  struct S { int d; } t = {};
  t.d = 0; // doesn't ICE if this line is commented out
  return t;
}

template<int>
int bar() {
  constexpr auto t = foo();
  return 0;
}
Comment 11 Jiu Fu Guo 2022-04-21 09:25:27 UTC
(In reply to Patrick Palka from comment #10)
> 
> Interestingly that doesn't seem to make a difference.  What seems to matter
> is whether the constexpr function modifies the CONSTRUCTOR that it returns:
> 
> constexpr auto foo() {
>   struct S { int d; } t = {};
>   t.d = 0; // doesn't ICE if this line is commented out
>   return t;
> }
> 
> template<int>
> int bar() {
>   constexpr auto t = foo();
>   return 0;
> }

Right, it is weird. Some PRs on Xtreme-* failure (including ICE) were also reported before. e.g. PR100052, PR101853, PR99910.  As commented in those PRs, these may be random failures, and changes in headers that could expose the ICE.
I'm also wondering if this may be an issue hidden inside somewhere (GC?).
Comment 12 GCC Commits 2022-04-21 12:35:54 UTC
The master branch has been updated by Patrick Palka <ppalka@gcc.gnu.org>:

https://gcc.gnu.org/g:1e6c0e69af8da436e1d1d2d23d8c38410d78ecf2

commit r12-8214-g1e6c0e69af8da436e1d1d2d23d8c38410d78ecf2
Author: Patrick Palka <ppalka@redhat.com>
Date:   Thu Apr 21 08:34:59 2022 -0400

    libstdc++: Work around modules ICE in <charconv> [PR105297]
    
    This makes the initializer for __table in __from_chars_alnum_to_val
    dependent in an artificial way, which works around the reported modules
    testsuite ICE by preventing the compiler from evaluating the initializer
    parse time.
    
    Compared to the alternative workaround of using a non-local class type
    for __table, this workaround has the advantage of slightly speeding up
    compilation of <charconv>, since now the table won't get built (via
    constexpr evaluation) until the integer std::from_chars overload is
    instantiated.
    
            PR c++/105297
            PR c++/105322
    
    libstdc++-v3/ChangeLog:
    
            * include/std/charconv (__from_chars_alnum_to_val): Make
            initializer for __table dependent in an artificial way.
Comment 13 Patrick Palka 2022-04-21 13:13:46 UTC
(In reply to Jiu Fu Guo from comment #11)
> (In reply to Patrick Palka from comment #10)
> > 
> > Interestingly that doesn't seem to make a difference.  What seems to matter
> > is whether the constexpr function modifies the CONSTRUCTOR that it returns:
> > 
> > constexpr auto foo() {
> >   struct S { int d; } t = {};
> >   t.d = 0; // doesn't ICE if this line is commented out
> >   return t;
> > }
> > 
> > template<int>
> > int bar() {
> >   constexpr auto t = foo();
> >   return 0;
> > }
> 
> Right, it is weird. Some PRs on Xtreme-* failure (including ICE) were also
> reported before. e.g. PR100052, PR101853, PR99910.  As commented in those
> PRs, these may be random failures, and changes in headers that could expose
> the ICE.
> I'm also wondering if this may be an issue hidden inside somewhere (GC?).

In this case I suspect it's just a bug in the modules code, I opened PR105322 to track it.
Comment 14 Patrick Palka 2022-04-21 13:14:20 UTC
Fixed, hopefully.
Comment 15 Jiu Fu Guo 2022-04-22 05:53:49 UTC
(In reply to Patrick Palka from comment #13)
> (In reply to Jiu Fu Guo from comment #11)
> > (In reply to Patrick Palka from comment #10)
> > > 
> > > Interestingly that doesn't seem to make a difference.  What seems to matter
> > > is whether the constexpr function modifies the CONSTRUCTOR that it returns:
> > > 
> > > constexpr auto foo() {
> > >   struct S { int d; } t = {};
> > >   t.d = 0; // doesn't ICE if this line is commented out
> > >   return t;
> > > }
> > > 
> > > template<int>
> > > int bar() {
> > >   constexpr auto t = foo();
> > >   return 0;
> > > }
> > 
> > Right, it is weird. Some PRs on Xtreme-* failure (including ICE) were also
> > reported before. e.g. PR100052, PR101853, PR99910.  As commented in those
> > PRs, these may be random failures, and changes in headers that could expose
> > the ICE.
> > I'm also wondering if this may be an issue hidden inside somewhere (GC?).
> 
> In this case I suspect it's just a bug in the modules code, I opened
> PR105322 to track it.

Oh, thanks!  This failure seems only about the module code on 'struct member cross functions'.