public inbox for gcc-bugs@sourceware.org
help / color / mirror / Atom feed
* [Bug c++/95823] New: [coroutines] compiler internal error in captures_temporary,
@ 2020-06-22 19:05 victor.burckel at gmail dot com
  2020-06-23  9:05 ` [Bug c++/95823] " iains at gcc dot gnu.org
                   ` (4 more replies)
  0 siblings, 5 replies; 6+ messages in thread
From: victor.burckel at gmail dot com @ 2020-06-22 19:05 UTC (permalink / raw)
  To: gcc-bugs

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=95823

            Bug ID: 95823
           Summary: [coroutines] compiler internal error in
                    captures_temporary,
           Product: gcc
           Version: 10.1.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: c++
          Assignee: unassigned at gcc dot gnu.org
          Reporter: victor.burckel at gmail dot com
  Target Milestone: ---

I get a compiler internal error when passing a value to a coroutine that is
retrieved through two inderections within smart pointers (I don't get the error
with raw pointers, that's the smallest piece of code I was able to produce).
> internal compiler error: in captures_temporary, at cp/coroutines.cc:2717

It can be easily worked around by making a local variable containing the value.
Of course the code would crash in runtime as the pointers are not initialized,
but I get the same error when initializing them.

I managed to reproduce with godbolt (both gcc10.1 and gcc trunk give me the
assertion)
https://godbolt.org/z/XarC_M

#include <coroutine>
#include <memory>

struct task {
  struct promise_type {
    auto initial_suspend() noexcept { return std::suspend_always{}; }
    auto final_suspend() noexcept { return std::suspend_always{}; }
    void return_void() {}
    task get_return_object() { return task{}; }
    void unhandled_exception() noexcept {}
  };

  ~task() noexcept {}

  bool await_ready() const noexcept { return false; }
  void await_suspend(std::coroutine_handle<>) noexcept {}
  void await_resume() noexcept {}
};

struct Id
{
    std::unique_ptr<int> value;
};

task g(int);

task f() {
    std::unique_ptr<Id> id;
    co_await g(*id->value);
}

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [Bug c++/95823] [coroutines] compiler internal error in captures_temporary,
  2020-06-22 19:05 [Bug c++/95823] New: [coroutines] compiler internal error in captures_temporary, victor.burckel at gmail dot com
@ 2020-06-23  9:05 ` iains at gcc dot gnu.org
  2020-07-16 21:02 ` cvs-commit at gcc dot gnu.org
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 6+ messages in thread
From: iains at gcc dot gnu.org @ 2020-06-23  9:05 UTC (permalink / raw)
  To: gcc-bugs

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=95823

Iain Sandoe <iains at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Last reconfirmed|                            |2020-06-23
           Keywords|                            |ice-on-valid-code
           Assignee|unassigned at gcc dot gnu.org      |iains at gcc dot gnu.org
   Target Milestone|---                         |10.2
             Status|UNCONFIRMED                 |NEW
     Ever confirmed|0                           |1

--- Comment #1 from Iain Sandoe <iains at gcc dot gnu.org> ---
thanks for the report.

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [Bug c++/95823] [coroutines] compiler internal error in captures_temporary,
  2020-06-22 19:05 [Bug c++/95823] New: [coroutines] compiler internal error in captures_temporary, victor.burckel at gmail dot com
  2020-06-23  9:05 ` [Bug c++/95823] " iains at gcc dot gnu.org
@ 2020-07-16 21:02 ` cvs-commit at gcc dot gnu.org
  2020-07-23  6:51 ` rguenth at gcc dot gnu.org
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 6+ messages in thread
From: cvs-commit at gcc dot gnu.org @ 2020-07-16 21:02 UTC (permalink / raw)
  To: gcc-bugs

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=95823

--- Comment #2 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Iain D Sandoe <iains@gcc.gnu.org>:

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

commit r11-2183-g0f66b8486cea8668020e4bd48f261b760cb579be
Author: Iain Sandoe <iain@sandoe.co.uk>
Date:   Sat Jul 11 08:49:33 2020 +0100

    coroutines: Correct frame capture of compiler temps [PR95591+4].

    When a full expression contains a co_await (or co_yield), this means
    that it might be suspended; which would destroy temporary variables
    (on a stack for example).  However the language guarantees that such
    temporaries are live until the end of the expression.

    In order to preserve this, we 'promote' temporaries where necessary
    so that they are saved in the coroutine frame (which allows them to
    remain live potentially until the frame is destroyed).  In addition,
    sub-expressions that produce control flow (such as TRUTH_AND/OR_IF
    or COND_EXPR) must be handled specifically to ensure that await
    expressions are properly expanded.

    This patch corrects two mistakes in which we were (a) failing to
    promote some temporaries and (b) we we failing to sequence DTORs for
    the captures properly. This manifests in a number of related (but not
    exact duplicate) PRs.

    The revised code collects the actions into one place and maps all the
    control flow into one form - a benefit of this is that co_returns are
    now expanded earlier (which provides an opportunity to address PR95517
    in some future patch).

    We replace a statement that contains await expression(s) with a bind
    scope that has a variable list describing the temporaries that have
    been 'promoted' and a statement list that contains a series of cleanup
    expressions for each of those.  Where we encounter nested conditional
    expressions, these are wrapped in a try-finally block with a guard var
    for each sub-expression variable that needs a DTOR.  The guards are all
    declared and initialized to false before the first conditional sub-
    expression.  The 'finally' block contains a series of if blocks (one
    per guard variable) enclosing the relevant DTOR.

    Variables listed in a bind scope in this manner are automatically moved
    to a coroutine frame version by existing code (so we re-use that rather
    than having a separate mechanism).

    gcc/cp/ChangeLog:

            PR c++/95591
            PR c++/95599
            PR c++/95823
            PR c++/95824
            PR c++/95895
            * coroutines.cc (struct coro_ret_data): Delete.
            (coro_maybe_expand_co_return): Delete.
            (co_return_expander): Delete.
            (expand_co_returns): Delete.
            (co_await_find_in_subtree): Remove unused name.
            (build_actor_fn): Remove unused parm, remove handling
            for co_return expansion.
            (register_await_info): Demote duplicate info message to a
            warning.
            (coro_make_frame_entry): Move closer to use site.
            (struct susp_frame_data): Add fields for final suspend label
            and a flag to indicate await expressions with initializers.
            (captures_temporary): Delete.
            (register_awaits): Remove unused code, update comments.
            (find_any_await): New.
            (tmp_target_expr_p): New.
            (struct interesting): New.
            (find_interesting_subtree): New.
            (struct var_nest_node): New.
            (flatten_await_stmt): New.
            (handle_nested_conditionals): New.
            (process_conditional): New.
            (replace_statement_captures): Rename to...
            (maybe_promote_temps): ... this.
            (maybe_promote_captured_temps): Delete.
            (analyze_expression_awaits): Check for await expressions with
            initializers.  Simplify handling for truth-and/or-if.
            (expand_one_truth_if): Simplify (map cases that need expansion
            to COND_EXPR).
            (await_statement_walker): Handle CO_RETURN_EXPR. Simplify the
            handling for truth-and/or-if expressions.
            (register_local_var_uses): Ensure that we create names in the
            implementation namespace.
            (morph_fn_to_coro): Add final suspend label to suspend frame
            callback data and remove it from the build_actor_fn call.

    gcc/testsuite/ChangeLog:

            PR c++/95591
            PR c++/95599
            PR c++/95823
            PR c++/95824
            PR c++/95895
            * g++.dg/coroutines/pr95591.C: New test.
            * g++.dg/coroutines/pr95599.C: New test.
            * g++.dg/coroutines/pr95823.C: New test.
            * g++.dg/coroutines/pr95824.C: New test.

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [Bug c++/95823] [coroutines] compiler internal error in captures_temporary,
  2020-06-22 19:05 [Bug c++/95823] New: [coroutines] compiler internal error in captures_temporary, victor.burckel at gmail dot com
  2020-06-23  9:05 ` [Bug c++/95823] " iains at gcc dot gnu.org
  2020-07-16 21:02 ` cvs-commit at gcc dot gnu.org
@ 2020-07-23  6:51 ` rguenth at gcc dot gnu.org
  2020-07-29  9:59 ` cvs-commit at gcc dot gnu.org
  2020-08-04 15:35 ` iains at gcc dot gnu.org
  4 siblings, 0 replies; 6+ messages in thread
From: rguenth at gcc dot gnu.org @ 2020-07-23  6:51 UTC (permalink / raw)
  To: gcc-bugs

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=95823

Richard Biener <rguenth at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|10.2                        |10.3

--- Comment #3 from Richard Biener <rguenth at gcc dot gnu.org> ---
GCC 10.2 is released, adjusting target milestone.

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [Bug c++/95823] [coroutines] compiler internal error in captures_temporary,
  2020-06-22 19:05 [Bug c++/95823] New: [coroutines] compiler internal error in captures_temporary, victor.burckel at gmail dot com
                   ` (2 preceding siblings ...)
  2020-07-23  6:51 ` rguenth at gcc dot gnu.org
@ 2020-07-29  9:59 ` cvs-commit at gcc dot gnu.org
  2020-08-04 15:35 ` iains at gcc dot gnu.org
  4 siblings, 0 replies; 6+ messages in thread
From: cvs-commit at gcc dot gnu.org @ 2020-07-29  9:59 UTC (permalink / raw)
  To: gcc-bugs

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=95823

--- Comment #4 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The releases/gcc-10 branch has been updated by Iain D Sandoe
<iains@gcc.gnu.org>:

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

commit r10-8545-gf43a1b1d1718969423337190ddbbbc9037c67783
Author: Iain Sandoe <iain@sandoe.co.uk>
Date:   Sun Jul 19 18:39:21 2020 +0100

    coroutines: Correct frame capture of compiler temps [PR95591+4].

    When a full expression contains a co_await (or co_yield), this means
    that it might be suspended; which would destroy temporary variables
    (on a stack for example).  However the language guarantees that such
    temporaries are live until the end of the expression.

    In order to preserve this, we 'promote' temporaries where necessary
    so that they are saved in the coroutine frame (which allows them to
    remain live potentially until the frame is destroyed).  In addition,
    sub-expressions that produce control flow (such as TRUTH_AND/OR_IF
    or COND_EXPR) must be handled specifically to ensure that await
    expressions are properly expanded.

    This patch corrects two mistakes in which we were (a) failing to
    promote some temporaries and (b) we we failing to sequence DTORs for
    the captures properly. This manifests in a number of related (but not
    exact duplicate) PRs.

    The revised code collects the actions into one place and maps all the
    control flow into one form - a benefit of this is that co_returns are
    now expanded earlier (which provides an opportunity to address PR95517
    in some future patch).

    We replace a statement that contains await expression(s) with a bind
    scope that has a variable list describing the temporaries that have
    been 'promoted' and a statement list that contains a series of cleanup
    expressions for each of those.  Where we encounter nested conditional
    expressions, these are wrapped in a try-finally block with a guard var
    for each sub-expression variable that needs a DTOR.  The guards are all
    declared and initialized to false before the first conditional sub-
    expression.  The 'finally' block contains a series of if blocks (one
    per guard variable) enclosing the relevant DTOR.

    Variables listed in a bind scope in this manner are automatically moved
    to a coroutine frame version by existing code (so we re-use that rather
    than having a separate mechanism).

    gcc/cp/ChangeLog:

            PR c++/95591
            PR c++/95599
            PR c++/95823
            PR c++/95824
            PR c++/95895
            * coroutines.cc (struct coro_ret_data): Delete.
            (coro_maybe_expand_co_return): Delete.
            (co_return_expander): Delete.
            (expand_co_returns): Delete.
            (co_await_find_in_subtree): Remove unused name.
            (build_actor_fn): Remove unused parm, remove handling
            for co_return expansion.
            (register_await_info): Demote duplicate info message to a
            warning.
            (coro_make_frame_entry): Move closer to use site.
            (struct susp_frame_data): Add fields for final suspend label
            and a flag to indicate await expressions with initializers.
            (captures_temporary): Delete.
            (register_awaits): Remove unused code, update comments.
            (find_any_await): New.
            (tmp_target_expr_p): New.
            (struct interesting): New.
            (find_interesting_subtree): New.
            (struct var_nest_node): New.
            (flatten_await_stmt): New.
            (handle_nested_conditionals): New.
            (process_conditional): New.
            (replace_statement_captures): Rename to...
            (maybe_promote_temps): ... this.
            (maybe_promote_captured_temps): Delete.
            (analyze_expression_awaits): Check for await expressions with
            initializers.  Simplify handling for truth-and/or-if.
            (expand_one_truth_if): Simplify (map cases that need expansion
            to COND_EXPR).
            (await_statement_walker): Handle CO_RETURN_EXPR. Simplify the
            handling for truth-and/or-if expressions.
            (register_local_var_uses): Ensure that we create names in the
            implementation namespace.
            (morph_fn_to_coro): Add final suspend label to suspend frame
            callback data and remove it from the build_actor_fn call.

    gcc/testsuite/ChangeLog:

            PR c++/95591
            PR c++/95599
            PR c++/95823
            PR c++/95824
            PR c++/95895
            * g++.dg/coroutines/pr95591.C: New test.
            * g++.dg/coroutines/pr95599.C: New test.
            * g++.dg/coroutines/pr95823.C: New test.
            * g++.dg/coroutines/pr95824.C: New test.

    (cherry picked from commit 0f66b8486cea8668020e4bd48f261b760cb579be)

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [Bug c++/95823] [coroutines] compiler internal error in captures_temporary,
  2020-06-22 19:05 [Bug c++/95823] New: [coroutines] compiler internal error in captures_temporary, victor.burckel at gmail dot com
                   ` (3 preceding siblings ...)
  2020-07-29  9:59 ` cvs-commit at gcc dot gnu.org
@ 2020-08-04 15:35 ` iains at gcc dot gnu.org
  4 siblings, 0 replies; 6+ messages in thread
From: iains at gcc dot gnu.org @ 2020-08-04 15:35 UTC (permalink / raw)
  To: gcc-bugs

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=95823

Iain Sandoe <iains at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
         Resolution|---                         |FIXED
             Status|NEW                         |RESOLVED

--- Comment #5 from Iain Sandoe <iains at gcc dot gnu.org> ---
fixed on master and for the 10.x branch.

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2020-08-04 15:35 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-06-22 19:05 [Bug c++/95823] New: [coroutines] compiler internal error in captures_temporary, victor.burckel at gmail dot com
2020-06-23  9:05 ` [Bug c++/95823] " iains at gcc dot gnu.org
2020-07-16 21:02 ` cvs-commit at gcc dot gnu.org
2020-07-23  6:51 ` rguenth at gcc dot gnu.org
2020-07-29  9:59 ` cvs-commit at gcc dot gnu.org
2020-08-04 15:35 ` iains at gcc dot gnu.org

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).