Skip to content

Commit 37f1372

Browse files
authored
Prevent view creation with underflowed size (#2927)
# Checklist - [x] The title and commit message(s) are descriptive. - [x] Small commits made to fix your PR have been squashed to avoid history pollution. - [x] Tests have been added for new features or bug fixes. - [ ] API of new functions and classes are documented. # Description When a slice with conditions such as `start > stop && step > 0` (or vice versa) is used to create a view (sliced or strided), the resulting array view can have an enormous size. Accessing its elements then leads to undefined behavior. Here is a minimal example: ```cpp #include <iostream> #include <xtensor/containers/xarray.hpp> #include <xtensor/views/xview.hpp> int main() { xt::xarray<int>::shape_type shape = {5}; xt::xarray<int> a(shape); // v := a[3:0:1] auto slice = xt::range(3, 0, 1); auto v = xt::view(a, slice); // Print the shape of v for (auto e : v.shape()) { std::cout << e << " "; } std::cout << std::endl; } ``` The expected output is `0`, but the actual output is `18446744073709551613`. The cause of this issue appears to be the size calculation in the `xstepped_range` constructor in `xtensor/include/xtensor/views/xslice.hpp`, shown below: ```cpp template <class T> inline xstepped_range<T>::xstepped_range(size_type start_val, size_type stop_val, size_type step) noexcept : m_start(start_val) , m_size(size_type(0)) , m_step(step) { size_type n = stop_val - start_val; m_size = n / step + (((n < 0) ^ (step > 0)) && (n % step)); } ``` When `start_val` is greater than `stop_val` (or more precisely, when `stop_val - start_val` has the opposite sign from `step`), `m_size` can become negative. This subsequently causes the resulting size to underflow. One simple way to fix this issue is to clamp `m_size` to a minimum value of `0`: ```cpp template <class T> inline xstepped_range<T>::xstepped_range(size_type start_val, size_type stop_val, size_type step) noexcept : m_start(start_val) , m_size(size_type(0)) , m_step(step) { size_type n = stop_val - start_val; m_size = std::max(n / step + (((n < 0) ^ (step > 0)) && (n % step)), size_type(0)); } ``` Note on terminology: > I used the term "underflow" to describe "integer underflow", the behavior where subtracting from a small unsigned integer results in a huge value.
1 parent 38cd7ec commit 37f1372

2 files changed

Lines changed: 41 additions & 1 deletion

File tree

include/xtensor/views/xslice.hpp

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1058,8 +1058,13 @@ namespace xt
10581058
, m_size(size_type(0))
10591059
, m_step(step)
10601060
{
1061+
if ((step > 0 && start_val >= stop_val) || (step < 0 && start_val <= stop_val))
1062+
{
1063+
m_size = 0;
1064+
return;
1065+
}
10611066
size_type n = stop_val - start_val;
1062-
m_size = n / step + (((n < 0) ^ (step > 0)) && (n % step));
1067+
m_size = n / step + static_cast<bool>(n % step);
10631068
}
10641069

10651070
template <class T>

test/test_xview.cpp

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -226,6 +226,41 @@ namespace xt
226226
EXPECT_EQ(view0, view1);
227227
}
228228

229+
TEST(xview, negative_step)
230+
{
231+
view_shape_type shape = {3, 4};
232+
xarray<double> a(shape);
233+
std::vector<double> data = {1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12};
234+
std::copy(data.cbegin(), data.cend(), a.template begin<layout_type::row_major>());
235+
236+
auto view0 = view(a, 1, range(3, 0, -1));
237+
EXPECT_EQ(size_t(1), view0.dimension());
238+
EXPECT_EQ(size_t(3), view0.shape(0));
239+
EXPECT_EQ(a(1, 3), view0(0));
240+
EXPECT_EQ(a(1, 2), view0(1));
241+
EXPECT_EQ(a(1, 1), view0(2));
242+
}
243+
244+
TEST(xview, size_zero)
245+
{
246+
view_shape_type shape = {3, 4};
247+
xarray<double> a(shape);
248+
std::vector<double> data = {1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12};
249+
std::copy(data.cbegin(), data.cend(), a.template begin<layout_type::row_major>());
250+
251+
auto view0 = view(a, 1, range(0, 0));
252+
EXPECT_EQ(size_t(1), view0.dimension());
253+
EXPECT_EQ(size_t(0), view0.shape(0));
254+
255+
auto view1 = view(a, 1, range(3, 0, 1));
256+
EXPECT_EQ(size_t(1), view1.dimension());
257+
EXPECT_EQ(size_t(0), view1.shape(0));
258+
259+
auto view2 = view(a, 1, range(0, 3, -1));
260+
EXPECT_EQ(size_t(1), view2.dimension());
261+
EXPECT_EQ(size_t(0), view2.shape(0));
262+
}
263+
229264
TEST(xview, stored_range)
230265
{
231266
view_shape_type shape = {3, 4};

0 commit comments

Comments
 (0)