You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by "dmitry-chirkov-dremio (via GitHub)" <gi...@apache.org> on 2026/03/09 02:45:35 UTC
[PR] GH-49470: [C++][Gandiva] Fix crashes in substring_index and truncate with extreme integer values [arrow]
dmitry-chirkov-dremio opened a new pull request, #49471:
URL: https://github.com/apache/arrow/pull/49471
### Rationale for this change
Two Gandiva functions crash when called with extreme integer parameter values:
1. `substring_index(VARCHAR, VARCHAR, INT)` crashes with SIGBUS when count is `INT_MIN`
2. `truncate(BIGINT, INT)` crashes with SIGSEGV when scale is `INT_MAX` or `INT_MIN`
### What changes are included in this PR?
**substring_index fix** (`gdv_string_function_stubs.cc`):
- Replace `abs(cnt)` with safe `int64_t` computation to avoid undefined behavior when `cnt == INT_MIN`
**truncate fix** (`precompiled/extended_math_ops.cc`):
- Return input unchanged for positive scales (no-op for integers)
- Return 0 for scales < -38 to prevent out-of-bounds access in `GetScaleMultiplier`
### Are these changes tested?
Yes. Added coverage for `INT_MAX`/`INT_MIN` values in `gdv_function_stubs_test.cc` and `extended_math_ops_test.cc`.
### Are there any user-facing changes?
No.
**This PR contains a "Critical Fix".** These changes fix crashes caused by:
- `abs(INT_MIN)` triggering undefined behavior (integer overflow) in `substring_index`
- Out-of-bounds array access in `GetScaleMultiplier` when `truncate` receives extreme scale values
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-49470: [C++][Gandiva] Fix crashes in substring_index and truncate with extreme integer values [arrow]
Posted by "zanmato1984 (via GitHub)" <gi...@apache.org>.
zanmato1984 merged PR #49471:
URL: https://github.com/apache/arrow/pull/49471
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-49470: [C++][Gandiva] Fix crashes in substring_index and truncate with extreme integer values [arrow]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #49471:
URL: https://github.com/apache/arrow/pull/49471#issuecomment-4106350286
Revision: 8b17dff793a46bee5c62ac9e272a2224e74ba963
Submitted crossbow builds: [ursacomputing/crossbow @ actions-90c6287765](https://github.com/ursacomputing/crossbow/branches/all?query=actions-90c6287765)
|Task|Status|
|----|------|
|test-r-linux-sanitizers|[](https://github.com/ursacomputing/crossbow/actions/runs/23405151040/job/68082741132)|
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-49470: [C++][Gandiva] Fix crashes in substring_index and truncate with extreme integer values [arrow]
Posted by "dmitry-chirkov-dremio (via GitHub)" <gi...@apache.org>.
dmitry-chirkov-dremio commented on code in PR #49471:
URL: https://github.com/apache/arrow/pull/49471#discussion_r2909018113
##########
cpp/src/gandiva/gdv_string_function_stubs.cc:
##########
@@ -407,14 +407,17 @@ const char* gdv_fn_substring_index(int64_t context, const char* txt, int32_t txt
}
}
- if (static_cast<int32_t>(abs(cnt)) <= static_cast<int32_t>(occ.size()) && cnt > 0) {
+ // Use int64_t to avoid undefined behavior with abs(INT_MIN)
+ int64_t abs_cnt = (cnt < 0) ? -static_cast<int64_t>(cnt) : static_cast<int64_t>(cnt);
Review Comment:
I'd leave as is:
1. The int64_t fix is more robust - it handles ALL negative values correctly, not just INT_MIN
2. It's the same number of lines - early exit adds 5 lines, current fix adds 3 lines
3. The current fix also simplifies the existing code - removes redundant static_cast<int32_t> casts
The early-exit approach only guards against the specific crash values, while the int64_t approach fixes the underlying type-safety issue.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-49470: [C++][Gandiva] Fix crashes in substring_index and truncate with extreme integer values [arrow]
Posted by "lriggs (via GitHub)" <gi...@apache.org>.
lriggs commented on code in PR #49471:
URL: https://github.com/apache/arrow/pull/49471#discussion_r2915196794
##########
cpp/src/gandiva/gdv_string_function_stubs.cc:
##########
@@ -407,14 +407,17 @@ const char* gdv_fn_substring_index(int64_t context, const char* txt, int32_t txt
}
}
- if (static_cast<int32_t>(abs(cnt)) <= static_cast<int32_t>(occ.size()) && cnt > 0) {
+ // Use int64_t to avoid undefined behavior with abs(INT_MIN)
+ int64_t abs_cnt = (cnt < 0) ? -static_cast<int64_t>(cnt) : static_cast<int64_t>(cnt);
+ int64_t occ_size = static_cast<int64_t>(occ.size());
+
+ if (abs_cnt <= occ_size && cnt > 0) {
memcpy(out, txt, occ[cnt - 1]);
*out_len = occ[cnt - 1];
return out;
- } else if (static_cast<int32_t>(abs(cnt)) <= static_cast<int32_t>(occ.size()) &&
- cnt < 0) {
+ } else if (abs_cnt <= occ_size && cnt < 0) {
int32_t sz = static_cast<int32_t>(occ.size());
- int32_t temp = static_cast<int32_t>(abs(cnt));
+ int32_t temp = static_cast<int32_t>(abs_cnt);
Review Comment:
Since this is dealing with edge cases I think you could use int64_t here instead and avoid the potential of an overflow in the cast.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-49470: [C++][Gandiva] Fix crashes in substring_index and truncate with extreme integer values [arrow]
Posted by "dmitry-chirkov-dremio (via GitHub)" <gi...@apache.org>.
dmitry-chirkov-dremio commented on code in PR #49471:
URL: https://github.com/apache/arrow/pull/49471#discussion_r2928788179
##########
cpp/src/gandiva/gdv_string_function_stubs.cc:
##########
@@ -407,14 +407,17 @@ const char* gdv_fn_substring_index(int64_t context, const char* txt, int32_t txt
}
}
- if (static_cast<int32_t>(abs(cnt)) <= static_cast<int32_t>(occ.size()) && cnt > 0) {
+ // Use int64_t to avoid undefined behavior with abs(INT_MIN)
+ int64_t abs_cnt = (cnt < 0) ? -static_cast<int64_t>(cnt) : static_cast<int64_t>(cnt);
+ int64_t occ_size = static_cast<int64_t>(occ.size());
+
+ if (abs_cnt <= occ_size && cnt > 0) {
memcpy(out, txt, occ[cnt - 1]);
*out_len = occ[cnt - 1];
return out;
- } else if (static_cast<int32_t>(abs(cnt)) <= static_cast<int32_t>(occ.size()) &&
- cnt < 0) {
+ } else if (abs_cnt <= occ_size && cnt < 0) {
int32_t sz = static_cast<int32_t>(occ.size());
- int32_t temp = static_cast<int32_t>(abs(cnt));
+ int32_t temp = static_cast<int32_t>(abs_cnt);
Review Comment:
@lriggs addresses. Resolving.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-49470: [C++][Gandiva] Fix crashes in substring_index and truncate with extreme integer values [arrow]
Posted by "conbench-apache-arrow[bot] (via GitHub)" <gi...@apache.org>.
conbench-apache-arrow[bot] commented on PR #49471:
URL: https://github.com/apache/arrow/pull/49471#issuecomment-4064953971
After merging your PR, Conbench analyzed the 3 benchmarking runs that have been run so far on merge-commit d6ce56e350633afc2fee16fb4e2347a18df1041a.
There were no benchmark performance regressions. 🎉
The [full Conbench report](https://github.com/apache/arrow/runs/67174445848) has more details. It also includes information about 1 possible false positive for unstable benchmarks that are known to sometimes produce them.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-49470: [C++][Gandiva] Fix crashes in substring_index and truncate with extreme integer values [arrow]
Posted by "jonkeane (via GitHub)" <gi...@apache.org>.
jonkeane commented on PR #49471:
URL: https://github.com/apache/arrow/pull/49471#issuecomment-4106347107
@github-actions crossbow submit test-r-linux-sanitizers
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-49470: [C++][Gandiva] Fix crashes in substring_index and truncate with extreme integer values [arrow]
Posted by "lriggs (via GitHub)" <gi...@apache.org>.
lriggs commented on code in PR #49471:
URL: https://github.com/apache/arrow/pull/49471#discussion_r2908367754
##########
cpp/src/gandiva/gdv_string_function_stubs.cc:
##########
@@ -407,14 +407,17 @@ const char* gdv_fn_substring_index(int64_t context, const char* txt, int32_t txt
}
}
- if (static_cast<int32_t>(abs(cnt)) <= static_cast<int32_t>(occ.size()) && cnt > 0) {
+ // Use int64_t to avoid undefined behavior with abs(INT_MIN)
+ int64_t abs_cnt = (cnt < 0) ? -static_cast<int64_t>(cnt) : static_cast<int64_t>(cnt);
Review Comment:
Is it possible to check the function parameter for the bad size and exit early like in the truncation fix? That seems simpler.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-49470: [C++][Gandiva] Fix crashes in substring_index and truncate with extreme integer values [arrow]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #49471:
URL: https://github.com/apache/arrow/pull/49471#issuecomment-4020735704
:warning: GitHub issue #49470 **has been automatically assigned in GitHub** to PR creator.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org