more fixes some reverts

Signed-off-by: Pratik Mankawde <3397372+pratikmankawde@users.noreply.github.com>
This commit is contained in:
Pratik Mankawde
2026-03-24 14:59:53 +00:00
parent 6468412621
commit 09ce8125a0
7 changed files with 49 additions and 66 deletions

View File

@@ -127,8 +127,7 @@ IOUAmount::operator-=(IOUAmount const& other)
inline IOUAmount
IOUAmount::operator-() const
{
// Negate in unsigned domain to avoid UB when mantissa_ == INT64_MIN
return {static_cast<mantissa_type>(-static_cast<std::uint64_t>(mantissa_)), exponent_};
return {-mantissa_, exponent_};
}
inline bool

View File

@@ -112,8 +112,7 @@ public:
XRPAmount
operator-() const
{
// Negate in unsigned domain to avoid UB when drops_ == INT64_MIN
return XRPAmount{static_cast<value_type>(-static_cast<std::uint64_t>(drops_))};
return XRPAmount{-drops_};
}
bool

View File

@@ -125,8 +125,19 @@ unsigned-integer-overflow:__chrono/duration.h
# Rippled code suppressions
# =============================================================================
# STAmount::operator+ uses unsigned domain for addition to avoid signed overflow
unsigned-integer-overflow:operator+*STAmount*
# Signed integer negation (-value) in amount types.
# INT64_MIN cannot occur in practice due to domain invariants (mantissa ranges
# are well within int64_t bounds), but UBSan flags the pattern as potential
# signed overflow.
signed-integer-overflow:IOUAmount
signed-integer-overflow:XRPAmount
signed-integer-overflow:MPTAmount
signed-integer-overflow:STAmount
# STAmount::operator+ signed addition — operands are bounded by total supply
# (~10^17 for XRP, ~10^18 for MPT) so overflow cannot occur in practice.
signed-integer-overflow:operator+*STAmount*
# STAmount::getRate uses unsigned shift and addition
unsigned-integer-overflow:getRate*
# STAmount::serialize uses unsigned bitwise operations

View File

@@ -91,8 +91,7 @@ IOUAmount::normalize()
bool const negative = (mantissa_ < 0);
if (negative)
// Negate in unsigned domain to avoid UB when mantissa_ == INT64_MIN
mantissa_ = static_cast<mantissa_type>(-static_cast<std::uint64_t>(mantissa_));
mantissa_ = -mantissa_;
while ((mantissa_ < minMantissa) && (exponent_ > minExponent))
{
@@ -119,8 +118,7 @@ IOUAmount::normalize()
Throw<std::overflow_error>("value overflow");
if (negative)
// Negate back in unsigned domain to restore sign
mantissa_ = static_cast<mantissa_type>(-static_cast<std::uint64_t>(mantissa_));
mantissa_ = -mantissa_;
}
IOUAmount::IOUAmount(Number const& other) : IOUAmount(fromNumber(other))

View File

@@ -19,8 +19,7 @@ MPTAmount::operator-=(MPTAmount const& other)
MPTAmount
MPTAmount::operator-() const
{
// Negate in unsigned domain to avoid UB when value_ == INT64_MIN
return MPTAmount{static_cast<value_type>(-static_cast<std::uint64_t>(value_))};
return MPTAmount{-value_};
}
bool

View File

@@ -63,18 +63,16 @@ getInt64Value(STAmount const& amount, bool valid, char const* error)
Throw<std::runtime_error>(error);
XRPL_ASSERT(amount.exponent() == 0, "xrpl::getInt64Value : exponent is zero");
// Verify mantissa fits in int64_t (roundtrip check)
auto ret = static_cast<std::int64_t>(amount.mantissa());
XRPL_ASSERT(
static_cast<std::uint64_t>(static_cast<std::int64_t>(amount.mantissa())) ==
amount.mantissa(),
static_cast<std::uint64_t>(ret) == amount.mantissa(),
"xrpl::getInt64Value : mantissa must roundtrip");
// Negate in unsigned domain then cast to signed to avoid undefined
// behavior when mantissa would represent INT64_MIN as signed
if (amount.negative())
return static_cast<std::int64_t>(-amount.mantissa());
ret = -ret;
return static_cast<std::int64_t>(amount.mantissa());
return ret;
}
static std::int64_t
@@ -224,12 +222,14 @@ STAmount::STAmount(std::uint64_t mantissa, bool negative)
STAmount::STAmount(XRPAmount const& amount)
: mAsset(xrpIssue()), mOffset(0), mIsNegative(amount < beast::zero)
{
// Negate in unsigned domain to avoid undefined behavior when
// amount.drops() == INT64_MIN
if (mIsNegative)
mValue = -static_cast<std::uint64_t>(amount.drops());
{
mValue = unsafe_cast<std::uint64_t>(-amount.drops());
}
else
mValue = static_cast<std::uint64_t>(amount.drops());
{
mValue = unsafe_cast<std::uint64_t>(amount.drops());
}
canonicalize();
}
@@ -263,12 +263,11 @@ STAmount::xrp() const
if (!native())
Throw<std::logic_error>("Cannot return non-native STAmount as XRPAmount");
auto drops = static_cast<XRPAmount::value_type>(mValue);
XRPL_ASSERT(mOffset == 0, "xrpl::STAmount::xrp : amount is canonical");
// Cast to unsigned, negate if needed, then cast to signed to avoid
// undefined behavior when mValue would represent INT64_MIN as signed
auto drops = mIsNegative ? static_cast<XRPAmount::value_type>(-mValue)
: static_cast<XRPAmount::value_type>(mValue);
if (mIsNegative)
drops = -drops;
return XRPAmount{drops};
}
@@ -279,12 +278,11 @@ STAmount::iou() const
if (integral())
Throw<std::logic_error>("Cannot return non-IOU STAmount as IOUAmount");
auto mantissa = static_cast<std::int64_t>(mValue);
auto exponent = mOffset;
// Negate in unsigned domain then cast to signed to avoid undefined
// behavior when mValue would represent INT64_MIN as signed
auto mantissa =
mIsNegative ? static_cast<std::int64_t>(-mValue) : static_cast<std::int64_t>(mValue);
if (mIsNegative)
mantissa = -mantissa;
return {mantissa, exponent};
}
@@ -295,12 +293,11 @@ STAmount::mpt() const
if (!holds<MPTIssue>())
Throw<std::logic_error>("Cannot return STAmount as MPTAmount");
auto value = static_cast<MPTAmount::value_type>(mValue);
XRPL_ASSERT(mOffset == 0, "xrpl::STAmount::mpt : amount is canonical");
// Negate in unsigned domain then cast to signed to avoid undefined
// behavior when mValue would represent INT64_MIN as signed
auto value = mIsNegative ? static_cast<MPTAmount::value_type>(-mValue)
: static_cast<MPTAmount::value_type>(mValue);
if (mIsNegative)
value = -value;
return MPTAmount{value};
}
@@ -311,9 +308,10 @@ STAmount::operator=(IOUAmount const& iou)
XRPL_ASSERT(integral() == false, "xrpl::STAmount::operator=(IOUAmount) : is not integral");
mOffset = iou.exponent();
mIsNegative = iou < beast::zero;
// Negate in unsigned domain to avoid UB when mantissa == INT64_MIN
if (mIsNegative)
{
mValue = static_cast<std::uint64_t>(-iou.mantissa());
}
else
{
mValue = static_cast<std::uint64_t>(iou.mantissa());
@@ -333,9 +331,7 @@ STAmount::operator=(Number const& number)
{
auto const originalMantissa = number.mantissa();
mIsNegative = originalMantissa < 0;
// Negate in unsigned domain to avoid UB when mantissa == INT64_MIN
mValue = mIsNegative ? -static_cast<std::uint64_t>(originalMantissa)
: static_cast<std::uint64_t>(originalMantissa);
mValue = mIsNegative ? -originalMantissa : originalMantissa;
mOffset = number.exponent();
}
canonicalize();
@@ -378,22 +374,9 @@ operator+(STAmount const& v1, STAmount const& v2)
}
if (v1.native())
{
// Perform addition in unsigned domain to avoid signed overflow UB,
// then cast back to signed for the STAmount constructor
auto const sum = static_cast<std::int64_t>(
static_cast<std::uint64_t>(getSNValue(v1)) +
static_cast<std::uint64_t>(getSNValue(v2)));
return {v1.getFName(), sum};
}
return {v1.getFName(), getSNValue(v1) + getSNValue(v2)};
if (v1.holds<MPTIssue>())
{
// Perform addition in unsigned domain to avoid signed overflow UB
auto const sum = static_cast<MPTAmount::value_type>(
static_cast<std::uint64_t>(v1.mpt().value()) +
static_cast<std::uint64_t>(v2.mpt().value()));
return {v1.mAsset, sum};
}
return {v1.mAsset, v1.mpt().value() + v2.mpt().value()};
if (getSTNumberSwitchover())
{
@@ -406,13 +389,11 @@ operator+(STAmount const& v1, STAmount const& v2)
std::int64_t vv1 = static_cast<std::int64_t>(v1.mantissa());
std::int64_t vv2 = static_cast<std::int64_t>(v2.mantissa());
// Negate in unsigned domain then cast back to signed to avoid UB
// when vv1 or vv2 would represent INT64_MIN
if (v1.negative())
vv1 = static_cast<std::int64_t>(-static_cast<std::uint64_t>(vv1));
vv1 = -vv1;
if (v2.negative())
vv2 = static_cast<std::int64_t>(-static_cast<std::uint64_t>(vv2));
vv2 = -vv2;
while (ov1 < ov2)
{
@@ -437,7 +418,7 @@ operator+(STAmount const& v1, STAmount const& v2)
if (fv >= 0)
return STAmount{v1.getFName(), v1.asset(), static_cast<std::uint64_t>(fv), ov1, false};
return STAmount{v1.getFName(), v1.asset(), -static_cast<std::uint64_t>(fv), ov1, true};
return STAmount{v1.getFName(), v1.asset(), static_cast<std::uint64_t>(-fv), ov1, true};
}
STAmount
@@ -877,9 +858,7 @@ STAmount::canonicalize()
auto set = [&](auto const& val) {
auto const value = val.value();
mIsNegative = value < 0;
// Negate in unsigned domain to avoid UB when value == INT64_MIN
mValue = mIsNegative ? -static_cast<std::uint64_t>(value)
: static_cast<std::uint64_t>(value);
mValue = mIsNegative ? -value : value;
};
if (native())
{
@@ -988,9 +967,7 @@ STAmount::set(std::int64_t v)
if (v < 0)
{
mIsNegative = true;
// Cast to unsigned before negating to avoid undefined behavior
// when v == INT64_MIN (negating INT64_MIN in signed is UB)
mValue = -static_cast<std::uint64_t>(v);
mValue = static_cast<std::uint64_t>(-v);
}
else
{

View File

@@ -155,7 +155,7 @@ private:
std::vector<Validator> list;
std::string uri;
FetchListConfig const& cfg;
bool isRetry{false};
bool isRetry{};
};
std::vector<publisher> servers;