c++: Properly mangle CONST_DECL without a INTEGER_CST value [PR116511]

We ICE upon the following *valid* code when mangling the requires
clause

=== cut here ===
template <int> struct s1 {
  enum { e1 = 1 };
};
template <int t2> struct s2 {
  enum { e1 = s1<t2>::e1 };
  s2() requires(0 != e1) {}
};
s2<8> a;
=== cut here ===

The problem is that the mangler wrongly assumes that the DECL_INITIAL of
a CONST_DECL is always an INTEGER_CST, and blindly passes it to
write_integer_cst.

I assume we should be able to actually compute the value of e1 and use
it when mangling, however from my investigation, it seems to be a pretty
involved change.

What's clear however is that we should not try to write a non-literal as
a literal. This patch adds a utility function to determine whether a
tree is a literal as per the definition in the ABI, and uses it to only
call write_template_arg_literal when we actually have a literal in hand.

Note that I had to change the expectation of an existing test, that was
expecting "[...]void (AF::*)(){}[...]" and now gets an equivalent
"[...](void (AF::*)())0[...]" (and FWIW is what clang and icx give; see
https://godbolt.org/z/hnjdeKEhW).

Successfully tested on x86_64-pc-linux-gnu.

	PR c++/116511

gcc/cp/ChangeLog:

	* mangle.cc (literal_p): New.
	(write_expression): Only call write_template_arg_literal for
	expressions with literal_p.
	(write_template_arg): Likewise.
	(write_template_arg_literal): Assert literal_p.

gcc/testsuite/ChangeLog:

	* g++.dg/abi/mangle72.C: Adjust test expectation.
	* g++.dg/abi/mangle80.C: New test.

---
 gcc/cp/mangle.cc                    | 33 ++++++++++++++++++++++++-----
 gcc/testsuite/g++.dg/abi/mangle72.C |  2 +-
 gcc/testsuite/g++.dg/abi/mangle80.C | 13 ++++++++++++
 3 files changed, 42 insertions(+), 6 deletions(-)
 create mode 100644 gcc/testsuite/g++.dg/abi/mangle80.C

On 9/6/24 7:15 AM, Simon Martin wrote:
> We ICE upon the following *valid* code when mangling the requires
> clause
> 
> === cut here ===
> template <int> struct s1 {
>    enum { e1 = 1 };
> };
> template <int t2> struct s2 {
>    enum { e1 = s1<t2>::e1 };
>    s2() requires(0 != e1) {}
> };
> s2<8> a;
> === cut here ===
> 
> The problem is that the mangler wrongly assumes that the DECL_INITIAL of
> a CONST_DECL is always an INTEGER_CST, and blindly passes it to
> write_integer_cst.
> 
> I assume we should be able to actually compute the value of e1 and use
> it when mangling, however from my investigation, it seems to be a pretty
> involved change.
> 
> What's clear however is that we should not try to write a non-literal as
> a literal. This patch adds a utility function to determine whether a
> tree is a literal as per the definition in the ABI, and uses it to only
> call write_template_arg_literal when we actually have a literal in hand.
> 
> Note that I had to change the expectation of an existing test, that was
> expecting "[...]void (AF::*)(){}[...]" and now gets an equivalent
> "[...](void (AF::*)())0[...]" (and FWIW is what clang and icx give; see
> https://godbolt.org/z/hnjdeKEhW).

Unfortunately we need to provide backward bug compatibility for 
-fabi-version=14, so this change needs to check abi_version_at_least (15).

> +/* Determine whether T is a literal per section 5.1.6.1 of the CXX ABI.  */
> +
> +static bool
> +literal_p (const tree t)
> +{
> +  if ((TREE_TYPE (t) && NULLPTR_TYPE_P (TREE_TYPE (t)))

This looks wrong; a random expression with type nullptr_t is not a 
literal, and can be instantiation-dependent.  And I don't see any test 
of mangling such a thing.

Jason

Hi Jason,

On Thu Apr 10, 2025 at 10:42 PM CEST, Jason Merrill wrote:
> On 9/6/24 7:15 AM, Simon Martin wrote:
>> We ICE upon the following *valid* code when mangling the requires
>> clause
>> 
>> === cut here ===
>> template <int> struct s1 {
>>    enum { e1 = 1 };
>> };
>> template <int t2> struct s2 {
>>    enum { e1 = s1<t2>::e1 };
>>    s2() requires(0 != e1) {}
>> };
>> s2<8> a;
>> === cut here ===
>> 
>> The problem is that the mangler wrongly assumes that the DECL_INITIAL of
>> a CONST_DECL is always an INTEGER_CST, and blindly passes it to
>> write_integer_cst.
>> 
>> I assume we should be able to actually compute the value of e1 and use
>> it when mangling, however from my investigation, it seems to be a pretty
>> involved change.
>> 
>> What's clear however is that we should not try to write a non-literal as
>> a literal. This patch adds a utility function to determine whether a
>> tree is a literal as per the definition in the ABI, and uses it to only
>> call write_template_arg_literal when we actually have a literal in hand.
>> 
>> Note that I had to change the expectation of an existing test, that was
>> expecting "[...]void (AF::*)(){}[...]" and now gets an equivalent
>> "[...](void (AF::*)())0[...]" (and FWIW is what clang and icx give; see
>> https://godbolt.org/z/hnjdeKEhW).
>
> Unfortunately we need to provide backward bug compatibility for 
> -fabi-version=14, so this change needs to check abi_version_at_least (15).
Good point, ack.

>> +/* Determine whether T is a literal per section 5.1.6.1 of the CXX ABI.  */
>> +
>> +static bool
>> +literal_p (const tree t)
>> +{
>> +  if ((TREE_TYPE (t) && NULLPTR_TYPE_P (TREE_TYPE (t)))
>
> This looks wrong; a random expression with type nullptr_t is not a 
> literal, and can be instantiation-dependent.  And I don't see any test 
> of mangling such a thing.
TBH I think there might be more than just this wrong with this patch :-)

I have been flip-flopping between "it's wrong to just mangle the
expression" and "but I don't think we can do much better" and never
settled on one; that's why I never pinged this 6+ month old patch.

Is the approach this took actually valid? I think that in an ideal
world, the enum value would have been tsubst'd (or we'd have all we need
to tsubst it) when we mangle, and I tried to hook things up so that it
happens, but I never succeeded.

Simon