Bug in Vector::operator=()
by Tom Spilman · in Torque Game Engine Advanced · 11/24/2004 (1:13 pm) · 7 replies
Seems that Vector::operator=() in tVector.h is bugged:
mElementCount can be greater than the length of p.mArray leading to an out of bounds array access. This error appeared when in PathManager::updatePath a position vector with a length of zero was assigned to PathEntry.positions. It seems that in that loop mElementCount should be changed to min( mElementCount, p.mElementCount ), but I haven't really considered the impact on construct().
template<class T> inline Vector<T>& Vector<T>::operator=(const Vector<T>& p)
{
if(mElementCount > p.mElementCount)
destroy(p.mElementCount, mElementCount);
U32 i;
for(i = 0; i < mElementCount; i++) // bugged loop!
mArray[i] = p.mArray[i];
resize(p.mElementCount);
if(i < p.mElementCount)
construct(i, p.mElementCount, p.mArray);
return *this;
}mElementCount can be greater than the length of p.mArray leading to an out of bounds array access. This error appeared when in PathManager::updatePath a position vector with a length of zero was assigned to PathEntry.positions. It seems that in that loop mElementCount should be changed to min( mElementCount, p.mElementCount ), but I haven't really considered the impact on construct().
About the author
Tom is a programmer and co-owner of Sickhead Games, LLC.
#2
11/24/2004 (8:07 pm)
Interesting. I've put this on my list to review. Thanks Tom.
#3
You can see it in a real situation by going into the TSE walkthru, open the mission editor, expand the OutdoorTransition script group, expand the Path, and try to delete all it's markers one at a time. When you delete the final marker you'll crash.
11/24/2004 (8:34 pm)
By the way a simple way to see this issue and crash Vector:Vector<int> test1, test2; test1.push_back( 1 ); test1 = test2; // crash!
You can see it in a real situation by going into the TSE walkthru, open the mission editor, expand the OutdoorTransition script group, expand the Path, and try to delete all it's markers one at a time. When you delete the final marker you'll crash.
#4
12/06/2004 (4:22 pm)
Thanks Tom, the fix is now checked in.
#5
12/07/2004 (9:15 pm)
On my todo list for TGE as well.
#6
12/08/2004 (11:03 am)
I checked it for TGE, that function is implemented differently - Mark had to change it for the refcount code. TGE is fine.
#7
03/15/2005 (12:02 am)
Great, thank you guys both. Another item off my todo list.
Associate Tom Spilman
Sickhead Games
template<class T> inline Vector<T>& Vector<T>::operator=(const Vector<T>& p) { if(mElementCount > p.mElementCount) destroy(p.mElementCount, mElementCount); U32 i, Count = getMin( mElementCount, p.mElementCount ); for(i = 0; i < Count; i++) mArray[i] = p.mArray[i]; resize(p.mElementCount); if(i < p.mElementCount) construct(i, p.mElementCount, p.mArray); return *this; }