Bug 116506 - [15 Regression] Destructors of temporary awaitables are executed too early
Summary: [15 Regression] Destructors of temporary awaitables are executed too early
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: c++ (show other bugs)
Version: 15.0
: P1 normal
Target Milestone: 15.0
Assignee: Iain Sandoe
URL:
Keywords: c++-coroutines, wrong-code
Depends on:
Blocks: 118574
  Show dependency treegraph
 
Reported: 2024-08-27 20:53 UTC by Dan Klishch
Modified: 2025-05-05 22:42 UTC (History)
8 users (show)

See Also:
Host:
Target:
Build:
Known to work:
Known to fail:
Last reconfirmed: 2024-08-29 00:00:00


Attachments
reduced reproducer for register_awaits ICE (1.54 KB, text/plain)
2024-09-05 00:06 UTC, Patrick Palka
Details
Updated patch addressing review comments (3.18 KB, patch)
2024-10-13 12:50 UTC, Iain Sandoe
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Dan Klishch 2024-08-27 20:53:09 UTC
After r15-3146-g47dbd69b1, destructors of temporary awaitables are executed too early (before suspending) if promise type has an await_transform. For example, the following snippet executes __builtin_trap if compiled with the current trunk but doesn't do so if compiled with GCC 14.2.1 or Clang 18.1.8.

```
$ cat ../test.cpp                                                                            
#include <coroutine>

bool g_too_early = true;
std::coroutine_handle<> g_handle;

struct Awaiter {
    Awaiter() {}
    ~Awaiter() {
        if (g_too_early) {
            __builtin_trap();
        }
    }

    bool await_ready() { return false; }
    void await_suspend(std::coroutine_handle<> handle) {
        g_handle = handle;
    }

    void await_resume() {}
};

struct SuspendNever {
    bool await_ready() noexcept { return true; }
    void await_suspend(std::coroutine_handle<>) noexcept {}
    void await_resume() noexcept {}
};

struct Coroutine {
    struct promise_type {
        Coroutine get_return_object() { return {}; }
        SuspendNever initial_suspend() { return {}; }
        SuspendNever final_suspend() noexcept { return {}; }
        void return_void() {}

        Awaiter&& await_transform(Awaiter&& u) {
            return static_cast<Awaiter&&>(u);
        }
    };
};

Coroutine foo() {
    co_await Awaiter{};
}

int main() {
    foo();
    g_too_early = false;
    g_handle.destroy();
}
$ ~/gcc-15-trunk/bin/g++ ../test.cpp -o ../test.elf -std=c++20 -fno-exceptions && ../test.elf
zsh: illegal hardware instruction (core dumped)  ../test.elf
$ g++ ../test.cpp -o ../test.elf -std=c++20 -fno-exceptions && ../test.elf 
$ clang++ ../test.cpp -o ../test.elf -std=c++20 -fno-exceptions -Wno-coroutine-missing-unhandled-exception && ../test.elf
```

CE link: https://godbolt.org/z/G7s35e4q8
Comment 1 Iain Sandoe 2024-08-29 08:27:06 UTC
thanks for the report, confirmed (I am working on a fix).
Comment 2 Patrick Palka 2024-09-05 00:06:12 UTC
Created attachment 59052 [details]
reduced reproducer for register_awaits ICE

FYI, the proposed patch https://gcc.gnu.org/pipermail/gcc-patches/2024-August/661807.html also fixes an ICE from register_awaits while building stdexec, reduced reproducer attached.
Comment 3 Iain Sandoe 2024-10-13 12:50:28 UTC
Created attachment 59334 [details]
Updated patch addressing review comments

To be retested and posted - please test if you have time,
Comment 4 Dan Klishch 2024-10-18 00:55:44 UTC
SerenityOS's on-host tests seem to work fine with the patch attached here (this was the original reproducer) and one from PR116914. Without the later, I hit ICE similar to one outlined in the gimplify_var_or_parm_decl report.
Comment 5 Eric Niebler 2024-12-30 01:57:25 UTC
will this fix make it for gcc-15? if not i need to find a workaround for stdexec. suggestion welcome.
Comment 6 Drea Pinski 2024-12-30 03:10:51 UTC
(In reply to Eric Niebler from comment #5)
> will this fix make it for gcc-15? if not i need to find a workaround for
> stdexec. suggestion welcome.

There is still another 3-4 months for a gcc 15 release. So there is plenty of time for this to get fixed.
Comment 7 GCC Commits 2025-02-03 22:03:43 UTC
The trunk branch has been updated by Jason Merrill <jason@gcc.gnu.org>:

https://gcc.gnu.org/g:4c743798b1d4530b327dad7c606c610f3811fdbf

commit r15-7338-g4c743798b1d4530b327dad7c606c610f3811fdbf
Author: Iain Sandoe <iains.gcc@gmail.com>
Date:   Thu Oct 31 08:40:08 2024 +0000

    c++/coroutines: Fix awaiter var creation [PR116506]
    
    Awaiters always need to have a coroutine state frame copy since
    they persist across potential supensions.  It simplifies the later
    analysis considerably to assign these early which we do when
    building co_await expressions.
    
    The cleanups in r15-3146-g47dbd69b1, unfortunately elided some of
    processing used to cater for cases where the var created from an
    xvalue, or is a pointer/reference type.
    
    Corrected thus.
    
            PR c++/116506
            PR c++/116880
    
    gcc/cp/ChangeLog:
    
            * coroutines.cc (build_co_await): Ensure that xvalues are
            materialised.  Handle references/pointer values in awaiter
            access expressions.
            (is_stable_lvalue): New.
            * decl.cc (cxx_maybe_build_cleanup): Handle null arg.
    
    gcc/testsuite/ChangeLog:
    
            * g++.dg/coroutines/pr116506.C: New test.
            * g++.dg/coroutines/pr116880.C: New test.
    
    Signed-off-by: Iain Sandoe <iain@sandoe.co.uk>
    Co-authored-by: Jason Merrill <jason@redhat.com>
Comment 8 Jason Merrill 2025-02-03 22:17:43 UTC
Fixed.
Comment 9 GCC Commits 2025-05-05 22:42:00 UTC
The releases/gcc-14 branch has been updated by Iain D Sandoe <iains@gcc.gnu.org>:

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

commit r14-11740-gb69209936680dabbb7bbe08e71646a2c25ece0bf
Author: Iain Sandoe <iains.gcc@gmail.com>
Date:   Thu Oct 31 08:40:08 2024 +0000

    c++/coroutines: Fix awaiter var creation [PR116506]
    
    Awaiters always need to have a coroutine state frame copy since
    they persist across potential supensions.  It simplifies the later
    analysis considerably to assign these early which we do when
    building co_await expressions.
    
    The cleanups in r15-3146-g47dbd69b1, unfortunately elided some of
    processing used to cater for cases where the var created from an
    xvalue, or is a pointer/reference type.
    
    Corrected thus.
    
            PR c++/116506
            PR c++/116880
    
    gcc/cp/ChangeLog:
    
            * coroutines.cc (build_co_await): Ensure that xvalues are
            materialised.  Handle references/pointer values in awaiter
            access expressions.
            (is_stable_lvalue): New.
            * decl.cc (cxx_maybe_build_cleanup): Handle null arg.
    
    gcc/testsuite/ChangeLog:
    
            * g++.dg/coroutines/pr116506.C: New test.
            * g++.dg/coroutines/pr116880.C: New test.
    
    Signed-off-by: Iain Sandoe <iain@sandoe.co.uk>
    Co-authored-by: Jason Merrill <jason@redhat.com>
    (cherry picked from commit 4c743798b1d4530b327dad7c606c610f3811fdbf)