diff mbox series

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

Message ID 20210923204253.903755-1-ppalka@redhat.com
State New
Headers
Series real: fix encoding of negative IEEE double/quad values [PR98216] |

Commit Message

Patrick Palka Sept. 23, 2021, 8:42 p.m. UTC 
  In encode_ieee_double/quad, the assignment

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

is intended to set the 31st bit of VAL whenever the given REAL_CST is
negative.  But on LP64 hosts it also unintentionally sets the upper 32
bits of VAL due to the promotion of r->sign from unsigned:1 to int and
the subsequent sign extension of the shifted value from int to long.

In the C++ frontend, this bug causes incorrect mangling of negative
double values due to the output of real_to_target during write_real_cst
unexpectedly having the upper 32 bits of each word set.  (I'm not sure
if/how this bug manifests itself outside of the frontend..)

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

Bootstrapped and regtested on x86_64-pc-linux-gnu, does this look OK for
trunk and perhaps the release branches?

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

gcc/ChangeLog:

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

gcc/testsuite/ChangeLog:

	* g++.dg/cpp2a/nontype-float2.C: New test.
---
 gcc/real.c                                  |  6 ++++--
 gcc/testsuite/g++.dg/cpp2a/nontype-float2.C | 13 +++++++++++++
 2 files changed, 17 insertions(+), 2 deletions(-)
 create mode 100644 gcc/testsuite/g++.dg/cpp2a/nontype-float2.C

Comments

Richard Biener Sept. 24, 2021, 7:28 a.m. UTC | #1 
On Thu, Sep 23, 2021 at 10:44 PM Patrick Palka via Gcc-patches
<gcc-patches@gcc.gnu.org> wrote:
>
> In encode_ieee_double/quad, the assignment
>
>   unsigned long VAL = r->sign << 31;
>
> is intended to set the 31st bit of VAL whenever the given REAL_CST is
> negative.  But on LP64 hosts it also unintentionally sets the upper 32
> bits of VAL due to the promotion of r->sign from unsigned:1 to int and
> the subsequent sign extension of the shifted value from int to long.
>
> In the C++ frontend, this bug causes incorrect mangling of negative
> double values due to the output of real_to_target during write_real_cst
> unexpectedly having the upper 32 bits of each word set.  (I'm not sure
> if/how this bug manifests itself outside of the frontend..)
>
> This patch fixes this by avoiding the unwanted sign extension.  Note
> that r0-53976 fixed the same bug in encode_ieee_single long ago.
>
> Bootstrapped and regtested on x86_64-pc-linux-gnu, does this look OK for
> trunk and perhaps the release branches?

OK for trunk and branches.

Thanks,
Richard.

>         PR c++/98216
>         PR c++/91292
>
> gcc/ChangeLog:
>
>         * real.c (encode_ieee_double): Avoid incorrect sign extension.
>         (encode_ieee_quad): Likewise.
>
> gcc/testsuite/ChangeLog:
>
>         * g++.dg/cpp2a/nontype-float2.C: New test.
> ---
>  gcc/real.c                                  |  6 ++++--
>  gcc/testsuite/g++.dg/cpp2a/nontype-float2.C | 13 +++++++++++++
>  2 files changed, 17 insertions(+), 2 deletions(-)
>  create mode 100644 gcc/testsuite/g++.dg/cpp2a/nontype-float2.C
>
> diff --git a/gcc/real.c b/gcc/real.c
> index 555cf44c142..8c7a47a69e6 100644
> --- a/gcc/real.c
> +++ b/gcc/real.c
> @@ -3150,9 +3150,10 @@ encode_ieee_double (const struct real_format *fmt, long *buf,
>                     const REAL_VALUE_TYPE *r)
>  {
>    unsigned long image_lo, image_hi, sig_lo, sig_hi, exp;
> +  unsigned long sign = r->sign;
>    bool denormal = (r->sig[SIGSZ-1] & SIG_MSB) == 0;
>
> -  image_hi = r->sign << 31;
> +  image_hi = sign << 31;
>    image_lo = 0;
>
>    if (HOST_BITS_PER_LONG == 64)
> @@ -3938,10 +3939,11 @@ encode_ieee_quad (const struct real_format *fmt, long *buf,
>                   const REAL_VALUE_TYPE *r)
>  {
>    unsigned long image3, image2, image1, image0, exp;
> +  unsigned long sign = r->sign;
>    bool denormal = (r->sig[SIGSZ-1] & SIG_MSB) == 0;
>    REAL_VALUE_TYPE u;
>
> -  image3 = r->sign << 31;
> +  image3 = sign << 31;
>    image2 = 0;
>    image1 = 0;
>    image0 = 0;
> diff --git a/gcc/testsuite/g++.dg/cpp2a/nontype-float2.C b/gcc/testsuite/g++.dg/cpp2a/nontype-float2.C
> new file mode 100644
> index 00000000000..5db208a05d1
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/cpp2a/nontype-float2.C
> @@ -0,0 +1,13 @@
> +// PR c++/98216
> +// { dg-do compile { target c++20 } }
> +
> +template<auto> void f() { }
> +
> +template void f<-1.0f>();
> +template void f<-2.0f>();
> +
> +template void f<-1.0>();
> +template void f<-2.0>();
> +
> +template void f<-1.0L>();
> +template void f<-2.0L>();
> --
> 2.33.0.514.g99c99ed825
>
diff mbox series

Patch

diff --git a/gcc/real.c b/gcc/real.c
index 555cf44c142..8c7a47a69e6 100644
--- a/gcc/real.c
+++ b/gcc/real.c
@@ -3150,9 +3150,10 @@  encode_ieee_double (const struct real_format *fmt, long *buf,
 		    const REAL_VALUE_TYPE *r)
 {
   unsigned long image_lo, image_hi, sig_lo, sig_hi, exp;
+  unsigned long sign = r->sign;
   bool denormal = (r->sig[SIGSZ-1] & SIG_MSB) == 0;
 
-  image_hi = r->sign << 31;
+  image_hi = sign << 31;
   image_lo = 0;
 
   if (HOST_BITS_PER_LONG == 64)
@@ -3938,10 +3939,11 @@  encode_ieee_quad (const struct real_format *fmt, long *buf,
 		  const REAL_VALUE_TYPE *r)
 {
   unsigned long image3, image2, image1, image0, exp;
+  unsigned long sign = r->sign;
   bool denormal = (r->sig[SIGSZ-1] & SIG_MSB) == 0;
   REAL_VALUE_TYPE u;
 
-  image3 = r->sign << 31;
+  image3 = sign << 31;
   image2 = 0;
   image1 = 0;
   image0 = 0;
diff --git a/gcc/testsuite/g++.dg/cpp2a/nontype-float2.C b/gcc/testsuite/g++.dg/cpp2a/nontype-float2.C
new file mode 100644
index 00000000000..5db208a05d1
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp2a/nontype-float2.C
@@ -0,0 +1,13 @@ 
+// PR c++/98216
+// { dg-do compile { target c++20 } }
+
+template<auto> void f() { }
+
+template void f<-1.0f>();
+template void f<-2.0f>();
+
+template void f<-1.0>();
+template void f<-2.0>();
+
+template void f<-1.0L>();
+template void f<-2.0L>();