Hi,

This patch fixes a case where the D language implementation was
comparing objects with uninitialized memory.

Explanation:

Floating-point emulation in the D front-end is done via a type named
`struct longdouble`, which in GDC is a small interface around the
real_value type. Because the D code cannot include gcc/real.h directly,
a big enough buffer is used for the data instead.

On x86_64, this buffer is actually bigger than real_value itself, so
when a new longdouble object is created with

    longdouble r;
    real_from_string3 (&r.rv (), buffer, mode);
    return r;

there is uninitialized padding at the end of `r`.  This was never a
problem when D was implemented in C++ (until GCC 12) as comparing two
longdouble objects with `==' would be forwarded to the relevant
operator== overload that extracted the underlying real_value.

However when the front-end was translated to D, such conditions were
instead rewritten into identity comparisons

    return exp.toReal() is CTFloat.zero

The `is` operator gets lowered as a call to `memcmp() == 0', which is
where the read of uninitialized memory occurs, as seen by valgrind.

==26778== Conditional jump or move depends on uninitialised value(s)
==26778==    at 0x911F41: dmd.dstruct._isZeroInit(dmd.expression.Expression) 
(dstruct.d:635)
==26778==    by 0x9123BE: StructDeclaration::finalizeSize() (dstruct.d:373)
==26778==    by 0x86747C: dmd.aggregate.AggregateDeclaration.determineSize(ref 
const(dmd.location.Loc)) (aggregate.d:226)
[...]

To avoid accidentally reading uninitialized data, explicitly initialize
all `longdouble` variables with an empty constructor on C++ side of the
implementation before initializing underlying real_value type it holds.

Bootstrapped and regression tested on x86_64-linux-gnu/-m32, committed
to mainline.  Will also backport to all releases since GCC 12 once the
patch has been regtested on them as well.

Regards,
Iain.

---
        PR d/116961

gcc/d/ChangeLog:

        * d-codegen.cc (build_float_cst): Change new_value type from real_t to
        real_value.
        * d-ctfloat.cc (CTFloat::fabs): Default initialize the return value.
        (CTFloat::ldexp): Likewise.
        (CTFloat::parse): Likewise.
        * d-longdouble.cc (longdouble::add): Likewise.
        (longdouble::sub): Likewise.
        (longdouble::mul): Likewise.
        (longdouble::div): Likewise.
        (longdouble::mod): Likewise.
        (longdouble::neg): Likewise.
        * d-port.cc (Port::isFloat32LiteralOutOfRange): Likewise.
        (Port::isFloat64LiteralOutOfRange): Likewise.

gcc/testsuite/ChangeLog:

        * gdc.dg/pr116961.d: New test.
---
 gcc/d/d-codegen.cc              |  6 +++---
 gcc/d/d-ctfloat.cc              |  6 +++---
 gcc/d/d-longdouble.cc           | 12 ++++++------
 gcc/d/d-port.cc                 |  4 ++--
 gcc/testsuite/gdc.dg/pr116961.d |  7 +++++++
 5 files changed, 21 insertions(+), 14 deletions(-)
 create mode 100644 gcc/testsuite/gdc.dg/pr116961.d

diff --git a/gcc/d/d-codegen.cc b/gcc/d/d-codegen.cc
index 66f06e9086e..50362e6f8f5 100644
--- a/gcc/d/d-codegen.cc
+++ b/gcc/d/d-codegen.cc
@@ -246,15 +246,15 @@ build_integer_cst (dinteger_t value, tree type)
 tree
 build_float_cst (const real_t &value, Type *totype)
 {
-  real_t new_value;
+  real_value new_value;
   TypeBasic *tb = totype->isTypeBasic ();
 
   gcc_assert (tb != NULL);
 
   tree type_node = build_ctype (tb);
-  real_convert (&new_value.rv (), TYPE_MODE (type_node), &value.rv ());
+  real_convert (&new_value, TYPE_MODE (type_node), &value.rv ());
 
-  return build_real (type_node, new_value.rv ());
+  return build_real (type_node, new_value);
 }
 
 /* Returns the .length component from the D dynamic array EXP.  */
diff --git a/gcc/d/d-ctfloat.cc b/gcc/d/d-ctfloat.cc
index 271885dcd9b..659d5616124 100644
--- a/gcc/d/d-ctfloat.cc
+++ b/gcc/d/d-ctfloat.cc
@@ -33,7 +33,7 @@ along with GCC; see the file COPYING3.  If not see
 real_t
 CTFloat::fabs (real_t r)
 {
-  real_t x;
+  real_t x = {};
   real_arithmetic (&x.rv (), ABS_EXPR, &r.rv (), NULL);
   return x.normalize ();
 }
@@ -43,7 +43,7 @@ CTFloat::fabs (real_t r)
 real_t
 CTFloat::ldexp (real_t r, int exp)
 {
-  real_t x;
+  real_t x = {};
   real_ldexp (&x.rv (), &r.rv (), exp);
   return x.normalize ();
 }
@@ -87,7 +87,7 @@ CTFloat::isInfinity (real_t r)
 real_t
 CTFloat::parse (const char *buffer, bool &overflow)
 {
-  real_t r;
+  real_t r = {};
   real_from_string3 (&r.rv (), buffer, TYPE_MODE (long_double_type_node));
 
   /* Front-end checks overflow to see if the value is representable.  */
diff --git a/gcc/d/d-longdouble.cc b/gcc/d/d-longdouble.cc
index 99ce8a19b6c..193a8283864 100644
--- a/gcc/d/d-longdouble.cc
+++ b/gcc/d/d-longdouble.cc
@@ -113,7 +113,7 @@ longdouble::to_bool (void) const
 longdouble
 longdouble::add (const longdouble &r) const
 {
-  longdouble x;
+  longdouble x = {};
   real_arithmetic (&x.rv (), PLUS_EXPR, &this->rv (), &r.rv ());
   return x.normalize ();
 }
@@ -121,7 +121,7 @@ longdouble::add (const longdouble &r) const
 longdouble
 longdouble::sub (const longdouble &r) const
 {
-  longdouble x;
+  longdouble x = {};
   real_arithmetic (&x.rv (), MINUS_EXPR, &this->rv (), &r.rv ());
   return x.normalize ();
 }
@@ -129,7 +129,7 @@ longdouble::sub (const longdouble &r) const
 longdouble
 longdouble::mul (const longdouble &r) const
 {
-  longdouble x;
+  longdouble x = {};
   real_arithmetic (&x.rv (), MULT_EXPR, &this->rv (), &r.rv ());
   return x.normalize ();
 }
@@ -137,7 +137,7 @@ longdouble::mul (const longdouble &r) const
 longdouble
 longdouble::div (const longdouble &r) const
 {
-  longdouble x;
+  longdouble x = {};
   real_arithmetic (&x.rv (), RDIV_EXPR, &this->rv (), &r.rv ());
   return x.normalize ();
 }
@@ -145,7 +145,7 @@ longdouble::div (const longdouble &r) const
 longdouble
 longdouble::mod (const longdouble &r) const
 {
-  longdouble x;
+  longdouble x = {};
   real_value q;
 
   if (r.rv ().cl == rvc_zero || REAL_VALUE_ISINF (this->rv ()))
@@ -172,7 +172,7 @@ longdouble::mod (const longdouble &r) const
 longdouble
 longdouble::neg (void) const
 {
-  longdouble x;
+  longdouble x = {};
   real_arithmetic (&x.rv (), NEGATE_EXPR, &this->rv (), NULL);
   return x.normalize ();
 }
diff --git a/gcc/d/d-port.cc b/gcc/d/d-port.cc
index a1636fc6cf8..a66fabccf33 100644
--- a/gcc/d/d-port.cc
+++ b/gcc/d/d-port.cc
@@ -74,7 +74,7 @@ Port::strupr (char *s)
 bool
 Port::isFloat32LiteralOutOfRange (const char *buffer)
 {
-  real_t r;
+  real_t r = {};
 
   real_from_string3 (&r.rv (), buffer, TYPE_MODE (float_type_node));
 
@@ -87,7 +87,7 @@ Port::isFloat32LiteralOutOfRange (const char *buffer)
 bool
 Port::isFloat64LiteralOutOfRange (const char *buffer)
 {
-  real_t r;
+  real_t r = {};
 
   real_from_string3 (&r.rv (), buffer, TYPE_MODE (double_type_node));
 
diff --git a/gcc/testsuite/gdc.dg/pr116961.d b/gcc/testsuite/gdc.dg/pr116961.d
new file mode 100644
index 00000000000..fd51308636c
--- /dev/null
+++ b/gcc/testsuite/gdc.dg/pr116961.d
@@ -0,0 +1,7 @@
+// { dg-do compile }
+struct S116961
+{
+    float thing = 0.0;
+}
+
+static assert(__traits(isZeroInit, S116961) == true);
-- 
2.43.0

Reply via email to