Bug 109556 - [13/14 Regression] internal compiler error: in iterative_hash_template_arg, at cp/pt.cc:1927
Summary: [13/14 Regression] internal compiler error: in iterative_hash_template_arg, a...
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: c++ (show other bugs)
Version: 13.0
: P3 normal
Target Milestone: 13.0
Assignee: Patrick Palka
URL:
Keywords: GC, ice-on-valid-code
Depends on:
Blocks:
 
Reported: 2023-04-19 12:50 UTC by Fabian Sauter
Modified: 2023-04-19 17:25 UTC (History)
4 users (show)

See Also:
Host:
Target:
Build:
Known to work:
Known to fail:
Last reconfirmed: 2023-04-19 00:00:00


Attachments
Preprocessed Source (256.32 KB, application/x-xz)
2023-04-19 13:00 UTC, Fabian Sauter
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Fabian Sauter 2023-04-19 12:50:28 UTC
I'm using gcc 13.0.1-0 on Fedora 38.
I hope this is the correct place to report this bug since in the log it states I should report it at RedHats bug tracker but I was not able to find a gcc section there.

Compiling mpunits (https://github.com/mpusz/units) fails with gcc13. With gcc12 it still works.

I'm still failing to reduce the problem to a single failing source header file.

Let me know if you need anything else.


## How to Reproduce:
git clone https://github.com/mpusz/units.git
cd units && mkdir build && cd build
cmake ..
cmake --build .

## Related Issues
https://github.com/mpusz/units/issues/450

## Output:
```
[ 30%] Building CXX object CMakeFiles/test_headers.dir/headers/src/systems/si/include/units/isq/si/absorbed_dose.cpp.o
In file included from /home/fabian/Repos/units/src/systems/si/include/units/isq/si/force.h:33,
                 from /home/fabian/Repos/units/src/systems/si/include/units/isq/si/energy.h:32,
                 from /home/fabian/Repos/units/./src/systems/si/include/units/isq/si/absorbed_dose.h:32,
                 from /home/fabian/Repos/units/build/headers/src/systems/si/include/units/isq/si/absorbed_dose.cpp:1:
/home/fabian/Repos/units/src/systems/si/include/units/isq/si/mass.h: In substitution of ‘template<class D, class U, class Rep>  requires (Dimension<D>) && (UnitOf<U, D>) && (Representation<Rep>) class units::quantity [with D = units::isq::si::dim_mass; U = units::isq::si::gigagram; Rep = long int]’:
/home/fabian/Repos/units/src/systems/si/include/units/isq/si/mass.h:88:7:   required by substitution of ‘template<class U, class Rep>  requires (UnitOf<U, units::isq::si::dim_mass>) && (Representation<Rep>) using units::isq::si::mass = units::quantity<units::isq::si::dim_mass, U, Rep> [with U = units::isq::si::gigagram; Rep = long int]’
/home/fabian/Repos/units/src/systems/si/include/units/isq/si/mass.h:218:37:   required from here
/home/fabian/Repos/units/src/systems/si/include/units/isq/si/mass.h:88:7: internal compiler error: in iterative_hash_template_arg, at cp/pt.cc:1927
   88 | using mass = quantity<dim_mass, U, Rep>;
      |       ^~~~
Please submit a full bug report, with preprocessed source.
See <http://bugzilla.redhat.com/bugzilla> for instructions.
Preprocessed source stored into /tmp/ccG8HXcN.out file, please attach this to your bugreport.
gmake[2]: *** [CMakeFiles/test_headers.dir/build.make:1182: CMakeFiles/test_headers.dir/headers/src/systems/si/include/units/isq/si/absorbed_dose.cpp.o] Error 1
gmake[1]: *** [CMakeFiles/Makefile2:575: CMakeFiles/test_headers.dir/all] Error 2
gmake: *** [Makefile:136: all] Error 2
```
Comment 1 Jakub Jelinek 2023-04-19 12:58:19 UTC
Preprocessed source stored into /tmp/ccG8HXcN.out file, please attach this to your bugreport.

^^^
Comment 2 Fabian Sauter 2023-04-19 13:00:56 UTC
Created attachment 54884 [details]
Preprocessed Source

Ah, it failed to upload with: "The file you are trying to attach is 2505 kilobytes (KB) in size. Attachments cannot be more than 1000 KB."

I compressed it!
Comment 3 Jakub Jelinek 2023-04-19 13:47:51 UTC
Reproduced, reducing now.
Comment 4 Patrick Palka 2023-04-19 14:12:07 UTC
It looks like the satisfaction cache is holding on to a TREE_VEC that's been ggc_free'd.  It got freed from

pt.cc:try_class_unification#L23897:

  if (TMPL_ARGS_HAVE_MULTIPLE_LEVELS (targs))
    for (tree level : tree_vec_range (targs))
      ggc_free (level);
  ggc_free (targs);

Removing these calls to ggc_free fixes the ICE, thus I'm pretty sure this started with r13-377-g3e948d645bc908.  In hindsight freeing this copy of 'targs' isn't generally safe because the call to unify in try_class_unification could perform satisfaction which in turn would capture parts of targs into the satisfaction cache.
Comment 5 Patrick Palka 2023-04-19 16:03:24 UTC
template<typename T, auto N>
concept C = (N != 0);

template<auto N, auto M>
struct A { };

template<auto N, C<N> auto M>
void f(A<N, M>);

int main() {
  f(A<1, 42>{});
  f(A<2, 42>{});
}

Bisection seems unreliable probably due to the use-after-free nature of the bug, but removing those calls to ggc_free fixes the issue so r13-377-g3e948d645bc908 is almost certainly the culprit.
Comment 6 Jakub Jelinek 2023-04-19 16:34:31 UTC
Indeed, the reduction got stuck at around 1.4M size and didn't reduce further until I've killed it.
Comment 7 GCC Commits 2023-04-19 17:08:30 UTC
The master branch has been updated by Patrick Palka <ppalka@gcc.gnu.org>:

https://gcc.gnu.org/g:5e284ebbc3082c5a8974d24e3a0977aa48f3cc60

commit r14-91-g5e284ebbc3082c5a8974d24e3a0977aa48f3cc60
Author: Patrick Palka <ppalka@redhat.com>
Date:   Wed Apr 19 13:07:46 2023 -0400

    c++: bad ggc_free in try_class_unification [PR109556]
    
    Aside from correcting how try_class_unification copies multi-dimensional
    'targs', r13-377-g3e948d645bc908 also made it ggc_free this copy as an
    optimization.  But this is wrong since the call to unify within might've
    captured the args in persistent memory such as the satisfaction cache
    (as part of constrained auto deduction).
    
            PR c++/109556
    
    gcc/cp/ChangeLog:
    
            * pt.cc (try_class_unification): Don't ggc_free the copy of
            'targs'.
    
    gcc/testsuite/ChangeLog:
    
            * g++.dg/cpp2a/concepts-placeholder13.C: New test.
Comment 8 GCC Commits 2023-04-19 17:14:54 UTC
The releases/gcc-13 branch has been updated by Patrick Palka <ppalka@gcc.gnu.org>:

https://gcc.gnu.org/g:90361bc6f4ffffedb444e86380b6d1e08475fa74

commit r13-7227-g90361bc6f4ffffedb444e86380b6d1e08475fa74
Author: Patrick Palka <ppalka@redhat.com>
Date:   Wed Apr 19 13:07:46 2023 -0400

    c++: bad ggc_free in try_class_unification [PR109556]
    
    Aside from correcting how try_class_unification copies multi-dimensional
    'targs', r13-377-g3e948d645bc908 also made it ggc_free this copy as an
    optimization.  But this is wrong since the call to unify within might've
    captured the args in persistent memory such as the satisfaction cache
    (as part of constrained auto deduction).
    
            PR c++/109556
    
    gcc/cp/ChangeLog:
    
            * pt.cc (try_class_unification): Don't ggc_free the copy of
            'targs'.
    
    gcc/testsuite/ChangeLog:
    
            * g++.dg/cpp2a/concepts-placeholder13.C: New test.
    
    (cherry picked from commit 5e284ebbc3082c5a8974d24e3a0977aa48f3cc60)
Comment 9 Patrick Palka 2023-04-19 17:25:18 UTC
Fixed for GCC 13.1, thanks for the bug report.

FWIW a potential workaround for this bug, which seems to work for me at least, is to remove the Magnitude constraint from to_prefix_base and to_base_scaled_unit.