aikhanjum opened a new pull request, #51477: URL: https://github.com/apache/arrow/pull/51477
### Rationale for this change The STL allocator computes `n * sizeof(T)` without checking for overflow. On a 64-bit platform, requesting `2^61` uint64_t elements wraps the byte count to zero. Requesting one more element wraps it to eight. Both allocations incorrectly succeed on upstream commit `5448aa7a916e73116ccc009454e3c19572dd5aad`, returning less storage than requested. Closes #31076. ### What changes are included in this PR? Check the element count before multiplication against both the size_t limit and the int64_t limit used by MemoryPool. Reject oversized requests with the existing BadAlloc exception, which derives from std::bad_alloc. Add one regression test covering wraparound to zero and eight bytes. The test frees any unexpectedly successful allocation before reporting the missing exception. ### Are these changes tested? Reproduce the original behavior with this code and the Arrow headers and library built from the upstream commit above. ```cpp arrow::stl::allocator<uint64_t> alloc; const size_t n = std::numeric_limits<size_t>::max() / sizeof(uint64_t) + 1; auto* data = alloc.allocate(n); alloc.deallocate(data, n); ``` The allocation should throw std::bad_alloc. Before the fix, it returns successfully. Repeating with `n + 1` returns an allocation of eight bytes on the tested platform. The new `allocator.AllocationSizeOverflow` test was run against unchanged production code and failed for both counts because no exception was thrown. It passes after the fix. Executed on macOS arm64 with Apple Clang 17, C++20, and BUILD_WARNING_LEVEL set to CHECKIN. - Debug CTest passed arrow-stl-test, arrow-misc-test, arrow-array-test, and arrow-utility-test. There were 2155 passing individual tests and 16 existing skips. - The AddressSanitizer and UndefinedBehaviorSanitizer build passed all 44 compiled STL tests, including the regression. - `pre-commit run --show-diff-on-failure --color=never --all-files cpp` passed all four configured C++ hooks. - `git diff --check upstream/main..HEAD` passed. Linux, Windows, 32-bit builds, LLVM 18 compilation, other allocation backends, and the complete CI matrix were not run. The regression directly exercises unsigned multiplication overflow. The separate signed byte-count bound was reviewed but not independently exercised with a custom MemoryPool. Bundled Boost emitted existing compiler warnings. ### Are there any user-facing changes? Impossible allocation sizes now throw std::bad_alloc instead of returning undersized storage. There are no public API signature changes. ### Was AI used for this PR? **PR code and description written by** - [ ] Human - [x] AI **Reviewed before submission by** - [x] Human - [x] AI - [ ] Not reviewed OpenAI Codex researched the issue and related PRs, wrote the reproducer, regression test, fix, and this description, ran the listed checks, and reviewed the diff with another AI agent. Grok was used for preliminary X research. No external implementation was copied into the patch. The human author reviewed the complete patch, discussed the overflow checks and regression test, and takes responsibility for the change. -- 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]
