Game Development Community

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:

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.


#1
11/24/2004 (1:52 pm)
It shouldn't impact construct() at all as i == p.mElementCount and nothing will or needs to be constructed. So the function should be:

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;
}
#2
11/24/2004 (8:07 pm)
Interesting. I've put this on my list to review. Thanks Tom.
#3
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.