Remove undefined behavior

* Taking the negative of a signed negative is UB, but
  taking the negative of an unsigned is not.
This commit is contained in:
Howard Hinnant
2022-09-15 11:31:15 -04:00
committed by Elliot Lee
parent d275a2ab72
commit 6fcd654bee
6 changed files with 255 additions and 183 deletions

View File

@@ -45,6 +45,12 @@ IOUAmount::minPositiveAmount()
void
IOUAmount::normalize()
{
if (mantissa_ == 0)
{
*this = beast::zero;
return;
}
if (*stNumberSwitchover)
{
Number v{mantissa_, exponent_};
@@ -56,11 +62,6 @@ IOUAmount::normalize()
*this = beast::zero;
return;
}
if (mantissa_ == 0)
{
*this = beast::zero;
return;
}
bool const negative = (mantissa_ < 0);
@@ -107,48 +108,46 @@ IOUAmount::IOUAmount(Number const& other)
IOUAmount&
IOUAmount::operator+=(IOUAmount const& other)
{
if (other == beast::zero)
return *this;
if (*this == beast::zero)
{
*this = other;
return *this;
}
if (*stNumberSwitchover)
{
*this = IOUAmount{Number{*this} + Number{other}};
return *this;
}
else
auto m = other.mantissa_;
auto e = other.exponent_;
while (exponent_ < e)
{
if (other == beast::zero)
return *this;
if (*this == beast::zero)
{
*this = other;
return *this;
}
auto m = other.mantissa_;
auto e = other.exponent_;
while (exponent_ < e)
{
mantissa_ /= 10;
++exponent_;
}
while (e < exponent_)
{
m /= 10;
++e;
}
// This addition cannot overflow an std::int64_t but we may throw from
// normalize if the result isn't representable.
mantissa_ += m;
if (mantissa_ >= -10 && mantissa_ <= 10)
{
*this = beast::zero;
return *this;
}
normalize();
mantissa_ /= 10;
++exponent_;
}
while (e < exponent_)
{
m /= 10;
++e;
}
// This addition cannot overflow an std::int64_t but we may throw from
// normalize if the result isn't representable.
mantissa_ += m;
if (mantissa_ >= -10 && mantissa_ <= 10)
{
*this = beast::zero;
return *this;
}
normalize();
return *this;
}

View File

@@ -25,7 +25,7 @@
#include <type_traits>
#include <utility>
#ifdef _MSVC_LANG
#ifdef BOOST_COMP_MSVC
#include <boost/multiprecision/cpp_int.hpp>
using uint128_t = boost::multiprecision::uint128_t;
#else // !defined(_MSVC_LANG)
@@ -130,34 +130,37 @@ int
Number::Guard::round() noexcept
{
auto mode = Number::getround();
switch (mode)
if (mode == towards_zero)
return -1;
if (mode == downward)
{
// round to nearest if mode is not one of the predefined values
default:
case to_nearest:
if (digits_ > 0x5000'0000'0000'0000)
return 1;
if (digits_ < 0x5000'0000'0000'0000)
return -1;
if (xbit_)
return 1;
return 0;
case towards_zero:
return -1;
case downward:
if (sbit_)
{
if (digits_ > 0 || xbit_)
return 1;
}
return -1;
case upward:
if (sbit_)
return -1;
if (sbit_)
{
if (digits_ > 0 || xbit_)
return 1;
return -1;
}
return -1;
}
if (mode == upward)
{
if (sbit_)
return -1;
if (digits_ > 0 || xbit_)
return 1;
return -1;
}
// assume round to nearest if mode is not one of the predefined values
if (digits_ > 0x5000'0000'0000'0000)
return 1;
if (digits_ < 0x5000'0000'0000'0000)
return -1;
if (xbit_)
return 1;
return 0;
}
// Number
@@ -173,9 +176,9 @@ Number::normalize()
return;
}
bool const negative = (mantissa_ < 0);
if (negative)
mantissa_ = -mantissa_;
auto m = static_cast<std::make_unsigned_t<rep>>(mantissa_);
if (negative)
m = -m;
while ((m < minMantissa) && (exponent_ > minExponent))
{
m *= 10;

View File

@@ -2092,37 +2092,53 @@ public:
using namespace jtx;
Env env{*this, features};
for (auto NumberSwitchOver : {false, true})
{
Env env{*this, features};
if (NumberSwitchOver)
env.enableFeature(fixUniversalNumber);
else
env.disableFeature(fixUniversalNumber);
auto const gw = Account{"gateway"};
auto const alice = Account{"alice"};
auto const bob = Account{"bob"};
auto const USD = gw["USD"];
auto const gw = Account{"gateway"};
auto const alice = Account{"alice"};
auto const bob = Account{"bob"};
auto const USD = gw["USD"];
env.fund(XRP(10000), gw, alice, bob);
env.fund(XRP(10000), gw, alice, bob);
env(rate(gw, 1.005));
env(rate(gw, 1.005));
env(trust(alice, USD(1000)));
env(trust(bob, USD(1000)));
env(trust(gw, alice["USD"](50)));
env(trust(alice, USD(1000)));
env(trust(bob, USD(1000)));
env(trust(gw, alice["USD"](50)));
env(pay(gw, bob, bob["USD"](1)));
env(pay(alice, gw, USD(50)));
env(pay(gw, bob, bob["USD"](1)));
env(pay(alice, gw, USD(50)));
env(trust(gw, alice["USD"](0)));
env(trust(gw, alice["USD"](0)));
env(offer(alice, USD(50), XRP(150000)));
env(offer(bob, XRP(100), USD(0.1)));
env(offer(alice, USD(50), XRP(150000)));
env(offer(bob, XRP(100), USD(0.1)));
auto jrr = ledgerEntryState(env, alice, gw, "USD");
BEAST_EXPECT(
jrr[jss::node][sfBalance.fieldName][jss::value] ==
"49.96666666666667");
jrr = ledgerEntryState(env, bob, gw, "USD");
BEAST_EXPECT(
jrr[jss::node][sfBalance.fieldName][jss::value] ==
"-0.9665000000333333");
auto jrr = ledgerEntryState(env, alice, gw, "USD");
BEAST_EXPECT(
jrr[jss::node][sfBalance.fieldName][jss::value] ==
"49.96666666666667");
jrr = ledgerEntryState(env, bob, gw, "USD");
if (NumberSwitchOver)
{
BEAST_EXPECT(
jrr[jss::node][sfBalance.fieldName][jss::value] ==
"-0.9665000000333333");
}
else
{
BEAST_EXPECT(
jrr[jss::node][sfBalance.fieldName][jss::value] ==
"-0.966500000033334");
}
}
}
void

View File

@@ -905,100 +905,143 @@ public:
{
testcase("IOU to IOU");
NumberSO stNumberSO{true};
Quality q1 = get_quality("1", "1");
for (auto NumberSwitchOver : {false, true})
{
NumberSO stNumberSO{NumberSwitchOver};
Quality q1 = get_quality("1", "1");
// Highly exaggerated 50% transfer rate for the input and output:
Rate const rate{parityRate.value + (parityRate.value / 2)};
// Highly exaggerated 50% transfer rate for the input and output:
Rate const rate{parityRate.value + (parityRate.value / 2)};
// TAKER OWNER
// QUAL OFFER FUNDS QUAL OFFER FUNDS
// EXPECTED
// EUR USD
attempt(
Sell,
"N:N",
q1,
{"2", "2"},
"10",
q1,
{"2", "2"},
"10",
{"2", "2"},
eur(),
usd(),
rate,
rate);
attempt(
Sell,
"N:B",
q1,
{"4", "4"},
"10",
q1,
{"4", "4"},
"4",
{"2.666666666666667", "2.666666666666667"},
eur(),
usd(),
rate,
rate);
attempt(
Buy,
"N:T",
q1,
{"1", "1"},
"10",
q1,
{"2", "2"},
"10",
{"1", "1"},
eur(),
usd(),
rate,
rate);
attempt(
Buy,
"N:BT",
q1,
{"2", "2"},
"10",
q1,
{"6", "6"},
"5",
{"2", "2"},
eur(),
usd(),
rate,
rate);
attempt(
Buy,
"N:TB",
q1,
{"2", "2"},
"2",
q1,
{"6", "6"},
"1",
{"0.6666666666666667", "0.6666666666666667"},
eur(),
usd(),
rate,
rate);
attempt(
Sell,
"A:N",
q1,
{"2", "2"},
"2.5",
q1,
{"2", "2"},
"10",
{"1.666666666666667", "1.666666666666667"},
eur(),
usd(),
rate,
rate);
// TAKER OWNER
// QUAL OFFER FUNDS QUAL OFFER FUNDS
// EXPECTED
// EUR USD
attempt(
Sell,
"N:N",
q1,
{"2", "2"},
"10",
q1,
{"2", "2"},
"10",
{"2", "2"},
eur(),
usd(),
rate,
rate);
if (NumberSwitchOver)
{
attempt(
Sell,
"N:B",
q1,
{"4", "4"},
"10",
q1,
{"4", "4"},
"4",
{"2.666666666666667", "2.666666666666667"},
eur(),
usd(),
rate,
rate);
}
else
{
attempt(
Sell,
"N:B",
q1,
{"4", "4"},
"10",
q1,
{"4", "4"},
"4",
{"2.666666666666666", "2.666666666666666"},
eur(),
usd(),
rate,
rate);
}
attempt(
Buy,
"N:T",
q1,
{"1", "1"},
"10",
q1,
{"2", "2"},
"10",
{"1", "1"},
eur(),
usd(),
rate,
rate);
attempt(
Buy,
"N:BT",
q1,
{"2", "2"},
"10",
q1,
{"6", "6"},
"5",
{"2", "2"},
eur(),
usd(),
rate,
rate);
attempt(
Buy,
"N:TB",
q1,
{"2", "2"},
"2",
q1,
{"6", "6"},
"1",
{"0.6666666666666667", "0.6666666666666667"},
eur(),
usd(),
rate,
rate);
if (NumberSwitchOver)
{
attempt(
Sell,
"A:N",
q1,
{"2", "2"},
"2.5",
q1,
{"2", "2"},
"10",
{"1.666666666666667", "1.666666666666667"},
eur(),
usd(),
rate,
rate);
}
else
{
attempt(
Sell,
"A:N",
q1,
{"2", "2"},
"2.5",
q1,
{"2", "2"},
"10",
{"1.666666666666666", "1.666666666666666"},
eur(),
usd(),
rate,
rate);
}
}
}
void

View File

@@ -543,6 +543,9 @@ public:
void
enableFeature(uint256 const feature);
void
disableFeature(uint256 const feature);
private:
void
fund(bool setDefaultRipple, STAmount const& amount, Account const& account);

View File

@@ -466,6 +466,14 @@ Env::enableFeature(uint256 const feature)
app().config().features.insert(feature);
}
void
Env::disableFeature(uint256 const feature)
{
// Env::close() must be called for feature
// enable to take place.
app().config().features.erase(feature);
}
} // namespace jtx
} // namespace test