On Wed, Apr 19, 2023 at 12:52:50PM -0400, Jason Merrill wrote:
> On 4/19/23 12:05, Patrick Palka wrote:
> > On Wed, 19 Apr 2023, Patrick Palka wrote:
> >
> > > 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 potentially wrong since the call to unify
> > > within might've captured the args in persistent memory such as the
> > > satisfaction cache (during constrained auto deduction).
> > >
> > > Bootstrapped and regtested on x86_64-pc-linux, does this look OK for
> > > trunk/13?
>
> OK.
>
> > > No testcase yet since the reduction is still in progress.
> > > The plan would be to push this with a reduced testcase, but I figured
> > > I'd send the actual fix for review now. Would this be OK for 13.1 or
> > > shall it wait until 13.2?
>
> Jakub's call, but this regression seems like a blocker to me.
Not doing ggc_free shouldn't really break stuff except increase memory
consumption, so I think this is ok for 13.1.
> > Now with a reduced testcase:
> >
> > -- >8 --
> >
> > Subject: [PATCH] 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 potentially wrong since the call to unify
> > within might've captured the args in persistent memory such as the
> > satisfaction cache (during constrained auto deduction).
> >
> > 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.
> > ---
> > gcc/cp/pt.cc | 5 -----
> > .../g++.dg/cpp2a/concepts-placeholder13.C | 16 ++++++++++++++++
> > 2 files changed, 16 insertions(+), 5 deletions(-)
> > create mode 100644 gcc/testsuite/g++.dg/cpp2a/concepts-placeholder13.C
> >
> > diff --git a/gcc/cp/pt.cc b/gcc/cp/pt.cc
> > index e065ace5c55..68a056acf8b 100644
> > --- a/gcc/cp/pt.cc
> > +++ b/gcc/cp/pt.cc
> > @@ -23895,11 +23895,6 @@ try_class_unification (tree tparms, tree targs,
> > tree parm, tree arg,
> > err = unify (tparms, targs, CLASSTYPE_TI_ARGS (parm),
> > CLASSTYPE_TI_ARGS (arg), UNIFY_ALLOW_NONE, explain_p);
> > - if (TMPL_ARGS_HAVE_MULTIPLE_LEVELS (targs))
> > - for (tree level : tree_vec_range (targs))
> > - ggc_free (level);
> > - ggc_free (targs);
> > -
> > return err ? NULL_TREE : arg;
> > }
> > diff --git a/gcc/testsuite/g++.dg/cpp2a/concepts-placeholder13.C
> > b/gcc/testsuite/g++.dg/cpp2a/concepts-placeholder13.C
> > new file mode 100644
> > index 00000000000..fd4a05c05e1
> > --- /dev/null
> > +++ b/gcc/testsuite/g++.dg/cpp2a/concepts-placeholder13.C
> > @@ -0,0 +1,16 @@
> > +// PR c++/109556
> > +// { dg-do compile { target c++20 } }
> > +
> > +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>{});
> > +}
Jakub
