Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ensure that switchover vars are initialized before use: #4527

Merged
merged 1 commit into from
May 23, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 11 additions & 4 deletions src/ripple/basics/IOUAmount.h
Original file line number Diff line number Diff line change
Expand Up @@ -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.
*/
Expand All @@ -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);
}
};

Expand Down
27 changes: 24 additions & 3 deletions src/ripple/basics/impl/IOUAmount.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -51,7 +72,7 @@ IOUAmount::normalize()
return;
}

if (*stNumberSwitchover)
if (getSTNumberSwitchover())
{
Number const v{mantissa_, exponent_};
mantissa_ = v.mantissa();
Expand Down Expand Up @@ -117,7 +138,7 @@ IOUAmount::operator+=(IOUAmount const& other)
return *this;
}

if (*stNumberSwitchover)
if (getSTNumberSwitchover())
{
*this = IOUAmount{Number{*this} + Number{other}};
return *this;
Expand Down
13 changes: 9 additions & 4 deletions src/ripple/protocol/STAmount.h
Original file line number Diff line number Diff line change
Expand Up @@ -536,22 +536,27 @@ 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.
*/

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:
Expand Down
35 changes: 28 additions & 7 deletions src/ripple/protocol/impl/STAmount.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -717,15 +738,15 @@ STAmount::canonicalize()
return;
}

if (*stAmountCanonicalizeSwitchover)
if (getSTAmountCanonicalizeSwitchover())
{
// log(cMaxNativeN, 10) == 17
if (mOffset > 17)
Throw<std::runtime_error>(
"Native currency amount out of range");
}

if (*stNumberSwitchover && *stAmountCanonicalizeSwitchover)
if (getSTNumberSwitchover() && getSTAmountCanonicalizeSwitchover())
{
Number num(
mIsNegative ? -mValue : mValue, mOffset, Number::unchecked{});
Expand All @@ -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
Expand All @@ -765,7 +786,7 @@ STAmount::canonicalize()

mIsNative = false;

if (*stNumberSwitchover)
if (getSTNumberSwitchover())
{
*this = iou();
return;
Expand Down Expand Up @@ -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();
Expand Down