Re: [PATCH] c++: Relax too strict assert in stabilize_expr [PR111160]

[Jason Merrill <jason@redhat.com>]

On 6/26/24 3:00 PM, Simon Martin wrote:
> The case in the ticket is an ICE on invalid due to an assert in stabilize_expr,
> but the underlying issue can actually trigger on this *valid* code:
> 
> === cut here ===
> struct TheClass {
>    TheClass() {}
>    TheClass(volatile TheClass& t) {}
>    TheClass operator=(volatile TheClass& t) volatile { return t; }
> };
> void the_func() {
>    volatile TheClass x, y, z;
>    (false ? x : y) = z;
> }
> === cut here ===
> 
> The problem is that stabilize_expr asserts that it returns an expression
> without TREE_SIDE_EFFECTS, which can't be if the involved type is volatile.
> 
> This patch relaxes the assert to accept having TREE_THIS_VOLATILE on the
> returned expression.
> 
> Successfully tested on x86_64-pc-linux-gnu.

OK.

> 	PR c++/111160
> 
> gcc/cp/ChangeLog:
> 
> 	* tree.cc (stabilize_expr): Stabilized expressions can have
> 	TREE_SIDE_EFFECTS if they're volatile.
> 
> gcc/testsuite/ChangeLog:
> 
> 	* g++.dg/overload/error8.C: New test.
> 	* g++.dg/overload/volatile2.C: New test.
> 
> ---
>   gcc/cp/tree.cc                            |  2 +-
>   gcc/testsuite/g++.dg/overload/error8.C    |  9 +++++++++
>   gcc/testsuite/g++.dg/overload/volatile2.C | 12 ++++++++++++
>   3 files changed, 22 insertions(+), 1 deletion(-)
>   create mode 100644 gcc/testsuite/g++.dg/overload/error8.C
>   create mode 100644 gcc/testsuite/g++.dg/overload/volatile2.C
> 
> diff --git a/gcc/cp/tree.cc b/gcc/cp/tree.cc
> index 28648c14c6d..dfd4a3a948b 100644
> --- a/gcc/cp/tree.cc
> +++ b/gcc/cp/tree.cc
> @@ -5969,7 +5969,7 @@ stabilize_expr (tree exp, tree* initp)
>       }
>     *initp = init_expr;
>   
> -  gcc_assert (!TREE_SIDE_EFFECTS (exp));
> +  gcc_assert (!TREE_SIDE_EFFECTS (exp) || TREE_THIS_VOLATILE (exp));
>     return exp;
>   }
>   
> diff --git a/gcc/testsuite/g++.dg/overload/error8.C b/gcc/testsuite/g++.dg/overload/error8.C
> new file mode 100644
> index 00000000000..a7e745860e0
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/overload/error8.C
> @@ -0,0 +1,9 @@
> +// PR c++/111160
> +// { dg-do compile { target c++11 } }
> +
> +class TheClass {}; // { dg-error "discards|bind|discards|bind" }
> +void the_func() {
> +  TheClass x;
> +  volatile TheClass y;
> +  (false ? x : x) = y; // { dg-error "ambiguous|ambiguous" }
> +}
> diff --git a/gcc/testsuite/g++.dg/overload/volatile2.C b/gcc/testsuite/g++.dg/overload/volatile2.C
> new file mode 100644
> index 00000000000..9f27357aed6
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/overload/volatile2.C
> @@ -0,0 +1,12 @@
> +// PR c++/111160
> +// { dg-do compile { target c++11 } }
> +
> +struct TheClass {
> +  TheClass() {}
> +  TheClass(volatile TheClass& t) {}
> +  TheClass operator=(volatile TheClass& t) volatile { return t; }
> +};
> +void the_func() {
> +  volatile TheClass x, y, z;
> +  (false ? x : y) = z;
> +}