This commit: c++: non-dependent .* operand folding [PR112427] https://gitlab.com/gnutools/gcc/-/commit/d3f48f68227 seems to cause a regression in some cases when computing the address of a member using a pointer-to-member. Origin: https://sourceware.org/bugzilla/show_bug.cgi?id=31281 I made this reproducer, which was then made more minimal by Tom de Vries. We get the address of the thread_info::node field in two different ways. One using through a pointer-to-member, the other is getting its address directly. We expect both to be equal. gcc 14/trunk gets it wrong. ... struct intrusive_list_node { void *next; }; struct dummy { void *base; }; struct thread_info : public dummy, public intrusive_list_node { intrusive_list_node node; }; static thread_info ti; int main (void) { auto thread_info::*MemberNode = &thread_info::node; auto node_ptr_1 = &(ti.*MemberNode); auto node_ptr_2 = &ti.node; return !(node_ptr_1 == node_ptr_2); } ... gcc-13: ... $ g++ test.cpp; ./a.out; echo $? 0 ... gcc-14: ... $ g++ test.cpp; ./a.out; echo $? 1 ...
Started with r14-5503-gd3f48f682271ed94ab6e9f6bc62418a62bd8ff26 Slightly shortened testcase which aborts on error: struct A { void *a; }; struct B { void *b; }; struct C : public B, public A { A c; }; static C d; int main () { auto C::*e = &C::c; auto f = &(d.*e); auto g = &d.c; if (f != g) __builtin_abort (); }
FWIW, the inherit order is relevant, after applying this change we get the expected result: ... -struct thread_info : public dummy, public intrusive_list_node { +struct thread_info : public intrusive_list_node, public dummy { ... This could be used as workaround.
The problem is in that typeck2.cc change: - datum = fold_build_pointer_plus (fold_convert (ptype, datum), component); + datum = cp_convert (ptype, datum, complain); + if (!processing_template_decl) + datum = build2 (POINTER_PLUS_EXPR, ptype, + datum, convert_to_ptrofftype (component)); datum is &d and ptype is A * from the #c1 testcase. So, previously, we would correctly build (and fold) a POINTER_PLUS_EXPR of (A *) &d and (sizetype) e (where e is 16, offsetof C::c in C). But, because C has A base, cp_convert of &d to A * doesn't create (A *) &d, but returns &d.D.2800 where D.2800 is the A base in C, which is at offset 8 in the structure. If we add to that 16, we get something at offset 24 rather than the offset 16. I understand the desire to get some error checking on the pointer to pointer conversion, but cp_convert in this case calls cp_convert_pointer which does the base lookups and build_base_path etc. If no checking is needed, then it could be just datum = build_nop (ptype, datum); if we don't want folding.
Created attachment 57213 [details] gcc14-pr113599.patch I'd go with this.
D'oh, sorry for the breakage. (In reply to Jakub Jelinek from comment #3) > If no checking is needed, then it could be just datum = build_nop (ptype, > datum); > if we don't want folding. Makes sense to me. We already checked that the object type types is compatible with the poniter-to-member class type via lookup_base, so a simple cast without any extra checking should suffice here. That's what calling 'convert' instead of 'cp_convert' would have done too.
The master branch has been updated by Jakub Jelinek <jakub@gcc.gnu.org>: https://gcc.gnu.org/g:fd620bd3351c6b9821c299035ed17e655d7954b5 commit r14-8439-gfd620bd3351c6b9821c299035ed17e655d7954b5 Author: Jakub Jelinek <jakub@redhat.com> Date: Fri Jan 26 00:08:36 2024 +0100 c++: Fix up build_m_component_ref [PR113599] The following testcase reduced from GDB is miscompiled starting with r14-5503 PR112427 change. The problem is in the build_m_component_ref hunk, which changed - datum = fold_build_pointer_plus (fold_convert (ptype, datum), component); + datum = cp_convert (ptype, datum, complain); + if (!processing_template_decl) + datum = build2 (POINTER_PLUS_EXPR, ptype, + datum, convert_to_ptrofftype (component)); + datum = cp_fully_fold (datum); Component is e, (sizetype) e is 16, offset of c inside of C. ptype is A *, pointer to type of C::c and datum is &d. Now, previously the above created ((A *) &d) p+ (sizetype) e which is correct, but in the new code cp_convert sees that C has A as base class and instead of returning (A *) &d, it returns &d.D.2800 where D.2800 is the FIELD_DECL for the A base at offset 8 into C. So, instead of computing ((A *) &d) p+ (sizetype) e it computes &d.D.2800 p+ (sizetype) e, which is ((A *) &d) p+ 24. The following patch fixes it by using convert instead of cp_convert which eventually calls build_nop (ptype, datum). 2024-01-26 Jakub Jelinek <jakub@redhat.com> PR c++/113599 * typeck2.cc (build_m_component_ref): Use convert instead of cp_convert for pointer conversion. * g++.dg/expr/ptrmem11.C: New test.
Fixed.