[Bug c++/116775] C++20 Coroutine await_suspend is unexpectedly executed when using in __builtin_constant_p

2025-07-29 Thread cvs-commit at gcc dot gnu.org via Gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=116775

--- Comment #17 from GCC Commits  ---
The releases/gcc-15 branch has been updated by Iain D Sandoe
:

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

commit r15-10098-ga94894ffd5ca5c607160f8fde8eccd60d681c108
Author: Iain Sandoe 
Date:   Mon Jun 9 11:00:47 2025 +0100

c++, coroutines: Handle builtin_constant_p [PR116775].

Since the folding of this builtin happens after the main coroutine FE
lowering, we need to account for await expressions in that lowering.

Since these expressions have a property of being not evaluated, but do
not have the full constraints of an unevaluatated context, we want to
apply the checks and then remove the await expressions so that they no
longer participate in the analysis and lowering.

When a builtin_constant_p call is encountered, and the operand contains
any await expression, we check to see if the operand can be a constant
and replace the call with its result.

PR c++/116775

gcc/cp/ChangeLog:

* coroutines.cc (analyze_expression_awaits): When we see
a builtin_constant_p call, and that contains one or more
await expressions, then replace the call with its result
and discard the unevaluated operand.

gcc/testsuite/ChangeLog:

* g++.dg/coroutines/pr116775.C: New test.

Signed-off-by: Iain Sandoe 
(cherry picked from commit ab3f04b73e5a1dd734d3bab64b4878d2d0cc29ad)

[Bug c++/116775] C++20 Coroutine await_suspend is unexpectedly executed when using in __builtin_constant_p

2025-06-13 Thread cvs-commit at gcc dot gnu.org via Gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=116775

--- Comment #16 from GCC Commits  ---
The master branch has been updated by Iain D Sandoe :

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

commit r16-1508-gab3f04b73e5a1dd734d3bab64b4878d2d0cc29ad
Author: Iain Sandoe 
Date:   Mon Jun 9 11:00:47 2025 +0100

c++, coroutines: Handle builtin_constant_p [PR116775].

Since the folding of this builtin happens after the main coroutine FE
lowering, we need to account for await expressions in that lowering.

Since these expressions have a property of being not evaluated, but do
not have the full constraints of an unevaluatated context, we want to
apply the checks and then remove the await expressions so that they no
longer participate in the analysis and lowering.

When a builtin_constant_p call is encountered, and the operand contains
any await expression, we check to see if the operand can be a constant
and replace the call with its result.

PR c++/116775

gcc/cp/ChangeLog:

* coroutines.cc (analyze_expression_awaits): When we see
a builtin_constant_p call, and that contains one or more
await expressions, then replace the call with its result
and discard the unevaluated operand.

gcc/testsuite/ChangeLog:

* g++.dg/coroutines/pr116775.C: New test.

Signed-off-by: Iain Sandoe 

[Bug c++/116775] C++20 Coroutine await_suspend is unexpectedly executed when using in __builtin_constant_p

2025-06-09 Thread iains at gcc dot gnu.org via Gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=116775

Iain Sandoe  changed:

   What|Removed |Added

 Status|NEW |ASSIGNED
   Assignee|unassigned at gcc dot gnu.org  |iains at gcc dot gnu.org

[Bug c++/116775] C++20 Coroutine await_suspend is unexpectedly executed when using in __builtin_constant_p

2025-06-09 Thread iains at gcc dot gnu.org via Gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=116775

Iain Sandoe  changed:

   What|Removed |Added

URL||https://gcc.gnu.org/piperma
   ||il/gcc-patches/2025-June/68
   ||6226.html

--- Comment #15 from Iain Sandoe  ---
posted a patch.

Note that I also posted a patch to handle [[assume... ]] but in the end
followed what clang has done and elide the assume with a warning that it has
been ignored because of the expression side-effects - as below.

https://gcc.gnu.org/pipermail/gcc-patches/2025-June/686227.html

[Bug c++/116775] C++20 Coroutine await_suspend is unexpectedly executed when using in __builtin_constant_p

2025-06-08 Thread iains at gcc dot gnu.org via Gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=116775

--- Comment #14 from Iain Sandoe  ---
(In reply to Jakub Jelinek from comment #13)
> For assume, note the r15-9124-ga6c2248cfd4bc change, just throwing away the
> expression doesn't work well when combining with computed gotos, it is
> invalid but we don't want to ICE.
> On the other side, assume is just an optimization, the compiler is always
> free to ignore any assumptions discovered from it, so I think changing the
> assume stuff from
> [[assume (exp)]] to [[assume (1 || exp)]] if exp contains co_* and then
> lowering it might work, assuming coro works fine for 1 || exp.
> The rest was just from skimming through extend.texi, haven't actually looked
> at what IL we get at coro lowering time (just that it might be a good idea
> to have some testcase).

Yeah, I guess the issue with eliding the expression completely is:
http://eel.is/c++draft/dcl.attr.assume#2
which means that even though it is not evaluated the content is ODR-used.

[Bug c++/116775] C++20 Coroutine await_suspend is unexpectedly executed when using in __builtin_constant_p

2025-06-08 Thread jakub at gcc dot gnu.org via Gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=116775

--- Comment #13 from Jakub Jelinek  ---
For assume, note the r15-9124-ga6c2248cfd4bc change, just throwing away the
expression doesn't work well when combining with computed gotos, it is invalid
but we don't want to ICE.
On the other side, assume is just an optimization, the compiler is always free
to ignore any assumptions discovered from it, so I think changing the assume
stuff from
[[assume (exp)]] to [[assume (1 || exp)]] if exp contains co_* and then
lowering it might work, assuming coro works fine for 1 || exp.
The rest was just from skimming through extend.texi, haven't actually looked at
what IL we get at coro lowering time (just that it might be a good idea to have
some testcase).

[Bug c++/116775] C++20 Coroutine await_suspend is unexpectedly executed when using in __builtin_constant_p

2025-06-08 Thread iains at gcc dot gnu.org via Gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=116775

--- Comment #12 from Iain Sandoe  ---
(In reply to Jakub Jelinek from comment #11)
> I think just folding the builtin to 0 right away might be easier.  Note,
> seems __builtin_constant_p (0 && ++i) is actually folded to 1, and ditto for
> __builtin_constant_p (({ 0 }) && ++i) so we are able to prove that the
> side-effects won't actually happen at runtime.  But I think we just don't
> want to fold the builtin with co_* in it to 0, users are trying something
> weird.  Even if they actually write __builtin_constant_p (0 && co_await ...)
> and we don't fold it away before coro lowering.

for __builtin_constant_p() I agree early folding to 0 (in the coroutine
lowering) seems the right solution.

for [[assume ((h(), co_await thing))] I have no idea what the user would even
mean by this .. I think although the std does not actually say so - I'd like to
make this ill-formed (at least for now).  ISTM that the await expression always
has to be false ? and that is runtime-UB according to
http://eel.is/c++draft/dcl.attr.assume#1 

for __builtin_has_attribute, when the first parm is an expression, then the
action pertains to the type of the expression - and I think that is reasonable
for an await / yield - will have to see whether that's already handled at a
higher level

for __builtin_choose_expr our doc. says it's C-only, so that bullet is dodged
for now.

[Bug c++/116775] C++20 Coroutine await_suspend is unexpectedly executed when using in __builtin_constant_p

2025-06-08 Thread iains at gcc dot gnu.org via Gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=116775

--- Comment #8 from Iain Sandoe  ---
(In reply to Iain Sandoe from comment #7)
> on a bit more examination - it seems to me that:
> 
> http://eel.is/c++draft/expr.await#2
> 
> says that this should be ill-formed 
> 
> Since the GCC doc says:
> 
> Built-in Function: int __builtin_constant_p (exp)
> You can use the built-in function __builtin_constant_p to determine if the
> expression exp is known to be constant at compile time and hence that GCC
> can perform constant-folding on expressions involving that value. The
> argument of the function is the expression to test. The expression is not
> evaluated, side-effects are discarded.

However, it seems that we do not treat the argument of __builtin_constantP() as
an unevaluated operand, and when I do - bootstrap fails... so there's some
conceptual difference between
"an operand that is not evaluated"
and a c++
"unevaluated operand"

(or the documentation for the builtin is in error)

[Bug c++/116775] C++20 Coroutine await_suspend is unexpectedly executed when using in __builtin_constant_p

2025-06-08 Thread jakub at gcc dot gnu.org via Gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=116775

--- Comment #11 from Jakub Jelinek  ---
I think just folding the builtin to 0 right away might be easier.  Note, seems
__builtin_constant_p (0 && ++i) is actually folded to 1, and ditto for
__builtin_constant_p (({ 0 }) && ++i) so we are able to prove that the
side-effects won't actually happen at runtime.  But I think we just don't want
to fold the builtin with co_* in it to 0, users are trying something weird. 
Even if they actually write __builtin_constant_p (0 && co_await ...) and we
don't fold it away before coro lowering.

[Bug c++/116775] C++20 Coroutine await_suspend is unexpectedly executed when using in __builtin_constant_p

2025-06-08 Thread iains at gcc dot gnu.org via Gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=116775

--- Comment #10 from Iain Sandoe  ---
(In reply to Jakub Jelinek from comment #9)
> Indeed, unevaluated for __builtin_constant_p is not unevaluated in the C++
> standard sense, just the behavior that we normally just fold the routine to
> false if TREE_SIDE_EFFECTS on the operands early.  Except for coros it isn't
> a big deal, it is ok just fold the builtin to 0.
> As I wrote, one option is when the coro lowering sees __builtin_constant_p
> look for co_await etc. in the argument and if it is there, fold the builtin
> manually to 0 instead of handling those and thus pretend co_* wasn't there
> at all.
> Or if we see those in __builtin_constant_p (exp), rewrite it into
> __builtin_constant_p (0 && exp) where we'd somehow force TREE_SIDE_EFFECTS.
> Note, __builtin_constant_p isn't the only problem, [[assume (exp)]]; is as
> well,
> __builtin_has_attribute I think too, maybe __builtin_choose_expr.

ack .. so then we do need to handle it manually - probably in the initial
co_await walk - we will also need to consider the other cases;

I've just made a patch for the unevaluated context cases (typeid, sizeof,
decltype) which seems OK - apart from the fact that we did not mark typeid
expressions as unevaluated - so will have to see if there's fallout from that.

[Bug c++/116775] C++20 Coroutine await_suspend is unexpectedly executed when using in __builtin_constant_p

2025-06-08 Thread jakub at gcc dot gnu.org via Gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=116775

--- Comment #9 from Jakub Jelinek  ---
Indeed, unevaluated for __builtin_constant_p is not unevaluated in the C++
standard sense, just the behavior that we normally just fold the routine to
false if TREE_SIDE_EFFECTS on the operands early.  Except for coros it isn't a
big deal, it is ok just fold the builtin to 0.
As I wrote, one option is when the coro lowering sees __builtin_constant_p look
for co_await etc. in the argument and if it is there, fold the builtin manually
to 0 instead of handling those and thus pretend co_* wasn't there at all.
Or if we see those in __builtin_constant_p (exp), rewrite it into
__builtin_constant_p (0 && exp) where we'd somehow force TREE_SIDE_EFFECTS.
Note, __builtin_constant_p isn't the only problem, [[assume (exp)]]; is as
well,
__builtin_has_attribute I think too, maybe __builtin_choose_expr.

[Bug c++/116775] C++20 Coroutine await_suspend is unexpectedly executed when using in __builtin_constant_p

2025-06-04 Thread iains at gcc dot gnu.org via Gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=116775

--- Comment #7 from Iain Sandoe  ---
on a bit more examination - it seems to me that:

http://eel.is/c++draft/expr.await#2

says that this should be ill-formed 

Since the GCC doc says:

Built-in Function: int __builtin_constant_p (exp)
You can use the built-in function __builtin_constant_p to determine if the
expression exp is known to be constant at compile time and hence that GCC can
perform constant-folding on expressions involving that value. The argument of
the function is the expression to test. The expression is not evaluated,
side-effects are discarded.

---

(although it seems that clang accepts it and does something reasonable - we can
implement something on the lines of the proposed solutions)

[Bug c++/116775] C++20 Coroutine await_suspend is unexpectedly executed when using in __builtin_constant_p

2024-09-24 Thread iains at gcc dot gnu.org via Gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=116775

--- Comment #6 from Iain Sandoe  ---
(In reply to Jakub Jelinek from comment #4)
> So, e.g. co_await_find_in_subtree/await_statement_expander/find_any_await
> and maybe other coroutines.cc cp_walk_tree callbacks could use some helper
> for CALL_EXPR and in that case if it is a BUILT_IN_CONSTANT_P call and the
> argument has TREE_SIDE_EFFECTS, set *walk_subtrees = 0.

that seems reasonable.

(In reply to Jakub Jelinek from comment #5)
> On the other side, perhaps completely ignoring CO_AWAIT_EXPR in there (dunno
> about others) might not be correct either, I guess the function is supposed
> to be still considered a coroutine.
> So perhaps it needs to be treated more like 0 && co_await ... or if (false)
> co_await ... or something similar.
> Also note there are other functions which ignore their argument
> side-effects, I think e.g. __builtin_classify_type does.
> And wonder about [[assume (co_await ...)]];

A lot of these interactions are not spelled out (and were probably not really
throught about much, I guess).

I am not sure that an await_expression can be BUILT_IN_CONSTANT_P because its
value is the output of awaiter.await_resume() - so that, even if
awaiter.await_ready() is always true - there is still a function call to obtain
the result.  Having said that ... I wonder about constexpr on await_resume ()
[coroutines cannot be constexpr, but thay can contain constexpr].

[Bug c++/116775] C++20 Coroutine await_suspend is unexpectedly executed when using in __builtin_constant_p

2024-09-24 Thread jakub at gcc dot gnu.org via Gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=116775

Jakub Jelinek  changed:

   What|Removed |Added

 CC||iains at gcc dot gnu.org

--- Comment #5 from Jakub Jelinek  ---
On the other side, perhaps completely ignoring CO_AWAIT_EXPR in there (dunno
about others) might not be correct either, I guess the function is supposed to
be still considered a coroutine.
So perhaps it needs to be treated more like 0 && co_await ... or if (false)
co_await ... or something similar.
Also note there are other functions which ignore their argument side-effects, I
think e.g. __builtin_classify_type does.
And wonder about [[assume (co_await ...)]];

[Bug c++/116775] C++20 Coroutine await_suspend is unexpectedly executed when using in __builtin_constant_p

2024-09-20 Thread jakub at gcc dot gnu.org via Gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=116775

--- Comment #4 from Jakub Jelinek  ---
So, e.g. co_await_find_in_subtree/await_statement_expander/find_any_await and
maybe other coroutines.cc cp_walk_tree callbacks could use some helper for
CALL_EXPR and in that case if it is a BUILT_IN_CONSTANT_P call and the argument
has TREE_SIDE_EFFECTS, set *walk_subtrees = 0.

[Bug c++/116775] C++20 Coroutine await_suspend is unexpectedly executed when using in __builtin_constant_p

2024-09-20 Thread jakub at gcc dot gnu.org via Gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=116775

Jakub Jelinek  changed:

   What|Removed |Added

 CC||jakub at gcc dot gnu.org

--- Comment #3 from Jakub Jelinek  ---
So, likely the coroutine lowering shouldn't walk into __builtin_constant_p
TREE_SIDE_EFFECTS argument subtrees at all, keep those as is and leave them to
be removed later.

[Bug c++/116775] C++20 Coroutine await_suspend is unexpectedly executed when using in __builtin_constant_p

2024-09-20 Thread arsen at gcc dot gnu.org via Gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=116775

Arsen Arsenović  changed:

   What|Removed |Added

 Ever confirmed|0   |1
   Last reconfirmed||2024-09-20
 Status|UNCONFIRMED |NEW
 CC||arsen at gcc dot gnu.org

--- Comment #2 from Arsen Arsenović  ---
indeed, this happens because the processing of constant_p happens after we
split the coroutine up, it'd seem, so we get this:

  if (<>)
{
  <;
  if (<::operator
std::__n4861::coroutine_handle (&_Coro_self_handle)>)>>)
{
  <;
}
  switch (<>)
{
  case 0:;
  <;
  case 1:;
  goto resume.4;
  default:;
  goto destroy.4;
}
  destroy.4:;
  goto coro.delete.promise;
}
  resume.4:;
  <;

... that last line is supposed to be an await_resume call, but it got folded to
zero (because it's not a constant, and the coroutine code generated
__builtin_constant_p (Aw0.await_resume ()).

confirmed

[Bug c++/116775] C++20 Coroutine await_suspend is unexpectedly executed when using in __builtin_constant_p

2024-09-18 Thread sunsijie at buaa dot edu.cn via Gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=116775

孙思杰  changed:

   What|Removed |Added

   Keywords||c++-coroutines

--- Comment #1 from 孙思杰  ---
build command:
g++ -std=c++20 -g  test.cc

gcc version:
Using built-in specs.
COLLECT_GCC=gcc
COLLECT_LTO_WRAPPER=/usr/lib/gcc/x86_64-pc-linux-gnu/14.2.1/lto-wrapper
Target: x86_64-pc-linux-gnu
Configured with: /build/gcc/src/gcc/configure
--enable-languages=ada,c,c++,d,fortran,go,lto,m2,objc,obj-c++,rust
--enable-bootstrap --prefix=/usr --libdir=/usr/lib --libexecdir=/usr/lib
--mandir=/usr/share/man --infodir=/usr/share/info
--with-bugurl=https://gitlab.archlinux.org/archlinux/packaging/packages/gcc/-/issues
--with-build-config=bootstrap-lto --with-linker-hash-style=gnu
--with-system-zlib --enable-__cxa_atexit --enable-cet=auto
--enable-checking=release --enable-clocale=gnu --enable-default-pie
--enable-default-ssp --enable-gnu-indirect-function --enable-gnu-unique-object
--enable-libstdcxx-backtrace --enable-link-serialization=1
--enable-linker-build-id --enable-lto --enable-multilib --enable-plugin
--enable-shared --enable-threads=posix --disable-libssp --disable-libstdcxx-pch
--disable-werror
Thread model: posix
Supported LTO compression algorithms: zlib zstd
gcc version 14.2.1 20240805 (GCC)

clang can handle it correctly