On Tue, Dec 17, 2024 at 10:36:09AM -0500, Jason Merrill wrote:
> On 12/16/24 6:48 PM, Marek Polacek wrote:
> > Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk/branches?
> > 
> > -- >8 --
> > This crash started with my r12-7803 but I believe the problem lies
> > elsewhere.
> > 
> > build_vec_init has cleanup_flags whose purpose is -- if I grok this
> > correctly -- to avoid destructing an object multiple times.  Let's
> > say we are initializing an array of A.  Then we might end up in
> > a scenario similar to initlist-eh1.C:
> > 
> >    try
> >      {
> >        call A::A in a loop
> >        // #0
> >        try
> >          {
> >       call a fn using the array
> >     }
> >        finally
> >     {
> >       // #1
> >       call A::~A in a loop
> >     }
> >      }
> >    catch
> >      {
> >        // #2
> >        call A::~A in a loop
> >      }
> > 
> > cleanup_flags makes us emit a statement like
> > 
> >    D.3048 = 2;
> > 
> > at #0 to disable performing the cleanup at #2, since #1 will take
> > care of the destruction of the array.
> > 
> > But if we are not emitting the loop because we can use a constant
> > initializer (and use a single { a, b, ...}), we shouldn't generate
> > the statement resetting the iterator to its initial value.  Otherwise
> > we crash in gimplify_var_or_parm_decl because it gets the stray decl
> > D.3048.
> > 
> >     PR c++/117985
> > 
> > gcc/cp/ChangeLog:
> > 
> >     * init.cc (build_vec_init): Clear CLEANUP_FLAGS if we're not
> >     generating the loop.
> 
> truncate seems dangerous, there might be other things in cleanup_flags that
> we want to leave alone.  I think it would be safer to assert that the last
> element in the vec is iterator, and then pop it.

Yeah, that does sound much safer.  Like so?

dg.exp passed.

-- >8 --
This crash started with my r12-7803 but I believe the problem lies
elsewhere.

build_vec_init has cleanup_flags whose purpose is -- if I grok this
correctly -- to avoid destructing an object multiple times.  Let's
say we are initializing an array of A.  Then we might end up in
a scenario similar to initlist-eh1.C:

  try
    {
      call A::A in a loop
      // #0
      try
        {
          call a fn using the array
        }
      finally
        {
          // #1
          call A::~A in a loop
        }
    }
  catch
    {
      // #2
      call A::~A in a loop
    }

cleanup_flags makes us emit a statement like

  D.3048 = 2;

at #0 to disable performing the cleanup at #2, since #1 will take
care of the destruction of the array.

But if we are not emitting the loop because we can use a constant
initializer (and use a single { a, b, ...}), we shouldn't generate
the statement resetting the iterator to its initial value.  Otherwise
we crash in gimplify_var_or_parm_decl because it gets the stray decl
D.3048.

        PR c++/117985

gcc/cp/ChangeLog:

        * init.cc (build_vec_init): Pop CLEANUP_FLAGS if we're not
        generating the loop.

gcc/testsuite/ChangeLog:

        * g++.dg/cpp0x/initlist-array23.C: New test.
        * g++.dg/cpp0x/initlist-array24.C: New test.
---
 gcc/cp/init.cc                                |  9 ++++++
 gcc/testsuite/g++.dg/cpp0x/initlist-array23.C | 28 +++++++++++++++++++
 gcc/testsuite/g++.dg/cpp0x/initlist-array24.C | 27 ++++++++++++++++++
 3 files changed, 64 insertions(+)
 create mode 100644 gcc/testsuite/g++.dg/cpp0x/initlist-array23.C
 create mode 100644 gcc/testsuite/g++.dg/cpp0x/initlist-array24.C

diff --git a/gcc/cp/init.cc b/gcc/cp/init.cc
index ae516407c92..7dcc1152c72 100644
--- a/gcc/cp/init.cc
+++ b/gcc/cp/init.cc
@@ -5109,6 +5109,15 @@ build_vec_init (tree base, tree maxindex, tree init,
     {
       if (!saw_non_const)
        {
+         /* If we're not generating the loop, we don't need to reset the
+            iterator.  */
+         if (cleanup_flags
+             && !vec_safe_is_empty (*cleanup_flags))
+           {
+             auto l = (*cleanup_flags)->last ();
+             gcc_assert (TREE_PURPOSE (l) == iterator);
+             (*cleanup_flags)->pop ();
+           }
          tree const_init = build_constructor (atype, const_vec);
          return build2 (INIT_EXPR, atype, obase, const_init);
        }
diff --git a/gcc/testsuite/g++.dg/cpp0x/initlist-array23.C 
b/gcc/testsuite/g++.dg/cpp0x/initlist-array23.C
new file mode 100644
index 00000000000..cda2afb9fcc
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp0x/initlist-array23.C
@@ -0,0 +1,28 @@
+// PR c++/117985
+// { dg-do compile { target c++11 } }
+
+struct _Vector_impl {
+  constexpr
+    _Vector_impl() {}
+};
+struct _Vector_base {
+  ~_Vector_base();
+  _Vector_impl _M_impl;
+};
+struct vector : private _Vector_base {};
+struct string {
+  string();
+};
+struct VEC {
+  vector pane{};
+};
+struct FOO {
+  VEC screen[1]{};
+  string debug_name;
+};
+
+int
+main ()
+{
+  FOO{};
+}
diff --git a/gcc/testsuite/g++.dg/cpp0x/initlist-array24.C 
b/gcc/testsuite/g++.dg/cpp0x/initlist-array24.C
new file mode 100644
index 00000000000..7dda00d5c0b
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp0x/initlist-array24.C
@@ -0,0 +1,27 @@
+// PR c++/117985
+// { dg-do compile { target c++20 } }
+
+struct _Vector_impl {
+  constexpr _Vector_impl() {}
+};
+struct _Vector_base {
+  constexpr ~_Vector_base() {}
+  _Vector_impl _M_impl;
+};
+struct vector : private _Vector_base {};
+struct string {
+  string();
+};
+struct VEC {
+  vector pane{};
+};
+struct FOO {
+  VEC screen[1]{};
+  string debug_name;
+};
+
+int
+main ()
+{
+  FOO{};
+}

base-commit: d17b09c07a1da0e3950718aabc2cbdb90cae402b
-- 
2.47.1

Reply via email to