SCI: Fix potential dangling pointer more robustly,

by changing the executionStack implementation to a list.

svn-id: r40971

engines/sci/engine/gc.cpp

@@ -97,8 +97,10 @@ reg_t_hash_map *find_all_used_references(EngineState *s) {
 #endif
 
 	// Init: Execution Stack
-	for (i = 0; i < s->_executionStack.size(); i++) {
+	Common::List<ExecStack>::iterator iter;
-		ExecStack &es = s->_executionStack[i];
+	for (iter = s->_executionStack.begin();
+	     iter != s->_executionStack.end(); ++iter) {
+		ExecStack &es = *iter;
 
 		if (es.type != EXEC_STACK_TYPE_KERNEL) {
 			wm.push(es.objp);

engines/sci/engine/kfile.cpp

@@ -654,7 +654,7 @@ reg_t kRestoreGame(EngineState *s, int funct_nr, int argc, reg_t *argv) {
 			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);

engines/sci/engine/kmisc.cpp

@@ -37,7 +37,9 @@ namespace Sci {
 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;
 }

engines/sci/engine/scriptdebug.cpp

@@ -1114,7 +1114,8 @@ int c_restore_game(EngineState *s, const Common::Array<cmd_param_t> &cmdParams)
 
 		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 @@ static int c_backtrace(EngineState *s, const Common::Array<cmd_param_t> &cmdPara
 	}
 
 	sciprintf("Call stack (current base: 0x%x):\n", s->execution_stack_base);
-	for (uint i = 0; i < s->_executionStack.size(); i++) {
+	Common::List<ExecStack>::iterator iter;
-		ExecStack &call = s->_executionStack[i];
+	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_set_acc(EngineState *s, const Common::Array<cmd_param_t> &cmdParams
 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 @@ static int c_send(EngineState *s, const Common::Array<cmd_param_t> &cmdParams) {
 	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,
+	xstack = add_exec_stack_entry(s, fptr,
-									s->_executionStack[0].sp - 1, 0, object, s->_executionStack.size()-1, SCI_XS_CALLEE_LOCALS);
+	                 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;
 

engines/sci/engine/state.h

@@ -202,7 +202,7 @@ public:
 
 	/* 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.

engines/sci/engine/vm.cpp

@@ -974,15 +974,13 @@ void run_vm(EngineState *s, int restoring) {
 			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 @@ void run_vm(EngineState *s, int restoring) {
 
 				// 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 @@ void run_vm(EngineState *s, int restoring) {
 
 //#ifndef DISABLE_VALIDATIONS
 		if (xs != &(s->_executionStack.back())) {
-			warning("xs is stale (%d/%p vs %d/%p); last command was %02x\n",
+			warning("xs is stale (%p vs %p); last command was %02x\n",
-					(int)(xs - &s->_executionStack[0]), (void *)xs,
+					(void *)xs, (void *)&(s->_executionStack.back()),
-					s->_executionStack.size()-1, (void *)&(s->_executionStack.back()),
 					opnumber);
 		}
 //#endif
@@ -2094,4 +2090,14 @@ void quit_vm() {
 	_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

engines/sci/engine/vm.h

@@ -1142,6 +1142,11 @@ Object *obj_get(EngineState *s, reg_t offset);
 ** 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