Game Development Community

Torque 3D Beta 2 Bug: setFieldComponent doing bad, bad things!

by Stephane Conde · in Torque 3D Professional · 05/26/2009 (11:16 pm) · 2 replies

I have encountered a crash bug in Torque 3D Beta 2 where setFieldComponent ends up causing an out of bounds access on a vector.

This is kind of a tricky one, but I believe the following code inside setFieldComponent makes some assumptions that don't always hold true:

// Set the value on an object field.
if( object && field )
   prevVal = object->getDataField( field, array );

// Set the value on a variable.
else if( gEvalState.currentVariable )
   prevVal = gEvalState.getStringVariable();

With certain script code (see below) gEvalState.currentVariable can be left as a dangling pointer. It seems like the issue of gEvalState.currentVariable possibly pointing to bad data has been around for a long time, but no one has accessed that bad data until now. Obviously, the best fix would be to set gEvalState.currentVariable to be NULL in those circumstances, but I am not familiar enough with the script system to suggest a fix for that. Alternatively, setFieldComponent might be able to be re-written to take this issue into account.

Here is an example of some TorqueScript that will crash a completely clean Torque 3D Beta 2 build (keep in mind that a release build seems to happily access the bad data without saying a word, so you need to run in debug for things to crash and burn horribly).

function testBadness(%test)
{
	return 0;
}
testBadness(0).test = 0;

This code is a contrived case to bring the issue to light, but it is 'valid' TorqueScript in that it certainly shouldn't cause bad memory accesses and that it compiles (and used to run) just fine. The variable being passed into testBadness seems to be what causes gEvalState.currentVariable to point to bad data (maybe because popping it off the stack causes whatever gEvalState.currentVariable was pointing at to be deleted?). Without the variable being passed into testBadness, gEvalState.currentVariable points to 'something', although I'm not sure if it is the intended 'something'.

Would someone be able to explain exactly what setFieldComponent and getFieldComponent do and why they were added?

Thanks in advance for any help you can provide!

Stephane

#1
05/27/2009 (2:02 pm)
I think i have a fix for this, but like with any other change in the TorqueScript core its always possible that we've broken something else as a side effect. So i would appreciate if you give it a good test.

Add the lines i've marked below with '// ADDED' in console/consoleInternal.cpp starting around line 481...
void ExprEvalState::pushFrame(StringTableEntry frameName, Namespace *ns)
{
   Dictionary *newFrame = new Dictionary(this);
   newFrame->scopeName = frameName;
   newFrame->scopeNamespace = ns;
   stack.push_back(newFrame);
   currentVariable = NULL; // ADDED
}

void ExprEvalState::popFrame()
{
   Dictionary *last = stack.last();
   stack.pop_back();
   delete last;
   currentVariable = NULL; // ADDED
}

void ExprEvalState::pushFrameRef(S32 stackIndex)
{
   AssertFatal( stackIndex >= 0 && stackIndex < stack.size(), "You must be asking for a valid frame!" );
   Dictionary *newFrame = new Dictionary(this, stack[stackIndex]);
   stack.push_back(newFrame);
   currentVariable = NULL; // ADDED
}

ExprEvalState::ExprEvalState()
{
   VECTOR_SET_ASSOCIATION(stack);
   globalVars.setState(this);
   thisObject = NULL;
   traceOn = false;
   currentVariable = NULL; // ADDED
}
#2
05/27/2009 (11:57 pm)
That worked great for me Tom. Thanks for the quick reply and fix.

Keep up the great work!