Hi Jason,
On 20 Sep 2024, at 18:06, Jason Merrill wrote:
> On 9/16/24 4:07 PM, Simon Martin wrote:
>> Hi Jason,
>>
>> On 14 Sep 2024, at 18:44, Simon Martin wrote:
>>
>>> 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
>>>> v14-15) 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”).
>> I’m attaching a revised version of the patch, that lets members
>> with an anonymous union type go through, and for operators,
>> introduces
>> a new ABI version under which it adds the missing "on”.
>
> We can add the missing "on" in v16, since previous releases of v16-19
> ICEd on that case.
Duh you’re right; amended in the latest revision of the patch.
>
>> @@ -3255,7 +3255,15 @@ write_member_name (tree member)
>> }
>> else if (DECL_P (member))
>> {
>> - gcc_assert (!DECL_OVERLOADED_OPERATOR_P (member));
>> + if (ANON_AGGR_TYPE_P (TREE_TYPE (member)))
>> + ;
>> + else if (DECL_OVERLOADED_OPERATOR_P (member))
>> + {
>> + if (abi_check (20))
>> + write_string ("on");
>> + }
>> + else
>> + gcc_assert (identifier_p (DECL_NAME (member)));
>
> This last else is redundant; checking DECL_OVERLOADED_OPERATOR_P
> already asserts that it's an identifier.
Thanks for pointing this out, fixed in the attached revision,
successfully tested on x86_64-pc-linux-gnu. OK for trunk?
Thanks, SimonFrom 79dfd8c8fc9e6c33549cc041c13c49acaf6b4994 Mon Sep 17 00:00:00 2001
From: Simon Martin <si...@nasilyan.com>
Date: Mon, 16 Sep 2024 13:45:32 +0200
Subject: [PATCH] c++: Don't crash when mangling member with anonymous union or
template type [PR100632, PR109790]
We currently crash upon mangling members that have an anonymous union or
a template operator type.
The problem is that before calling write_unqualified_name,
write_member_name asserts that it has a declaration whose DECL_NAME is
an identifier node that is not that of an operator. This is wrong:
- In PR100632, it's an anonymous union declaration, hence a 0 DECL_NAME
- In PR109790, it's a legitimate template declaration for an operator
(this was accepted up to GCC 10)
This assert was added via r11-6301, to be sure that we do write the "on"
marker for operator members.
This patch removes that assert and instead
- Lets members with an anonymous union type go through
- For operators, adds the missing "on" marker for ABI versions greater
than the highest usable with GCC 10
Successfully tested on x86_64-pc-linux-gnu.
PR c++/109790
PR c++/100632
gcc/cp/ChangeLog:
* mangle.cc (write_member_name): Handle members whose type is an
anonymous union member. Write missing "on" marker for operators
when ABI version is at least 16.
gcc/testsuite/ChangeLog:
* g++.dg/cpp0x/decltype83.C: New test.
* g++.dg/cpp0x/decltype83a.C: New test.
* g++.dg/cpp1y/lambda-ice3.C: New test.
* g++.dg/cpp1y/lambda-ice3a.C: New test.
* g++.dg/cpp2a/nontype-class67.C: New test.
---
gcc/cp/mangle.cc | 8 +++++++-
gcc/testsuite/g++.dg/cpp0x/decltype83.C | 20 ++++++++++++++++++++
gcc/testsuite/g++.dg/cpp0x/decltype83a.C | 18 ++++++++++++++++++
gcc/testsuite/g++.dg/cpp1y/lambda-ice3.C | 19 +++++++++++++++++++
gcc/testsuite/g++.dg/cpp1y/lambda-ice3a.C | 17 +++++++++++++++++
gcc/testsuite/g++.dg/cpp2a/nontype-class67.C | 9 +++++++++
6 files changed, 90 insertions(+), 1 deletion(-)
create mode 100644 gcc/testsuite/g++.dg/cpp0x/decltype83.C
create mode 100644 gcc/testsuite/g++.dg/cpp0x/decltype83a.C
create mode 100644 gcc/testsuite/g++.dg/cpp1y/lambda-ice3.C
create mode 100644 gcc/testsuite/g++.dg/cpp1y/lambda-ice3a.C
create mode 100644 gcc/testsuite/g++.dg/cpp2a/nontype-class67.C
diff --git a/gcc/cp/mangle.cc b/gcc/cp/mangle.cc
index 46dc6923add..17988d69e1e 100644
--- a/gcc/cp/mangle.cc
+++ b/gcc/cp/mangle.cc
@@ -3255,7 +3255,13 @@ write_member_name (tree member)
}
else if (DECL_P (member))
{
- gcc_assert (!DECL_OVERLOADED_OPERATOR_P (member));
+ if (ANON_AGGR_TYPE_P (TREE_TYPE (member)))
+ ;
+ else if (DECL_OVERLOADED_OPERATOR_P (member))
+ {
+ if (abi_check (16))
+ write_string ("on");
+ }
write_unqualified_name (member);
}
else if (TREE_CODE (member) == TEMPLATE_ID_EXPR)
diff --git a/gcc/testsuite/g++.dg/cpp0x/decltype83.C
b/gcc/testsuite/g++.dg/cpp0x/decltype83.C
new file mode 100644
index 00000000000..b71a302d5eb
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp0x/decltype83.C
@@ -0,0 +1,20 @@
+// PR c++/109790
+// This used to work until GCC 10; force the usage of ABI 15 (the highest
+// usable in GCC 10) and check that the mangling (actually wrong; see
+// decltyp83a.C) matches that of GCC 10's default ABI version (14).
+
+// { dg-do compile { target c++11 } }
+// { dg-additional-options "-fabi-version=15" }
+
+struct A {
+ template<class T> void operator+(T);
+};
+
+template<class T>
+decltype(&A::operator+<T>) f();
+
+int main() {
+ f<int>();
+}
+
+// { dg-final { scan-assembler "_Z1fIiEDTadsr1AplIT_EEv" } }
diff --git a/gcc/testsuite/g++.dg/cpp0x/decltype83a.C
b/gcc/testsuite/g++.dg/cpp0x/decltype83a.C
new file mode 100644
index 00000000000..27c363651f1
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp0x/decltype83a.C
@@ -0,0 +1,18 @@
+// PR c++/109790
+// Up to GCC 10, the mangling would be missing the "on" marker, hence be wrong.
+// Check that this is fixed with the latest ABI.
+
+// { dg-do compile { target c++11 } }
+
+struct A {
+ template<class T> void operator+(T);
+};
+
+template<class T>
+decltype(&A::operator+<T>) f();
+
+int main() {
+ f<int>();
+}
+
+// { dg-final { scan-assembler "_Z1fIiEDTadsr1AonplIT_EEv" } }
diff --git a/gcc/testsuite/g++.dg/cpp1y/lambda-ice3.C
b/gcc/testsuite/g++.dg/cpp1y/lambda-ice3.C
new file mode 100644
index 00000000000..b6a2056724f
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp1y/lambda-ice3.C
@@ -0,0 +1,19 @@
+// PR c++/109790
+// This used to work until GCC 10; force the usage of ABI 15 (the highest
+// usable in GCC 10) and check that the mangling (actually wrong; see
+// lambda-ice3a.C) matches that of GCC 10's default ABI version (14).
+
+// { dg-do compile { target c++14 } }
+// { dg-additional-options "-fabi-version=15" }
+
+auto ll = [](auto ... ){};
+template <class _Impl, class _Args>
+ void mm(void (_Impl::*__p)(_Args) const);
+template <class _Ts>
+using __impl_for = decltype(mm(&decltype(ll)::operator()<_Ts>));
+template <class _Ts> __impl_for<_Ts> f() { }
+void aaa() {
+ f<int>();
+}
+
+// { dg-final { scan-assembler "_Z1fIiEDTcl2mmadsrN2llMUlDpT_E_EclIT_EEEv" } }
diff --git a/gcc/testsuite/g++.dg/cpp1y/lambda-ice3a.C
b/gcc/testsuite/g++.dg/cpp1y/lambda-ice3a.C
new file mode 100644
index 00000000000..6394e6c4914
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp1y/lambda-ice3a.C
@@ -0,0 +1,17 @@
+// PR c++/109790
+// Up to GCC 10, the mangling would be missing the "on" marker, hence be wrong.
+// Check that this is fixed with the latest ABI.
+
+// { dg-do compile { target c++14 } }
+
+auto ll = [](auto ... ){};
+template <class _Impl, class _Args>
+ void mm(void (_Impl::*__p)(_Args) const);
+template <class _Ts>
+using __impl_for = decltype(mm(&decltype(ll)::operator()<_Ts>));
+template <class _Ts> __impl_for<_Ts> f() { }
+void aaa() {
+ f<int>();
+}
+
+// { dg-final { scan-assembler "_Z1fIiEDTcl2mmadsrN2llMUlDpT_E_EonclIT_EEEv" }
}
diff --git a/gcc/testsuite/g++.dg/cpp2a/nontype-class67.C
b/gcc/testsuite/g++.dg/cpp2a/nontype-class67.C
new file mode 100644
index 00000000000..accf4284883
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp2a/nontype-class67.C
@@ -0,0 +1,9 @@
+// PR c++/100632
+// { dg-do compile { target c++20 } }
+
+struct B { const int* p; };
+template<B> void f() {}
+
+struct Nested { union { int k; }; } nested;
+
+template void f<B{&nested.k}>();
--
2.44.0
