admin/webrtc_m130
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Fix backwards logic in rtc::Buffer::OnMovedFrom()

Browse Source 
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}
This commit is contained in:
Karl Wiberg 2018-10-10 12:44:12 +02:00 committed by Commit Bot
parent 0213786b39
commit b3b017950a
2 changed files with 18 additions and 6 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

11
rtc_base/buffer.h
View File

@ -149,7 +149,6 @@ class BufferT {
}
BufferT& operator=(BufferT&& buf) {
RTC_DCHECK(IsConsistent());
RTC_DCHECK(buf.IsConsistent());
size_ = buf.size_;
capacity_ = buf.capacity_;
@ -389,7 +388,7 @@ class BufferT {
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 @@ class BufferT {
// 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
}

13
rtc_base/buffer_unittest.cc
View File

@ -434,6 +434,19 @@ TEST(BufferTest, TestStruct) {
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();