Hi,
On 14 Feb 2025, at 18:08, Simon Martin wrote:
> We have been miscompiling the following valid code since GCC8, and
> r8-3497-g281e6c1d8f1b4c
>
> === cut here ===
> struct span {
> span (const int (&__first)[1]) : _M_ptr (__first) {}
> int operator[] (long __i) { return _M_ptr[__i]; }
> const int *_M_ptr;
> };
> void foo () {
> constexpr int a_vec[]{1};
> auto vec{[&a_vec]() -> span { return a_vec; }()};
> }
> === cut here ===
>
> The problem is that perform_implicit_conversion_flags (via
> mark_rvalue_use) replaces "a_vec" in the return statement by a
> CONSTRUCTOR representing a_vec's constant value, and then takes its
> address when invoking span's constructor. So we end up with an
> instance
> that points to garbage instead of a_vec's storage.
>
> I've tried many things to somehow recover from this replacement, but I
> actually think we should not do it when converting to a class type: we
> have no idea whether the conversion will involve a constructor taking
> an
> address or reference. So we should assume it's the case, and call
> mark_lvalue_use, not mark_rvalue_use (I might very weel be overseeing
> things, and feedback is more than welcome).
>
> This is what the patch does, successfully tested on
> x86_64-pc-linux-gnu.
Friendly ping.
Thanks! Simon
> PR c++/117504
>
> gcc/cp/ChangeLog:
>
> * call.cc (perform_implicit_conversion_flags): When possibly
> converting to a class, call mark_lvalue_use.
>
> gcc/testsuite/ChangeLog:
>
> * g++.dg/cpp2a/constexpr-117504.C: New test.
> * g++.dg/cpp2a/constexpr-117504a.C: New test.
>
> ---
> gcc/cp/call.cc | 4 ++
> gcc/testsuite/g++.dg/cpp2a/constexpr-117504.C | 60
> +++++++++++++++++++
> .../g++.dg/cpp2a/constexpr-117504a.C | 12 ++++
> 3 files changed, 76 insertions(+)
> create mode 100644 gcc/testsuite/g++.dg/cpp2a/constexpr-117504.C
> create mode 100644 gcc/testsuite/g++.dg/cpp2a/constexpr-117504a.C
>
> diff --git a/gcc/cp/call.cc b/gcc/cp/call.cc
> index 38a8f7fdcda..097b1fa55a4 100644
> --- a/gcc/cp/call.cc
> +++ b/gcc/cp/call.cc
> @@ -13973,6 +13973,10 @@ perform_implicit_conversion_flags (tree type,
> tree expr,
>
> if (TYPE_REF_P (type))
> expr = mark_lvalue_use (expr);
> + else if (MAYBE_CLASS_TYPE_P (type))
> + /* We might convert using a constructor that takes the address of
> EXPR, so
> + assume that it will be the case. */
> + expr = mark_lvalue_use (expr);
> else
> expr = mark_rvalue_use (expr);
>
> diff --git a/gcc/testsuite/g++.dg/cpp2a/constexpr-117504.C
> b/gcc/testsuite/g++.dg/cpp2a/constexpr-117504.C
> new file mode 100644
> index 00000000000..290d3dfd61e
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/cpp2a/constexpr-117504.C
> @@ -0,0 +1,60 @@
> +// PR c++/117504 - Initial report
> +// { dg-do "run" { target c++20 } }
> +
> +struct span {
> + span (const int (&__first)[1]) : _M_ptr (__first) {}
> + int operator[] (long __i) { return _M_ptr[__i]; }
> + const int *_M_ptr;
> +};
> +
> +constexpr int a_global_vec[]{1};
> +span myFunctor() {
> + return a_global_vec;
> +}
> +
> +int main() {
> + constexpr int a_vec[]{1};
> +
> + //
> + // This PR's case, that used to be miscompiled.
> + //
> + auto lambda_1 = [&a_vec] () -> span { return a_vec; };
> + auto vec_1 { lambda_1 () };
> + if (vec_1[0] != 1)
> + __builtin_abort ();
> +
> + // Variant that used to be miscompiled as well.
> + auto lambda_2 = [&] () -> span { return a_vec; };
> + auto vec_2 { lambda_2 () };
> + if (vec_2[0] != 1)
> + __builtin_abort ();
> +
> + //
> + // Related cases that worked already.
> + //
> + auto lambda_3 = [&a_vec] () /* -> span */ { return a_vec; };
> + auto vec_3 { lambda_3 () };
> + if (vec_3[0] != 1)
> + __builtin_abort ();
> +
> + auto lambda_4 = [&] () /* -> span */ { return a_vec; };
> + auto vec_4 { lambda_4 () };
> + if (vec_4[0] != 1)
> + __builtin_abort ();
> +
> + const int (&vec_5)[1] = a_vec;
> + if (vec_5[0] != 1)
> + __builtin_abort ();
> +
> + span vec_6 (a_vec);
> + if (vec_6[0] != 1)
> + __builtin_abort ();
> +
> + auto vec_7 = myFunctor ();
> + if (vec_7[0] != 1)
> + __builtin_abort ();
> +
> + const int (&vec_8)[1] { a_vec };
> + if (vec_8[0] != 1)
> + __builtin_abort ();
> +}
> diff --git a/gcc/testsuite/g++.dg/cpp2a/constexpr-117504a.C
> b/gcc/testsuite/g++.dg/cpp2a/constexpr-117504a.C
> new file mode 100644
> index 00000000000..f6d4dc8cbc5
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/cpp2a/constexpr-117504a.C
> @@ -0,0 +1,12 @@
> +// PR c++/117504 - ICE discovered by ppalka@ when reducing.
> +// { dg-do "compile" { target c++20 } }
> +
> +struct span {
> + span (const int* __first) : _M_ptr (__first) {}
> + int operator[] (long __i) { return _M_ptr[__i]; }
> + const int *_M_ptr;
> +};
> +int main() {
> + constexpr int a_vec[]{1};
> + auto vec { [&a_vec]() -> span { return a_vec; } () };
> +}
> --
> 2.44.0
