Fix ASAN failure in `array.rend()`. (#219)
The iterator constructor used `array[index]` to initialize an internal
cached view (necessary so that `operator->` can return a pointer), which
caused pointer arithmetic overflow when `index` was -1 -- as in the case
of `rend()`.
This change uses a checked `array.at(index)` method to get a null view
when `index` is out of bounds.
I considered changing `array[index]` to return a null view for
out-of-bounds indexes, but user code may be inadvertently relying on
that behavior, so I am leaving it out of this bug fix.
diff --git a/runtime/cpp/emboss_array_view.h b/runtime/cpp/emboss_array_view.h
index 674b2ad..fa8ccd3 100644
--- a/runtime/cpp/emboss_array_view.h
+++ b/runtime/cpp/emboss_array_view.h
@@ -56,7 +56,7 @@
explicit ElementViewIterator(const GenericArrayView array_view,
::std::ptrdiff_t index)
- : array_view_(array_view), view_(array_view[index]), index_(index) {}
+ : array_view_(array_view), view_(array_view.at(index)), index_(index) {}
ElementViewIterator() = default;
@@ -66,7 +66,7 @@
ElementViewIterator &operator+=(difference_type d) {
index_ += (kDirection == ElementViewIteratorDirection::kForward ? d : -d);
- view_ = array_view_[index_];
+ view_ = array_view_.at(index_);
return *this;
}
@@ -178,9 +178,15 @@
: parameters_{parameters...}, buffer_{buffer} {}
ElementView operator[](::std::size_t index) const {
- return IndexOperatorHelper<sizeof...(ElementViewParameterTypes) ==
- 0>::ConstructElement(parameters_, buffer_,
- index);
+ return IndexOperatorHelper<(sizeof...(ElementViewParameterTypes) ==
+ 0)>::UncheckedConstructElement(parameters_,
+ buffer_, index);
+ }
+
+ ElementView at(::std::size_t index) const {
+ return IndexOperatorHelper<(sizeof...(ElementViewParameterTypes) ==
+ 0)>::ConstructElement(parameters_, buffer_,
+ index, ElementCount());
}
ForwardIterator begin() const { return ForwardIterator(*this, 0); }
@@ -316,10 +322,18 @@
struct IndexOperatorHelper {
static ElementView ConstructElement(
const ::std::tuple<ElementViewParameterTypes...> ¶meters,
+ BufferType buffer, ::std::size_t index, ::std::size_t size) {
+ return IndexOperatorHelper<
+ (sizeof...(ElementViewParameterTypes) == 1 + sizeof...(N)), N...,
+ sizeof...(N)>::ConstructElement(parameters, buffer, index, size);
+ }
+
+ static ElementView UncheckedConstructElement(
+ const ::std::tuple<ElementViewParameterTypes...> ¶meters,
BufferType buffer, ::std::size_t index) {
return IndexOperatorHelper<
- sizeof...(ElementViewParameterTypes) == 1 + sizeof...(N), N...,
- sizeof...(N)>::ConstructElement(parameters, buffer, index);
+ (sizeof...(ElementViewParameterTypes) == 1 + sizeof...(N)), N...,
+ sizeof...(N)>::UncheckedConstructElement(parameters, buffer, index);
}
};
@@ -327,6 +341,18 @@
struct IndexOperatorHelper<true, N...> {
static ElementView ConstructElement(
const ::std::tuple<ElementViewParameterTypes...> ¶meters,
+ BufferType buffer, ::std::size_t index, ::std::size_t size) {
+ return ElementView(
+ ::std::get<N>(parameters)...,
+ index < 0 || index >= size
+ ? typename BufferType::template OffsetStorageType<kElementSize,
+ 0>(nullptr)
+ : buffer.template GetOffsetStorage<kElementSize, 0>(
+ kElementSize * index, kElementSize));
+ }
+
+ static ElementView UncheckedConstructElement(
+ const ::std::tuple<ElementViewParameterTypes...> ¶meters,
BufferType buffer, ::std::size_t index) {
return ElementView(::std::get<N>(parameters)...,
buffer.template GetOffsetStorage<kElementSize, 0>(