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 1828740989 MINOR: [C++][Gandiva] cast to unsigned char before ctype
calls (#50124)
1828740989 is described below
commit 1828740989d8feb8abdd4a2beed2d48257da4359
Author: metsw24-max <[email protected]>
AuthorDate: Mon Jul 6 18:45:54 2026 +0530
MINOR: [C++][Gandiva] cast to unsigned char before ctype calls (#50124)
### Rationale for this change
The Gandiva precompiled string and time helpers feed raw `char` bytes into
`isdigit`/`isxdigit`/`isalpha`/`toupper`. Where `char` is signed, any byte
above 0x7f (so any multi-byte UTF-8 value coming from untrusted column data) is
sign-extended to a negative `int`, which sits outside the `unsigned char`/`EOF`
domain those functions are defined for. glibc tolerates it, but a strict ctype
implementation indexes its lookup table before the start of the array, which is
undefined behaviour. [...]
### What changes are included in this PR?
Cast each ctype argument to `unsigned char` at the call site:
* `time.cc`: `castDATE_utf8`, `castTIMESTAMP_utf8`, `castTIME_utf8`
* `string_ops.cc`: `binary_string`, the `cast<INT>_varbinary` macro,
`from_hex_utf8`, `soundex_utf8`
The sweep covers both files so no call site is left behind. There is no
behaviour change where `char` is unsigned or the ctype implementation is
lenient; it just keeps the argument in range everywhere.
### Are these changes tested?
No new test. This is a localised undefined-behaviour fix with no observable
change on the platforms CI runs (`char` is unsigned or the ctype lookup is
lenient there), and the existing Gandiva precompiled tests already exercise
these functions.
### Are there any user-facing changes?
No.
Authored-by: metsw24-max <[email protected]>
Signed-off-by: Sutou Kouhei <[email protected]>
---
cpp/src/gandiva/precompiled/string_ops.cc | 16 +++++++++-------
cpp/src/gandiva/precompiled/time.cc | 6 +++---
2 files changed, 12 insertions(+), 10 deletions(-)
diff --git a/cpp/src/gandiva/precompiled/string_ops.cc
b/cpp/src/gandiva/precompiled/string_ops.cc
index 0dd02cb1d8..90484f2970 100644
--- a/cpp/src/gandiva/precompiled/string_ops.cc
+++ b/cpp/src/gandiva/precompiled/string_ops.cc
@@ -2357,7 +2357,8 @@ const char* binary_string(gdv_int64 context, const char*
text, gdv_int32 text_le
(text[i + 1] == 'x' || text[i + 1] == 'X')) {
char hd1 = text[i + 2];
char hd2 = text[i + 3];
- if (isxdigit(hd1) && isxdigit(hd2)) {
+ if (isxdigit(static_cast<unsigned char>(hd1)) &&
+ isxdigit(static_cast<unsigned char>(hd2))) {
// [a-fA-F0-9]
ret[j] = to_binary_from_hex(hd1) * 16 + to_binary_from_hex(hd2);
i += 3;
@@ -2406,7 +2407,7 @@ const char* binary_string(gdv_int64 context, const char*
text, gdv_int32 text_le
int read_index = 0;
\
while (read_index < in_len) {
\
char c1 = in[read_index];
\
- if (isxdigit(c1)) {
\
+ if (isxdigit(static_cast<unsigned char>(c1))) {
\
digit = to_binary_from_hex(c1);
\
\
OUT_TYPE next = result * 16 - digit;
\
@@ -2956,7 +2957,8 @@ const char* from_hex_utf8(int64_t context, const char*
text, int32_t text_len,
for (int32_t i = 0; i < text_len; i += 2) {
char b1 = text[i];
char b2 = text[i + 1];
- if (isxdigit(b1) && isxdigit(b2)) {
+ if (isxdigit(static_cast<unsigned char>(b1)) &&
+ isxdigit(static_cast<unsigned char>(b2))) {
// [a-fA-F0-9]
ret[j++] = to_binary_from_hex(b1) * 16 + to_binary_from_hex(b2);
} else {
@@ -3024,9 +3026,9 @@ const char* soundex_utf8(gdv_int64 context, const char*
in, gdv_int32 in_len,
int start_idx = 0;
for (int i = 0; i < in_len; ++i) {
- if (isalpha(in[i]) > 0) {
+ if (isalpha(static_cast<unsigned char>(in[i])) > 0) {
// Retain the first letter
- ret[0] = toupper(in[i]);
+ ret[0] = toupper(static_cast<unsigned char>(in[i]));
start_idx = i + 1;
break;
}
@@ -3042,8 +3044,8 @@ const char* soundex_utf8(gdv_int64 context, const char*
in, gdv_int32 in_len,
soundex[0] = '\0';
// Replace consonants with digits and special letters with 0
for (int i = start_idx; i < in_len; i++) {
- if (isalpha(in[i]) > 0) {
- c = toupper(in[i]) - 65;
+ if (isalpha(static_cast<unsigned char>(in[i])) > 0) {
+ c = toupper(static_cast<unsigned char>(in[i])) - 65;
if (mappings[c] != soundex[si - 1]) {
soundex[si] = mappings[c];
si++;
diff --git a/cpp/src/gandiva/precompiled/time.cc
b/cpp/src/gandiva/precompiled/time.cc
index f7028730eb..2b60f63651 100644
--- a/cpp/src/gandiva/precompiled/time.cc
+++ b/cpp/src/gandiva/precompiled/time.cc
@@ -648,7 +648,7 @@ gdv_date64 castDATE_utf8(int64_t context, const char*
input, gdv_int32 length) {
int dateIndex = 0, index = 0, value = 0;
int year_str_len = 0;
while (dateIndex < 3 && index < length) {
- if (!isdigit(input[index])) {
+ if (!isdigit(static_cast<unsigned char>(input[index]))) {
dateFields[dateIndex++] = value;
value = 0;
} else {
@@ -718,7 +718,7 @@ gdv_timestamp castTIMESTAMP_utf8(int64_t context, const
char* input, gdv_int32 l
int year_str_len = 0, sub_seconds_len = 0;
int ts_field_index = TimeFields::kYear, index = 0, value = 0;
while (ts_field_index < TimeFields::kMax && index < length) {
- if (isdigit(input[index])) {
+ if (isdigit(static_cast<unsigned char>(input[index]))) {
value = (value * 10) + (input[index] - '0');
if (ts_field_index == TimeFields::kYear) {
year_str_len++;
@@ -847,7 +847,7 @@ gdv_time32 castTIME_utf8(int64_t context, const char*
input, int32_t length) {
bool has_invalid_digit = false;
while (time_field_idx < TimeFields::kDisplacementHours && index < length) {
- if (isdigit(input[index])) {
+ if (isdigit(static_cast<unsigned char>(input[index]))) {
value = (value * 10) + (input[index] - '0');
if (time_field_idx == TimeFields::kSubSeconds) {
