Ensure that switchover vars are initialized before use: (#4527)

Global variables in different TUs are initialized in an undefined order.
At least one global variable was accessing a global switchover variable.
This caused the switchover variable to be accessed in an uninitialized
state.

Since the switchover is always explicitly set before transaction
processing, this bug can not effect transaction processing, but could
effect unit tests (and potentially the value of some global variables).
Note: at the time of this patch the offending bug is not yet in
production.

src/ripple/basics/IOUAmount.h

@@ -186,7 +186,14 @@ mulRatio(
     std::uint32_t den,
     bool roundUp);
 
-extern LocalValue<bool> stNumberSwitchover;
+// Since many uses of the number class do not have access to a ledger,
+// getSTNumberSwitchover needs to be globally accessible.
+
+bool
+getSTNumberSwitchover();
+
+void
+setSTNumberSwitchover(bool v);
 
 /** RAII class to set and restore the Number switchover.
  */
@@ -198,16 +205,16 @@ class NumberSO
 public:
     ~NumberSO()
     {
-        *stNumberSwitchover = saved_;
+        setSTNumberSwitchover(saved_);
     }
 
     NumberSO(NumberSO const&) = delete;
     NumberSO&
     operator=(NumberSO const&) = delete;
 
-    explicit NumberSO(bool v) : saved_(*stNumberSwitchover)
+    explicit NumberSO(bool v) : saved_(getSTNumberSwitchover())
     {
-        *stNumberSwitchover = v;
+        setSTNumberSwitchover(v);
     }
 };
 

src/ripple/basics/impl/IOUAmount.cpp

@@ -27,7 +27,28 @@
 
 namespace ripple {
 
-LocalValue<bool> stNumberSwitchover(true);
+namespace {
+
+// Use a static inside a function to help prevent order-of-initialzation issues
+LocalValue<bool>&
+getStaticSTNumberSwitchover()
+{
+    static LocalValue<bool> r{true};
+    return r;
+}
+}  // namespace
+
+bool
+getSTNumberSwitchover()
+{
+    return *getStaticSTNumberSwitchover();
+}
+
+void
+setSTNumberSwitchover(bool v)
+{
+    *getStaticSTNumberSwitchover() = v;
+}
 
 /* The range for the mantissa when normalized */
 static std::int64_t constexpr minMantissa = 1000000000000000ull;
@@ -51,7 +72,7 @@ IOUAmount::normalize()
         return;
     }
 
-    if (*stNumberSwitchover)
+    if (getSTNumberSwitchover())
     {
         Number const v{mantissa_, exponent_};
         mantissa_ = v.mantissa();
@@ -117,7 +138,7 @@ IOUAmount::operator+=(IOUAmount const& other)
         return *this;
     }
 
-    if (*stNumberSwitchover)
+    if (getSTNumberSwitchover())
     {
         *this = IOUAmount{Number{*this} + Number{other}};
         return *this;

src/ripple/protocol/STAmount.h

@@ -536,7 +536,12 @@ isXRP(STAmount const& amount)
 // the low-level routine stAmountCanonicalize on an amendment switch. Only
 // transactions need to use this switchover. Outside of a transaction it's safe
 // to unconditionally use the new behavior.
-extern LocalValue<bool> stAmountCanonicalizeSwitchover;
+
+bool
+getSTAmountCanonicalizeSwitchover();
+
+void
+setSTAmountCanonicalizeSwitchover(bool v);
 
 /** RAII class to set and restore the STAmount canonicalize switchover.
  */
@@ -544,14 +549,14 @@ extern LocalValue<bool> stAmountCanonicalizeSwitchover;
 class STAmountSO
 {
 public:
-    explicit STAmountSO(bool v) : saved_(*stAmountCanonicalizeSwitchover)
+    explicit STAmountSO(bool v) : saved_(getSTAmountCanonicalizeSwitchover())
     {
-        *stAmountCanonicalizeSwitchover = v;
+        setSTAmountCanonicalizeSwitchover(v);
     }
 
     ~STAmountSO()
     {
-        *stAmountCanonicalizeSwitchover = saved_;
+        setSTAmountCanonicalizeSwitchover(saved_);
     }
 
 private:

src/ripple/protocol/impl/STAmount.cpp

@@ -34,7 +34,28 @@
 
 namespace ripple {
 
-LocalValue<bool> stAmountCanonicalizeSwitchover(true);
+namespace {
+
+// Use a static inside a function to help prevent order-of-initialzation issues
+LocalValue<bool>&
+getStaticSTAmountCanonicalizeSwitchover()
+{
+    static LocalValue<bool> r{true};
+    return r;
+}
+}  // namespace
+
+bool
+getSTAmountCanonicalizeSwitchover()
+{
+    return *getStaticSTAmountCanonicalizeSwitchover();
+}
+
+void
+setSTAmountCanonicalizeSwitchover(bool v)
+{
+    *getStaticSTAmountCanonicalizeSwitchover() = v;
+}
 
 static const std::uint64_t tenTo14 = 100000000000000ull;
 static const std::uint64_t tenTo14m1 = tenTo14 - 1;
@@ -395,7 +416,7 @@ operator+(STAmount const& v1, STAmount const& v2)
     if (v1.native())
         return {v1.getFName(), getSNValue(v1) + getSNValue(v2)};
 
-    if (*stNumberSwitchover)
+    if (getSTNumberSwitchover())
     {
         auto x = v1;
         x = v1.iou() + v2.iou();
@@ -717,7 +738,7 @@ STAmount::canonicalize()
             return;
         }
 
-        if (*stAmountCanonicalizeSwitchover)
+        if (getSTAmountCanonicalizeSwitchover())
         {
             // log(cMaxNativeN, 10) == 17
             if (mOffset > 17)
@@ -725,7 +746,7 @@ STAmount::canonicalize()
                     "Native currency amount out of range");
         }
 
-        if (*stNumberSwitchover && *stAmountCanonicalizeSwitchover)
+        if (getSTNumberSwitchover() && getSTAmountCanonicalizeSwitchover())
         {
             Number num(
                 mIsNegative ? -mValue : mValue, mOffset, Number::unchecked{});
@@ -744,7 +765,7 @@ STAmount::canonicalize()
 
             while (mOffset > 0)
             {
-                if (*stAmountCanonicalizeSwitchover)
+                if (getSTAmountCanonicalizeSwitchover())
                 {
                     // N.B. do not move the overflow check to after the
                     // multiplication
@@ -765,7 +786,7 @@ STAmount::canonicalize()
 
     mIsNative = false;
 
-    if (*stNumberSwitchover)
+    if (getSTNumberSwitchover())
     {
         *this = iou();
         return;
@@ -1208,7 +1229,7 @@ multiply(STAmount const& v1, STAmount const& v2, Issue const& issue)
         return STAmount(v1.getFName(), minV * maxV);
     }
 
-    if (*stNumberSwitchover)
+    if (getSTNumberSwitchover())
         return {IOUAmount{Number{v1} * Number{v2}}, issue};
 
     std::uint64_t value1 = v1.mantissa();