Skip to content

Commit

Permalink
Fix ASAN failure in array.rend().
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
reventlov committed Jan 30, 2025
1 parent 1827594 commit ee4138d
Showing 1 changed file with 33 additions and 7 deletions.
40 changes: 33 additions & 7 deletions runtime/cpp/emboss_array_view.h
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ class ElementViewIterator {

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;

Expand All @@ -66,7 +66,7 @@ class ElementViewIterator {

ElementViewIterator &operator+=(difference_type d) {
index_ += (kDirection == ElementViewIteratorDirection::kForward ? d : -d);
view_ = array_view_[index_];
view_ = array_view_.at(index_);
return *this;
}

Expand Down Expand Up @@ -178,9 +178,15 @@ class GenericArrayView final {
: 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); }
Expand Down Expand Up @@ -315,17 +321,37 @@ class GenericArrayView final {
template <bool, ::std::size_t... N>
struct IndexOperatorHelper {
static ElementView ConstructElement(
const ::std::tuple<ElementViewParameterTypes...> &parameters,
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...> &parameters,
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);
}
};

template </**/ ::std::size_t... N>
struct IndexOperatorHelper<true, N...> {
static ElementView ConstructElement(
const ::std::tuple<ElementViewParameterTypes...> &parameters,
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...> &parameters,
BufferType buffer, ::std::size_t index) {
return ElementView(::std::get<N>(parameters)...,
Expand Down

0 comments on commit ee4138d

Please sign in to comment.