Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix issue where console stack values were getting overwritten #1026

Merged
merged 2 commits into from Dec 21, 2014
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
40 changes: 23 additions & 17 deletions Engine/source/console/compiledEval.cpp
Expand Up @@ -470,11 +470,11 @@ ConsoleValueRef CodeBlock::exec(U32 ip, const char *functionName, Namespace *thi
dSprintf(traceBuffer + dStrlen(traceBuffer), sizeof(traceBuffer) - dStrlen(traceBuffer),
"%s(", thisFunctionName);
}
for (i = 0; i < wantedArgc; i++)
for(i = 0; i < wantedArgc; i++)
{
dStrcat(traceBuffer, argv[i + 1]);
if (i != wantedArgc - 1)
dStrcat(traceBuffer, ", ");
dStrcat(traceBuffer, argv[i+1]);
if(i != wantedArgc - 1)
dStrcat(traceBuffer, ", ");
}
dStrcat(traceBuffer, ")");
Con::printf("%s", traceBuffer);
Expand Down Expand Up @@ -533,9 +533,16 @@ ConsoleValueRef CodeBlock::exec(U32 ip, const char *functionName, Namespace *thi
}
}

// Reset the console stack frame which at this point will contain
// either nothing or argv[] which we just copied
CSTK.resetFrame();
bool doResetValueStack = !gEvalState.mResetLocked;
gEvalState.mResetLocked = true;

if (gEvalState.mShouldReset)
{
// Ensure all stacks are clean in case anything became unbalanced during the previous execution
STR.clearFrames();
CSTK.clearFrames();
gEvalState.mShouldReset = false;
}

// Grab the state of the telenet debugger here once
// so that the push and pop frames are always balanced.
Expand Down Expand Up @@ -1817,7 +1824,7 @@ ConsoleValueRef CodeBlock::exec(U32 ip, const char *functionName, Namespace *thi
ConsoleValueRef ret;
if(nsEntry->mFunctionOffset)
ret = nsEntry->mCode->exec(nsEntry->mFunctionOffset, fnName, nsEntry->mNamespace, callArgc, callArgv, false, nsEntry->mPackage);

STR.popFrame();
// Functions are assumed to return strings, so look ahead to see if we can skip the conversion
if(code[ip] == OP_STR_TO_UINT)
Expand All @@ -1837,7 +1844,7 @@ ConsoleValueRef CodeBlock::exec(U32 ip, const char *functionName, Namespace *thi
}
else
STR.setStringValue((const char*)ret);

// This will clear everything including returnValue
CSTK.popFrame();
STR.clearFunctionOffset();
Expand Down Expand Up @@ -2219,22 +2226,21 @@ ConsoleValueRef CodeBlock::exec(U32 ip, const char *functionName, Namespace *thi
Con::printf("%s", traceBuffer);
}
}
else
{
delete[] globalStrings;
globalStringsMaxLen = 0;

delete[] globalFloats;
globalStrings = NULL;
globalFloats = NULL;
}
smCurrentCodeBlock = saveCodeBlock;
if(saveCodeBlock && saveCodeBlock->name)
{
Con::gCurrentFile = saveCodeBlock->name;
Con::gCurrentRoot = saveCodeBlock->modPath;
}

// Mark the reset flag for the next run if we've finished execution
if (doResetValueStack)
{
gEvalState.mShouldReset = true;
gEvalState.mResetLocked = false;
}

decRefCount();

#ifdef TORQUE_DEBUG
Expand Down
1 change: 1 addition & 0 deletions Engine/source/console/consoleInternal.cpp
Expand Up @@ -796,6 +796,7 @@ ExprEvalState::ExprEvalState()
currentVariable = NULL;
mStackDepth = 0;
stack.reserve( 64 );
mShouldReset = false;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should mResetLocked be initialized here as well?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, must have missed that. Whoopsie

}

ExprEvalState::~ExprEvalState()
Expand Down
2 changes: 2 additions & 0 deletions Engine/source/console/consoleInternal.h
Expand Up @@ -468,6 +468,8 @@ class ExprEvalState
bool traceOn;

U32 mStackDepth;
bool mShouldReset; ///< Designates if the value stack should be reset
bool mResetLocked; ///< mShouldReset will be set at the end

ExprEvalState();
~ExprEvalState();
Expand Down
8 changes: 7 additions & 1 deletion Engine/source/console/stringStack.cpp
Expand Up @@ -162,7 +162,7 @@ ConsoleValue* ConsoleValueStack::pop()

void ConsoleValueStack::pushFrame()
{
//Con::printf("CSTK pushFrame");
//Con::printf("CSTK pushFrame[%i] (%i)", mFrame, mStackPos);
mStackFrames[mFrame++] = mStackPos;
}

Expand All @@ -181,6 +181,12 @@ void ConsoleValueStack::resetFrame()
//Con::printf("CSTK resetFrame to %i", mStackPos);
}

void ConsoleValueStack::clearFrames()
{
mStackPos = 0;
mFrame = 0;
}

void ConsoleValueStack::popFrame()
{
//Con::printf("CSTK popFrame");
Expand Down
19 changes: 18 additions & 1 deletion Engine/source/console/stringStack.h
Expand Up @@ -74,6 +74,7 @@ struct StringStack
mBuffer = (char *) dRealloc(mBuffer, mBufferSize);
}
}

void validateArgBufferSize(U32 size)
{
if(size > mArgBufferSize)
Expand All @@ -82,6 +83,7 @@ struct StringStack
mArgBuffer = (char *) dRealloc(mArgBuffer, mArgBufferSize);
}
}

StringStack()
{
mBufferSize = 0;
Expand All @@ -95,6 +97,8 @@ struct StringStack
mFunctionOffset = 0;
validateBufferSize(8192);
validateArgBufferSize(2048);
dMemset(mBuffer, '\0', mBufferSize);
dMemset(mArgBuffer, '\0', mArgBufferSize);
}
~StringStack()
{
Expand Down Expand Up @@ -141,6 +145,7 @@ struct StringStack
/// Clear the function offset.
void clearFunctionOffset()
{
//Con::printf("StringStack mFunctionOffset = 0 (from %i)", mFunctionOffset);
mFunctionOffset = 0;
}

Expand Down Expand Up @@ -262,9 +267,9 @@ struct StringStack
return ret;
}


void pushFrame()
{
//Con::printf("StringStack pushFrame [frame=%i, start=%i]", mNumFrames, mStartStackSize);
mFrameOffsets[mNumFrames++] = mStartStackSize;
mStartOffsets[mStartStackSize++] = mStart;
mStart += ReturnBufferSpace;
Expand All @@ -273,11 +278,22 @@ struct StringStack

void popFrame()
{
//Con::printf("StringStack popFrame [frame=%i, start=%i]", mNumFrames, mStartStackSize);
mStartStackSize = mFrameOffsets[--mNumFrames];
mStart = mStartOffsets[mStartStackSize];
mLen = 0;
}

void clearFrames()
{
//Con::printf("StringStack clearFrames");
mNumFrames = 0;
mStart = 0;
mLen = 0;
mStartStackSize = 0;
mFunctionOffset = 0;
}

/// Get the arguments for a function call from the stack.
void getArgcArgv(StringTableEntry name, U32 *argc, const char ***in_argv, bool popStackFrame = false);
};
Expand Down Expand Up @@ -309,6 +325,7 @@ class ConsoleValueStack
void popFrame();

void resetFrame();
void clearFrames();

void getArgcArgv(StringTableEntry name, U32 *argc, ConsoleValueRef **in_argv, bool popStackFrame = false);

Expand Down