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.
Simon
