pitrou commented on a change in pull request #12699:
URL: https://github.com/apache/arrow/pull/12699#discussion_r836438769
##########
File path: cpp/src/arrow/python/numpy_to_arrow.cc
##########
@@ -404,8 +404,13 @@ class NumPyStridedConverter {
ARROW_ASSIGN_OR_RAISE(buffer_, AllocateBuffer(sizeof(T) * length_, pool_));
const int64_t stride = PyArray_STRIDES(arr)[0];
- if (stride % sizeof(T) == 0) {
- const int64_t stride_elements = stride / sizeof(T);
+ // ARROW-16013: covert sizeof(T) to signed int64 first, otherwise dividing
by it would
+ // do an unsigned division. This cannot be caught by tests without ubsan,
since common
+ // signed overflow behavoir and the fact that the sizeof(T) is currently
always a
+ // power of two here cause CopyStridedNatural to still produce correct
results
Review comment:
```suggestion
// ARROW-16013: convert sizeof(T) to signed int64 first, otherwise
dividing by it would
// do an unsigned division. This cannot be caught by tests without
ubsan, since common
// signed overflow behavior and the fact that the sizeof(T) is currently
always a
// power of two here cause CopyStridedNatural to still produce correct
results
```
--
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]
