From 3d218d76e7e0206f4eb476f91361af8ec243ee30 Mon Sep 17 00:00:00 2001 From: Jay Date: Fri, 31 Oct 2025 12:21:00 +0800 Subject: [PATCH] butil: fix undefined behaviors There are two kinds of problems: 1. signed number overflow is undefined behavior; 2. vsnprintfT may return E2BIG instead of EOVERFLOW. --- src/butil/fast_rand.cpp | 24 +++++++++++---- src/butil/numerics/safe_conversions.h | 25 ++++++++++++++++ .../strings/string_number_conversions.cc | 29 ++----------------- src/butil/strings/stringprintf.cc | 2 +- 4 files changed, 46 insertions(+), 34 deletions(-) diff --git a/src/butil/fast_rand.cpp b/src/butil/fast_rand.cpp index 36e0e83105..cef4585428 100644 --- a/src/butil/fast_rand.cpp +++ b/src/butil/fast_rand.cpp @@ -23,6 +23,7 @@ #include "butil/macros.h" #include "butil/time.h" // gettimeofday_us() #include "butil/fast_rand.h" +#include "butil/numerics/safe_conversions.h" // safe_abs namespace butil { @@ -110,20 +111,31 @@ int64_t fast_rand_in_64(int64_t min, int64_t max) { if (need_init(_tls_seed)) { init_fast_rand_seed(&_tls_seed); } - if (min >= max) { + if (BAIDU_UNLIKELY(min >= max)) { if (min == max) { return min; } - const int64_t tmp = min; - min = max; - max = tmp; + std::swap(min, max); + } + uint64_t range; + if (min >= 0) { + // Always safe to do subtraction. + range = (uint64_t)(max - min) + 1; + return min + (int64_t)fast_rand_impl(range, &_tls_seed); + } + + uint64_t abs_min = safe_abs(min); + if (max >= 0) { + range = abs_min + (uint64_t)(max) + 1; + } else { + range = abs_min - safe_abs(max) + 1; } - int64_t range = max - min + 1; if (range == 0) { // max = INT64_MAX, min = INT64_MIN return (int64_t)xorshift128_next(&_tls_seed); } - return min + (int64_t)fast_rand_impl(max - min + 1, &_tls_seed); + uint64_t r = fast_rand_impl(range, &_tls_seed); + return r >= abs_min ? (int64_t)(r - abs_min) : -((int64_t)(abs_min - r)); } uint64_t fast_rand_in_u64(uint64_t min, uint64_t max) { diff --git a/src/butil/numerics/safe_conversions.h b/src/butil/numerics/safe_conversions.h index 677aa4af0a..9a48811714 100644 --- a/src/butil/numerics/safe_conversions.h +++ b/src/butil/numerics/safe_conversions.h @@ -58,6 +58,31 @@ inline Dst saturated_cast(Src value) { return static_cast(value); } +inline uint64_t safe_abs(uint64_t x) { + return x; +} + +inline uint64_t safe_abs(int64_t x) { + return (x >= 0) ? (uint64_t)x : ((~(uint64_t)(x)) + 1); +} + +inline uint32_t safe_abs(uint32_t x) { + return x; +} + +inline uint32_t safe_abs(int32_t x) { + return (uint32_t)safe_abs((int64_t)x); +} + +#if defined(__APPLE__) +inline unsigned long safe_abs(unsigned long x) { + return x; +} +inline unsigned long safe_abs(long x) { + return (x >= 0) ? (unsigned long)x : ((~(unsigned long)(x)) + 1); +} +#endif + } // namespace butil #endif // BUTIL_SAFE_CONVERSIONS_H_ diff --git a/src/butil/strings/string_number_conversions.cc b/src/butil/strings/string_number_conversions.cc index 29645dec89..bcf3f49ce4 100644 --- a/src/butil/strings/string_number_conversions.cc +++ b/src/butil/strings/string_number_conversions.cc @@ -12,6 +12,7 @@ #include #include "butil/logging.h" +#include "butil/numerics/safe_conversions.h" // safe_abs #include "butil/scoped_clear_errno.h" #include "butil/strings/utf_string_conversions.h" #include "butil/third_party/dmg_fp/dmg_fp.h" @@ -22,30 +23,6 @@ namespace { template struct IntToStringT { - // This is to avoid a compiler warning about unary minus on unsigned type. - // For example, say you had the following code: - // template - // INT abs(INT value) { return value < 0 ? -value : value; } - // Even though if INT is unsigned, it's impossible for value < 0, so the - // unary minus will never be taken, the compiler will still generate a - // warning. We do a little specialization dance... - template - struct ToUnsignedT {}; - - template - struct ToUnsignedT { - static UINT2 ToUnsigned(INT2 value) { - return static_cast(value); - } - }; - - template - struct ToUnsignedT { - static UINT2 ToUnsigned(INT2 value) { - return static_cast(value < 0 ? -value : value); - } - }; - // This set of templates is very similar to the above templates, but // for testing whether an integer is negative. template @@ -74,9 +51,7 @@ struct IntToStringT { STR outbuf(kOutputBufSize, 0); bool is_neg = TestNegT::TestNeg(value); - // Even though is_neg will never be true when INT is parameterized as - // unsigned, even the presence of the unary operation causes a warning. - UINT res = ToUnsignedT::ToUnsigned(value); + UINT res = safe_abs(value); typename STR::iterator it(outbuf.end()); do { diff --git a/src/butil/strings/stringprintf.cc b/src/butil/strings/stringprintf.cc index 3f40e7247d..0ca6366cc8 100644 --- a/src/butil/strings/stringprintf.cc +++ b/src/butil/strings/stringprintf.cc @@ -78,7 +78,7 @@ static void StringAppendVT(StringType* dst, // wrong and no amount of buffer-doubling is going to fix it. return; #else - if (errno != 0 && errno != EOVERFLOW) + if (errno != 0 && errno != EOVERFLOW && errno != E2BIG) return; // Try doubling the buffer size. mem_length *= 2;