Skip to content

Commit

Permalink
Fix bad probe on span assignment operator.
Browse files Browse the repository at this point in the history
Empty spans with data pointers one past the end of an object are
allowed, but we can't allow UnownedPtr<T> to probe for validity of
this location during an assignment for these spans.

See e.g. https://pdfium-review.googlesource.com/c/pdfium/+/96971
for an example of how this goes wrong.

Change-Id: I7ab60800a4d1584aeeec8343362337cb6c137672
Reviewed-on: https://pdfium-review.googlesource.com/c/pdfium/+/97031
Reviewed-by: Lei Zhang <[email protected]>
Commit-Queue: Tom Sepez <[email protected]>
  • Loading branch information
tsepez authored and Pdfium LUCI CQ committed Aug 29, 2022
1 parent c9c2180 commit 8479a08
Show file tree
Hide file tree
Showing 2 changed files with 22 additions and 6 deletions.
8 changes: 8 additions & 0 deletions core/fxcrt/span_util_unittest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -81,3 +81,11 @@ TEST(Spanmove, FitsWithin) {
EXPECT_EQ(dst[2], 'A');
EXPECT_EQ(dst[3], 'B');
}

TEST(Span, AssignOverOnePastEnd) {
std::vector<char> src(2, 'A');
pdfium::span<char> span = pdfium::make_span(src);
span = span.subspan(2);
span = pdfium::make_span(src);
EXPECT_EQ(span.size(), 2u);
}
20 changes: 14 additions & 6 deletions third_party/base/span.h
Original file line number Diff line number Diff line change
Expand Up @@ -209,14 +209,15 @@ class span {
// seamlessly used as a span<const T>, but not the other way around.
template <typename U, typename = internal::EnableIfLegalSpanConversion<U, T>>
constexpr span(const span<U>& other) : span(other.data(), other.size()) {}
span& operator=(const span& other) noexcept = default;
~span() noexcept {
if (!size_) {
// Empty spans might point to byte N+1 of a N-byte object, legal for
// C pointers but not UnownedPtrs.
data_.ReleaseBadPointer();
span& operator=(const span& other) noexcept {
if (this != &other) {
ReleaseEmptySpan();
data_ = other.data_;
size_ = other.size_;
}
return *this;
}
~span() noexcept { ReleaseEmptySpan(); }

// [span.sub], span subviews
const span first(size_t count) const {
Expand Down Expand Up @@ -281,6 +282,13 @@ class span {
}

private:
void ReleaseEmptySpan() noexcept {
// Empty spans might point to byte N+1 of a N-byte object, legal for
// C pointers but not UnownedPtrs.
if (!size_)
data_.ReleaseBadPointer();
}

UnownedPtr<T> data_;
size_t size_;
};
Expand Down

0 comments on commit 8479a08

Please sign in to comment.