[APFloat] Fix uninitialized variable in IEEEFloat constructors

[nullptr.cpp]

Summary

Some constructors of IEEEFloat do not initialize member variable exponent.
Fix it by initializing exponent with the following values:

For NaNs, the `exponent` is `maxExponent+1`.
For Infinities, the `exponent` is `maxExponent+1`.
For Zeroes, the `exponent` is `maxExponent-1`.

[craig.topper]

What problem does this cause? IEEEFloat::initFromFloatAPInt which is called for IEEEFloat::IEEEFloat(float f) also doesn't assign exponent from what I can see.

[nullptr.cpp]

In D86997#2251233, @craig.topper wrote:

What problem does this cause? IEEEFloat::initFromFloatAPInt which is called for IEEEFloat::IEEEFloat(float f) also doesn't assign exponent from what I can see.

I found this by a static analyzer checker, I’m just worried if an uninitialized variable is used somewhere before initialization.
Should we initialize it or just leave it as it is?

[RKSimon]

https://llvm.org/reports/scan-build/ lists a large number of APFloat warnings - it may be possible to create test cases from one of those analysis paths.

[lattner]

This patch LGTM. I think that conservatively init the members is a good idea.

[nullptr.cpp]

Also fix other constructors.

[lattner]

LGTM, thanks for doing this.

[nullptr.cpp]

I don't have commit access, can anyone help commit this?
Yang Fan <[e-mail]>
Thanks!

[RKSimon]

In D86997#2255580, @nullptr.cpp wrote:

I don't have commit access, can anyone help commit this?
Yang Fan <[e-mail]>
Thanks!

Thanks for looking at this @nullptr.cpp - I'll merge this later today

llvm/include/llvm/ADT/APFloat.h

 
 namespace detail {
 
 class IEEEFloat final : public APFloatBase {
 public:
   /// \name Constructors
   /// @{
 
-  IEEEFloat(const fltSemantics &); // Default construct to 0.0
+  IEEEFloat(const fltSemantics &); // Default construct to +0.0
   IEEEFloat(const fltSemantics &, integerPart);
   IEEEFloat(const fltSemantics &, uninitializedTag);
   IEEEFloat(const fltSemantics &, const APInt &);
   explicit IEEEFloat(double d);
   explicit IEEEFloat(float f);
   IEEEFloat(const IEEEFloat &);
   IEEEFloat(IEEEFloat &&);
   ~IEEEFloat();
private:
   opStatus convertFromUnsignedParts(const integerPart *, unsigned int,
                                     roundingMode);
   Expected<opStatus> convertFromHexadecimalString(StringRef, roundingMode);
   Expected<opStatus> convertFromDecimalString(StringRef, roundingMode);
   char *convertNormalToHexString(char *, unsigned int, bool,
                                  roundingMode) const;
   opStatus roundSignificandWithExponent(const integerPart *, unsigned int, int,
                                         roundingMode);
+  ExponentType exponentNaN() const;
+  ExponentType exponentInf() const;
+  ExponentType exponentZero() const;
 
   /// @}
 
   APInt convertHalfAPFloatToAPInt() const;
   APInt convertBFloatAPFloatToAPInt() const;
   APInt convertFloatAPFloatToAPInt() const;
   APInt convertDoubleAPFloatToAPInt() const;
   APInt convertQuadrupleAPFloatToAPInt() const;

llvm/lib/Support/APFloat.cpp

 }
 
 /* Make this number a NaN, with an arbitrary but deterministic value
    for the significand.  If double or longer, this is a signalling NaN,
    which may not be ideal.  If float, this is QNaN(0).  */
 void IEEEFloat::makeNaN(bool SNaN, bool Negative, const APInt *fill) {
   category = fcNaN;
   sign = Negative;
+  exponent = exponentNaN();
 
   integerPart *significand = significandParts();
   unsigned numParts = partCount();
 
   // Set the significand bits to the fill.
   if (!fill || fill->getNumWords() < numParts)
     APInt::tcSet(significand, 0, numParts);
   if (fill) {
IEEEFloat::IEEEFloat(const fltSemantics &ourSemantics, integerPart value) {
   zeroSignificand();
   exponent = ourSemantics.precision - 1;
   significandParts()[0] = value;
   normalize(rmNearestTiesToEven, lfExactlyZero);
 }
 
 IEEEFloat::IEEEFloat(const fltSemantics &ourSemantics) {
   initialize(&ourSemantics);
-  category = fcZero;
-  sign = false;
+  makeZero(false);
 }
 
 // Delegate to the previous constructor, because later copy constructor may
 // actually inspects category, which can't be garbage.
 IEEEFloat::IEEEFloat(const fltSemantics &ourSemantics, uninitializedTag tag)
     : IEEEFloat(ourSemantics) {}
 
 IEEEFloat::IEEEFloat(const IEEEFloat &rhs) {
void IEEEFloat::initFromF80LongDoubleAPInt(const APInt &api) {
   uint64_t mysignificand = i1;
   uint8_t myintegerbit = mysignificand >> 63;
 
   initialize(&semX87DoubleExtended);
   assert(partCount()==2);
 
   sign = static_cast<unsigned int>(i2>>15);
   if (myexponent == 0 && mysignificand == 0) {
-    // exponent, significand meaningless
-    category = fcZero;
+    makeZero(sign);
   } else if (myexponent==0x7fff && mysignificand==0x8000000000000000ULL) {
-    // exponent, significand meaningless
-    category = fcInfinity;
+    makeInf(sign);
   } else if ((myexponent == 0x7fff && mysignificand != 0x8000000000000000ULL) ||
              (myexponent != 0x7fff && myexponent != 0 && myintegerbit == 0)) {
-    // exponent meaningless
     category = fcNaN;
+    exponent = exponentNaN();
     significandParts()[0] = mysignificand;
     significandParts()[1] = 0;
   } else {
     category = fcNormal;
     exponent = myexponent - 16383;
     significandParts()[0] = mysignificand;
     significandParts()[1] = 0;
     if (myexponent==0)          // denormal
void IEEEFloat::initFromQuadrupleAPInt(const APInt &api) {
   uint64_t mysignificand2 = i2 & 0xffffffffffffLL;
 
   initialize(&semIEEEquad);
   assert(partCount()==2);
 
   sign = static_cast<unsigned int>(i2>>63);
   if (myexponent==0 &&
       (mysignificand==0 && mysignificand2==0)) {
-    // exponent, significand meaningless
-    category = fcZero;
+    makeZero(sign);
   } else if (myexponent==0x7fff &&
              (mysignificand==0 && mysignificand2==0)) {
-    // exponent, significand meaningless
-    category = fcInfinity;
+    makeInf(sign);
   } else if (myexponent==0x7fff &&
              (mysignificand!=0 || mysignificand2 !=0)) {
-    // exponent meaningless
     category = fcNaN;
+    exponent = exponentNaN();
     significandParts()[0] = mysignificand;
     significandParts()[1] = mysignificand2;
   } else {
     category = fcNormal;
     exponent = myexponent - 16383;
     significandParts()[0] = mysignificand;
     significandParts()[1] = mysignificand2;
     if (myexponent==0)          // denormal
void IEEEFloat::initFromDoubleAPInt(const APInt &api) {
   uint64_t myexponent = (i >> 52) & 0x7ff;
   uint64_t mysignificand = i & 0xfffffffffffffLL;
 
   initialize(&semIEEEdouble);
   assert(partCount()==1);
 
   sign = static_cast<unsigned int>(i>>63);
   if (myexponent==0 && mysignificand==0) {
-    // exponent, significand meaningless
-    category = fcZero;
+    makeZero(sign);
   } else if (myexponent==0x7ff && mysignificand==0) {
-    // exponent, significand meaningless
-    category = fcInfinity;
+    makeInf(sign);
   } else if (myexponent==0x7ff && mysignificand!=0) {
-    // exponent meaningless
     category = fcNaN;
+    exponent = exponentNaN();
     *significandParts() = mysignificand;
   } else {
     category = fcNormal;
     exponent = myexponent - 1023;
     *significandParts() = mysignificand;
     if (myexponent==0)          // denormal
       exponent = -1022;
     else
       *significandParts() |= 0x10000000000000LL;  // integer bit
   }
 }
 
 void IEEEFloat::initFromFloatAPInt(const APInt &api) {
   assert(api.getBitWidth()==32);
   uint32_t i = (uint32_t)*api.getRawData();
   uint32_t myexponent = (i >> 23) & 0xff;
   uint32_t mysignificand = i & 0x7fffff;
 
   initialize(&semIEEEsingle);
   assert(partCount()==1);
 
   sign = i >> 31;
   if (myexponent==0 && mysignificand==0) {
-    // exponent, significand meaningless
-    category = fcZero;
+    makeZero(sign);
   } else if (myexponent==0xff && mysignificand==0) {
-    // exponent, significand meaningless
-    category = fcInfinity;
+    makeInf(sign);
   } else if (myexponent==0xff && mysignificand!=0) {
-    // sign, exponent, significand meaningless
     category = fcNaN;
+    exponent = exponentNaN();
     *significandParts() = mysignificand;
   } else {
     category = fcNormal;
     exponent = myexponent - 127;  //bias
     *significandParts() = mysignificand;
     if (myexponent==0)    // denormal
       exponent = -126;
     else
       *significandParts() |= 0x800000; // integer bit
   }
 }
 
 void IEEEFloat::initFromBFloatAPInt(const APInt &api) {
   assert(api.getBitWidth() == 16);
   uint32_t i = (uint32_t)*api.getRawData();
   uint32_t myexponent = (i >> 7) & 0xff;
   uint32_t mysignificand = i & 0x7f;
 
   initialize(&semBFloat);
   assert(partCount() == 1);
 
   sign = i >> 15;
   if (myexponent == 0 && mysignificand == 0) {
-    // exponent, significand meaningless
-    category = fcZero;
+    makeZero(sign);
   } else if (myexponent == 0xff && mysignificand == 0) {
-    // exponent, significand meaningless
-    category = fcInfinity;
+    makeInf(sign);
   } else if (myexponent == 0xff && mysignificand != 0) {
-    // sign, exponent, significand meaningless
     category = fcNaN;
+    exponent = exponentNaN();
     *significandParts() = mysignificand;
   } else {
     category = fcNormal;
     exponent = myexponent - 127; // bias
     *significandParts() = mysignificand;
     if (myexponent == 0) // denormal
       exponent = -126;
     else
       *significandParts() |= 0x80; // integer bit
   }
 }
 
 void IEEEFloat::initFromHalfAPInt(const APInt &api) {
   assert(api.getBitWidth()==16);
   uint32_t i = (uint32_t)*api.getRawData();
   uint32_t myexponent = (i >> 10) & 0x1f;
   uint32_t mysignificand = i & 0x3ff;
 
   initialize(&semIEEEhalf);
   assert(partCount()==1);
 
   sign = i >> 15;
   if (myexponent==0 && mysignificand==0) {
-    // exponent, significand meaningless
-    category = fcZero;
+    makeZero(sign);
   } else if (myexponent==0x1f && mysignificand==0) {
-    // exponent, significand meaningless
-    category = fcInfinity;
+    makeInf(sign);
   } else if (myexponent==0x1f && mysignificand!=0) {
-    // sign, exponent, significand meaningless
     category = fcNaN;
+    exponent = exponentNaN();
     *significandParts() = mysignificand;
   } else {
     category = fcNormal;
     exponent = myexponent - 15;  //bias
     *significandParts() = mysignificand;
     if (myexponent==0)    // denormal
       exponent = -14;
     else
IEEEFloat::opStatus IEEEFloat::next(bool nextDown) {
 
   // If we are performing nextDown, swap sign so we have -nextUp(-x)
   if (nextDown)
     changeSign();
 
   return result;
 }
 
+APFloatBase::ExponentType IEEEFloat::exponentNaN() const {
+  return semantics->maxExponent + 1;
+}
+
+APFloatBase::ExponentType IEEEFloat::exponentInf() const {
+  return semantics->maxExponent + 1;
+}
+
+APFloatBase::ExponentType IEEEFloat::exponentZero() const {
+  return semantics->minExponent - 1;
+}
+
 void IEEEFloat::makeInf(bool Negative) {
   category = fcInfinity;
   sign = Negative;
-  exponent = semantics->maxExponent + 1;
+  exponent = exponentInf();
   APInt::tcSet(significandParts(), 0, partCount());
 }
 
 void IEEEFloat::makeZero(bool Negative) {
   category = fcZero;
   sign = Negative;
-  exponent = semantics->minExponent-1;
+  exponent = exponentZero();
   APInt::tcSet(significandParts(), 0, partCount());
 }
 
 void IEEEFloat::makeQuiet() {
   assert(isNaN());
   APInt::tcSetBit(significandParts(), semantics->precision - 2);
 }