On Wed Mar 5, 2025 at 10:32 PM CET, Jason Merrill wrote:
> On 3/5/25 6:58 AM, Simon Martin wrote:
>> Hi Jason,
>> 
>> On Tue Mar 4, 2025 at 11:47 PM CET, Jason Merrill wrote:
>>> 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?
>> Thanks for the suggestions; simply removing those calls actually works. This
>> is what the attached updated patch does.
>> 
>> Successfully tested on x86_64-pc-linux-gnu. OK for trunk? And if so, OK for
>> branches after 2-3 weeks since it's a wrong code bug?
>
> OK, yes.
Thanks, merged as r15-7849-gfdf846fdddcc04.

I'll reply to this thread when I've backported this patch to active branches,
in 2-3 weeks.

Simon


Reply via email to