[Bug c++/91292] Mangler incorrectly handles negative numbers in expressions

[Bug c++/91292] Mangler incorrectly handles negative numbers in expressions

[ppalka at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=91292

Patrick Palka <ppalka at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |ppalka at gcc dot gnu.org

--- Comment #3 from Patrick Palka <ppalka at gcc dot gnu.org> ---
> According to http://itanium-cxx-abi.github.io/cxx-abi/abi.html#expressions , a negative number isn't treated as a negative literal; it is a negated number.  This means that it is mangled as though it were positive, and then its negation is mangled.  So -1 and -(1) are treated the same.

Hmm, but according to
http://itanium-cxx-abi.github.io/cxx-abi/abi.html#mangling.literal the mangling
of a negative integer literal is prefixed with "n", and according to
http://itanium-cxx-abi.github.io/cxx-abi/abi.html#mangle.operator-name the
mangling of -<expr> is prefixed with "ng".  So I don't see why -1 and -(1)
should be given the same mangling.  ISTM GCC is already getting the mangling
right.

[Bug c++/91292] Mangler incorrectly handles negative numbers in expressions

[ppalka at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=91292

--- Comment #4 from Patrick Palka <ppalka at gcc dot gnu.org> ---
And if -(1) is to be mangled the same as -1, then shouldn't

  template<typename T>
  typename std::enable_if<(int)sizeof(T) >= -(1), int>::type size1(T *t);

  template<typename T>
  typename std::enable_if<(int)sizeof(T) >= -1, int>::type size1(T *t);

be considered two declarations of the same function?  But IIUC that would
contradict [temp.over.link]p5, which says

  Two expressions involving template parameters are considered equivalent if
two function definitions containing the expressions would satisfy the
one-definition rule

but IIUC the one-definition rule fails here because -1 is not the same
(token-wise) as -(1).

[Bug c++/91292] Mangler incorrectly handles negative numbers in expressions

[ppalka at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=91292

Patrick Palka <ppalka at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           See Also|                            |https://gcc.gnu.org/bugzill
                   |                            |a/show_bug.cgi?id=98216

--- Comment #5 from Patrick Palka <ppalka at gcc dot gnu.org> ---
The wrong mangling of floating point numbers is also being tracked at PR98216

[Bug c++/91292] Mangler incorrectly handles negative numbers in expressions

[cvs-commit at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=91292

--- Comment #6 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Patrick Palka <ppalka@gcc.gnu.org>:

https://gcc.gnu.org/g:34947d4e97ee72b26491cfe5ff4fa8258fadbe95

commit r12-3882-g34947d4e97ee72b26491cfe5ff4fa8258fadbe95
Author: Patrick Palka <ppalka@redhat.com>
Date:   Fri Sep 24 12:36:26 2021 -0400

    real: fix encoding of negative IEEE double/quad values [PR98216]

    In encode_ieee_double/quad, the assignment

      unsigned long WORD = r->sign << 31;

    is intended to set the 31st bit of WORD whenever the sign bit is set.
    But on LP64 hosts it also unintentionally sets the upper 32 bits of WORD,
    because r->sign gets promoted from unsigned:1 to int and then the result
    of the shift (equal to INT_MIN) gets sign extended from int to long.

    In the C++ frontend, this bug causes incorrect mangling of negative
    floating point values because the output of real_to_target called from
    write_real_cst unexpectedly has the upper 32 bits of this word set,
    which the caller doesn't mask out.

    This patch fixes this by avoiding the unwanted sign extension.  Note
    that r0-53976 fixed the same bug in encode_ieee_single long ago.

            PR c++/98216
            PR c++/91292

    gcc/ChangeLog:

            * real.c (encode_ieee_double): Avoid unwanted sign extension.
            (encode_ieee_quad): Likewise.

    gcc/testsuite/ChangeLog:

            * g++.dg/cpp2a/nontype-float2.C: New test.

[Bug c++/91292] Mangler incorrectly handles negative numbers in expressions

[richard-gccbugzilla at metafoo dot co.uk]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=91292

--- Comment #7 from Richard Smith <richard-gccbugzilla at metafoo dot co.uk> ---
(In reply to Patrick Palka from comment #3)
> Hmm, but according to
> http://itanium-cxx-abi.github.io/cxx-abi/abi.html#mangling.literal the
> mangling of a negative integer literal is prefixed with "n",

There is no such thing as a negative integer literal.

The ABI document says "negative integer *values* are preceded with "n""; this
case is reached when mangling fully-resolved template arguments via the

<template-arg> ::= <expr-primary>

production, not when mangling an instantiation-dependent expression. For
example, given

template<int> struct X {};
template<int N> void f(X<N * -1>, X<-1>) {}
template void f<1>(X<-1>, X<-1>);

the proper mangling is

_Z1fILi1EEv1XIXmlT_ngLi1EEES0_ILin1EE

using ngLi1E for the instantiation-dependent expression -1 and Lin1E for the
non-instantiation-dependent value -1.


(In reply to Patrick Palka from comment #4)
> And if -(1) is to be mangled the same as -1, then shouldn't
> 
>   template<typename T>
>   typename std::enable_if<(int)sizeof(T) >= -(1), int>::type size1(T *t);
> 
>   template<typename T>
>   typename std::enable_if<(int)sizeof(T) >= -1, int>::type size1(T *t);
> 
> be considered two declarations of the same function?  But IIUC that would
> contradict [temp.over.link]p5, which says
> 
>   Two expressions involving template parameters are considered equivalent if
> two function definitions containing the expressions would satisfy the
> one-definition rule
> 
> but IIUC the one-definition rule fails here because -1 is not the same
> (token-wise) as -(1).

These declarations are functionally-equivalent but not equivalent, so a program
is not permitted to contain both. That language rule exists in order to allow
implementations to do things like ignore parentheses in mangling, as the
Itanium C++ ABI does.

Note that parentheses are never mangled (except for a weird corner case
involving pointers to members), so if your argument were correct it would apply
very broadly. For example, that argument would imply that -1 and (-1) would
need different manglings.

[Bug c++/91292] Mangler incorrectly handles negative numbers in expressions

[ppalka at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=91292

--- Comment #8 from Patrick Palka <ppalka at gcc dot gnu.org> ---
I see, thanks very much for that insightful explanation.

[Bug c++/91292] Mangler incorrectly handles negative numbers in expressions

[cvs-commit at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=91292

--- Comment #9 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The releases/gcc-11 branch has been updated by Patrick Palka
<ppalka@gcc.gnu.org>:

https://gcc.gnu.org/g:1682576e62d41cd761472943372b83aee514254a

commit r11-9083-g1682576e62d41cd761472943372b83aee514254a
Author: Patrick Palka <ppalka@redhat.com>
Date:   Fri Sep 24 12:36:26 2021 -0400

    real: fix encoding of negative IEEE double/quad values [PR98216]

    In encode_ieee_double/quad, the assignment

      unsigned long WORD = r->sign << 31;

    is intended to set the 31st bit of WORD whenever the sign bit is set.
    But on LP64 hosts it also unintentionally sets the upper 32 bits of WORD,
    because r->sign gets promoted from unsigned:1 to int and then the result
    of the shift (equal to INT_MIN) gets sign extended from int to long.

    In the C++ frontend, this bug causes incorrect mangling of negative
    floating point values because the output of real_to_target called from
    write_real_cst unexpectedly has the upper 32 bits of this word set,
    which the caller doesn't mask out.

    This patch fixes this by avoiding the unwanted sign extension.  Note
    that r0-53976 fixed the same bug in encode_ieee_single long ago.

            PR c++/98216
            PR c++/91292

    gcc/ChangeLog:

            * real.c (encode_ieee_double): Avoid unwanted sign extension.
            (encode_ieee_quad): Likewise.

    gcc/testsuite/ChangeLog:

            * g++.dg/cpp2a/nontype-float2.C: New test.

    (cherry picked from commit 34947d4e97ee72b26491cfe5ff4fa8258fadbe95)