Hi,
On Tue Mar 25, 2025 at 8:33 PM CET, Simon Martin wrote:
> Hi,
>
> On Tue Mar 25, 2025 at 6:52 PM CET, Jason Merrill wrote:
>> On 3/25/25 1:50 PM, Marek Polacek wrote:
>>> On Tue, Mar 25, 2025 at 05:18:23PM +0000, Simon Martin wrote:
>>>> We've been miscompiling the following since r0-51314-gd6b4ea8592e338 (I
>>>> did not go compile something that old, and identified this change via
>>>> git blame, so might be wrong)
>>>>
>>>> === cut here ===
>>>> struct Foo { int x; };
>>>> Foo& get (Foo &v) { return v; }
>>>> void bar () {
>>>> Foo v; v.x = 1;
>>>> (true ? get (v) : get (v)).*(&Foo::x) = 2;
>>>> // v.x still equals 1 here...
>>>> }
>>>> === cut here ===
>>>>
>>>> The problem lies in build_m_component_ref, that computes the address of
>>>> the COND_EXPR using build_address to build the representation of
>>>> (true ? get (v) : get (v)).*(&Foo::x);
>>>> and gets something like
>>>> &(true ? get (v) : get (v)) // #1
>>>> instead of
>>>> (true ? &get (v) : &get (v)) // #2
>>>> and the write does not go where want it to, hence the miscompile.
>>>>
>>>> This patch replaces the call to build_address by a call to
>>>> cp_build_addr_expr, which gives #2, that is properly handled.
>>>>
>>>> Successfully tested on x86_64-pc-linux-gnu. OK for trunk? And for active
>>>> branches after 2-3 weeks since it's a nasty one (albeit very old)?
>>>>
>>>> PR c++/114525
>>>>
>>>> gcc/cp/ChangeLog:
>>>>
>>>> * typeck2.cc (build_m_component_ref): Call cp_build_addr_expr
>>>> instead of build_address.
>>>>
>>>> gcc/testsuite/ChangeLog:
>>>>
>>>> * g++.dg/parse/pr114525.C: New test.
>>>
>>> g++.dg/expr/cond18.C seems like a more appropriate place, but the
>>> patch itself LGTM.
> Good call out, thanks Marek.
>
> I've merged the patch with the suggested test rename as
> r15-8911-g35ce9afc84a63f.
>
> I'll reply to this thread in 2-3 weeks when I've backported to 13 and
> 14.
For information I have just backported this to 14 (r14-11602) and 13
(r13-9523).
Simon
