On 2/14/25 12:08 PM, 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).
Yeah, those mark_*_use calls seem misplaced, they should be called instead by the code that actually does the conversion.
What if we replace all of them here with just mark_exp_read? Or nothing?
This is what the patch does, successfully tested on x86_64-pc-linux-gnu. 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.Cnew 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; } () }; +}
