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.

Author: BusyJay

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) {

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_

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 {

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;