Skip to content

Commit 08b3cc9

Browse files
authored
GH-47926: [C++] Adopt alternative safe arithmetic library (#47958)
### Rationale for this change Our safe math arithmetic currently use the [portable-snippets library](https://github.com/nemequ/portable-snippets/blob/master/safe-math/safe-math.h). Besides being unmaintained, this library includes Windows headers, which has unfortunate effects such as defining multiple unwanted macros (`GetObject`, etc.). ### What changes are included in this PR? Replace usage of the portable snippets safe-math header with the [SafeInt](https://github.com/dcleblanc/SafeInt) library, and gcc/clang builtins. ### Are these changes tested? Yes, by existing CI tests. ### Are there any user-facing changes? No. * GitHub Issue: #47926 Authored-by: Antoine Pitrou <[email protected]> Signed-off-by: Antoine Pitrou <[email protected]>
1 parent 177ec45 commit 08b3cc9

File tree

8 files changed

+3784
-1111
lines changed

8 files changed

+3784
-1111
lines changed

cpp/src/arrow/compute/kernels/scalar_arithmetic_test.cc

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -241,7 +241,10 @@ class TestUnaryArithmetic : public TestBaseUnaryArithmetic<T, ArithmeticOptions>
241241
protected:
242242
using Base = TestBaseUnaryArithmetic<T, ArithmeticOptions>;
243243
using Base::options_;
244-
void SetOverflowCheck(bool value) { options_.check_overflow = value; }
244+
void SetOverflowCheck(bool value) {
245+
ARROW_SCOPED_TRACE("check_overflow = ", value);
246+
options_.check_overflow = value;
247+
}
245248
};
246249

247250
template <typename T>
@@ -421,7 +424,10 @@ class TestBinaryArithmetic : public TestBaseArithmetic<T> {
421424
AssertArraysApproxEqual(*expected, *actual, /*verbose=*/true, equal_options_);
422425
}
423426

424-
void SetOverflowCheck(bool value = true) { options_.check_overflow = value; }
427+
void SetOverflowCheck(bool value) {
428+
ARROW_SCOPED_TRACE("check_overflow = ", value);
429+
options_.check_overflow = value;
430+
}
425431

426432
void SetNansEqual(bool value = true) {
427433
this->equal_options_ = equal_options_.nans_equal(value);

cpp/src/arrow/util/int_util_overflow.h

Lines changed: 126 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -25,10 +25,7 @@
2525
#include "arrow/util/macros.h"
2626
#include "arrow/util/visibility.h"
2727

28-
// "safe-math.h" includes <intsafe.h> from the Windows headers.
29-
#include "arrow/vendored/portable-snippets/safe-math.h"
30-
// clang-format off (avoid include reordering)
31-
// clang-format on
28+
#include "arrow/vendored/safeint/safe_math.h"
3229

3330
namespace arrow {
3431
namespace internal {
@@ -38,49 +35,141 @@ namespace internal {
3835
// On overflow, these functions return true. Otherwise, false is returned
3936
// and `out` is updated with the result of the operation.
4037

41-
#define OP_WITH_OVERFLOW(_func_name, _psnip_op, _type, _psnip_type) \
42-
[[nodiscard]] static inline bool _func_name(_type u, _type v, _type* out) { \
43-
return !psnip_safe_##_psnip_type##_##_psnip_op(out, u, v); \
38+
#define SAFE_INT_OP_WITH_OVERFLOW(_func_name, _op_name, _c_type, _type) \
39+
[[nodiscard]] static inline bool _func_name(_c_type u, _c_type v, _c_type* out) { \
40+
return !check_##_op_name##_##_type##_##_type(u, v, out); \
4441
}
4542

46-
#define OPS_WITH_OVERFLOW(_func_name, _psnip_op) \
47-
OP_WITH_OVERFLOW(_func_name, _psnip_op, int8_t, int8) \
48-
OP_WITH_OVERFLOW(_func_name, _psnip_op, int16_t, int16) \
49-
OP_WITH_OVERFLOW(_func_name, _psnip_op, int32_t, int32) \
50-
OP_WITH_OVERFLOW(_func_name, _psnip_op, int64_t, int64) \
51-
OP_WITH_OVERFLOW(_func_name, _psnip_op, uint8_t, uint8) \
52-
OP_WITH_OVERFLOW(_func_name, _psnip_op, uint16_t, uint16) \
53-
OP_WITH_OVERFLOW(_func_name, _psnip_op, uint32_t, uint32) \
54-
OP_WITH_OVERFLOW(_func_name, _psnip_op, uint64_t, uint64)
43+
#define SAFE_INT_OPS_WITH_OVERFLOW(_func_name, _op_name) \
44+
SAFE_INT_OP_WITH_OVERFLOW(_func_name, _op_name, int32_t, int32) \
45+
SAFE_INT_OP_WITH_OVERFLOW(_func_name, _op_name, int64_t, int64) \
46+
SAFE_INT_OP_WITH_OVERFLOW(_func_name, _op_name, uint32_t, uint32) \
47+
SAFE_INT_OP_WITH_OVERFLOW(_func_name, _op_name, uint64_t, uint64)
48+
49+
SAFE_INT_OPS_WITH_OVERFLOW(SafeIntAddWithOverflow, add)
50+
SAFE_INT_OPS_WITH_OVERFLOW(SafeIntSubtractWithOverflow, sub)
51+
SAFE_INT_OPS_WITH_OVERFLOW(SafeIntMultiplyWithOverflow, mul)
52+
53+
#undef SAFE_INT_OP_WITH_OVERFLOW
54+
#undef SAFE_INT_OPS_WITH_OVERFLOW
55+
56+
template <typename Int, typename SignedRet, typename UnsignedRet>
57+
using transformed_int_t =
58+
std::conditional_t<std::is_signed_v<Int>, SignedRet, UnsignedRet>;
59+
60+
template <typename Int>
61+
using upscaled_int32_t = transformed_int_t<Int, int32_t, uint32_t>;
62+
63+
// Use GCC/CLang builtins for checked arithmetic, promising better performance
64+
// than SafeInt's hand-written implementations.
65+
#if defined __has_builtin
66+
# if __has_builtin(__builtin_object_size)
67+
# define USE_CHECKED_ARITHMETIC_BUILTINS 1
68+
# else
69+
# define USE_CHECKED_ARITHMETIC_BUILTINS 0
70+
# endif
71+
#endif
72+
73+
template <typename Int>
74+
[[nodiscard]] bool AddWithOverflowGeneric(Int u, Int v, Int* out) {
75+
#if USE_CHECKED_ARITHMETIC_BUILTINS
76+
return __builtin_add_overflow(u, v, out);
77+
#else
78+
if constexpr (sizeof(Int) < 4) {
79+
using UpscaledInt = upscaled_int32_t<Int>;
80+
auto r = static_cast<UpscaledInt>(u) + static_cast<UpscaledInt>(v);
81+
*out = static_cast<Int>(r);
82+
return r != *out;
83+
} else {
84+
return SafeIntAddWithOverflow(u, v, out);
85+
}
86+
#endif
87+
}
5588

56-
OPS_WITH_OVERFLOW(AddWithOverflow, add)
57-
OPS_WITH_OVERFLOW(SubtractWithOverflow, sub)
58-
OPS_WITH_OVERFLOW(MultiplyWithOverflow, mul)
59-
OPS_WITH_OVERFLOW(DivideWithOverflow, div)
89+
template <typename Int>
90+
[[nodiscard]] bool SubtractWithOverflowGeneric(Int u, Int v, Int* out) {
91+
#if USE_CHECKED_ARITHMETIC_BUILTINS
92+
return __builtin_sub_overflow(u, v, out);
93+
#else
94+
if constexpr (sizeof(Int) < 4) {
95+
using UpscaledInt = upscaled_int32_t<Int>;
96+
auto r = static_cast<UpscaledInt>(u) - static_cast<UpscaledInt>(v);
97+
*out = static_cast<Int>(r);
98+
return r != *out;
99+
} else {
100+
return SafeIntSubtractWithOverflow(u, v, out);
101+
}
102+
#endif
103+
}
60104

61-
#undef OP_WITH_OVERFLOW
62-
#undef OPS_WITH_OVERFLOW
105+
template <typename Int>
106+
[[nodiscard]] bool MultiplyWithOverflowGeneric(Int u, Int v, Int* out) {
107+
#if USE_CHECKED_ARITHMETIC_BUILTINS
108+
return __builtin_mul_overflow(u, v, out);
109+
#else
110+
if constexpr (sizeof(Int) < 4) {
111+
using UpscaledInt = upscaled_int32_t<Int>;
112+
auto r = static_cast<UpscaledInt>(u) * static_cast<UpscaledInt>(v);
113+
*out = static_cast<Int>(r);
114+
return r != *out;
115+
} else {
116+
return SafeIntMultiplyWithOverflow(u, v, out);
117+
}
118+
#endif
119+
}
63120

64-
// Define function NegateWithOverflow with the signature `bool(T u, T* out)`
65-
// where T is a signed integer type. On overflow, these functions return true.
66-
// Otherwise, false is returned and `out` is updated with the result of the
67-
// operation.
121+
template <typename Int>
122+
[[nodiscard]] bool DivideWithOverflowGeneric(Int u, Int v, Int* out) {
123+
if (v == 0) {
124+
*out = Int{};
125+
return true;
126+
}
127+
if constexpr (std::is_signed_v<Int>) {
128+
constexpr auto kMin = std::numeric_limits<Int>::min();
129+
if (u == kMin && v == -1) {
130+
*out = kMin;
131+
return true;
132+
}
133+
}
134+
*out = u / v;
135+
return false;
136+
}
68137

69-
#define UNARY_OP_WITH_OVERFLOW(_func_name, _psnip_op, _type, _psnip_type) \
70-
[[nodiscard]] static inline bool _func_name(_type u, _type* out) { \
71-
return !psnip_safe_##_psnip_type##_##_psnip_op(out, u); \
138+
// Define non-generic versions of the above so as to benefit from automatic
139+
// integer conversion, to allow for mixed-type calls such as
140+
// AddWithOverflow(int32_t, int64_t, int64_t*).
141+
142+
#define NON_GENERIC_OP_WITH_OVERFLOW(_func_name, _c_type) \
143+
[[nodiscard]] inline bool _func_name(_c_type u, _c_type v, _c_type* out) { \
144+
return ARROW_PREDICT_FALSE(_func_name##Generic(u, v, out)); \
72145
}
73146

74-
#define SIGNED_UNARY_OPS_WITH_OVERFLOW(_func_name, _psnip_op) \
75-
UNARY_OP_WITH_OVERFLOW(_func_name, _psnip_op, int8_t, int8) \
76-
UNARY_OP_WITH_OVERFLOW(_func_name, _psnip_op, int16_t, int16) \
77-
UNARY_OP_WITH_OVERFLOW(_func_name, _psnip_op, int32_t, int32) \
78-
UNARY_OP_WITH_OVERFLOW(_func_name, _psnip_op, int64_t, int64)
147+
#define NON_GENERIC_OPS_WITH_OVERFLOW(_func_name) \
148+
NON_GENERIC_OP_WITH_OVERFLOW(_func_name, int8_t) \
149+
NON_GENERIC_OP_WITH_OVERFLOW(_func_name, uint8_t) \
150+
NON_GENERIC_OP_WITH_OVERFLOW(_func_name, int16_t) \
151+
NON_GENERIC_OP_WITH_OVERFLOW(_func_name, uint16_t) \
152+
NON_GENERIC_OP_WITH_OVERFLOW(_func_name, int32_t) \
153+
NON_GENERIC_OP_WITH_OVERFLOW(_func_name, uint32_t) \
154+
NON_GENERIC_OP_WITH_OVERFLOW(_func_name, int64_t) \
155+
NON_GENERIC_OP_WITH_OVERFLOW(_func_name, uint64_t)
156+
157+
NON_GENERIC_OPS_WITH_OVERFLOW(AddWithOverflow)
158+
NON_GENERIC_OPS_WITH_OVERFLOW(SubtractWithOverflow)
159+
NON_GENERIC_OPS_WITH_OVERFLOW(MultiplyWithOverflow)
160+
NON_GENERIC_OPS_WITH_OVERFLOW(DivideWithOverflow)
79161

80-
SIGNED_UNARY_OPS_WITH_OVERFLOW(NegateWithOverflow, neg)
162+
#undef NON_GENERIC_OPS_WITH_OVERFLOW
163+
#undef NON_GENERIC_OP_WITH_OVERFLOW
81164

82-
#undef UNARY_OP_WITH_OVERFLOW
83-
#undef SIGNED_UNARY_OPS_WITH_OVERFLOW
165+
// Define function NegateWithOverflow with the signature `bool(T u, T* out)`
166+
// where T is a signed integer type. On overflow, these functions return true.
167+
// Otherwise, false is returned and `out` is updated with the result of the
168+
// operation.
169+
template <typename Int>
170+
[[nodiscard]] bool NegateWithOverflow(Int v, Int* out) {
171+
return SubtractWithOverflow(Int{}, v, out);
172+
}
84173

85174
/// Signed addition with well-defined behaviour on overflow (as unsigned)
86175
template <typename SignedInt>

cpp/src/arrow/vendored/CMakeLists.txt

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,4 +21,5 @@ add_subdirectory(datetime)
2121
add_subdirectory(double-conversion)
2222
add_subdirectory(pcg)
2323
add_subdirectory(portable-snippets)
24+
add_subdirectory(safeint)
2425
add_subdirectory(xxhash)

0 commit comments

Comments
 (0)