Fix backwards logic in rtc::Buffer::OnMovedFrom()
The logic in rtc::Buffer::OnMovedFrom was backwards w.r.t.
RTC_DCHECK_IS_ON. We intended to provoke bugs when DCHECKs are on and
play it safe when DCHECKs are off, but actually we did the reverse.
This CL fixes that.
It also adds a death test that would have caught the bug.
Bug: webrtc:9856
Change-Id: Ib6a4b07d12732e5a66e93b36b885abdab93e55c7
Reviewed-on: https://webrtc-review.googlesource.com/c/105040
Reviewed-by: Danil Chapovalov <danilchap@webrtc.org>
Commit-Queue: Karl Wiberg <kwiberg@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#25091}diff --git a/rtc_base/buffer.h b/rtc_base/buffer.h
index 22f8937..9bf96cd 100644
--- a/rtc_base/buffer.h
+++ b/rtc_base/buffer.h
@@ -149,7 +149,6 @@
}
BufferT& operator=(BufferT&& buf) {
- RTC_DCHECK(IsConsistent());
RTC_DCHECK(buf.IsConsistent());
size_ = buf.size_;
capacity_ = buf.capacity_;
@@ -389,7 +388,7 @@
ExplicitZeroMemory(data_.get() + size_, count * sizeof(T));
}
- // Precondition for all methods except Clear and the destructor.
+ // Precondition for all methods except Clear, operator= and the destructor.
// Postcondition for all methods except move construction and move
// assignment, which leave the moved-from object in a possibly inconsistent
// state.
@@ -401,14 +400,14 @@
// can mutate the state slightly to help subsequent sanity checks catch bugs.
void OnMovedFrom() {
#if RTC_DCHECK_IS_ON
+ // Ensure that *this is always inconsistent, to provoke bugs.
+ size_ = 1;
+ capacity_ = 0;
+#else
// Make *this consistent and empty. Shouldn't be necessary, but better safe
// than sorry.
size_ = 0;
capacity_ = 0;
-#else
- // Ensure that *this is always inconsistent, to provoke bugs.
- size_ = 1;
- capacity_ = 0;
#endif
}
diff --git a/rtc_base/buffer_unittest.cc b/rtc_base/buffer_unittest.cc
index ae976f1..1c3abfd 100644
--- a/rtc_base/buffer_unittest.cc
+++ b/rtc_base/buffer_unittest.cc
@@ -434,6 +434,19 @@
EXPECT_EQ(kObsidian, buf[2].stone);
}
+TEST(BufferTest, DieOnUseAfterMove) {
+ Buffer buf(17);
+ Buffer buf2 = std::move(buf);
+ EXPECT_EQ(buf2.size(), 17u);
+#if RTC_DCHECK_IS_ON
+#if GTEST_HAS_DEATH_TEST && !defined(WEBRTC_ANDROID)
+ EXPECT_DEATH(buf.empty(), "");
+#endif
+#else
+ EXPECT_TRUE(buf.empty());
+#endif
+}
+
TEST(ZeroOnFreeBufferTest, TestZeroOnSetData) {
ZeroOnFreeBuffer<uint8_t> buf(kTestData, 7);
const uint8_t* old_data = buf.data();