Bug 118574 - [15 regression] Crashes with coroutines in KDE Plasma
Summary: [15 regression] Crashes with coroutines in KDE Plasma
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: c++ (show other bugs)
Version: 15.0
: P3 normal
Target Milestone: 15.0
Assignee: Jason Merrill
URL:
Keywords: c++-coroutines, needs-bisection, needs-source, wrong-code
Depends on: 116506 116914 117231
Blocks:
  Show dependency treegraph
 
Reported: 2025-01-20 22:57 UTC by Sam James
Modified: 2025-02-15 16:43 UTC (History)
5 users (show)

See Also:
Host:
Target:
Build:
Known to work:
Known to fail:
Last reconfirmed: 2025-02-10 00:00:00


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Sam James 2025-01-20 22:57:02 UTC
I'm filing this as a placeholder for now as we're now in stage 4 and it's important this one not be forgotten.

When using KDE Plasma compiled w/ trunk, I see a bunch of coro-related crashes:
* one machine consistently has plasmashell crash if I disconnect from a wifi network in plasma-nm
* another machine crashes easily on plasmashell startup in powerdevil when checking display info for brightness

Building with GCC 14 or Clang works fine.

This is with Iain's patches for PR116506 and PR116914 applied as well. I've already discussed it with Iain a bit and the hope is that this is actually a dupe of one of the other open coro issues.

I don't yet have a testcase for it. I'll post backtraces when I next get a chance (but obviously both current reproducers are disruptive if there's any ongoing work).
Comment 1 Jakub Jelinek 2025-01-27 10:33:38 UTC
Is KDE Plasma built with -std=gnu++20 or -std=gnu++17?
Perhaps as a workaround -fno-range-for-ext-temps option could help.
If it is built with -std=c++23/gnu++23 or later, that won't work though.
Comment 2 Sam James 2025-01-27 15:32:43 UTC
(In reply to Jakub Jelinek from comment #1)
> Is KDE Plasma built with -std=gnu++20 or -std=gnu++17?

-std=gnu++20

> Perhaps as a workaround -fno-range-for-ext-temps option could help.
> If it is built with -std=c++23/gnu++23 or later, that won't work though.

Let me try.

Meanwhile, here's a backtrace for completeness:
```
t
#0  __pthread_kill_implementation (threadid=<optimized out>, signo=11, no_tid=0) at pthread_kill.c:44
#1  __pthread_kill_internal (threadid=<optimized out>, signo=11) at pthread_kill.c:89
#2  __GI___pthread_kill (threadid=<optimized out>, signo=signo@entry=11) at pthread_kill.c:100
#3  0x00007f3930e20706 in __GI_raise (sig=11) at ../sysdeps/posix/raise.c:26
#4  0x00007f3934bc759d in KCrash::defaultCrashHandler (sig=11) at /usr/src/debug/kde-frameworks/kcrash-6.10.0/kcrash-6.10.0/src/kcrash.cpp:596
#5  0x00007f3930e20840 in <signal handler called> () at /usr/lib64/libc.so.6
#6  std::__atomic_base<int>::load (this=0x55ff1a0eda4004, __m=std::memory_order::relaxed) at /usr/lib/gcc/x86_64-pc-linux-gnu/15/include/g++-v15/bits/atomic_base.h:500
#7  QAtomicOps<int>::loadRelaxed<int> (_q_value=<error reading variable: Cannot access memory at address 0x55ff1a0eda4004>) at /usr/include/qt6/QtCore/qatomic_cxx11.h:202
#8  QBasicAtomicInteger<int>::loadRelaxed (this=0x55ff1a0eda4004) at /usr/include/qt6/QtCore/qbasicatomic.h:36
#9  QWeakPointer<QObject>::internalData (this=0x55ff1a1b409f) at /usr/include/qt6/QtCore/qsharedpointer_impl.h:752
#10 QPointer<Kirigami::Platform::PlatformTheme>::data (this=0x55ff1a1b409f) at /usr/include/qt6/QtCore/qpointer.h:75
#11 comparesEqual<Kirigami::Platform::PlatformTheme const> (lhs=..., rhs=0x55ff1a0efe70) at /usr/include/qt6/QtCore/qpointer.h:103
#12 operator==<Kirigami::Platform::PlatformTheme const> (lhs=..., rhs=<synthetic pointer>: <optimized out>) at /usr/include/qt6/QtCore/qpointer.h:104
#13 Kirigami::Platform::PlatformThemePrivate::color (this=<optimized out>, theme=0x55ff1a0efe70, color=Kirigami::Platform::PlatformThemeData::TextColor)
    at /usr/src/debug/kde-frameworks/kirigami-6.10.0/kirigami-6.10.0/src/platform/platformtheme.cpp:281
#14 Kirigami::Platform::PlatformTheme::textColor (this=0x55ff1a0efe70) at /usr/src/debug/kde-frameworks/kirigami-6.10.0/kirigami-6.10.0/src/platform/platformtheme.cpp:465
#15 0x00007f38e40f0cc5 in Icon::loadFromTheme (this=0x55ff1a0ea650, iconName=...) at /usr/src/debug/kde-frameworks/kirigami-6.10.0/kirigami-6.10.0/src/primitives/icon.cpp:642
#16 Icon::findIcon (this=this@entry=0x55ff1a0ea650, size=...) at /usr/src/debug/kde-frameworks/kirigami-6.10.0/kirigami-6.10.0/src/primitives/icon.cpp:533
#17 0x00007f38e40f4a68 in Icon::updatePolish (this=0x55ff1a0ea650) at /usr/src/debug/kde-frameworks/kirigami-6.10.0/kirigami-6.10.0/src/primitives/icon.cpp:376
#18 0x00007f393366c47c in QQuickWindowPrivate::polishItems (this=0x55ff189d64c0)
    at /usr/src/debug/dev-qt/qtdeclarative-6.8.1/qtdeclarative-everywhere-src-6.8.1/src/quick/items/qquickwindow.cpp:348
#19 0x00007f393339fd28 in QSGThreadedRenderLoop::polishAndSync (this=this@entry=0x55ff17a4e270, w=w@entry=0x55ff1cae3640, inExpose=inExpose@entry=true)
    at /usr/src/debug/dev-qt/qtdeclarative-6.8.1/qtdeclarative-everywhere-src-6.8.1/src/quick/scenegraph/qsgthreadedrenderloop.cpp:1636
#20 0x00007f39333ad825 in QSGThreadedRenderLoop::handleExposure (this=0x55ff17a4e270, window=<optimized out>)
    at /usr/src/debug/dev-qt/qtdeclarative-6.8.1/qtdeclarative-everywhere-src-6.8.1/src/quick/scenegraph/qsgthreadedrenderloop.cpp:1337
#21 0x00007f3931f5c3b8 in QWindow::event (this=0x55ff190f7050, ev=0x7fff214f3480) at /usr/src/debug/dev-qt/qtbase-6.8.1/qtbase-everywhere-src-6.8.1/src/gui/kernel/qwindow.cpp:2691
#22 0x00007f39343946ff in QApplicationPrivate::notify_helper (this=0x55ff1723dab0, receiver=0x55ff190f7050, e=0x7fff214f3480)
    at /usr/src/debug/dev-qt/qtbase-6.8.1/qtbase-everywhere-src-6.8.1/src/widgets/kernel/qapplication.cpp:3296
#23 0x00007f3931948df8 in QCoreApplication::notifyInternal2 (receiver=0x55ff190f7050, event=0x7fff214f3480)
    at /usr/src/debug/dev-qt/qtbase-6.8.1/qtbase-everywhere-src-6.8.1/src/corelib/kernel/qcoreapplication.cpp:1168
#24 0x00007f3931ee8577 in QGuiApplicationPrivate::processExposeEvent (e=<optimized out>)
    at /usr/src/debug/dev-qt/qtbase-6.8.1/qtbase-everywhere-src-6.8.1/src/gui/kernel/qguiapplication.cpp:3464
#25 0x00007f3931f606da in QWindowSystemInterface::sendWindowSystemEvents (flags=...)
    at /usr/src/debug/dev-qt/qtbase-6.8.1/qtbase-everywhere-src-6.8.1/src/gui/kernel/qwindowsysteminterface.cpp:1114
#26 0x00007f393257dab4 in userEventSourceDispatch (source=<optimized out>)
    at /usr/src/debug/dev-qt/qtbase-6.8.1/qtbase-everywhere-src-6.8.1/src/gui/platform/unix/qeventdispatcher_glib.cpp:38
#27 0x00007f39304a8017 in g_main_dispatch (context=0x7f3918000f30) at ../glib-2.82.4/glib/gmain.c:3357
#28 0x00007f39304a92e8 in g_main_context_dispatch_unlocked (context=0x7f3918000f30) at ../glib-2.82.4/glib/gmain.c:4208
#29 g_main_context_iterate_unlocked (context=context@entry=0x7f3918000f30, block=block@entry=1, dispatch=dispatch@entry=1, self=<optimized out>) at ../glib-2.82.4/glib/gmain.c:4273
#30 0x00007f39304a93c8 in g_main_context_iteration (context=0x7f3918000f30, may_block=1) at ../glib-2.82.4/glib/gmain.c:4338
#31 0x00007f3931643304 in QEventDispatcherGlib::processEvents (this=0x55ff17240620, flags=...)
    at /usr/src/debug/dev-qt/qtbase-6.8.1/qtbase-everywhere-src-6.8.1/src/corelib/kernel/qeventdispatcher_glib.cpp:399
#32 0x00007f393197dc46 in QEventLoop::processEvents (this=0x7fff214f37a0, flags=...)
    at /usr/src/debug/dev-qt/qtbase-6.8.1/qtbase-everywhere-src-6.8.1/src/corelib/kernel/qeventloop.cpp:103
#33 QEventLoop::exec (this=0x7fff214f37a0, flags=...) at /usr/src/debug/dev-qt/qtbase-6.8.1/qtbase-everywhere-src-6.8.1/src/corelib/kernel/qeventloop.cpp:194
#34 0x00007f393197e595 in QCoreApplication::exec () at /usr/src/debug/dev-qt/qtbase-6.8.1/qtbase-everywhere-src-6.8.1/src/corelib/kernel/qcoreapplication.cpp:1513
#35 0x00007f3931ed0ab0 in QGuiApplication::exec () at /usr/src/debug/dev-qt/qtbase-6.8.1/qtbase-everywhere-src-6.8.1/src/gui/kernel/qguiapplication.cpp:1975
#36 0x00007f39342c7499 in QApplication::exec () at /usr/src/debug/dev-qt/qtbase-6.8.1/qtbase-everywhere-src-6.8.1/src/widgets/kernel/qapplication.cpp:2564
#37 0x000055fee2651130 in main (argc=<optimized out>, argv=<optimized out>) at /usr/src/debug/kde-plasma/plasma-workspace-6.3.49.9999/plasma-workspace-6.3.49.9999/shell/main.cpp:191
```
Comment 3 Jason Merrill 2025-02-03 22:19:06 UTC
Can you check whether this still occurs after the two commits I just pushed?
Comment 4 Sam James 2025-02-04 00:12:37 UTC
-fno-range-for-ext-temps seems to have helped with an older trunk (from before the weekend). I'll try Jason's fixes now.
Comment 5 Sam James 2025-02-08 01:34:10 UTC
Unfortunately, it looks like the fixes weren't enough and the crashes are back when dropping -fno-range-for-ext-temps.
Comment 6 Iain Sandoe 2025-02-08 08:34:13 UTC
(In reply to Sam James from comment #5)
> Unfortunately, it looks like the fixes weren't enough and the crashes are
> back when dropping -fno-range-for-ext-temps.

ack

does https://gcc.gnu.org/bugzilla/show_bug.cgi?id=116914#c18 make any difference?

If not, then I will take a look at this after the WG21 meeting.
Comment 7 Jakub Jelinek 2025-02-08 09:16:15 UTC
(In reply to Sam James from comment #5)
> Unfortunately, it looks like the fixes weren't enough and the crashes are
> back when dropping -fno-range-for-ext-temps.

Can you bisect to one TU using -f{,no-}range-for-ext-temps?
Comment 8 Jakub Jelinek 2025-02-09 18:22:07 UTC
In the rhbz bug Alessandro Astone came up with:
#include <iostream>
#include <string>
#include <vector>
#include <chrono>
#include <thread>
#include <coroutine>

using namespace std::chrono_literals;

struct TimerAwaiter {
    std::chrono::milliseconds duration;
    bool await_ready() const noexcept { return false; }
    void await_suspend(std::coroutine_handle<> h) const noexcept {
        std::this_thread::sleep_for(duration);
        h.resume();
    }
    void await_resume() const noexcept {}
};

struct Task {
    struct promise_type {
        std::string value;
        std::suspend_never initial_suspend() { return {}; }
        std::suspend_always final_suspend() noexcept { return {}; }
        void return_value(std::string v) { value = std::move(v); }
        void unhandled_exception() { std::terminate(); }
        Task get_return_object() { return Task{this}; }
    };
    promise_type *p;
    explicit Task(promise_type *p) : p(p) {}
    std::string get() { return p->value; }
};

std::vector<std::string> getStringList() {
    std::vector<std::string> a;
    a.push_back("foo");
    a.push_back("bar");
    return a;
}

Task processStringList() {
    std::vector<std::string> ret;
    for (const auto &item : getStringList()) {
        co_await TimerAwaiter{200ms};
        ret.push_back(item);
    }
    co_return "foobar";
}

int main() {
    auto task = processStringList();
    std::cout << task.get() << std::endl;
    return 0;
}

$ ./g++ -std=gnu++20 -O2 -g -o /tmp/coro{,.C} -fno-range-for-ext-temps; LD_LIBRARY_PATH=../lib64 /tmp/coro
foobar
$ ./g++ -std=gnu++20 -O2 -g -o /tmp/coro{,.C} -frange-for-ext-temps; LD_LIBRARY_PATH=../lib64 /tmp/coro
Segmentation fault (core dumped)
Comment 9 Sam James 2025-02-09 18:23:35 UTC
(In reply to Jakub Jelinek from comment #7)
> Can you bisect to one TU using -f{,no-}range-for-ext-temps?

I started last night, but the crashers are plugins which makes it a nuisance. I'll work more on it if this reproducer from Alessandro ends up being insufficient.
Comment 10 Jakub Jelinek 2025-02-10 10:34:23 UTC
No iostream/string:
#include <vector>
#include <chrono>
#include <thread>
#include <coroutine>

using namespace std::chrono_literals;

struct TimerAwaiter {
    std::chrono::milliseconds duration;
    bool await_ready() const noexcept { return false; }
    void await_suspend(std::coroutine_handle<> h) const noexcept {
        std::this_thread::sleep_for(duration);
        h.resume();
    }
    void await_resume() const noexcept {}
};

struct Task {
    struct promise_type {
        const char *value;
        std::suspend_never initial_suspend() { return {}; }
        std::suspend_always final_suspend() noexcept { return {}; }
        void return_value(const char *v) { value = std::move(v); }
        void unhandled_exception() { std::terminate(); }
        Task get_return_object() { return Task{this}; }
    };
    promise_type *p;
    explicit Task(promise_type *p) : p(p) {}
    const char *get() { return p->value; }
};

std::vector<const char *> getStringList() {
    std::vector<const char *> a;
    a.push_back("foo");
    a.push_back("bar");
    return a;
}

Task processStringList() {
    std::vector<const char *> ret;
    for (const auto &item : getStringList()) {
        co_await TimerAwaiter{200ms};
        ret.push_back(item);
    }
    co_return "foobar";
}

int main() {
    auto task = processStringList();
    if (__builtin_strcmp (task.get(), "foobar"))
      __builtin_abort();
    return 0;
}
Comment 11 Jakub Jelinek 2025-02-10 10:36:17 UTC
No thread/chrono:
#include <vector>
#include <time.h>
#include <cerrno>
#include <coroutine>

struct TimerAwaiter {
    long ns;
    bool await_ready() const noexcept { return false; }
    void await_suspend(std::coroutine_handle<> h) const noexcept {
        struct ::timespec __ts = { 0, ns };
        while (::nanosleep(&__ts, &__ts) == -1 && errno == EINTR)
          ;
        h.resume();
    }
    void await_resume() const noexcept {}
};

struct Task {
    struct promise_type {
        const char *value;
        std::suspend_never initial_suspend() { return {}; }
        std::suspend_always final_suspend() noexcept { return {}; }
        void return_value(const char *v) { value = std::move(v); }
        void unhandled_exception() { __builtin_abort(); }
        Task get_return_object() { return Task{this}; }
    };
    promise_type *p;
    explicit Task(promise_type *p) : p(p) {}
    const char *get() { return p->value; }
};

std::vector<const char *> getStringList() {
    std::vector<const char *> a;
    a.push_back("foo");
    a.push_back("bar");
    return a;
}

Task processStringList() {
    std::vector<const char *> ret;
    for (const auto &item : getStringList()) {
        co_await TimerAwaiter{200000};
        ret.push_back(item);
    }
    co_return "foobar";
}

int main() {
    auto task = processStringList();
    if (__builtin_strcmp (task.get(), "foobar"))
      __builtin_abort();
}
Comment 12 Jakub Jelinek 2025-02-10 10:50:52 UTC
Finally testcase with only <coroutine>:
#include <coroutine>

struct A {
  const char **a = nullptr;
  int n = 0;
  void push_back (const char *x) { if (!a) a = new const char *[2]; a[n++] = x; }
  const char **begin () const { return a; }
  const char **end () const { return a + n; }
  ~A () { delete[] a; }
};

struct B {
  long ns;
  bool await_ready () const noexcept { return false; }
  void await_suspend (std::coroutine_handle<> h) const noexcept {
    volatile int v = 0;
    while (v < ns)
      v = v + 1;
    h.resume ();
  }
  void await_resume () const noexcept {}
};

struct C {
  struct promise_type {
    const char *value;
    std::suspend_never initial_suspend () { return {}; }
    std::suspend_always final_suspend () noexcept { return {}; }
    void return_value (const char *v) { value = v; }
    void unhandled_exception () { __builtin_abort (); }
    C get_return_object () { return C{this}; }
  };
  promise_type *p;
  explicit C (promise_type *p) : p(p) {}
  const char *get () { return p->value; }
};

A
foo ()
{
  A a;
  a.push_back ("foo");
  a.push_back ("bar");
  return a;
}

C
bar ()
{
  A ret;
  for (const auto &item : foo ())
    {
      co_await B{200000};
      ret.push_back (item);
    }
  co_return "foobar";
}

int
main ()
{
  auto task = bar ();
  if (__builtin_strcmp (task.get (), "foobar"))
    __builtin_abort ();
}
Comment 13 Jason Merrill 2025-02-10 12:15:01 UTC
From the gimple, it's clear that the problem is that with -frange-for-ext-temps the reference temp for __for_range isn't getting a slot in the coro frame:

-                    frame_ptr->_D2_3_0 = getStringList (); [return slot optimization]
-                    _13 = &frame_ptr->_D2_3_0;
-                    frame_ptr->__for_range_2_3 = _13;

+  struct vector D.139648;
+                        D.139648 = getStringList (); [return slot optimization]
+                            frame_ptr->__for_range_2_3 = &D.139648;
+                            _13 = frame_ptr->__for_range_2_3;

This temporary should not be affected by the flag, it's already extended because it's bound to the reference variable.
Comment 14 Jakub Jelinek 2025-02-10 12:26:43 UTC
Looking at gimple with co_await B{200000}; line commented out, the difference -fno-range-for-ext-temps vs -frange-for-ext-temps is
@@ -77,7 +77,6 @@ void A::~A (struct A * const this)
 
 void bar ()
 {
+  struct A D.2934;
   struct A ret;
 
   try
@@ -89,15 +88,14 @@ void bar ()
           {
             const char * const & item;
             struct A & __for_range;
-            struct A D.2634;
             const char * * __for_begin;
             const char * * __for_end;
 
             try
               {
                 D.2934 = foo (); [return slot optimization]
-                __for_range = &D.2634;
                 try
                   {
+                    __for_range = &D.2934;
                     __for_begin = A::begin (__for_range);
                     __for_end = A::end (__for_range);
(changed D.2657 to D.2634 in one case because it is the same thing).  So the temporary is declared in a different scope and __for_range initialized once inside of the try/finally and once outside of it.  Everything else is the same.
Comment 15 Jakub Jelinek 2025-02-10 12:49:04 UTC
BTW, it might be also interesting to test behavior of the other temporaries extended newly in the range for, so say something like
#include <coroutine>

[[gnu::noipa]] void
baz (int *)
{
}

struct D {
  D () : d (new int) { *d = 42; }
  ~D () { *d = 0; baz (d); delete d; }
  int *d;
};

struct A {
  const char **a = nullptr;
  int n = 0;
  const D *d = nullptr;
  void push_back (const char *x) { if (!a) a = new const char *[2]; a[n++] = x; }
  const char **begin () const { return a; }
  const char **end () const { return a + n; }
  ~A () { if (d && *d->d != 42) __builtin_abort (); delete[] a; }
};

struct B {
  long ns;
  bool await_ready () const noexcept { return false; }
  void await_suspend (std::coroutine_handle<> h) const noexcept {
    volatile int v = 0;
    while (v < ns)
      v = v + 1;
    h.resume ();
  }
  void await_resume () const noexcept {}
};

struct C {
  struct promise_type {
    const char *value;
    std::suspend_never initial_suspend () { return {}; }
    std::suspend_always final_suspend () noexcept { return {}; }
    void return_value (const char *v) { value = v; }
    void unhandled_exception () { __builtin_abort (); }
    C get_return_object () { return C{this}; }
  };
  promise_type *p;
  explicit C (promise_type *p) : p(p) {}
  const char *get () { return p->value; }
};

A
foo (const D &d)
{
  A a;
  a.d = &d;
  a.push_back ("foo");
  a.push_back ("bar");
  return a;
}

C
bar ()
{
  A ret;
  for (const auto &item : foo ({}))
    {
      co_await B{200000};
      ret.push_back (item);
    }
  co_return "foobar";
}

int
main ()
{
  auto task = bar ();
  if (__builtin_strcmp (task.get (), "foobar"))
    __builtin_abort ();
}

Obviously, this one will fail without -frange-for-ext-temps because the D temporary is destructed there too early.
Comment 16 Iain Sandoe 2025-02-10 12:53:29 UTC
I wonder if this https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94897 is of any relevance?
Comment 17 Jakub Jelinek 2025-02-10 13:11:35 UTC
Note, even the D {} temporary goes into function local vars rather than in the narrower scope.
While for the -std=c++20 case the temporary is created and pushdecl called on it
#0  pushdecl (decl=<var_decl 0x7fffea2df5f0>, hiding=false) at ../../gcc/cp/name-lookup.cc:3893
#1  0x00000000004435fd in make_temporary_var_for_ref_to_temp (decl=<var_decl 0x7fffea2df558 __for_range >, type=<record_type 0x7fffea2e5000 A>) at ../../gcc/cp/call.cc:14154
#2  0x000000000044365c in set_up_extended_ref_temp (decl=<var_decl 0x7fffea2df558 __for_range >, expr=<target_expr 0x7fffea2e4700>, Python Exception <class 'RuntimeError'>: Null type
cleanups=0x7fffffffcdd8, initp=0x7fffffffcbb0,
    cond_guard=0x0) at ../../gcc/cp/call.cc:14173
#3  0x00000000004464a6 in extend_ref_init_temps_1 (decl=<var_decl 0x7fffea2df558 __for_range >, init=<nop_expr 0x7fffea2f4ea0>, Python Exception <class 'RuntimeError'>: Null type nam
cleanups=0x7fffffffcdd8, cond_guard=0x0)
    at ../../gcc/cp/call.cc:14804
#4  0x0000000000446639 in extend_ref_init_temps (decl=<var_decl 0x7fffea2df558 __for_range >, init=<nop_expr 0x7fffea2f4ea0>, Python Exception <class 'RuntimeError'>: Null type name.
cleanups=0x7fffffffcdd8, cond_guard=0x0)
    at ../../gcc/cp/call.cc:14835
#5  0x00000000008c55cb in store_init_value (decl=<var_decl 0x7fffea2df558 __for_range >, init=<nop_expr 0x7fffea2f4ea0>, Python Exception <class 'RuntimeError'>: Null type name.
cleanups=0x7fffffffcdd8, flags=8196)
    at ../../gcc/cp/typeck2.cc:867
#6  0x000000000054cc49 in check_initializer (decl=<var_decl 0x7fffea2df558 __for_range >, init=<nop_expr 0x7fffea2f4ea0>, flags=8196, Python Exception <class 'RuntimeError'>: Null ty
cleanups=0x7fffffffcdd8)
    at ../../gcc/cp/decl.cc:8022
#7  0x0000000000554b0c in cp_finish_decl (decl=<var_decl 0x7fffea2df558 __for_range >, init=<target_expr 0x7fffea2e4700>, init_const_expr_p=false, asmspec_tree=<tree 0x0>, flags=4,
    decomp=0x0) at ../../gcc/cp/decl.cc:9200
#8  0x00000000006e575b in cp_convert_range_for (statement=<for_stmt 0x7fffea2d2948>, range_decl=<var_decl 0x7fffea2df428 item>, range_expr=<target_expr 0x7fffea2e4700>, decomp=0x0,
    ivdep=false, unroll=<tree 0x0>, novector=false) at ../../gcc/cp/parser.cc:14856
#9  0x00000000006e4e5a in cp_parser_range_for (parser=0x7fffea2cc2a0, scope=<statement_list 0x7fffea2f4bc0>, init=<tree 0x0>, range_decl=<var_decl 0x7fffea2df428 item>,
    ivdep=false, unroll=<tree 0x0>, novector=false, is_omp=false) at ../../gcc/cp/parser.cc:14643
in the ref init extension code, for -frange-for-ext-temps extend_ref_init_temps intentionally ignores it for __for_range:
extend_ref_init_temps (tree decl, tree init, vec<tree, va_gc> **cleanups,                                                                                    
   if (processing_template_decl)                                                                                                                                                      
     return init;                                                                                                                                                                     
                                                                                                                                                                                      
+  /* P2718R0 - ignore temporaries in C++23 for-range-initializer, those                                                                                                              
+     have all extended lifetime.  */                                                                                                                                                 
+  if (DECL_NAME (decl) == for_range__identifier                                                                                                                                      
+      && flag_range_for_ext_temps)                                                                                                                                                   
+    return init;                                                                                                                                                                     
+                                                                                                                                                                                     

because in that case all the temporaries are extended until end of range for body, so treating __range_for differently would just mess up with the ordering.
But it means that pushdecl on those isn't called.  The corresponding temporary is in this case created by
#1  0x000000000160b42d in build_decl (loc=4611686018427387914, code=VAR_DECL, name=<tree 0x0>, type=<record_type 0x7fffea300000 A>) at ../../gcc/tree.cc:5421
5421	  t = make_node (code PASS_MEM_STAT);
(gdb) 
#2  0x0000000000863fe5 in build_local_temp (type=<record_type 0x7fffea300000 A>) at ../../gcc/cp/tree.cc:579
579	  tree slot = build_decl (input_location,
(gdb) 
#3  0x000000000086451b in build_aggr_init_expr (type=<record_type 0x7fffea300000 A>, init=<call_expr 0x7fffea15ab70>) at ../../gcc/cp/tree.cc:689
689	      slot = build_local_temp (type);
(gdb) 
#4  0x0000000000864b3f in build_cplus_new (type=<record_type 0x7fffea300000 A>, init=<call_expr 0x7fffea15ab70>, complain=3) at ../../gcc/cp/tree.cc:741
741	  tree rval = build_aggr_init_expr (type, init);
(gdb) 
#5  0x0000000000438e56 in build_cxx_call (fn=<call_expr 0x7fffea15ab70>, nargs=0, argarray=0x7fffffffb6e0, complain=3, orig_fndecl=<function_decl 0x7fffea30a300 foo>)
    at ../../gcc/cp/call.cc:11463
11463		  fn = build_cplus_new (TREE_TYPE (fn), fn, complain);
(gdb) 
#6  0x0000000000436f26 in build_over_call (cand=0x43e0fb0, flags=1, complain=3) at ../../gcc/cp/call.cc:10933
10933	  tree call = build_cxx_call (fn, nargs, argarray, complain|decltype_flag);
(gdb) 
#7  0x000000000042002b in build_new_function_call (fn=<function_decl 0x7fffea30a300 foo>, Python Exception <class 'RuntimeError'>: Null type name.
args=0x7fffffffbc58, complain=3) at ../../gcc/cp/call.cc:5243
5243	      result = build_over_call (cand, LOOKUP_NORMAL, complain);
(gdb) 
#8  0x0000000000826e19 in finish_call_expr (fn=<function_decl 0x7fffea30a300 foo>, Python Exception <class 'RuntimeError'>: Null type name.
args=0x7fffffffbc58, disallow_virtual=false, koenig_p=false, complain=3)
    at ../../gcc/cp/semantics.cc:3457
3457		  result = build_new_function_call (fn, args, complain);
(gdb) 
#9  0x00000000006d5635 in cp_parser_postfix_expression (parser=0x7fffea2e62a0, address_p=false, cast_p=false, member_access_only_p=false, decltype_p=false, pidk_return=0x0)
    at ../../gcc/cp/parser.cc:8474

8474			  = finish_call_expr (postfix_expression, &args,
(gdb) 
#10 0x00000000006d8d0d in cp_parser_unary_expression (parser=0x7fffea2e62a0, pidk=0x0, address_p=false, cast_p=false, decltype_p=false) at ../../gcc/cp/parser.cc:9733
9733	  return cp_parser_postfix_expression (parser, address_p, cast_p,
(gdb) 
#11 0x00000000006da4f0 in cp_parser_cast_expression (parser=0x7fffea2e62a0, address_p=false, cast_p=false, decltype_p=false, pidk=0x0) at ../../gcc/cp/parser.cc:10648
10648	  return cp_parser_unary_expression (parser, pidk, address_p,
(gdb) 
#12 0x00000000006da5f3 in cp_parser_binary_expression (parser=0x7fffea2e62a0, cast_p=false, no_toplevel_fold_p=false, decltype_p=false, prec=PREC_NOT_OPERATOR, pidk=0x0)
    at ../../gcc/cp/parser.cc:10751
10751	  current.lhs = cp_parser_cast_expression (parser, /*address_p=*/false,
(gdb) 
#13 0x00000000006db7a1 in cp_parser_assignment_expression (parser=0x7fffea2e62a0, pidk=0x0, cast_p=false, decltype_p=false) at ../../gcc/cp/parser.cc:11096
11096	      expr = cp_parser_binary_expression (parser, cast_p, false,
(gdb) 
#14 0x00000000006dbcdd in cp_parser_expression (parser=0x7fffea2e62a0, pidk=0x0, cast_p=false, decltype_p=false, warn_comma_p=false) at ../../gcc/cp/parser.cc:11279
11279		  = cp_parser_assignment_expression (parser, pidk, cast_p, decltype_p);
(gdb) 
#15 0x00000000006e4b3c in cp_parser_range_for (parser=0x7fffea2e62a0, scope=<statement_list 0x7fffea30fe80>, init=<tree 0x0>, range_decl=<var_decl 0x7fffea2fc428 item>, 
    ivdep=false, unroll=<tree 0x0>, novector=false, is_omp=false) at ../../gcc/cp/parser.cc:14606
14606	    range_expr = cp_parser_expression (parser);

So, can coro handle even such temporaries which didn't have pushdecl called on them, or would we need to somehow find out all such temporaries and call pushdecl on them at some point?
Comment 18 Jason Merrill 2025-02-10 13:26:28 UTC
Before my PR118491 commit, this temporary was added to the frame, apparently that simple change was inadequate to deal with this category of issues.
Comment 19 Iain Sandoe 2025-02-10 13:28:47 UTC
(In reply to Jakub Jelinek from comment #17)
> Note, even the D {} temporary goes into function local vars rather than in
> the narrower scope.

hmm.  This might be an underlying problem, the coroutines code wants to be able to reason about the lifetime of variables that must exist across an await expression.
 
> So, can coro handle even such temporaries which didn't have pushdecl called
> on them, or would we need to somehow find out all such temporaries and call
> pushdecl on them at some point?

It should be able to - temporaries are 'promoted' when they exist within a cleanup point statement/expression and that statement/expression contains an await expression.

so cleanup point statements should be morphed into a BIND_EXPR with the temporaries given a compiler-local names and contained in the BIND_EXPR_VARS.  The latter step is what causes them to be preserved in the frame.
Comment 20 Jakub Jelinek 2025-02-10 13:35:19 UTC
Note, even just
  foo ({});
outside of range-for has its temporaries without pushdecl.
  const struct D D.2692;

...
              try
                {
                  D::D (&D.2692);
                  try
                    {
                      D.2693 = foo (&D.2692); [return slot optimization]
                      A::~A (&D.2693);
                    }
                  finally
                    {
                      D::~D (&D.2692);
                    }
                }
              finally
                {
                  D.2692 = {CLOBBER(eos)};
                }

  Guess for coro that would be about co_await qux ({}); or similar for qux with const D & operand.
But in the range-for case, the lifetime of those temporaries doesn't end at the end of the statement but is until the end of the range for body.
Comment 21 Iain Sandoe 2025-02-10 13:38:48 UTC
(In reply to Jakub Jelinek from comment #20)
> Note, even just


> But in the range-for case, the lifetime of those temporaries doesn't end at
> the end of the statement but is until the end of the range for body.

.. failing to handle this could be the bug (although I'm not sure what exactly changed to regress it).
Comment 22 Jason Merrill 2025-02-10 14:12:06 UTC
We might change the P2718 strategy to use set_up_extended_ref_temp for all temps in the range_for initializer, rather than none.
Comment 23 Jakub Jelinek 2025-02-10 14:17:56 UTC
Or could we try to walk all the cleanups queued at the end of the range-for and pushdecl any temporaries mentioned in there if they don't have DECL_NAME yet?
Comment 24 Iain Sandoe 2025-02-10 15:10:28 UTC
(In reply to Jakub Jelinek from comment #23)
> Or could we try to walk all the cleanups queued at the end of the range-for
> and pushdecl any temporaries mentioned in there if they don't have DECL_NAME
> yet?

we should not need a solution that requires us pushdecl - these vars do not need to be visible to lookup etc. - we have a mechanism to give temporaries a compiler-local name when they are added to the BIND_EXPR .. the important thing is for the coroutines code to be able to reason about _which_ temporaries to process in this way.
Comment 25 GCC Commits 2025-02-11 23:08:50 UTC
The trunk branch has been updated by Jason Merrill <jason@gcc.gnu.org>:

https://gcc.gnu.org/g:0d2a5f3cb715fd95f1fa4a13b5d67c7eea28f178

commit r15-7481-g0d2a5f3cb715fd95f1fa4a13b5d67c7eea28f178
Author: Jason Merrill <jason@redhat.com>
Date:   Mon Feb 10 15:44:13 2025 +0100

    c++: change implementation of -frange-for-ext-temps [PR118574]
    
    The implementation in r15-3840 used a novel technique of wrapping the entire
    range-for loop in a CLEANUP_POINT_EXPR, which confused the coroutines
    transformation.  Instead let's use the existing extend_ref_init_temps
    mechanism.
    
    This does not revert all of r15-3840, only the parts that change how
    CLEANUP_POINT_EXPRs are applied to range-for declarations.
    
            PR c++/118574
            PR c++/107637
    
    gcc/cp/ChangeLog:
    
            * call.cc (struct extend_temps_data): New.
            (extend_temps_r, extend_all_temps): New.
            (set_up_extended_ref_temp): Handle tree walk case.
            (extend_ref_init_temps): Cal extend_all_temps.
            * decl.cc (initialize_local_var): Revert ext-temps change.
            * parser.cc (cp_convert_range_for): Likewise.
            (cp_parser_omp_loop_nest): Likewise.
            * pt.cc (tsubst_stmt): Likewise.
            * semantics.cc (finish_for_stmt): Likewise.
    
    gcc/testsuite/ChangeLog:
    
            * g++.dg/coroutines/range-for1.C: New test.
Comment 26 Jason Merrill 2025-02-12 14:12:00 UTC
This should be fixed now.
Comment 27 Sam James 2025-02-12 14:55:15 UTC
I'll test with the option re-enabled to confirm but I'm sure it is. I'll only comment/reopen if problems persist.
Comment 28 GCC Commits 2025-02-13 10:55:27 UTC
The master branch has been updated by Jakub Jelinek <jakub@gcc.gnu.org>:

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

commit r15-7503-ga1855a3e0bcf0f94bc9fa421ccc9ebd0565c62cd
Author: Jakub Jelinek <jakub@redhat.com>
Date:   Thu Feb 13 11:53:04 2025 +0100

    testsuite: Add another range for coroutines testcase [PR118574]
    
    This patch adds another range for coroutine testcase, which doesn't
    extend (across co_await) just the __for_range var and what it binds
    to (so passes even without -frange-for-ext-temps), but also some other
    temporaries and verifies they are destructed in the right order.
    
    2025-02-13  Jakub Jelinek  <jakub@redhat.com>
    
            PR c++/118574
            * g++.dg/coroutines/range-for2.C: New test.
Comment 29 Sam James 2025-02-15 16:43:02 UTC
Indeed all seems well now. Thanks!

Some fixes were made on KDE side too.