This is an automated email from the ASF dual-hosted git repository.
kou pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/arrow.git
The following commit(s) were added to refs/heads/main by this push:
new d26582eee4e GH-50544: [C++][Gandiva] Fix out-of-bounds read in round
with most negative precision (#50545)
d26582eee4e is described below
commit d26582eee4e07c1ad3e840757ed35e835596bb5d
Author: Abdul Rawoof Khan <[email protected]>
AuthorDate: Fri Oct 16 06:42:45 2026 +0530
GH-50544: [C++][Gandiva] Fix out-of-bounds read in round with most negative
precision (#50545)
### Rationale for this change
`round_int32_int32` and `round_int64_int32` negate the caller-supplied
precision into `abs_precision` before range-checking it, so the check
`abs_precision > 9` (and `> 18` for the int64 overload) never fires for
`precision == INT32_MIN`, where the negation leaves the value negative. That
value reaches `get_power_of_10`, which indexes a 19-element static table behind
`DCHECK_GE`/`DCHECK_LE` only. The DCHECKs compile out in release builds, so
`round(int_col, -2147483648)` reads far out [...]
`truncate_int64_int32` in the same file already compares the signed scale
directly (`out_scale < -38`) and is unaffected; the two `round` overloads are
the only sites that negate first.
### What changes are included in this PR?
Check the bound against the signed `precision` before negating, at both
sites. Every precision that was in range before still takes the same path, and
the negation now only runs on values known to be within 9 (or 18) of zero.
### Are these changes tested?
Yes. Added two cases to `TestExtendedMathOps.TestRound` covering
`std::numeric_limits<int32_t>::min()` for both overloads, matching the existing
`truncate_int64_int32` case in the same file. On the unpatched tree the debug
build aborts with `extended_math_ops.cc:372: Check failed: (exp) >= (0)`; with
the fix the full `gandiva-precompiled-test` suite passes (134 tests).
### Are there any user-facing changes?
No.
**This PR contains a "Critical Fix".** It fixes an out-of-bounds read
reachable from a user-supplied `round` precision argument.
* GitHub Issue: #50544
Authored-by: abdul rawoof <[email protected]>
Signed-off-by: Sutou Kouhei <[email protected]>
---
cpp/src/gandiva/precompiled/extended_math_ops.cc | 14 ++++++++------
cpp/src/gandiva/precompiled/extended_math_ops_test.cc | 12 ++++++++++++
2 files changed, 20 insertions(+), 6 deletions(-)
diff --git a/cpp/src/gandiva/precompiled/extended_math_ops.cc
b/cpp/src/gandiva/precompiled/extended_math_ops.cc
index 348d595faf4..206295f06dd 100644
--- a/cpp/src/gandiva/precompiled/extended_math_ops.cc
+++ b/cpp/src/gandiva/precompiled/extended_math_ops.cc
@@ -324,12 +324,13 @@ gdv_int32 round_int32_int32(gdv_int32 number, gdv_int32
precision) {
if (precision >= 0) {
return number;
}
- gdv_int32 abs_precision = -precision;
// This is to ensure that there is no overflow while calculating
10^precision, 9 is
- // the smallest N for which 10^N does not fit into 32 bits, so we can safely
return 0
- if (abs_precision > 9) {
+ // the smallest N for which 10^N does not fit into 32 bits, so we can safely
return 0.
+ // The bound is checked before negating because -INT32_MIN is still negative.
+ if (precision < -9) {
return 0;
}
+ gdv_int32 abs_precision = -precision;
gdv_int32 num_sign = (number > 0) ? 1 : -1;
gdv_int32 abs_number = number * num_sign;
gdv_int32 power_of_10 =
static_cast<gdv_int32>(get_power_of_10(abs_precision));
@@ -349,12 +350,13 @@ gdv_int64 round_int64_int32(gdv_int64 number, gdv_int32
precision) {
if (precision >= 0) {
return number;
}
- gdv_int32 abs_precision = -precision;
// This is to ensure that there is no overflow while calculating
10^precision, 19 is
- // the smallest N for which 10^N does not fit into 64 bits, so we can safely
return 0
- if (abs_precision > 18) {
+ // the smallest N for which 10^N does not fit into 64 bits, so we can safely
return 0.
+ // The bound is checked before negating because -INT32_MIN is still negative.
+ if (precision < -18) {
return 0;
}
+ gdv_int32 abs_precision = -precision;
gdv_int32 num_sign = (number > 0) ? 1 : -1;
gdv_int64 abs_number = number * num_sign;
gdv_int64 power_of_10 = get_power_of_10(abs_precision);
diff --git a/cpp/src/gandiva/precompiled/extended_math_ops_test.cc
b/cpp/src/gandiva/precompiled/extended_math_ops_test.cc
index e7a0f74c1b4..f9c92dd0911 100644
--- a/cpp/src/gandiva/precompiled/extended_math_ops_test.cc
+++ b/cpp/src/gandiva/precompiled/extended_math_ops_test.cc
@@ -203,6 +203,18 @@ TEST(TestExtendedMathOps, TestRound) {
EXPECT_EQ(round_int64_int32(-23453462343, -4), -23453460000);
EXPECT_EQ(round_int64_int32(-23453462343, -5), -23453500000);
EXPECT_EQ(round_int64_int32(345353425343, -12), 0);
+
+ // The last in-range precision still rounds; one past it returns 0. These
guard
+ // against an off-by-one in the range check at both overloads' boundary.
+ EXPECT_EQ(round_int32_int32(1234567890, -9), 1000000000);
+ EXPECT_EQ(round_int32_int32(1234567890, -10), 0);
+ EXPECT_EQ(round_int64_int32(1234567890123456789, -18), 1000000000000000000);
+ EXPECT_EQ(round_int64_int32(1234567890123456789, -19), 0);
+
+ // The most negative precision must be rejected by the range check rather
than
+ // negated into an out-of-range table index.
+ EXPECT_EQ(round_int32_int32(1234, std::numeric_limits<int32_t>::min()), 0);
+ EXPECT_EQ(round_int64_int32(345353425343,
std::numeric_limits<int32_t>::min()), 0);
}
TEST(TestExtendedMathOps, TestTruncate) {
