From 4dccb740488abe20c130b84b98e4b5e7ce38dad8 Mon Sep 17 00:00:00 2001 From: Loic Gouarin Date: Thu, 29 Oct 2026 18:50:04 +0100 Subject: [PATCH] Validate the arguments of pad and tile pad and tile take one value, or one pair of values, per axis of the expression and index their argument by axis. The sizes were only checked by XTENSOR_ASSERT, which is off in release builds, and the limits of each pad mode were not checked at all in release: - pad with fewer pairs than axes, or pairs with fewer than two values, read past the end of pad_width (heap-buffer-overflow); - pad with more pairs than axes ignored the extra ones; - wrap, periodic and symmetric with a width larger than the axis, or reflect with a width of at least its size, and edge on an empty axis, returned uninitialized values; - tile with more repetitions than axes wrote past the std::array shape of an xtensor (stack-buffer-overflow), and with fewer it built a garbage shape; - tile(e, n) on a 0-D expression wrote to an empty std::vector; - negative widths or repetitions wrapped around to huge sizes. These cases now throw std::runtime_error before the result is allocated. The loops of tile use the dimension of the expression for every buffer they index, which also removes the GCC 13 -O3 -Wstringop-overflow warning on its slice vector. --- include/xtensor/misc/xpad.hpp | 101 +++++++++++++++++++++++++++------- test/test_xpad.cpp | 51 +++++++++++++++++ 2 files changed, 132 insertions(+), 20 deletions(-) diff --git a/include/xtensor/misc/xpad.hpp b/include/xtensor/misc/xpad.hpp index 1204632f3..809aa322c 100644 --- a/include/xtensor/misc/xpad.hpp +++ b/include/xtensor/misc/xpad.hpp @@ -10,6 +10,12 @@ #ifndef XTENSOR_PAD_HPP #define XTENSOR_PAD_HPP +#include +#include +#include +#include +#include + #include "../containers/xarray.hpp" #include "../containers/xtensor.hpp" #include "../views/xstrided_view.hpp" @@ -47,15 +53,67 @@ namespace xt namespace detail { + template + inline std::size_t checked_count(S value, const char* error) + { + if constexpr (std::is_signed_v) + { + if (value < S(0)) + { + XTENSOR_THROW(std::runtime_error, error); + } + } + return static_cast(value); + } + + // pad_width is indexed by axis, and the modes other than constant read the + // padding back from the values of the array: without these checks, invalid + // widths read past the end of pad_width or return uninitialized values. template - inline bool check_pad_width(const std::vector>& pad_width, const T& shape) + inline void check_pad_width(const std::vector>& pad_width, const T& shape, pad_mode mode) { if (pad_width.size() != shape.size()) { - return false; + XTENSOR_THROW(std::runtime_error, "pad: pad_width must have one {before, after} pair per axis."); + } + for (std::size_t axis = 0; axis < shape.size(); ++axis) + { + if (pad_width[axis].size() != 2) + { + XTENSOR_THROW( + std::runtime_error, + "pad: pad_width must have one {before, after} pair per axis." + ); + } + const char* negative = "pad: pad widths must not be negative."; + const std::size_t before = checked_count(pad_width[axis][0], negative); + const std::size_t after = checked_count(pad_width[axis][1], negative); + const std::size_t size = static_cast(shape[axis]); + std::size_t max_width = (std::numeric_limits::max)(); + switch (mode) + { + case pad_mode::constant: + break; + case pad_mode::symmetric: + case pad_mode::wrap: + case pad_mode::periodic: + max_width = size; + break; + case pad_mode::reflect: + max_width = size == 0 ? 0 : size - 1; + break; + case pad_mode::edge: + max_width = size == 0 ? 0 : max_width; + break; + } + if (before > max_width || after > max_width) + { + XTENSOR_THROW( + std::runtime_error, + "pad: pad width too large for the size of the axis in this mode." + ); + } } - - return true; } } @@ -77,7 +135,7 @@ namespace xt pad_mode mode = pad_mode::constant, V constant_value = 0) { - XTENSOR_ASSERT(detail::check_pad_width(pad_width, e.shape())); + detail::check_pad_width(pad_width, e.shape(), mode); using size_type = typename std::decay_t::size_type; using return_type = temporary_type_t; @@ -122,19 +180,16 @@ namespace xt if (mode == pad_mode::wrap || mode == pad_mode::periodic) { - XTENSOR_ASSERT(nb <= e.shape(axis)); svs[axis] = xt::range(e.shape(axis), nb + e.shape(axis)); xt::strided_view(out, svt) = xt::strided_view(out, svs); } else if (mode == pad_mode::symmetric) { - XTENSOR_ASSERT(nb <= e.shape(axis)); svs[axis] = xt::range(2 * nb - 1, nb - 1, -1); xt::strided_view(out, svt) = xt::strided_view(out, svs); } else if (mode == pad_mode::reflect) { - XTENSOR_ASSERT(nb <= e.shape(axis) - 1); svs[axis] = xt::range(2 * nb, nb, -1); xt::strided_view(out, svt) = xt::strided_view(out, svs); } @@ -154,13 +209,11 @@ namespace xt if (mode == pad_mode::wrap || mode == pad_mode::periodic) { - XTENSOR_ASSERT(ne <= e.shape(axis)); svs[axis] = xt::range(nb, nb + ne); xt::strided_view(out, svt) = xt::strided_view(out, svs); } else if (mode == pad_mode::symmetric) { - XTENSOR_ASSERT(ne <= e.shape(axis)); if (ne == nb + e.shape(axis)) { svs[axis] = xt::range(nb + e.shape(axis) - 1, _, -1); @@ -173,7 +226,6 @@ namespace xt } else if (mode == pad_mode::reflect) { - XTENSOR_ASSERT(ne <= e.shape(axis) - 1); if (ne == nb + e.shape(axis) - 1) { svs[axis] = xt::range(nb + e.shape(axis) - 2, _, -1); @@ -249,26 +301,31 @@ namespace xt using return_type = temporary_type_t; - XTENSOR_ASSERT(e.shape().size() == reps.size()); + const size_type dim = e.dimension(); + if (reps.size() != dim) + { + XTENSOR_THROW(std::runtime_error, "tile: reps must have one value per axis."); + } using new_shape_type = typename return_type::shape_type; - auto new_shape = xtl::make_sequence(e.shape().size()); + auto new_shape = xtl::make_sequence(dim); - xt::xstrided_slice_vector sv(reps.size()); + xt::xstrided_slice_vector sv(dim); - for (size_type axis = 0; axis < reps.size(); ++axis) + for (size_type axis = 0; axis < dim; ++axis) { - new_shape[axis] = e.shape(axis) * reps[axis]; + const size_type n = checked_count(reps[axis], "tile: reps must not be negative."); + new_shape[axis] = e.shape(axis) * n; sv[axis] = xt::range(0, e.shape(axis)); } return_type out(new_shape); xt::strided_view(out, sv) = e; - xt::xstrided_slice_vector svs(e.shape().size(), xt::all()); - xt::xstrided_slice_vector svt(e.shape().size(), xt::all()); + xt::xstrided_slice_vector svs(dim, xt::all()); + xt::xstrided_slice_vector svt(dim, xt::all()); - for (size_type axis = 0; axis < e.shape().size(); ++axis) + for (size_type axis = 0; axis < dim; ++axis) { for (size_type i = 1; i < static_cast(reps[axis]); ++i) { @@ -314,7 +371,11 @@ namespace xt template ::size_type, XTL_REQUIRES(xtl::is_integral)> inline auto tile(E&& e, S reps) { - std::vector tw(e.shape().size(), static_cast(1)); + if (e.dimension() == 0) + { + XTENSOR_THROW(std::runtime_error, "tile: a 0-D expression has no first axis to repeat along."); + } + std::vector tw(e.dimension(), static_cast(1)); tw[0] = reps; return detail::tile(std::forward(e), tw); } diff --git a/test/test_xpad.cpp b/test/test_xpad.cpp index 32f684f2c..1e7a42c77 100644 --- a/test/test_xpad.cpp +++ b/test/test_xpad.cpp @@ -361,4 +361,55 @@ namespace xt }; EXPECT_EQ(m, expected); } + + TEST(xpad, invalid_pad_width) + { + xt::xtensor a = {{1, 2, 3}, {4, 5, 6}}; + using widths = std::vector>; + + XT_EXPECT_THROW(xt::pad(a, widths{{1, 1}}), std::runtime_error); + XT_EXPECT_THROW(xt::pad(a, widths{{1, 1}, {1, 1}, {1, 1}}), std::runtime_error); + XT_EXPECT_THROW(xt::pad(a, widths{{1}, {1}}), std::runtime_error); + XT_EXPECT_THROW(xt::pad(a, std::vector{1}), std::runtime_error); + XT_EXPECT_THROW(xt::pad(a, std::vector>{{1, -1}, {0, 0}}), std::runtime_error); + XT_EXPECT_THROW(xt::pad(a, -1), std::runtime_error); + + // The other modes read the padding from the array, which limits the widths. + XT_EXPECT_THROW(xt::pad(a, widths{{3, 0}, {0, 0}}, xt::pad_mode::wrap), std::runtime_error); + XT_EXPECT_THROW(xt::pad(a, widths{{0, 3}, {0, 0}}, xt::pad_mode::periodic), std::runtime_error); + XT_EXPECT_THROW(xt::pad(a, widths{{0, 0}, {4, 0}}, xt::pad_mode::symmetric), std::runtime_error); + XT_EXPECT_THROW(xt::pad(a, widths{{0, 0}, {0, 3}}, xt::pad_mode::reflect), std::runtime_error); + + xt::xtensor empty = xt::xtensor::from_shape({0, 2}); + XT_EXPECT_THROW(xt::pad(empty, widths{{1, 0}, {0, 0}}, xt::pad_mode::edge), std::runtime_error); + XT_EXPECT_THROW(xt::pad(empty, widths{{1, 0}, {0, 0}}, xt::pad_mode::reflect), std::runtime_error); + EXPECT_EQ( + xt::pad(empty, widths{{1, 0}, {0, 0}}, xt::pad_mode::constant, 7), + (xt::xtensor{{7, 7}}) + ); + } + + TEST(xpad, largest_pad_width) + { + xt::xtensor a = {1, 2, 3}; + + EXPECT_EQ(xt::pad(a, 3, xt::pad_mode::wrap), (xt::xtensor{1, 2, 3, 1, 2, 3, 1, 2, 3})); + EXPECT_EQ(xt::pad(a, 3, xt::pad_mode::symmetric), (xt::xtensor{3, 2, 1, 1, 2, 3, 3, 2, 1})); + EXPECT_EQ(xt::pad(a, 2, xt::pad_mode::reflect), (xt::xtensor{3, 2, 1, 2, 3, 2, 1})); + EXPECT_EQ(xt::pad(a, 4, xt::pad_mode::edge), (xt::xtensor{1, 1, 1, 1, 1, 2, 3, 3, 3, 3, 3})); + } + + TEST(xpad, tile_invalid_reps) + { + xt::xtensor a = {1, 2, 3}; + xt::xarray b = {{1, 2}, {3, 4}}; + xt::xarray scalar = 5; + + XT_EXPECT_THROW(xt::tile(a, {2u, 3u}), std::runtime_error); + XT_EXPECT_THROW(xt::tile(b, {2u, 3u, 4u}), std::runtime_error); + XT_EXPECT_THROW(xt::tile(b, std::vector{2}), std::runtime_error); + XT_EXPECT_THROW(xt::tile(b, std::vector{2, -1}), std::runtime_error); + XT_EXPECT_THROW(xt::tile(scalar, 3u), std::runtime_error); + XT_EXPECT_THROW(xt::tile(a, -2), std::runtime_error); + } }