Skip to content

Commit 5901d12

Browse files
authored
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.
1 parent 1827594 commit 5901d12

File tree

1 file changed

+33
-7
lines changed

1 file changed

+33
-7
lines changed

runtime/cpp/emboss_array_view.h

Lines changed: 33 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,7 @@ class ElementViewIterator {
5656

5757
explicit ElementViewIterator(const GenericArrayView array_view,
5858
::std::ptrdiff_t index)
59-
: array_view_(array_view), view_(array_view[index]), index_(index) {}
59+
: array_view_(array_view), view_(array_view.at(index)), index_(index) {}
6060

6161
ElementViewIterator() = default;
6262

@@ -66,7 +66,7 @@ class ElementViewIterator {
6666

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

@@ -178,9 +178,15 @@ class GenericArrayView final {
178178
: parameters_{parameters...}, buffer_{buffer} {}
179179

180180
ElementView operator[](::std::size_t index) const {
181-
return IndexOperatorHelper<sizeof...(ElementViewParameterTypes) ==
182-
0>::ConstructElement(parameters_, buffer_,
183-
index);
181+
return IndexOperatorHelper<(sizeof...(ElementViewParameterTypes) ==
182+
0)>::UncheckedConstructElement(parameters_,
183+
buffer_, index);
184+
}
185+
186+
ElementView at(::std::size_t index) const {
187+
return IndexOperatorHelper<(sizeof...(ElementViewParameterTypes) ==
188+
0)>::ConstructElement(parameters_, buffer_,
189+
index, ElementCount());
184190
}
185191

186192
ForwardIterator begin() const { return ForwardIterator(*this, 0); }
@@ -315,17 +321,37 @@ class GenericArrayView final {
315321
template <bool, ::std::size_t... N>
316322
struct IndexOperatorHelper {
317323
static ElementView ConstructElement(
324+
const ::std::tuple<ElementViewParameterTypes...> &parameters,
325+
BufferType buffer, ::std::size_t index, ::std::size_t size) {
326+
return IndexOperatorHelper<
327+
(sizeof...(ElementViewParameterTypes) == 1 + sizeof...(N)), N...,
328+
sizeof...(N)>::ConstructElement(parameters, buffer, index, size);
329+
}
330+
331+
static ElementView UncheckedConstructElement(
318332
const ::std::tuple<ElementViewParameterTypes...> &parameters,
319333
BufferType buffer, ::std::size_t index) {
320334
return IndexOperatorHelper<
321-
sizeof...(ElementViewParameterTypes) == 1 + sizeof...(N), N...,
322-
sizeof...(N)>::ConstructElement(parameters, buffer, index);
335+
(sizeof...(ElementViewParameterTypes) == 1 + sizeof...(N)), N...,
336+
sizeof...(N)>::UncheckedConstructElement(parameters, buffer, index);
323337
}
324338
};
325339

326340
template </**/ ::std::size_t... N>
327341
struct IndexOperatorHelper<true, N...> {
328342
static ElementView ConstructElement(
343+
const ::std::tuple<ElementViewParameterTypes...> &parameters,
344+
BufferType buffer, ::std::size_t index, ::std::size_t size) {
345+
return ElementView(
346+
::std::get<N>(parameters)...,
347+
index < 0 || index >= size
348+
? typename BufferType::template OffsetStorageType<kElementSize,
349+
0>(nullptr)
350+
: buffer.template GetOffsetStorage<kElementSize, 0>(
351+
kElementSize * index, kElementSize));
352+
}
353+
354+
static ElementView UncheckedConstructElement(
329355
const ::std::tuple<ElementViewParameterTypes...> &parameters,
330356
BufferType buffer, ::std::size_t index) {
331357
return ElementView(::std::get<N>(parameters)...,

0 commit comments

Comments
 (0)