From 0e238cc9ac8cb4523bcc892dfa2fb6de62db8f87 Mon Sep 17 00:00:00 2001
From: Pyry Haulos <phaulos@google.com>
Date: Tue, 29 Mar 2016 19:21:04 -0700
Subject: [PATCH] Fix bugs in de::AppendList

 * de::AppendList was not calling destructor for elements in the last
block. This would result in resource leak if elements have non-trivial
destructor.

 * de::AppendList::clear() failed to set m_first->next to null resulting
use of freed blocks.

Bug: 27909093
Change-Id: Id012bd6c76eb31058b302d0540891e5280a2d39f
---
 framework/delibs/decpp/deAppendList.cpp | 65 +++++++++++++++++++++++++++++++++
 framework/delibs/decpp/deAppendList.hpp |  5 ++-
 2 files changed, 68 insertions(+), 2 deletions(-)

diff --git a/framework/delibs/decpp/deAppendList.cpp b/framework/delibs/decpp/deAppendList.cpp
index ed7ef74..7433173 100644
--- a/framework/delibs/decpp/deAppendList.cpp
+++ b/framework/delibs/decpp/deAppendList.cpp
@@ -132,6 +132,62 @@ void runAppendListTest (deUint32 numThreads, deUint32 numElements, deUint32 numE
 	}
 }
 
+class ObjCountElem
+{
+public:
+	ObjCountElem (int* liveCount)
+		: m_liveCount(liveCount)
+	{
+		*m_liveCount += 1;
+	}
+
+	~ObjCountElem (void)
+	{
+		*m_liveCount -= 1;
+	}
+
+	ObjCountElem (const ObjCountElem& other)
+		: m_liveCount(other.m_liveCount)
+	{
+		*m_liveCount += 1;
+	}
+
+	ObjCountElem& operator= (const ObjCountElem& other)
+	{
+		m_liveCount = other.m_liveCount;
+		*m_liveCount += 1;
+		return *this;
+	}
+
+private:
+	int* m_liveCount;
+};
+
+void runClearTest (deUint32 numElements1, deUint32 numElements2, deUint32 numElementsHint)
+{
+	int		liveCount	= 0;
+
+	{
+		de::AppendList<ObjCountElem>	testList	(numElementsHint);
+
+		for (deUint32 ndx = 0; ndx < numElements1; ++ndx)
+			testList.append(ObjCountElem(&liveCount));
+
+		DE_TEST_ASSERT(liveCount == (int)numElements1);
+
+		testList.clear();
+
+		DE_TEST_ASSERT(liveCount == 0);
+
+		for (deUint32 ndx = 0; ndx < numElements2; ++ndx)
+			testList.append(ObjCountElem(&liveCount));
+
+		DE_TEST_ASSERT(liveCount == (int)numElements2);
+	}
+
+	DE_TEST_ASSERT(liveCount == 0);
+}
+
 } // anonymous
 
 void AppendList_selfTest (void)
@@ -150,6 +206,15 @@ void AppendList_selfTest (void)
 		runAppendListTest(4, 10000, 500);
 		runAppendListTest(4, 100, 10);
 	}
+
+	// Dtor + clear()
+	runClearTest(1, 1, 1);
+	runClearTest(1, 2, 10);
+	runClearTest(50, 25, 10);
+	runClearTest(9, 50, 10);
+	runClearTest(10, 50, 10);
+	runClearTest(50, 9, 10);
+	runClearTest(50, 10, 10);
 }
 
 } // de
diff --git a/framework/delibs/decpp/deAppendList.hpp b/framework/delibs/decpp/deAppendList.hpp
index 6ad3bb4..21c9f67 100644
--- a/framework/delibs/decpp/deAppendList.hpp
+++ b/framework/delibs/decpp/deAppendList.hpp
@@ -182,7 +182,7 @@ AppendList<ElementType>::~AppendList (void)
 		curBlock = delBlock->next;
 
 		// Call destructor for allocated elements
-		for (; elementNdx < min(m_numElements, delBlock->blockNdx*m_blockSize); ++elementNdx)
+		for (; elementNdx < min(m_numElements, (delBlock->blockNdx+1)*m_blockSize); ++elementNdx)
 			delBlock->elements[elementNdx%m_blockSize].~ElementType();
 
 		delete delBlock;
@@ -206,7 +206,7 @@ void AppendList<ElementType>::clear (void)
 		curBlock = delBlock->next;
 
 		// Call destructor for allocated elements
-		for (; elementNdx < min(m_numElements, delBlock->blockNdx*m_blockSize); ++elementNdx)
+		for (; elementNdx < min(m_numElements, (delBlock->blockNdx+1)*m_blockSize); ++elementNdx)
 			delBlock->elements[elementNdx%m_blockSize].~ElementType();
 
 		if (delBlock != m_first)
@@ -216,6 +216,7 @@ void AppendList<ElementType>::clear (void)
 	DE_ASSERT(elementNdx == m_numElements);
 
 	m_numElements	= 0;
+	m_first->next	= DE_NULL;
 	m_last			= m_first;
 }
 
-- 
2.7.4