This is an automated email from the ASF dual-hosted git repository.
guangmingchen pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/brpc.git
The following commit(s) were added to refs/heads/master by this push:
new f58ec398 butil: fix undefined behaviors (#3135)
f58ec398 is described below
commit f58ec3981d33a2f9229e44618ae243a97a8fe541
Author: Jay <[email protected]>
AuthorDate: Sun Dec 7 22:27:06 2025 +0800
butil: fix undefined behaviors (#3135)
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 ++++++++++++++++++++++
src/butil/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 36e0e831..cef45854 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 677aa4af..9a488117 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<Dst>(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 29645dec..bcf3f49c 100644
--- a/src/butil/strings/string_number_conversions.cc
+++ b/src/butil/strings/string_number_conversions.cc
@@ -12,6 +12,7 @@
#include <limits>
#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 <typename STR, typename INT, typename UINT, bool NEG>
struct IntToStringT {
- // This is to avoid a compiler warning about unary minus on unsigned type.
- // For example, say you had the following code:
- // template <typename INT>
- // 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 <typename INT2, typename UINT2, bool NEG2>
- struct ToUnsignedT {};
-
- template <typename INT2, typename UINT2>
- struct ToUnsignedT<INT2, UINT2, false> {
- static UINT2 ToUnsigned(INT2 value) {
- return static_cast<UINT2>(value);
- }
- };
-
- template <typename INT2, typename UINT2>
- struct ToUnsignedT<INT2, UINT2, true> {
- static UINT2 ToUnsigned(INT2 value) {
- return static_cast<UINT2>(value < 0 ? -value : value);
- }
- };
-
// This set of templates is very similar to the above templates, but
// for testing whether an integer is negative.
template <typename INT2, bool NEG2>
@@ -74,9 +51,7 @@ struct IntToStringT {
STR outbuf(kOutputBufSize, 0);
bool is_neg = TestNegT<INT, NEG>::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<INT, UINT, NEG>::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 3f40e724..0ca6366c 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;
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]