pearu opened a new issue, #50840:
URL: https://github.com/apache/arrow/issues/50840
### Describe the bug, including details regarding any error messages,
version, and platform.
`compute::Take` on a binary-like array (`string`/`binary`, i.e. 32-bit
offsets) silently overflows the output offsets buffer when the selected data
exceeds `INT32_MAX` bytes. It returns `Status::OK()` with a corrupt array
rather than raising, which leads to garbage values and segfaults downstream.
There *is* a guard for exactly this in
`VarBinarySelectionImpl::GenerateOutput`, but it never fires on GCC/Clang
because of a misplaced closing parenthesis.
#### Reproducer (Python)
This is how the problem typically surfaces — decoding a dictionary whose
dense form exceeds 2 GiB:
```python
import numpy as np, pyarrow as pa
arr = pa.DictionaryArray.from_arrays(
np.zeros(50_000_000, dtype=np.int16),
pa.array(["a" * 50]),
)
np.asarray(arr) # 50_000_000 * 50 = 2.5e9 bytes > INT32_MAX
```
Result is either a nonsense error or a hard crash:
```
ArrowException: Unknown error: Wrapping <garbage bytes> failed
```
```
Segmentation fault (core dumped)
```
`np.asarray` → `Array.to_numpy(zero_copy_only=False)` sets
`decode_dictionaries=True`, and `arrow_to_pandas.cc` `DecodeDictionaries()`
casts to the dictionary's *value* type (`string`, 32-bit offsets) via
`compute::Cast`, which dispatches to `UnpackDictionary` → `Take`.
#### Reproducer (C++, minimal)
The Python layer is incidental; `Take` alone is enough. 2048 × 1 MiB = 2 GiB
of output, one value past the limit:
```cpp
StringBuilder values_builder;
ASSERT_OK(values_builder.Append(std::string(1 << 20, 'x'))); // 1 MiB
ASSERT_OK_AND_ASSIGN(auto values, values_builder.Finish());
Int32Builder indices_builder;
ASSERT_OK(indices_builder.Reserve(2048));
for (int64_t i = 0; i < 2048; ++i) indices_builder.UnsafeAppend(0);
ASSERT_OK_AND_ASSIGN(auto indices, indices_builder.Finish());
ASSERT_OK_AND_ASSIGN(auto out, Take(Datum(*values), Datum(*indices))); //
succeeds!
```
Inspecting the result of the dictionary-decode form on current `main`
(`42694575d0`):
```
decoded size = 2500000000 bytes (INT32_MAX = 2147483647)
RESULT: Cast returned OK (NO error raised)
length = 50000000
negative offsets = 7050328
last offset = -1794967296 <- 2500000000 - 2^32
```
#### Root cause
`cpp/src/arrow/compute/kernels/vector_selection_internal.cc:520`:
```cpp
if (Adapter::is_take &&
ARROW_PREDICT_FALSE(static_cast<int64_t>(offset) +
static_cast<int64_t>(val_size)) > kOffsetLimit) {
// ^ closes around the
SUM
return Status::Invalid("Take operation overflowed binary array capacity");
}
```
On GCC/Clang, `ARROW_PREDICT_FALSE(x)` expands to `(__builtin_expect(!!(x),
0))` (`cpp/src/arrow/util/macros.h:79`). The `!!` collapses the sum to **0 or
1**, which is then compared against `kOffsetLimit` (2147483646) — so the
condition is **always false** and the guard is dead code.
Standalone confirmation:
```
sum = 2147500000
kOffsetLimit = 2147483646
as-written guard fires : false
intended guard fires : true
```
Under MSVC and the fallback definitions, `ARROW_PREDICT_FALSE(x)` is `(x)`
(`macros.h:105`, `macros.h:113`), so the guard works correctly there. **This
affects GCC/Clang builds only** — Linux and macOS, not Windows.
Introduced in `c07486c29f` (ARROW-5760, 2020-06-11), so it has been present
since Arrow 1.0.0. It went unnoticed because there is no large-memory test
covering `Take` in `vector_selection_test.cc`.
#### Expected behavior
`Take` should raise a proper error rather than returning a corrupt array.
With the parenthesis corrected, the reproducer yields:
```
Invalid: Take operation overflowed binary array capacity
```
which propagates cleanly up through `Cast` → `DecodeDictionaries` → a normal
Python exception, instead of a segfault.
Making the *dictionary-decode* case actually succeed (rather than raise) is
a separate enhancement — the 32-bit `string` type genuinely cannot represent >2
GiB, so `Take` refusing is correct. I'll file that separately.
#### Platform
- Reproduced on `main` @ `42694575d0`, Linux x86_64, GCC 14.3.0
- Python symptom also seen on pyarrow 24.0.0 and a 25.0.0 dev build
I have a fix and a `LARGE_MEMORY_TEST` regression test ready and will open a
PR.
### Component(s)
C++
---
_🤖 Drafted by Claude Code (an AI agent) and reviewed & approved by pearu._
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
