From 6d3f4411e01aecdc82893483cfb5b88f3cab19cb Mon Sep 17 00:00:00 2001 From: Bryan Bernhart Date: Thu, 23 Jun 2022 14:40:41 -0700 Subject: [PATCH] Fix AV after moving a LinkedList. Destroying a LinkedList after being moved would AV. This change fixes the issue by checking if the root is still valid before being removed upon destruction. --- src/gpgmm/utils/LinkedList.h | 8 ++++++-- src/tests/unittests/LinkedListTests.cpp | 19 +++++++++++++++++++ 2 files changed, 25 insertions(+), 2 deletions(-) diff --git a/src/gpgmm/utils/LinkedList.h b/src/gpgmm/utils/LinkedList.h index da3fe31d7..9b7445353 100644 --- a/src/gpgmm/utils/LinkedList.h +++ b/src/gpgmm/utils/LinkedList.h @@ -175,12 +175,16 @@ namespace gpgmm { // If any LinkNodes still exist in the LinkedList, there will be outstanding references // to root_ even after it has been freed. We should remove root_ from the list to // prevent any future access. - root_.RemoveFromList(); + if (root_.IsInList()) { + root_.RemoveFromList(); + } } // Using LinkedList in std::vector or STL container requires the move constructor to not // throw. - LinkedList(LinkedList&& other) noexcept : root_(std::move(other.root_)) { + LinkedList(LinkedList&& other) noexcept + : root_(std::move(other.root_)), size_(other.size_) { + other.size_ = 0; } // Appends |e| to the end of the linked list. diff --git a/src/tests/unittests/LinkedListTests.cpp b/src/tests/unittests/LinkedListTests.cpp index e42895f7f..7bee0eb9f 100644 --- a/src/tests/unittests/LinkedListTests.cpp +++ b/src/tests/unittests/LinkedListTests.cpp @@ -81,3 +81,22 @@ TEST(LinkedListTests, Clear) { EXPECT_TRUE(list.empty()); EXPECT_EQ(list.size(), 0u); } + +TEST(LinkedListTests, Move) { + LinkNode* objectInFirstList = new FakeObject(); + + LinkedList firstList; + firstList.push_back(objectInFirstList); + EXPECT_EQ(firstList.size(), 1u); + EXPECT_FALSE(firstList.empty()); + + EXPECT_EQ(firstList.head(), objectInFirstList); + EXPECT_EQ(firstList.tail(), objectInFirstList); + + LinkedList secondList(std::move(firstList)); + EXPECT_EQ(secondList.size(), 1u); + EXPECT_FALSE(secondList.empty()); + + EXPECT_EQ(secondList.head(), objectInFirstList); + EXPECT_EQ(secondList.tail(), objectInFirstList); +}