[Scummvm-cvs-logs] scummvm master -> d86504ef887c5f10fedbc081476a050295241cb4

bluegr md5 at scummvm.org
Fri Mar 25 12:40:00 CET 2011


This automated email contains information about 2 new commits which have been
pushed to the 'scummvm' repo located at https://github.com/scummvm/scummvm .

Summary:
4ceb4838ed SCI: Cleaned up send_selector()
d86504ef88 SCI: Cleaned up the BreakpointType enum and documented the bpe command


Commit: 4ceb4838ed9c153337a7a07fc03d3d1e78351af4
    https://github.com/scummvm/scummvm/commit/4ceb4838ed9c153337a7a07fc03d3d1e78351af4
Author: md5 (md5 at scummvm.org)
Date: 2011-03-25T04:25:38-07:00

Commit Message:
SCI: Cleaned up send_selector()

- Placed all of the associated debug code in a separate function
- Unified debug output

Changed paths:
    engines/sci/engine/vm.cpp



diff --git a/engines/sci/engine/vm.cpp b/engines/sci/engine/vm.cpp
index 24f3c96..fdd4a46 100644
--- a/engines/sci/engine/vm.cpp
+++ b/engines/sci/engine/vm.cpp
@@ -373,22 +373,92 @@ bool SciEngine::checkSelectorBreakpoint(BreakpointType breakpointType, reg_t sen
 	return false;
 }
 
+void debugSelectorCall(reg_t send_obj, Selector selector, int argc, StackPtr argp, ObjVarRef &varp, reg_t funcp, SegManager *segMan, SelectorType selectorType) {
+	int activeBreakpointTypes = g_sci->_debugState._activeBreakpointTypes;
+	const char *objectName = segMan->getObjectName(send_obj);
+	const char *selectorName = g_sci->getKernel()->getSelectorName(selector).c_str();
+	Console *con = g_sci->getSciDebugger();
+
+#ifdef VM_DEBUG_SEND
+		debugN("Send to %04x:%04x (%s), selector %04x (%s):", PRINT_REG(send_obj), 
+			s->_segMan->getObjectName(send_obj), selector, 
+			g_sci->getKernel()->getSelectorName(selector).c_str());
+#endif // VM_DEBUG_SEND
+
+	switch (selectorType) {
+	case kSelectorNone:
+		break;
+	case kSelectorVariable:
+#ifdef VM_DEBUG_SEND
+		if (argc)
+			debugN("Varselector: Write %04x:%04x\n", PRINT_REG(argp[1]));
+		else
+			debugN("Varselector: Read\n");
+#endif // VM_DEBUG_SEND
+
+		// argc == 0: read selector
+		// argc == 1: write selector
+		// argc can be bigger than 1 in some cases, because of a script bug.
+		// Usually, these aren't fatal.
+		if ((activeBreakpointTypes & BREAK_SELECTORREAD) ||
+			(activeBreakpointTypes & BREAK_SELECTORWRITE) ||
+			argc > 1) {
+
+			reg_t selectorValue = *varp.getPointer(segMan);
+			if (!argc && (activeBreakpointTypes & BREAK_SELECTORREAD)) {
+				if (g_sci->checkSelectorBreakpoint(BREAK_SELECTORREAD, send_obj, selector))
+					con->DebugPrintf("Read from selector (%s:%s): %04x:%04x\n", 
+							objectName, selectorName,
+							PRINT_REG(selectorValue));
+			} else if (argc && (activeBreakpointTypes & BREAK_SELECTORWRITE)) {
+				if (g_sci->checkSelectorBreakpoint(BREAK_SELECTORWRITE, send_obj, selector))
+					con->DebugPrintf("Write to selector (%s:%s): change %04x:%04x to %04x:%04x\n", 
+							objectName, selectorName,
+							PRINT_REG(selectorValue), PRINT_REG(argp[1]));
+			}
+
+			if (argc > 1)
+				debug(kDebugLevelScripts, "Write to selector (%s:%s): change %04x:%04x to %04x:%04x, argc == %d\n", 
+							objectName, selectorName,
+							PRINT_REG(selectorValue), PRINT_REG(argp[1]), argc);
+		}
+		break;
+	case kSelectorMethod:
+#ifndef VM_DEBUG_SEND
+			if (activeBreakpointTypes & BREAK_SELECTOREXEC) {
+				if (g_sci->checkSelectorBreakpoint(BREAK_SELECTOREXEC, send_obj, selector)) {
+#else
+			if (true) {
+				if (true) {
+#endif
+					con->DebugPrintf("%s::%s(", objectName, selectorName);
+					for (int i = 0; i < argc; i++) {
+						con->DebugPrintf("%04x:%04x", PRINT_REG(argp[i+1]));
+						if (i + 1 < argc)
+							con->DebugPrintf(", ");
+					}
+					con->DebugPrintf(") at %04x:%04x\n", PRINT_REG(funcp));
+				}
+			}
+		break;
+	}	// switch
+}
+
 ExecStack *send_selector(EngineState *s, reg_t send_obj, reg_t work_obj, StackPtr sp, int framesize, StackPtr argp) {
-// send_obj and work_obj are equal for anything but 'super'
-// Returns a pointer to the TOS exec_stack element
+	// send_obj and work_obj are equal for anything but 'super'
+	// Returns a pointer to the TOS exec_stack element
 	assert(s);
 
 	reg_t funcp;
-	int selector;
+	Selector selector;
 	int argc;
-	int origin = s->_executionStack.size()-1; // Origin: Used for debugging
+	int origin = s->_executionStack.size() - 1; // Origin: Used for debugging
+	int activeBreakpointTypes = g_sci->_debugState._activeBreakpointTypes;
 	// We return a pointer to the new active ExecStack
 
 	// The selector calls we catch are stored below:
 	Common::Stack<CallsStruct> sendCalls;
 
-	int activeBreakpointTypes = g_sci->_debugState._activeBreakpointTypes;
-
 	while (framesize > 0) {
 		selector = argp->requireUint16();
 		argp++;
@@ -397,145 +467,49 @@ ExecStack *send_selector(EngineState *s, reg_t send_obj, reg_t work_obj, StackPt
 		if (argc > 0x800)	// More arguments than the stack could possibly accomodate for
 			error("send_selector(): More than 0x800 arguments to function call");
 
-#ifdef VM_DEBUG_SEND
-		debugN("Send to %04x:%04x (%s), selector %04x (%s):", PRINT_REG(send_obj), 
-			s->_segMan->getObjectName(send_obj), selector, 
-			g_sci->getKernel()->getSelectorName(selector).c_str());
-#endif // VM_DEBUG_SEND
-
 		ObjVarRef varp;
-		switch (lookupSelector(s->_segMan, send_obj, selector, &varp, &funcp)) {
+		CallsStruct call;
+		call.argp = argp;
+		call.argc = argc;
+		call.selector = selector;
+		SelectorType selectorType = lookupSelector(s->_segMan, send_obj, selector, &varp, &funcp);
+		if (activeBreakpointTypes || DebugMan.isDebugChannelEnabled(kDebugLevelScripts))
+			debugSelectorCall(send_obj, selector, argc, argp, varp, funcp, s->_segMan, selectorType);
+
+		switch (selectorType) {
 		case kSelectorNone:
 			error("Send to invalid selector 0x%x of object at %04x:%04x", 0xffff & selector, PRINT_REG(send_obj));
 			break;
-
 		case kSelectorVariable:
-
-#ifdef VM_DEBUG_SEND
-			if (argc)
-				debugN("Varselector: Write %04x:%04x\n", PRINT_REG(argp[1]));
-			else
-				debugN("Varselector: Read\n");
-#endif // VM_DEBUG_SEND
-
-			// argc == 0: read selector
-			// argc != 0: write selector
-			if (!argc) {
-				// read selector
-				if (activeBreakpointTypes & BREAK_SELECTORREAD) {
-					if (g_sci->checkSelectorBreakpoint(BREAK_SELECTORREAD, send_obj, selector))
-						debug("[read selector]\n");
-				}
-			} else {
-				// write selector
-				if (activeBreakpointTypes & BREAK_SELECTORWRITE) {
-					if (g_sci->checkSelectorBreakpoint(BREAK_SELECTORWRITE, send_obj, selector)) {
-						reg_t oldReg = *varp.getPointer(s->_segMan);
-						reg_t newReg = argp[1];
-						warning("[write to selector (%s:%s): change %04x:%04x to %04x:%04x]\n", 
-							s->_segMan->getObjectName(send_obj), g_sci->getKernel()->getSelectorName(selector).c_str(), 
-							PRINT_REG(oldReg), PRINT_REG(newReg));
-					}
-				}
-			}
-
-			if (argc > 1) {
-				// argc can indeed be bigger than 1 in some cases, and it's usually the
-				// result of a script bug. Usually these aren't fatal.
-
-				const char *objectName = s->_segMan->getObjectName(send_obj);
-
-				reg_t oldReg = *varp.getPointer(s->_segMan);
-				reg_t newReg = argp[1];
-				const char *selectorName = g_sci->getKernel()->getSelectorName(selector).c_str();
-				debug(2, "send_selector(): argc = %d while modifying variable selector "
-						"%x (%s) of object %04x:%04x (%s) from %04x:%04x to %04x:%04x",
-						argc, selector, selectorName, PRINT_REG(send_obj),
-						objectName, PRINT_REG(oldReg), PRINT_REG(newReg));
-			}
-
-			{
-				CallsStruct call;
-				call.address.var = varp; // register the call
-				call.argp = argp;
-				call.argc = argc;
-				call.selector = selector;
-				call.type = EXEC_STACK_TYPE_VARSELECTOR; // Register as a varselector
-				sendCalls.push(call);
-			}
-
+			call.address.var = varp; // register the call
+			call.type = EXEC_STACK_TYPE_VARSELECTOR; // Register as a varselector
 			break;
-
 		case kSelectorMethod:
-
-#ifndef VM_DEBUG_SEND
-			if (activeBreakpointTypes & BREAK_SELECTOREXEC) {
-				if (g_sci->checkSelectorBreakpoint(BREAK_SELECTOREXEC, send_obj, selector)) {
-					debugN("[execute selector]");
-
-					int displaySize = 0;
-					for (int argNr = 1; argNr <= argc; argNr++) {
-						if (argNr == 1)
-							debugN(" - ");
-						reg_t curParam = argp[argNr];
-						if (curParam.isPointer()) {
-							debugN("[%04x:%04x] ", PRINT_REG(curParam));
-							displaySize += 12;
-						} else {
-							debugN("[%04x] ", curParam.offset);
-							displaySize += 7;
-						}
-						if (displaySize > 50) {
-							if (argNr < argc)
-								debugN("...");
-							break;
-						}
-					}
-					debugN("\n");
-				}
-			}
-#else // VM_DEBUG_SEND
-			if (activeBreakpointTypes & BREAK_SELECTOREXEC)
-				g_sci->checkSelectorBreakpoint(BREAK_SELECTOREXEC, send_obj, selector);
-			debugN("Funcselector(");
-			for (int i = 0; i < argc; i++) {
-				debugN("%04x:%04x", PRINT_REG(argp[i+1]));
-				if (i + 1 < argc)
-					debugN(", ");
-			}
-			debugN(") at %04x:%04x\n", PRINT_REG(funcp));
-#endif // VM_DEBUG_SEND
-
-			{
-				CallsStruct call;
-				call.address.func = funcp; // register call
-				call.argp = argp;
-				call.argc = argc;
-				call.selector = selector;
-				call.type = EXEC_STACK_TYPE_CALL;
-				call.sp = sp;
-				sp = CALL_SP_CARRY; // Destroy sp, as it will be carried over
-				sendCalls.push(call);
-			}
-
+			call.address.func = funcp; // register call
+			call.type = EXEC_STACK_TYPE_CALL;
+			call.sp = sp;
+			sp = CALL_SP_CARRY; // Destroy sp, as it will be carried over
 			break;
 		} // switch (lookupSelector())
 
+		sendCalls.push(call);
+
 		framesize -= (2 + argc);
 		argp += argc + 1;
-	}
+	}	// while (framesize > 0)
 
 	// Iterate over all registered calls in the reverse order. This way, the first call is
 	// placed on the TOS; as soon as it returns, it will cause the second call to be executed.
 	while (!sendCalls.empty()) {
 		CallsStruct call = sendCalls.pop();
 		if (call.type == EXEC_STACK_TYPE_VARSELECTOR) // Write/read variable?
-			add_exec_stack_varselector(s->_executionStack, work_obj, call.argc, call.argp,
-			                                    call.selector, call.address.var, origin);
-		else
+			add_exec_stack_varselector(s->_executionStack, work_obj,
+										call.argc, call.argp, call.selector,
+										call.address.var, origin);
+		else	// Invoke function
 			add_exec_stack_entry(s->_executionStack, call.address.func, call.sp, work_obj,
-			                         call.argc, call.argp,
-			                         call.selector, -1, -1, send_obj, origin, SCI_XS_CALLEE_LOCALS);
+									call.argc, call.argp, call.selector,
+									-1, -1, send_obj, origin, SCI_XS_CALLEE_LOCALS);
 	}
 
 	_exec_varselectors(s);


Commit: d86504ef887c5f10fedbc081476a050295241cb4
    https://github.com/scummvm/scummvm/commit/d86504ef887c5f10fedbc081476a050295241cb4
Author: md5 (md5 at scummvm.org)
Date: 2011-03-25T04:37:00-07:00

Commit Message:
SCI: Cleaned up the BreakpointType enum and documented the bpe command

Changed paths:
    engines/sci/console.cpp
    engines/sci/debug.h
    engines/sci/engine/vm.cpp



diff --git a/engines/sci/console.cpp b/engines/sci/console.cpp
index 93d21c3..e2e5ee9 100644
--- a/engines/sci/console.cpp
+++ b/engines/sci/console.cpp
@@ -3154,10 +3154,9 @@ bool Console::cmdBreakpointKernel(int argc, const char **argv) {
 }
 
 bool Console::cmdBreakpointFunction(int argc, const char **argv) {
-	// TODO/FIXME: Why does this accept 2 parameters (the high and the low part of the address)?"
 	if (argc != 3) {
 		DebugPrintf("Sets a breakpoint on the execution of the specified exported function.\n");
-		DebugPrintf("Usage: %s <addr1> <addr2>\n", argv[0]);
+		DebugPrintf("Usage: %s <script number> <export number\n", argv[0]);
 		return true;
 	}
 
@@ -3166,6 +3165,7 @@ bool Console::cmdBreakpointFunction(int argc, const char **argv) {
 	   A breakpoint set on an invalid method name will just never trigger. */
 	Breakpoint bp;
 	bp.type = BREAK_EXPORT;
+	// script number, export number
 	bp.address = (atoi(argv[1]) << 16 | atoi(argv[2]));
 
 	_debugState._breakpoints.push_back(bp);
diff --git a/engines/sci/debug.h b/engines/sci/debug.h
index d9959f0..8ddbbd0 100644
--- a/engines/sci/debug.h
+++ b/engines/sci/debug.h
@@ -34,18 +34,18 @@ namespace Sci {
 // These types are used both as identifiers and as elements of bitfields
 enum BreakpointType {
 	/**
-	 * Break when selector is executed. data contains (char *) selector name
+	 * Break when a selector is executed. Data contains (char *) selector name
 	 * (in the format Object::Method)
 	 */
-	BREAK_SELECTOREXEC = 1 << 0, // break when selector gets executed
-	BREAK_SELECTORREAD = 1 << 1, // break when selector gets executed
-	BREAK_SELECTORWRITE = 1 << 2, // break when selector gets executed
+	BREAK_SELECTOREXEC  = 1 << 0, // break when a function selector is executed
+	BREAK_SELECTORREAD  = 1 << 1, // break when a variable selector is read
+	BREAK_SELECTORWRITE = 1 << 2, // break when a variable selector is written
 
 	/**
-	 * Break when an exported function is called. data contains
+	 * Break when an exported function is called. Data contains
 	 * script_no << 16 | export_no.
 	 */
-	BREAK_EXPORT = 1 << 3
+	BREAK_EXPORT        = 1 << 3
 };
 
 struct Breakpoint {
diff --git a/engines/sci/engine/vm.cpp b/engines/sci/engine/vm.cpp
index fdd4a46..1eb22ef 100644
--- a/engines/sci/engine/vm.cpp
+++ b/engines/sci/engine/vm.cpp
@@ -272,9 +272,7 @@ static void validate_write_var(reg_t *r, reg_t *stack_base, int type, int max, i
 
 bool SciEngine::checkExportBreakpoint(uint16 script, uint16 pubfunct) {
 	if (_debugState._activeBreakpointTypes & BREAK_EXPORT) {
-		uint32 bpaddress;
-
-		bpaddress = (script << 16 | pubfunct);
+		uint32 bpaddress = (script << 16 | pubfunct);
 
 		Common::List<Breakpoint>::const_iterator bp;
 		for (bp = _debugState._breakpoints.begin(); bp != _debugState._breakpoints.end(); ++bp) {






More information about the Scummvm-git-logs mailing list