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.
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.
Successfully tested on x86_64-pc-linux-gnu. OK?
Thanks again, Simon
>
>> This patch fixes this by relaxing the assert to accept members that
>> are
>> not identifiers, that are handled by write_unqualified_name just
>> fine.
From c5985057d6769ae2d54ebffd703d5508624479dd Mon Sep 17 00:00:00 2001
From: Simon Martin <si...@nasilyan.com>
Date: Mon, 9 Sep 2024 09:31:10 +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 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, is a legitimate template declaration for an operator
This assert was added via r11-6301, to be sure that we do write the "on"
marker for operator members.
This patch refactors write_member_name to add this marker whenever it's
needed (including cases where it was missing; see decltype83.C), and
removes the assert altogether since it's not needed anymore.
Successfully tested on x86_64-pc-linux-gnu.
PR c++/109790
PR c++/100632
gcc/cp/ChangeLog:
* mangle.cc (write_member_name): Add "on" marker for all
operators. Remove now unnecessary gcc_assert.
gcc/testsuite/ChangeLog:
* g++.dg/cpp0x/decltype83.C: New test.
* g++.dg/cpp1y/lambda-ice3.C: New test.
* g++.dg/cpp2a/nontype-class67.C: New test.
---
gcc/cp/mangle.cc | 23 ++++++++++----------
gcc/testsuite/g++.dg/cpp0x/decltype83.C | 18 +++++++++++++++
gcc/testsuite/g++.dg/cpp1y/lambda-ice3.C | 12 ++++++++++
gcc/testsuite/g++.dg/cpp2a/nontype-class67.C | 9 ++++++++
4 files changed, 51 insertions(+), 11 deletions(-)
create mode 100644 gcc/testsuite/g++.dg/cpp0x/decltype83.C
create mode 100644 gcc/testsuite/g++.dg/cpp1y/lambda-ice3.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..d2548d69991 100644
--- a/gcc/cp/mangle.cc
+++ b/gcc/cp/mangle.cc
@@ -3244,20 +3244,21 @@ write_template_args (tree args, tree parms /*=
NULL_TREE*/)
static void
write_member_name (tree member)
{
- if (identifier_p (member))
+ /* Make sure to add the "on" marker if MEMBER is an operator. */
+ if ((identifier_p (member) || DECL_DECLARES_FUNCTION_P (member))
+ && abi_check (11))
{
- if (IDENTIFIER_ANY_OP_P (member))
- {
- if (abi_check (11))
- write_string ("on");
- }
- write_unqualified_id (member);
+ tree name = member;
+ if (DECL_DECLARES_FUNCTION_P (name))
+ name = DECL_NAME (name);
+ if (IDENTIFIER_ANY_OP_P (name))
+ write_string ("on");
}
+
+ if (identifier_p (member))
+ write_unqualified_id (member);
else if (DECL_P (member))
- {
- gcc_assert (!DECL_OVERLOADED_OPERATOR_P (member));
- write_unqualified_name (member);
- }
+ write_unqualified_name (member);
else if (TREE_CODE (member) == TEMPLATE_ID_EXPR)
{
tree name = TREE_OPERAND (member, 0);
diff --git a/gcc/testsuite/g++.dg/cpp0x/decltype83.C
b/gcc/testsuite/g++.dg/cpp0x/decltype83.C
new file mode 100644
index 00000000000..75e9c081b27
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp0x/decltype83.C
@@ -0,0 +1,18 @@
+// PR c++/109790
+// { dg-do compile { target c++11 } }
+// This would ICE due to an assert, and if the assert were removed, it'd still
+// be wrongly mangling "decltype (&(A::operator+<int>)) f<int>()": the "on"
+// marker for operators would be missing.
+
+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..49261aaabb7
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp1y/lambda-ice3.C
@@ -0,0 +1,12 @@
+// PR c++/109790
+// { 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>();
+}
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
