From 309f80b25f4a20940598e5118b1a98691793a841 Mon Sep 17 00:00:00 2001 From: melex750 Date: Fri, 11 Jun 2021 22:46:23 -0400 Subject: [PATCH] Fix some bugs and memory leaks in CBOT * Added CBotStack::StackOver() calls where it was possible to go out-of-bounds when calling CBotStack::AddStack(). * Fixed some bugs in CBotExternalCallClass --- src/CBot/CBotDefParam.cpp | 1 + src/CBot/CBotExternalCall.cpp | 39 +++++++++++++------------ src/CBot/CBotInstr/CBotDefClass.cpp | 1 + src/CBot/CBotInstr/CBotFunction.cpp | 2 +- src/CBot/CBotInstr/CBotInstrCall.cpp | 1 + src/CBot/CBotInstr/CBotInstrMethode.cpp | 1 + src/CBot/CBotInstr/CBotNew.cpp | 1 + src/CBot/CBotInstr/CBotTwoOpExpr.cpp | 2 +- src/CBot/CBotProgram.cpp | 7 +++-- src/CBot/CBotProgram.h | 10 ++++--- 10 files changed, 39 insertions(+), 26 deletions(-) diff --git a/src/CBot/CBotDefParam.cpp b/src/CBot/CBotDefParam.cpp index de3fd503..75be07eb 100644 --- a/src/CBot/CBotDefParam.cpp +++ b/src/CBot/CBotDefParam.cpp @@ -137,6 +137,7 @@ bool CBotDefParam::Execute(CBotVar** ppVars, CBotStack* &pj) while ( p != nullptr ) { pile = pile->AddStack(); + if (pile->StackOver()) return pj->Return(pile); if (pile->GetState() == 1) // already done? { if (ppVars != nullptr && ppVars[i] != nullptr) ++i; diff --git a/src/CBot/CBotExternalCall.cpp b/src/CBot/CBotExternalCall.cpp index 8862212b..5b1b4c73 100644 --- a/src/CBot/CBotExternalCall.cpp +++ b/src/CBot/CBotExternalCall.cpp @@ -82,19 +82,23 @@ int CBotExternalCallList::DoCall(CBotToken* token, CBotVar* thisVar, CBotVar** p CBotExternalCall* pt = m_list[token->GetString()].get(); - if (pStack->IsCallFinished()) return true; - CBotStack* pile = pStack->AddStackExternalCall(pt); + if (thisVar == nullptr && pStack->IsCallFinished()) return true; // only for non-method external call - // lists the parameters depending on the contents of the stack (pStackVar) - CBotVar* pVar = MakeListVars(ppVar, true); + // if this is a method call we need to use AddStack() + CBotStack* pile = (thisVar != nullptr) ? pStack->AddStack() : pStack->AddStackExternalCall(pt); - // creates a variable to the result - CBotVar* pResult = rettype.Eq(CBotTypVoid) ? nullptr : CBotVar::Create("", rettype); + if (pile->GetState() == 0) // the first time? + { + // lists the parameters depending on the contents of the stack + CBotVar* pVar = MakeListVars(ppVar, true); + pile->SetVar(pVar); - pile->SetVar(pVar); - - CBotStack* pile2 = pile->AddStack(); - pile2->SetVar(pResult); + CBotStack* pile2 = pile->AddStack(); + // creates a variable to the result + CBotVar* pResult = rettype.Eq(CBotTypVoid) ? nullptr : CBotVar::Create("", rettype); + pile2->SetVar(pResult); + pile->IncState(); // increment state to mark this step done + } pile->SetError(CBotNoErr, token); // save token for the position in case of error return pt->Run(thisVar, pStack); @@ -107,7 +111,8 @@ bool CBotExternalCallList::RestoreCall(CBotToken* token, CBotVar* thisVar, CBotV CBotExternalCall* pt = m_list[token->GetString()].get(); - CBotStack* pile = pStack->RestoreStackEOX(pt); + // if this is a method call we need to use RestoreStack() + CBotStack* pile = (thisVar != nullptr) ? pStack->RestoreStack() : pStack->RestoreStackEOX(pt); if (pile == nullptr) return true; pile->RestoreStack(); @@ -163,8 +168,7 @@ bool CBotExternalCallDefault::Run(CBotVar* thisVar, CBotStack* pStack) return false; } - if (result != nullptr) pStack->SetCopyVar(result); - + pStack->Return(pile2); // return 'result' and clear extra stack return true; } @@ -187,8 +191,8 @@ CBotTypResult CBotExternalCallClass::Compile(CBotVar* thisVar, CBotVar* args, vo bool CBotExternalCallClass::Run(CBotVar* thisVar, CBotStack* pStack) { - if (pStack->IsCallFinished()) return true; - CBotStack* pile = pStack->AddStackExternalCall(this); + assert(thisVar != nullptr); + CBotStack* pile = pStack->AddStack(); CBotVar* args = pile->GetVar(); CBotStack* pile2 = pile->AddStack(); @@ -207,9 +211,8 @@ bool CBotExternalCallClass::Run(CBotVar* thisVar, CBotStack* pStack) return false; } - if (result != nullptr) pStack->SetCopyVar(result); - + pStack->Return(pile2); // return 'result' and clear extra stack return true; } -} +} // namespace CBot diff --git a/src/CBot/CBotInstr/CBotDefClass.cpp b/src/CBot/CBotInstr/CBotDefClass.cpp index c7711703..640b56eb 100644 --- a/src/CBot/CBotInstr/CBotDefClass.cpp +++ b/src/CBot/CBotInstr/CBotDefClass.cpp @@ -360,6 +360,7 @@ bool CBotDefClass::Execute(CBotStack* &pj) if ( p != nullptr) while ( true ) { pile2 = pile2->AddStack(); // place on the stack for the results + if (pile2->StackOver()) return pj->Return(pile2); if ( pile2->GetState() == 0 ) { if (!p->Execute(pile2)) return false; // interrupted here? diff --git a/src/CBot/CBotInstr/CBotFunction.cpp b/src/CBot/CBotInstr/CBotFunction.cpp index 478b9196..e4a62e95 100644 --- a/src/CBot/CBotInstr/CBotFunction.cpp +++ b/src/CBot/CBotInstr/CBotFunction.cpp @@ -445,7 +445,7 @@ bool CBotFunction::Execute(CBotVar** ppVars, CBotStack* &pj, CBotVar* pInstance) pile->IncState(); } - if ( !m_block->Execute(pile) ) + if (!pile->GetRetVar(m_block->Execute(pile))) { if ( pile->GetError() < 0 ) pile->SetError( CBotNoErr ); diff --git a/src/CBot/CBotInstr/CBotInstrCall.cpp b/src/CBot/CBotInstr/CBotInstrCall.cpp index 60689cb1..b3d151a7 100644 --- a/src/CBot/CBotInstr/CBotInstrCall.cpp +++ b/src/CBot/CBotInstr/CBotInstrCall.cpp @@ -138,6 +138,7 @@ bool CBotInstrCall::Execute(CBotStack* &pj) if ( p != nullptr) while ( true ) { pile = pile->AddStack(); // place on the stack for the results + if (pile->StackOver()) return pj->Return(pile); if ( pile->GetState() == 0 ) { if (!p->Execute(pile)) return false; // interrupted here? diff --git a/src/CBot/CBotInstr/CBotInstrMethode.cpp b/src/CBot/CBotInstr/CBotInstrMethode.cpp index 12c50466..a69ae7c9 100644 --- a/src/CBot/CBotInstr/CBotInstrMethode.cpp +++ b/src/CBot/CBotInstr/CBotInstrMethode.cpp @@ -164,6 +164,7 @@ bool CBotInstrMethode::ExecuteVar(CBotVar* &pVar, CBotStack* &pj, CBotToken* pre } ppVars[i++] = pile2->GetVar(); // construct the list of pointers pile2 = pile2->AddStack(); // space on the stack for the result + if (pile2->StackOver()) return pj->Return(pile2); p = p->GetNext(); if ( p == nullptr) break; } diff --git a/src/CBot/CBotInstr/CBotNew.cpp b/src/CBot/CBotInstr/CBotNew.cpp index 5f7539b1..b97f4e68 100644 --- a/src/CBot/CBotInstr/CBotNew.cpp +++ b/src/CBot/CBotInstr/CBotNew.cpp @@ -189,6 +189,7 @@ bool CBotNew::Execute(CBotStack* &pj) if (p != nullptr) while ( true) { pile2 = pile2->AddStack(); // space on the stack for the result + if (pile2->StackOver()) return pj->Return(pile2); if (pile2->GetState() == 0) { if (!p->Execute(pile2)) return false; // interrupted here? diff --git a/src/CBot/CBotInstr/CBotTwoOpExpr.cpp b/src/CBot/CBotInstr/CBotTwoOpExpr.cpp index a1339724..f57633d5 100644 --- a/src/CBot/CBotInstr/CBotTwoOpExpr.cpp +++ b/src/CBot/CBotInstr/CBotTwoOpExpr.cpp @@ -357,6 +357,7 @@ bool CBotTwoOpExpr::Execute(CBotStack* &pStack) CBotStack* pStk2 = pStk1->AddStack(); // adds an item to the stack // or return in case of recovery + if (pStk2->StackOver()) return pStack->Return(pStk2); // 2nd state, evalute right operand if ( pStk2->GetState() == 0 ) @@ -514,7 +515,6 @@ bool CBotTwoOpExpr::Execute(CBotStack* &pStack) pStk2->SetVar(result); // puts the result on the stack if ( err ) pStk2->SetError(err, &m_token); // and the possible error (division by zero) -// pStk1->Return(pStk2); // releases the stack return pStack->Return(pStk2); // transmits the result } diff --git a/src/CBot/CBotProgram.cpp b/src/CBot/CBotProgram.cpp index 428f1086..c55bf06f 100644 --- a/src/CBot/CBotProgram.cpp +++ b/src/CBot/CBotProgram.cpp @@ -34,7 +34,7 @@ namespace CBot { -CBotExternalCallList* CBotProgram::m_externalCalls = new CBotExternalCallList(); +std::unique_ptr CBotProgram::m_externalCalls; CBotProgram::CBotProgram() { @@ -395,6 +395,8 @@ int CBotProgram::GetVersion() void CBotProgram::Init() { + m_externalCalls.reset(new CBotExternalCallList); + CBotProgram::DefineNum("CBotErrZeroDiv", CBotErrZeroDiv); // division by zero CBotProgram::DefineNum("CBotErrNotInit", CBotErrNotInit); // uninitialized variable CBotProgram::DefineNum("CBotErrBadThrow", CBotErrBadThrow); // throw a negative value @@ -420,9 +422,10 @@ void CBotProgram::Free() CBotToken::ClearDefineNum(); m_externalCalls->Clear(); CBotClass::ClearPublic(); + m_externalCalls.reset(); } -CBotExternalCallList* CBotProgram::GetExternalCalls() +const std::unique_ptr& CBotProgram::GetExternalCalls() { return m_externalCalls; } diff --git a/src/CBot/CBotProgram.h b/src/CBot/CBotProgram.h index 8d7576f2..1ac41450 100644 --- a/src/CBot/CBotProgram.h +++ b/src/CBot/CBotProgram.h @@ -19,11 +19,12 @@ #pragma once -#include "CBot/CBotTypResult.h" #include "CBot/CBotEnums.h" -#include #include +#include +#include +#include namespace CBot { @@ -31,6 +32,7 @@ namespace CBot class CBotFunction; class CBotClass; class CBotStack; +class CBotTypResult; class CBotVar; class CBotExternalCallList; @@ -335,11 +337,11 @@ public: /** * \brief Returns static list of all registered external calls */ - static CBotExternalCallList* GetExternalCalls(); + static const std::unique_ptr& GetExternalCalls(); private: //! All external calls - static CBotExternalCallList* m_externalCalls; + static std::unique_ptr m_externalCalls; //! All user-defined functions std::list m_functions{}; //! The entry point function