Hi,
On Thu Mar 6, 2025 at 10:15 AM CET, Simon Martin wrote:
> 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.
To close the loop here, I've backported this fix as
r13-9450-gf10853a0087bc115c8ee1ddb5fc641bffdb7f1a4
and
r14-11445-gf078a613bf85eff138c2567b599779dee6ae4b22.
Simon
