Hi Jason, On 14 Sep 2024, at 18:11, Jason Merrill wrote:
> On 9/13/24 11:06 AM, Simon Martin wrote: >> Hi Jason, >> >> On 12 Sep 2024, at 16:48, Jason Merrill wrote: >> >>> On 9/12/24 7:23 AM, Simon Martin wrote: >>>> Hi, >>>> >>>> While looking at more open PRs, I have discovered that the problem >>>> reported in PR109790 is very similar to that in PR100632, so I’m >>>> combining both in a single patch attached here. The fix is similar >>>> to >>>> the one I initially submitted, only more general and I believe >>>> better. >>> >>>> We currently crash upon mangling members that have an anonymous >>>> union >>>> or a template type. >>>> >>>> The problem is that before calling write_unqualified_name, >>>> write_member_name has an assert that assumes that it has an >>>> IDENTIFIER_NODE in its hand. However it's incorrect: it has an >>>> anonymous union in PR100632, and a template in PR109790. >>> >>> The assert does not assume it has an IDENTIFIER_NODE; it assumes it >>> has a _DECL, and expects its DECL_NAME to be an IDENTIFIER_NODE. >>> >>> !identifier_p will always be true for a _DECL, making the assert >>> useless. >> Indeed, my bad. Thanks for catching and explaining this! >>> >>> How about checking !DECL_NAME (member) instead of !identifier_p? >> Unfortunately it does not fix PR100632, that actually involves >> legitimate operators. >> >> I checked why the assert was added in the first place (via r11-6301), >> >> and the idea was to catch any case where we’d be missing the >> “on” >> marker - PR100632 contains such cases. > > I assume you mean 109790? Yes :-/ > >> So I took the approach to refactor write_member_name a bit to first >> write the marker in all the cases required, and then actually write >> the >> member name; and the assert is not needed anymore there. > > Refactoring code in mangle.cc is tricky given the intent to retain > backward bug-compatibility. > > Specifically, adding the "on" in ABI v11 is wrong since GCC 10 (ABI > v10) didn't emit it for the 109790 testcase; we can add it for v16, > since GCC 11 ICEd on the testcase. > > I would prefer to fix the bug locally rather than refactor. Understood, that makes sense. I’ll work on a more local patch and resubmit (by the way you can also ignore https://gcc.gnu.org/pipermail/gcc-patches/2024-September/662496.html, that is also “too wide”). Thanks, Simon
