[Scummvm-cvs-logs] SF.net SVN: scummvm:[40971] scummvm/trunk/engines/sci/engine
wjpalenstijn at users.sourceforge.net
wjpalenstijn at users.sourceforge.net
Fri May 29 00:42:18 CEST 2009
Revision: 40971
http://scummvm.svn.sourceforge.net/scummvm/?rev=40971&view=rev
Author: wjpalenstijn
Date: 2009-05-28 22:42:18 +0000 (Thu, 28 May 2009)
Log Message:
-----------
SCI: Fix potential dangling pointer more robustly,
by changing the executionStack implementation to a list.
Modified Paths:
--------------
scummvm/trunk/engines/sci/engine/gc.cpp
scummvm/trunk/engines/sci/engine/kfile.cpp
scummvm/trunk/engines/sci/engine/kmisc.cpp
scummvm/trunk/engines/sci/engine/scriptdebug.cpp
scummvm/trunk/engines/sci/engine/state.h
scummvm/trunk/engines/sci/engine/vm.cpp
scummvm/trunk/engines/sci/engine/vm.h
Modified: scummvm/trunk/engines/sci/engine/gc.cpp
===================================================================
--- scummvm/trunk/engines/sci/engine/gc.cpp 2009-05-28 22:28:23 UTC (rev 40970)
+++ scummvm/trunk/engines/sci/engine/gc.cpp 2009-05-28 22:42:18 UTC (rev 40971)
@@ -97,8 +97,10 @@
#endif
// Init: Execution Stack
- for (i = 0; i < s->_executionStack.size(); i++) {
- ExecStack &es = s->_executionStack[i];
+ Common::List<ExecStack>::iterator iter;
+ for (iter = s->_executionStack.begin();
+ iter != s->_executionStack.end(); ++iter) {
+ ExecStack &es = *iter;
if (es.type != EXEC_STACK_TYPE_KERNEL) {
wm.push(es.objp);
Modified: scummvm/trunk/engines/sci/engine/kfile.cpp
===================================================================
--- scummvm/trunk/engines/sci/engine/kfile.cpp 2009-05-28 22:28:23 UTC (rev 40970)
+++ scummvm/trunk/engines/sci/engine/kfile.cpp 2009-05-28 22:42:18 UTC (rev 40971)
@@ -654,7 +654,7 @@
if (newstate) {
s->successor = newstate;
script_abort_flag = SCRIPT_ABORT_WITH_REPLAY; // Abort current game
- s->_executionStack.resize(s->execution_stack_base + 1);
+ shrink_execution_stack(s, s->execution_stack_base + 1);
} else {
s->r_acc = make_reg(0, 1);
sciprintf("Restoring failed (game_id = '%s').\n", game_id);
Modified: scummvm/trunk/engines/sci/engine/kmisc.cpp
===================================================================
--- scummvm/trunk/engines/sci/engine/kmisc.cpp 2009-05-28 22:28:23 UTC (rev 40970)
+++ scummvm/trunk/engines/sci/engine/kmisc.cpp 2009-05-28 22:42:18 UTC (rev 40971)
@@ -37,7 +37,9 @@
reg_t kRestartGame(EngineState *s, int funct_nr, int argc, reg_t *argv) {
s->restarting_flags |= SCI_GAME_IS_RESTARTING_NOW;
s->restarting_flags &= ~SCI_GAME_WAS_RESTARTED_AT_LEAST_ONCE; // This appears to help
- s->_executionStack.resize(s->execution_stack_base + 1);
+
+ shrink_execution_stack(s, s->execution_stack_base + 1);
+
script_abort_flag = 1; // Force vm to abort ASAP
return NULL_REG;
}
Modified: scummvm/trunk/engines/sci/engine/scriptdebug.cpp
===================================================================
--- scummvm/trunk/engines/sci/engine/scriptdebug.cpp 2009-05-28 22:28:23 UTC (rev 40970)
+++ scummvm/trunk/engines/sci/engine/scriptdebug.cpp 2009-05-28 22:42:18 UTC (rev 40971)
@@ -1114,7 +1114,8 @@
script_abort_flag = SCRIPT_ABORT_WITH_REPLAY; // Abort current game
_debugstate_valid = 0;
- s->_executionStack.resize(s->execution_stack_base + 1);
+
+ shrink_execution_stack(s, s->execution_stack_base + 1);
return 0;
} else {
sciprintf("Restoring gamestate '%s' failed.\n", cmdParams[0].str);
@@ -1539,8 +1540,11 @@
}
sciprintf("Call stack (current base: 0x%x):\n", s->execution_stack_base);
- for (uint i = 0; i < s->_executionStack.size(); i++) {
- ExecStack &call = s->_executionStack[i];
+ Common::List<ExecStack>::iterator iter;
+ uint i = 0;
+ for (iter = s->_executionStack.begin();
+ iter != s->_executionStack.end(); ++iter, ++i) {
+ ExecStack &call = *iter;
const char *objname = obj_get_name(s, call.sendp);
int paramc, totalparamc;
@@ -2146,7 +2150,7 @@
static int c_send(EngineState *s, const Common::Array<cmd_param_t> &cmdParams) {
reg_t object = cmdParams[0].reg;
const char *selector_name = cmdParams[1].str;
- StackPtr stackframe = s->_executionStack[0].sp;
+ StackPtr stackframe = s->_executionStack.front().sp;
int selector_id;
unsigned int i;
ExecStack *xstack;
@@ -2180,8 +2184,11 @@
for (i = 2; i < cmdParams.size(); i++)
stackframe[i] = cmdParams[i].reg;
- xstack = add_exec_stack_entry(s, fptr, s->_executionStack[0].sp + cmdParams.size(), object, cmdParams.size() - 2,
- s->_executionStack[0].sp - 1, 0, object, s->_executionStack.size()-1, SCI_XS_CALLEE_LOCALS);
+ xstack = add_exec_stack_entry(s, fptr,
+ s->_executionStack.front().sp + cmdParams.size(),
+ object, cmdParams.size() - 2,
+ s->_executionStack.front().sp - 1, 0, object,
+ s->_executionStack.size()-1, SCI_XS_CALLEE_LOCALS);
xstack->selector = selector_id;
xstack->type = selector_type == kSelectorVariable ? EXEC_STACK_TYPE_VARSELECTOR : EXEC_STACK_TYPE_CALL;
Modified: scummvm/trunk/engines/sci/engine/state.h
===================================================================
--- scummvm/trunk/engines/sci/engine/state.h 2009-05-28 22:28:23 UTC (rev 40970)
+++ scummvm/trunk/engines/sci/engine/state.h 2009-05-28 22:42:18 UTC (rev 40971)
@@ -202,7 +202,7 @@
/* VM Information */
- Common::Array<ExecStack> _executionStack; /**< The execution stack */
+ Common::List<ExecStack> _executionStack; /**< The execution stack */
/**
* When called from kernel functions, the vm is re-started recursively on
* the same stack. This variable contains the stack base for the current vm.
Modified: scummvm/trunk/engines/sci/engine/vm.cpp
===================================================================
--- scummvm/trunk/engines/sci/engine/vm.cpp 2009-05-28 22:28:23 UTC (rev 40970)
+++ scummvm/trunk/engines/sci/engine/vm.cpp 2009-05-28 22:42:18 UTC (rev 40971)
@@ -974,15 +974,13 @@
int argc = (opparams[1] >> 1) // Given as offset, but we need count
+ 1 + restadjust;
StackPtr call_base = xs->sp - argc;
- StackPtr cur_sp = xs->sp;
xs->sp[1].offset += restadjust;
- xs->sp = call_base;
- // NB: add_exec_stack_entry can re-allocate the execution stacks
xs_new = add_exec_stack_entry(s, make_reg(xs->addr.pc.segment, xs->addr.pc.offset + opparams[0]),
- cur_sp, xs->objp, (validate_arithmetic(*call_base)) + restadjust,
+ xs->sp, xs->objp, (validate_arithmetic(*call_base)) + restadjust,
call_base, NULL_SELECTOR, xs->objp, s->_executionStack.size()-1, xs->local_segment);
restadjust = 0; // Used up the &rest adjustment
+ xs->sp = call_base;
s->_executionStackPosChanged = true;
break;
@@ -1078,7 +1076,6 @@
// Not reached the base, so let's do a soft return
s->_executionStack.pop_back();
- xs = old_xs - 1;
s->_executionStackPosChanged = true;
xs = &(s->_executionStack.back());
@@ -1445,9 +1442,8 @@
//#ifndef DISABLE_VALIDATIONS
if (xs != &(s->_executionStack.back())) {
- warning("xs is stale (%d/%p vs %d/%p); last command was %02x\n",
- (int)(xs - &s->_executionStack[0]), (void *)xs,
- s->_executionStack.size()-1, (void *)&(s->_executionStack.back()),
+ warning("xs is stale (%p vs %p); last command was %02x\n",
+ (void *)xs, (void *)&(s->_executionStack.back()),
opnumber);
}
//#endif
@@ -2094,4 +2090,14 @@
_debug_step_running = 0;
}
+void shrink_execution_stack(EngineState *s, uint size) {
+ assert(s->_executionStack.size() >= size);
+ Common::List<ExecStack>::iterator iter;
+ iter = s->_executionStack.begin();
+ for (uint i = 0; i < size; ++i)
+ ++iter;
+ s->_executionStack.erase(iter, s->_executionStack.end());
+}
+
+
} // End of namespace Sci
Modified: scummvm/trunk/engines/sci/engine/vm.h
===================================================================
--- scummvm/trunk/engines/sci/engine/vm.h 2009-05-28 22:28:23 UTC (rev 40970)
+++ scummvm/trunk/engines/sci/engine/vm.h 2009-05-28 22:42:18 UTC (rev 40971)
@@ -1142,6 +1142,11 @@
** Returns : (Object *) The object in question, or NULL if there is none
*/
+void shrink_execution_stack(EngineState *s, uint size);
+/* Shrink execution stack to size.
+** Contains an assert it is not already smaller.
+*/
+
} // End of namespace Sci
#endif // SCI_ENGINE_VM_H
This was sent by the SourceForge.net collaborative development platform, the world's largest Open Source development site.
More information about the Scummvm-git-logs
mailing list