Repository navigation
Conversation
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.
Contributor
Merging this PR will not alter performance1 warning
Comparing |
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Checklist
Description
padandtileindex their argument by axis, but only checked its size withXTENSOR_ASSERT, which is off in release builds. The limits of each pad mode were not checked in release either:Too many pairs were silently ignored,
reflectwith a width of at least the axis size andedgeon an empty axis returned uninitialized values, and negative widths or repetitions wrapped to huge sizes.These cases now throw
std::runtime_errorbefore the result is allocated. The loops intileuse the dimension of the expression for every buffer they index, which also removes the GCC 13-O3-Wstringop-overflowwarning on its slice vector.xpad.invalid_pad_widthandxpad.tile_invalid_repsfail on master under AddressSanitizer and pass with the fix.xpad.largest_pad_widthchecks that the largest valid width of each mode still works.