[Scummvm-git-logs] scummvm master -> c7d631cb6697a4c8c6d9cc0e32caba79f9f7b922

wjp wjp at usecode.org
Sat Jun 10 21:36:51 CEST 2017


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

Summary:
3554875c7a SCI: Fix wildcard selector breakpoints
1e5c965a2b SCI: Move scriptdebug declarations to separate header
e9867356f5 SCI: Handle selector read/write breakpoints from opcodes
b56a49c53e SCI: Move backtrace output to scriptdebug.cpp
6d143e6da3 SCI: Remove union from Breakpoint
cb69d10e96 SCI: Add underscores to Breakpoint member variables
8e683db72b SCI: Add break/log/backtrace actions for triggered breakpoints
423ecde8e0 SCI: Move printObject from console to scriptdebug
be84cfdb59 SCI: Add inspect, none breakpoint actions
b3866aa3d5 SCI: Allow multiple breakpoints with same trigger but different action
4d34d586a6 SCI: Allow setting bp action directly on creation
0f0ecff0b8 SCI: Print breakpoint info on creation
e2e3f7c4c5 SCI: Move bpk/logkernel to main breakpoint infrastructure
4102a77964 SCI: Clean up breakpoint code (indentation, consistency)
e7b6a257b9 SCI: Change 'none' breakpoint action to 'ignore' for consistency
c7d631cb66 SCI: Expand kernel breakpoint pattern matching for negative matches


Commit: 3554875c7a583f03aea0c8192b8c0b53b76ddde7
    https://github.com/scummvm/scummvm/commit/3554875c7a583f03aea0c8192b8c0b53b76ddde7
Author: Willem Jan Palenstijn (wjp at usecode.org)
Date: 2017-06-10T21:32:35+02:00

Commit Message:
SCI: Fix wildcard selector breakpoints

0f9c33e02f1cb2c740c1eb0dcaad96dd22ec29e7 in 2011 broke selector
breakpoints of the type "ObjName::", which previously caught all
selector sends of the named object.

Thanks to TMM and snover for noticing.

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


diff --git a/engines/sci/engine/scriptdebug.cpp b/engines/sci/engine/scriptdebug.cpp
index 09b38d1..6a2324e 100644
--- a/engines/sci/engine/scriptdebug.cpp
+++ b/engines/sci/engine/scriptdebug.cpp
@@ -681,11 +681,14 @@ void Kernel::dissectScript(int scriptNumber, Vocabulary *vocab) {
 
 bool SciEngine::checkSelectorBreakpoint(BreakpointType breakpointType, reg_t send_obj, int selector) {
 	Common::String methodName = _gamestate->_segMan->getObjectName(send_obj);
-	methodName += ("::" + getKernel()->getSelectorName(selector));
+	methodName += "::" + getKernel()->getSelectorName(selector);
 
 	Common::List<Breakpoint>::const_iterator bpIter;
 	for (bpIter = _debugState._breakpoints.begin(); bpIter != _debugState._breakpoints.end(); ++bpIter) {
-		if ((*bpIter).type == breakpointType && (*bpIter).name == methodName) {
+		if (bpIter->type != breakpointType)
+			continue;
+		if (bpIter->name == methodName ||
+		    (bpIter->name.hasSuffix("::") && methodName.hasPrefix(bpIter->name))) {
 			_console->debugPrintf("Break on %s (in [%04x:%04x])\n", methodName.c_str(), PRINT_REG(send_obj));
 			_debugState.debugging = true;
 			_debugState.breakpointWasHit = true;


Commit: 1e5c965a2bf038193af93663bfe99d65a4b0f7cc
    https://github.com/scummvm/scummvm/commit/1e5c965a2bf038193af93663bfe99d65a4b0f7cc
Author: Willem Jan Palenstijn (wjp at usecode.org)
Date: 2017-06-10T21:32:35+02:00

Commit Message:
SCI: Move scriptdebug declarations to separate header

Changed paths:
  A engines/sci/engine/scriptdebug.h
    engines/sci/engine/vm.cpp


diff --git a/engines/sci/engine/scriptdebug.h b/engines/sci/engine/scriptdebug.h
new file mode 100644
index 0000000..15e7134
--- /dev/null
+++ b/engines/sci/engine/scriptdebug.h
@@ -0,0 +1,38 @@
+/* ScummVM - Graphic Adventure Engine
+ *
+ * ScummVM is the legal property of its developers, whose names
+ * are too numerous to list here. Please refer to the COPYRIGHT
+ * file distributed with this source distribution.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; either version 2
+ * of the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
+ *
+ */
+
+#ifndef SCI_ENGINE_SCRIPTDEBUG_H
+#define SCI_ENGINE_SCRIPTDEBUG_H
+
+namespace Sci {
+
+#ifndef REDUCE_MEMORY_USAGE
+extern const char *opcodeNames[];
+#endif
+
+void debugSelectorCall(reg_t send_obj, Selector selector, int argc, StackPtr argp, ObjVarRef &varp, reg_t funcp, SegManager *segMan, SelectorType selectorType);
+
+void logKernelCall(const KernelFunction *kernelCall, const KernelSubFunction *kernelSubCall, EngineState *s, int argc, reg_t *argv, reg_t result);
+
+}
+
+#endif
diff --git a/engines/sci/engine/vm.cpp b/engines/sci/engine/vm.cpp
index 12a9c3b..edffa60 100644
--- a/engines/sci/engine/vm.cpp
+++ b/engines/sci/engine/vm.cpp
@@ -36,6 +36,7 @@
 #include "sci/engine/selector.h"	// for SELECTOR
 #include "sci/engine/gc.h"
 #include "sci/engine/workarounds.h"
+#include "sci/engine/scriptdebug.h"
 
 namespace Sci {
 
@@ -114,10 +115,6 @@ static bool validate_variable(reg_t *r, reg_t *stack_base, int type, int max, in
 	return true;
 }
 
-#ifndef REDUCE_MEMORY_USAGE
-extern const char *opcodeNames[]; // from scriptdebug.cpp
-#endif
-
 static reg_t read_var(EngineState *s, int type, int index) {
 	if (validate_variable(s->variables[type], s->stack_base, type, s->variablesMax[type], index)) {
 		if (s->variables[type][index].getSegment() == kUninitializedSegment) {
@@ -257,8 +254,6 @@ static void _exec_varselectors(EngineState *s) {
 	}
 }
 
-// from scriptdebug.cpp
-extern void debugSelectorCall(reg_t send_obj, Selector selector, int argc, StackPtr argp, ObjVarRef &varp, reg_t funcp, SegManager *segMan, SelectorType selectorType);
 
 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'
@@ -340,9 +335,6 @@ static void addKernelCallToExecStack(EngineState *s, int kernelCallNr, int kerne
 	s->_executionStack.push_back(xstack);
 }
 
-// from scriptdebug.cpp
-extern void logKernelCall(const KernelFunction *kernelCall, const KernelSubFunction *kernelSubCall, EngineState *s, int argc, reg_t *argv, reg_t result);
-
 static void callKernelFunc(EngineState *s, int kernelCallNr, int argc) {
 	Kernel *kernel = g_sci->getKernel();
 


Commit: e9867356f5ccedf5d9a8a087852230b416d8d458
    https://github.com/scummvm/scummvm/commit/e9867356f5ccedf5d9a8a087852230b416d8d458
Author: Willem Jan Palenstijn (wjp at usecode.org)
Date: 2017-06-10T21:32:35+02:00

Commit Message:
SCI: Handle selector read/write breakpoints from opcodes

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


diff --git a/engines/sci/engine/scriptdebug.cpp b/engines/sci/engine/scriptdebug.cpp
index 6a2324e..28c7b95 100644
--- a/engines/sci/engine/scriptdebug.cpp
+++ b/engines/sci/engine/scriptdebug.cpp
@@ -804,6 +804,37 @@ void debugSelectorCall(reg_t send_obj, Selector selector, int argc, StackPtr arg
 	}	// switch
 }
 
+void debugPropertyAccess(Object *obj, reg_t objp, unsigned int index, reg_t curValue, reg_t newValue, SegManager *segMan, BreakpointType breakpointType) {
+	const Object *var_container = obj;
+	if (!obj->isClass() && getSciVersion() != SCI_VERSION_3)
+		var_container = segMan->getObject(obj->getSuperClassSelector());
+	if ((index >> 1) >= var_container->getVarCount()) {
+		// TODO: error, warning, debug?
+		return;
+	}
+	uint16 varSelector = var_container->getVarSelector(index >> 1);
+	if (g_sci->checkSelectorBreakpoint(breakpointType, objp, varSelector)) {
+		// checkSelectorBreakpoint has already triggered the breakpoint.
+		// We just output the relevant data here.
+
+		Console *con = g_sci->getSciDebugger();
+		const char *objectName = segMan->getObjectName(objp);
+		const char *selectorName = g_sci->getKernel()->getSelectorName(varSelector).c_str();
+		if (breakpointType == BREAK_SELECTORWRITE) {
+			con->debugPrintf("Write to selector (%s:%s): change %04x:%04x to %04x:%04x\n",
+								objectName, selectorName,
+								PRINT_REG(curValue), PRINT_REG(newValue));
+		} else if (breakpointType == BREAK_SELECTORREAD) {
+			con->debugPrintf("Read from selector (%s:%s): %04x:%04x\n",
+								objectName, selectorName,
+								PRINT_REG(curValue));
+
+		} else {
+			assert(false);
+		}
+	}
+}
+
 void logKernelCall(const KernelFunction *kernelCall, const KernelSubFunction *kernelSubCall, EngineState *s, int argc, reg_t *argv, reg_t result) {
 	Kernel *kernel = g_sci->getKernel();
 	if (!kernelSubCall) {
diff --git a/engines/sci/engine/scriptdebug.h b/engines/sci/engine/scriptdebug.h
index 15e7134..964b8a4 100644
--- a/engines/sci/engine/scriptdebug.h
+++ b/engines/sci/engine/scriptdebug.h
@@ -31,6 +31,8 @@ extern const char *opcodeNames[];
 
 void debugSelectorCall(reg_t send_obj, Selector selector, int argc, StackPtr argp, ObjVarRef &varp, reg_t funcp, SegManager *segMan, SelectorType selectorType);
 
+void debugPropertyAccess(Object *obj, reg_t objp, unsigned int index, reg_t curValue, reg_t newValue, SegManager *segMan, BreakpointType breakpointType);
+
 void logKernelCall(const KernelFunction *kernelCall, const KernelSubFunction *kernelSubCall, EngineState *s, int argc, reg_t *argv, reg_t result);
 
 }
diff --git a/engines/sci/engine/vm.cpp b/engines/sci/engine/vm.cpp
index edffa60..1e088d6 100644
--- a/engines/sci/engine/vm.cpp
+++ b/engines/sci/engine/vm.cpp
@@ -1129,29 +1129,60 @@ void run_vm(EngineState *s) {
 
 		case op_pToa: // 0x31 (49)
 			// Property To Accumulator
+			if (g_sci->_debugState._activeBreakpointTypes & BREAK_SELECTORREAD) {
+				debugPropertyAccess(obj, s->xs->objp, opparams[0],
+				                    validate_property(s, obj, opparams[0]), NULL_REG,
+				                    s->_segMan, BREAK_SELECTORREAD);
+			}
 			s->r_acc = validate_property(s, obj, opparams[0]);
 			break;
 
 		case op_aTop: // 0x32 (50)
+			{
 			// Accumulator To Property
-			validate_property(s, obj, opparams[0]) = s->r_acc;
+			reg_t &opProperty = validate_property(s, obj, opparams[0]);
+			if (g_sci->_debugState._activeBreakpointTypes & BREAK_SELECTORWRITE) {
+				debugPropertyAccess(obj, s->xs->objp, opparams[0],
+				                    opProperty, s->r_acc,
+				                    s->_segMan, BREAK_SELECTORWRITE);
+			}
+
+			opProperty = s->r_acc;
 #ifdef ENABLE_SCI32
 			updateInfoFlagViewVisible(obj, opparams[0], true);
 #endif
 			break;
+		}
 
 		case op_pTos: // 0x33 (51)
+			{
 			// Property To Stack
-			PUSH32(validate_property(s, obj, opparams[0]));
+			reg_t value = validate_property(s, obj, opparams[0]);
+			if (g_sci->_debugState._activeBreakpointTypes & BREAK_SELECTORREAD) {
+				debugPropertyAccess(obj, s->xs->objp, opparams[0],
+				                    value, NULL_REG,
+				                    s->_segMan, BREAK_SELECTORREAD);
+			}
+			PUSH32(value);
 			break;
+		}
 
 		case op_sTop: // 0x34 (52)
+			{
 			// Stack To Property
-			validate_property(s, obj, opparams[0]) = POP32();
+			reg_t newValue = POP32();
+			reg_t &opProperty = validate_property(s, obj, opparams[0]);
+			if (g_sci->_debugState._activeBreakpointTypes & BREAK_SELECTORWRITE) {
+				debugPropertyAccess(obj, s->xs->objp, opparams[0],
+				                    opProperty, newValue,
+				                    s->_segMan, BREAK_SELECTORWRITE);
+			}
+			opProperty = newValue;
 #ifdef ENABLE_SCI32
 			updateInfoFlagViewVisible(obj, opparams[0], true);
 #endif
 			break;
+		}
 
 		case op_ipToa: // 0x35 (53)
 		case op_dpToa: // 0x36 (54)
@@ -1161,10 +1192,25 @@ void run_vm(EngineState *s) {
 			// Increment/decrement a property and copy to accumulator,
 			// or push to stack
 			reg_t &opProperty = validate_property(s, obj, opparams[0]);
+			reg_t oldValue = opProperty;
+
+			if (g_sci->_debugState._activeBreakpointTypes & BREAK_SELECTORREAD) {
+				debugPropertyAccess(obj, s->xs->objp, opparams[0],
+				                    oldValue, NULL_REG,
+				                    s->_segMan, BREAK_SELECTORREAD);
+			}
+
 			if (opcode & 1)
 				opProperty += 1;
 			else
 				opProperty -= 1;
+
+			if (g_sci->_debugState._activeBreakpointTypes & BREAK_SELECTORWRITE) {
+				debugPropertyAccess(obj, s->xs->objp, opparams[0],
+				                    oldValue, opProperty,
+				                    s->_segMan, BREAK_SELECTORWRITE);
+			}
+
 #ifdef ENABLE_SCI32
 			updateInfoFlagViewVisible(obj, opparams[0], true);
 #endif


Commit: b56a49c53e90acc334d8c86d3e3c1f97662e881b
    https://github.com/scummvm/scummvm/commit/b56a49c53e90acc334d8c86d3e3c1f97662e881b
Author: Willem Jan Palenstijn (wjp at usecode.org)
Date: 2017-06-10T21:32:35+02:00

Commit Message:
SCI: Move backtrace output to scriptdebug.cpp

Changed paths:
    engines/sci/console.cpp
    engines/sci/engine/scriptdebug.cpp
    engines/sci/engine/scriptdebug.h


diff --git a/engines/sci/console.cpp b/engines/sci/console.cpp
index 211aab0..ec54642 100644
--- a/engines/sci/console.cpp
+++ b/engines/sci/console.cpp
@@ -34,6 +34,7 @@
 #include "sci/engine/savegame.h"
 #include "sci/engine/gc.h"
 #include "sci/engine/features.h"
+#include "sci/engine/scriptdebug.h"
 #include "sci/sound/midiparser_sci.h"
 #include "sci/sound/music.h"
 #include "sci/sound/drivers/mididriver.h"
@@ -3260,75 +3261,7 @@ void Console::printOffsets(int scriptNr, uint16 showType) {
 }
 
 bool Console::cmdBacktrace(int argc, const char **argv) {
-	debugPrintf("Call stack (current base: 0x%x):\n", _engine->_gamestate->executionStackBase);
-	Common::List<ExecStack>::const_iterator iter;
-	uint i = 0;
-
-	for (iter = _engine->_gamestate->_executionStack.begin();
-	     iter != _engine->_gamestate->_executionStack.end(); ++iter, ++i) {
-		const ExecStack &call = *iter;
-		const char *objname = _engine->_gamestate->_segMan->getObjectName(call.sendp);
-		int paramc, totalparamc;
-
-		switch (call.type) {
-		case EXEC_STACK_TYPE_CALL: // Normal function
-			if (call.type == EXEC_STACK_TYPE_CALL)
-			debugPrintf(" %x: script %d - ", i, (*(Script *)_engine->_gamestate->_segMan->_heap[call.addr.pc.getSegment()]).getScriptNumber());
-			if (call.debugSelector != -1) {
-				debugPrintf("%s::%s(", objname, _engine->getKernel()->getSelectorName(call.debugSelector).c_str());
-			} else if (call.debugExportId != -1) {
-				debugPrintf("export %d (", call.debugExportId);
-			} else if (call.debugLocalCallOffset != -1) {
-				debugPrintf("call %x (", call.debugLocalCallOffset);
-			}
-			break;
-
-		case EXEC_STACK_TYPE_KERNEL: // Kernel function
-			if (call.debugKernelSubFunction == -1)
-				debugPrintf(" %x:[%x]  k%s(", i, call.debugOrigin, _engine->getKernel()->getKernelName(call.debugKernelFunction).c_str());
-			else
-				debugPrintf(" %x:[%x]  k%s(", i, call.debugOrigin, _engine->getKernel()->getKernelName(call.debugKernelFunction, call.debugKernelSubFunction).c_str());
-			break;
-
-		case EXEC_STACK_TYPE_VARSELECTOR:
-			debugPrintf(" %x:[%x] vs%s %s::%s (", i, call.debugOrigin, (call.argc) ? "write" : "read",
-			          objname, _engine->getKernel()->getSelectorName(call.debugSelector).c_str());
-			break;
-		}
-
-		totalparamc = call.argc;
-
-		if (totalparamc > 16)
-			totalparamc = 16;
-
-		for (paramc = 1; paramc <= totalparamc; paramc++) {
-			debugPrintf("%04x:%04x", PRINT_REG(call.variables_argp[paramc]));
-
-			if (paramc < call.argc)
-				debugPrintf(", ");
-		}
-
-		if (call.argc > 16)
-			debugPrintf("...");
-
-		debugPrintf(")\n     ");
-		if (call.debugOrigin != -1)
-			debugPrintf("by %x ", call.debugOrigin);
-		debugPrintf("obj@%04x:%04x", PRINT_REG(call.objp));
-		if (call.type == EXEC_STACK_TYPE_CALL) {
-			debugPrintf(" pc=%04x:%04x", PRINT_REG(call.addr.pc));
-			if (call.sp == CALL_SP_CARRY)
-				debugPrintf(" sp,fp:carry");
-			else {
-				debugPrintf(" sp=ST:%04x", (unsigned)(call.sp - _engine->_gamestate->stack_base));
-				debugPrintf(" fp=ST:%04x", (unsigned)(call.fp - _engine->_gamestate->stack_base));
-			}
-		} else
-			debugPrintf(" pc:none");
-
-		debugPrintf(" argp:ST:%04x", (unsigned)(call.variables_argp - _engine->_gamestate->stack_base));
-		debugPrintf("\n");
-	}
+	logBacktrace();
 
 	return true;
 }
diff --git a/engines/sci/engine/scriptdebug.cpp b/engines/sci/engine/scriptdebug.cpp
index 28c7b95..17fc271 100644
--- a/engines/sci/engine/scriptdebug.cpp
+++ b/engines/sci/engine/scriptdebug.cpp
@@ -915,4 +915,84 @@ void logKernelCall(const KernelFunction *kernelCall, const KernelSubFunction *ke
 		debugN(" = %d\n", result.getOffset());
 }
 
+
+void logBacktrace() {
+	Console *con = g_sci->getSciDebugger();
+	EngineState *s = g_sci->getEngineState();
+
+	con->debugPrintf("Call stack (current base: 0x%x):\n", s->executionStackBase);
+	Common::List<ExecStack>::const_iterator iter;
+	uint i = 0;
+
+
+	for (iter = s->_executionStack.begin();
+	     iter != s->_executionStack.end(); ++iter, ++i) {
+		const ExecStack &call = *iter;
+		const char *objname = s->_segMan->getObjectName(call.sendp);
+		int paramc, totalparamc;
+
+		switch (call.type) {
+		case EXEC_STACK_TYPE_CALL: // Normal function
+			if (call.type == EXEC_STACK_TYPE_CALL)
+			con->debugPrintf(" %x: script %d - ", i, s->_segMan->getScript(call.addr.pc.getSegment())->getScriptNumber());
+			if (call.debugSelector != -1) {
+				con->debugPrintf("%s::%s(", objname, g_sci->getKernel()->getSelectorName(call.debugSelector).c_str());
+			} else if (call.debugExportId != -1) {
+				con->debugPrintf("export %d (", call.debugExportId);
+			} else if (call.debugLocalCallOffset != -1) {
+				con->debugPrintf("call %x (", call.debugLocalCallOffset);
+			}
+			break;
+
+		case EXEC_STACK_TYPE_KERNEL: // Kernel function
+			if (call.debugKernelSubFunction == -1)
+				con->debugPrintf(" %x:[%x]  k%s(", i, call.debugOrigin, g_sci->getKernel()->getKernelName(call.debugKernelFunction).c_str());
+			else
+				con->debugPrintf(" %x:[%x]  k%s(", i, call.debugOrigin, g_sci->getKernel()->getKernelName(call.debugKernelFunction, call.debugKernelSubFunction).c_str());
+			break;
+
+		case EXEC_STACK_TYPE_VARSELECTOR:
+			con->debugPrintf(" %x:[%x] vs%s %s::%s (", i, call.debugOrigin, (call.argc) ? "write" : "read",
+			          objname, g_sci->getKernel()->getSelectorName(call.debugSelector).c_str());
+			break;
+		}
+
+		totalparamc = call.argc;
+
+		if (totalparamc > 16)
+			totalparamc = 16;
+
+		for (paramc = 1; paramc <= totalparamc; paramc++) {
+			con->debugPrintf("%04x:%04x", PRINT_REG(call.variables_argp[paramc]));
+
+			if (paramc < call.argc)
+				con->debugPrintf(", ");
+		}
+
+		if (call.argc > 16)
+			con->debugPrintf("...");
+
+		con->debugPrintf(")\n     ");
+		if (call.debugOrigin != -1)
+			con->debugPrintf("by %x ", call.debugOrigin);
+		con->debugPrintf("obj@%04x:%04x", PRINT_REG(call.objp));
+		if (call.type == EXEC_STACK_TYPE_CALL) {
+			con->debugPrintf(" pc=%04x:%04x", PRINT_REG(call.addr.pc));
+			if (call.sp == CALL_SP_CARRY)
+				con->debugPrintf(" sp,fp:carry");
+			else {
+				con->debugPrintf(" sp=ST:%04x", (unsigned)(call.sp - s->stack_base));
+				con->debugPrintf(" fp=ST:%04x", (unsigned)(call.fp - s->stack_base));
+			}
+		} else
+			con->debugPrintf(" pc:none");
+
+		con->debugPrintf(" argp:ST:%04x", (unsigned)(call.variables_argp - s->stack_base));
+		con->debugPrintf("\n");
+	}
+}
+
+
+
+
 } // End of namespace Sci
diff --git a/engines/sci/engine/scriptdebug.h b/engines/sci/engine/scriptdebug.h
index 964b8a4..1d8a2ef 100644
--- a/engines/sci/engine/scriptdebug.h
+++ b/engines/sci/engine/scriptdebug.h
@@ -35,6 +35,8 @@ void debugPropertyAccess(Object *obj, reg_t objp, unsigned int index, reg_t curV
 
 void logKernelCall(const KernelFunction *kernelCall, const KernelSubFunction *kernelSubCall, EngineState *s, int argc, reg_t *argv, reg_t result);
 
+void logBacktrace();
+
 }
 
 #endif


Commit: 6d143e6da3e972fcf69c73028db702e89c53e278
    https://github.com/scummvm/scummvm/commit/6d143e6da3e972fcf69c73028db702e89c53e278
Author: Willem Jan Palenstijn (wjp at usecode.org)
Date: 2017-06-10T21:32:35+02:00

Commit Message:
SCI: Remove union from Breakpoint

Changed paths:
    engines/sci/debug.h


diff --git a/engines/sci/debug.h b/engines/sci/debug.h
index 60fad26..d7aa1b6 100644
--- a/engines/sci/debug.h
+++ b/engines/sci/debug.h
@@ -48,10 +48,8 @@ enum BreakpointType {
 
 struct Breakpoint {
 	BreakpointType type;
-	union {
-		uint32 address;     ///< Breakpoints on exports
-		reg32_t regAddress; ///< Breakpoints on addresses
-	};
+	uint32 address;     ///< Breakpoints on exports
+	reg32_t regAddress; ///< Breakpoints on addresses
 	Common::String name; ///< Breakpoints on selector names
 };
 


Commit: cb69d10e962e2cea604175fdd35189726f6a6436
    https://github.com/scummvm/scummvm/commit/cb69d10e962e2cea604175fdd35189726f6a6436
Author: Willem Jan Palenstijn (wjp at usecode.org)
Date: 2017-06-10T21:32:35+02:00

Commit Message:
SCI: Add underscores to Breakpoint member variables

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


diff --git a/engines/sci/console.cpp b/engines/sci/console.cpp
index ec54642..2274dac 100644
--- a/engines/sci/console.cpp
+++ b/engines/sci/console.cpp
@@ -3729,22 +3729,22 @@ bool Console::cmdBreakpointList(int argc, const char **argv) {
 	Common::List<Breakpoint>::const_iterator end = _debugState._breakpoints.end();
 	for (; bp != end; ++bp) {
 		debugPrintf("  #%i: ", i);
-		switch (bp->type) {
+		switch (bp->_type) {
 		case BREAK_SELECTOREXEC:
-			debugPrintf("Execute %s\n", bp->name.c_str());
+			debugPrintf("Execute %s\n", bp->_name.c_str());
 			break;
 		case BREAK_SELECTORREAD:
-			debugPrintf("Read %s\n", bp->name.c_str());
+			debugPrintf("Read %s\n", bp->_name.c_str());
 			break;
 		case BREAK_SELECTORWRITE:
-			debugPrintf("Write %s\n", bp->name.c_str());
+			debugPrintf("Write %s\n", bp->_name.c_str());
 			break;
 		case BREAK_EXPORT:
-			bpdata = bp->address;
+			bpdata = bp->_address;
 			debugPrintf("Execute script %d, export %d\n", bpdata >> 16, bpdata & 0xFFFF);
 			break;
 		case BREAK_ADDRESS:
-			debugPrintf("Execute address %04x:%04x\n", PRINT_REG(bp->regAddress));
+			debugPrintf("Execute address %04x:%04x\n", PRINT_REG(bp->_regAddress));
 		}
 
 		i++;
@@ -3790,7 +3790,7 @@ bool Console::cmdBreakpointDelete(int argc, const char **argv) {
 	// Update EngineState::_activeBreakpointTypes.
 	int type = 0;
 	for (bp = _debugState._breakpoints.begin(); bp != end; ++bp) {
-		type |= bp->type;
+		type |= bp->_type;
 	}
 
 	_debugState._activeBreakpointTypes = type;
@@ -3812,8 +3812,8 @@ bool Console::cmdBreakpointMethod(int argc, const char **argv) {
 	   Thus, we can't check whether the command argument is a valid method name.
 	   A breakpoint set on an invalid method name will just never trigger. */
 	Breakpoint bp;
-	bp.type = BREAK_SELECTOREXEC;
-	bp.name = argv[1];
+	bp._type = BREAK_SELECTOREXEC;
+	bp._name = argv[1];
 
 	_debugState._breakpoints.push_back(bp);
 	_debugState._activeBreakpointTypes |= BREAK_SELECTOREXEC;
@@ -3829,8 +3829,8 @@ bool Console::cmdBreakpointRead(int argc, const char **argv) {
 	}
 
 	Breakpoint bp;
-	bp.type = BREAK_SELECTORREAD;
-	bp.name = argv[1];
+	bp._type = BREAK_SELECTORREAD;
+	bp._name = argv[1];
 
 	_debugState._breakpoints.push_back(bp);
 	_debugState._activeBreakpointTypes |= BREAK_SELECTORREAD;
@@ -3846,8 +3846,8 @@ bool Console::cmdBreakpointWrite(int argc, const char **argv) {
 	}
 
 	Breakpoint bp;
-	bp.type = BREAK_SELECTORWRITE;
-	bp.name = argv[1];
+	bp._type = BREAK_SELECTORWRITE;
+	bp._name = argv[1];
 
 	_debugState._breakpoints.push_back(bp);
 	_debugState._activeBreakpointTypes |= BREAK_SELECTORWRITE;
@@ -3891,9 +3891,9 @@ bool Console::cmdBreakpointFunction(int argc, const char **argv) {
 	   Thus, we can't check whether the command argument is a valid method name.
 	   A breakpoint set on an invalid method name will just never trigger. */
 	Breakpoint bp;
-	bp.type = BREAK_EXPORT;
+	bp._type = BREAK_EXPORT;
 	// script number, export number
-	bp.address = (atoi(argv[1]) << 16 | atoi(argv[2]));
+	bp._address = (atoi(argv[1]) << 16 | atoi(argv[2]));
 
 	_debugState._breakpoints.push_back(bp);
 	_debugState._activeBreakpointTypes |= BREAK_EXPORT;
@@ -3917,8 +3917,8 @@ bool Console::cmdBreakpointAddress(int argc, const char **argv) {
 	}
 
 	Breakpoint bp;
-	bp.type = BREAK_ADDRESS;
-	bp.regAddress = make_reg32(addr.getSegment(), addr.getOffset());
+	bp._type = BREAK_ADDRESS;
+	bp._regAddress = make_reg32(addr.getSegment(), addr.getOffset());
 
 	_debugState._breakpoints.push_back(bp);
 	_debugState._activeBreakpointTypes |= BREAK_ADDRESS;
diff --git a/engines/sci/debug.h b/engines/sci/debug.h
index d7aa1b6..b8557bb 100644
--- a/engines/sci/debug.h
+++ b/engines/sci/debug.h
@@ -47,10 +47,10 @@ enum BreakpointType {
 };
 
 struct Breakpoint {
-	BreakpointType type;
-	uint32 address;     ///< Breakpoints on exports
-	reg32_t regAddress; ///< Breakpoints on addresses
-	Common::String name; ///< Breakpoints on selector names
+	BreakpointType _type;
+	uint32 _address;     ///< Breakpoints on exports
+	reg32_t _regAddress; ///< Breakpoints on addresses
+	Common::String _name; ///< Breakpoints on selector names
 };
 
 enum DebugSeeking {
diff --git a/engines/sci/engine/scriptdebug.cpp b/engines/sci/engine/scriptdebug.cpp
index 17fc271..a439298 100644
--- a/engines/sci/engine/scriptdebug.cpp
+++ b/engines/sci/engine/scriptdebug.cpp
@@ -685,10 +685,10 @@ bool SciEngine::checkSelectorBreakpoint(BreakpointType breakpointType, reg_t sen
 
 	Common::List<Breakpoint>::const_iterator bpIter;
 	for (bpIter = _debugState._breakpoints.begin(); bpIter != _debugState._breakpoints.end(); ++bpIter) {
-		if (bpIter->type != breakpointType)
+		if (bpIter->_type != breakpointType)
 			continue;
-		if (bpIter->name == methodName ||
-		    (bpIter->name.hasSuffix("::") && methodName.hasPrefix(bpIter->name))) {
+		if (bpIter->_name == methodName ||
+		    (bpIter->_name.hasSuffix("::") && methodName.hasPrefix(bpIter->_name))) {
 			_console->debugPrintf("Break on %s (in [%04x:%04x])\n", methodName.c_str(), PRINT_REG(send_obj));
 			_debugState.debugging = true;
 			_debugState.breakpointWasHit = true;
@@ -704,7 +704,7 @@ bool SciEngine::checkExportBreakpoint(uint16 script, uint16 pubfunct) {
 
 		Common::List<Breakpoint>::const_iterator bp;
 		for (bp = _debugState._breakpoints.begin(); bp != _debugState._breakpoints.end(); ++bp) {
-			if (bp->type == BREAK_EXPORT && bp->address == bpaddress) {
+			if (bp->_type == BREAK_EXPORT && bp->_address == bpaddress) {
 				_console->debugPrintf("Break on script %d, export %d\n", script, pubfunct);
 				_debugState.debugging = true;
 				_debugState.breakpointWasHit = true;
@@ -720,7 +720,7 @@ bool SciEngine::checkAddressBreakpoint(const reg32_t &address) {
 	if (_debugState._activeBreakpointTypes & BREAK_ADDRESS) {
 		Common::List<Breakpoint>::const_iterator bp;
 		for (bp = _debugState._breakpoints.begin(); bp != _debugState._breakpoints.end(); ++bp) {
-			if (bp->type == BREAK_ADDRESS && bp->regAddress == address) {
+			if (bp->_type == BREAK_ADDRESS && bp->_regAddress == address) {
 				_console->debugPrintf("Break at %04x:%04x\n", PRINT_REG(address));
 				_debugState.debugging = true;
 				_debugState.breakpointWasHit = true;


Commit: 8e683db72b280fbe4319d68466c7f3c61a6c107d
    https://github.com/scummvm/scummvm/commit/8e683db72b280fbe4319d68466c7f3c61a6c107d
Author: Willem Jan Palenstijn (wjp at usecode.org)
Date: 2017-06-10T21:32:35+02:00

Commit Message:
SCI: Add break/log/backtrace actions for triggered breakpoints

The action can be set using the new console command bp_action/bpact.

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


diff --git a/engines/sci/console.cpp b/engines/sci/console.cpp
index 2274dac..6c9be97 100644
--- a/engines/sci/console.cpp
+++ b/engines/sci/console.cpp
@@ -205,6 +205,8 @@ Console::Console(SciEngine *engine) : GUI::Debugger(),
 	registerCmd("bp_del",				WRAP_METHOD(Console, cmdBreakpointDelete));
 	registerCmd("bpdel",				WRAP_METHOD(Console, cmdBreakpointDelete));			// alias
 	registerCmd("bc",					WRAP_METHOD(Console, cmdBreakpointDelete));			// alias
+	registerCmd("bp_action",				WRAP_METHOD(Console, cmdBreakpointAction));
+	registerCmd("bpact",				WRAP_METHOD(Console, cmdBreakpointAction));			// alias
 	registerCmd("bp_address",			WRAP_METHOD(Console, cmdBreakpointAddress));
 	registerCmd("bpa",					WRAP_METHOD(Console, cmdBreakpointAddress));		// alias
 	registerCmd("bp_method",			WRAP_METHOD(Console, cmdBreakpointMethod));
@@ -443,6 +445,7 @@ bool Console::cmdHelp(int argc, const char **argv) {
 	debugPrintf("Breakpoints:\n");
 	debugPrintf(" bp_list / bplist / bl - Lists the current breakpoints\n");
 	debugPrintf(" bp_del / bpdel / bc - Deletes a breakpoint with the specified index\n");
+	debugPrintf(" bp_action / bpact - Set action to be performed when breakpoint is triggered\n");
 	debugPrintf(" bp_address / bpa - Sets a breakpoint on a script address\n");
 	debugPrintf(" bp_method / bpx - Sets a breakpoint on the execution of a specified method/selector\n");
 	debugPrintf(" bp_read / bpr - Sets a breakpoint on reading of a specified selector\n");
@@ -3729,22 +3732,34 @@ bool Console::cmdBreakpointList(int argc, const char **argv) {
 	Common::List<Breakpoint>::const_iterator end = _debugState._breakpoints.end();
 	for (; bp != end; ++bp) {
 		debugPrintf("  #%i: ", i);
+		const char *bpaction;
+
+		switch (bp->_action) {
+		case BREAK_LOG:
+			bpaction = " (action: log only)";
+			break;
+		case BREAK_BACKTRACE:
+			bpaction = " (action: show backtrace)";
+			break;
+		default:
+			bpaction = "";
+		}
 		switch (bp->_type) {
 		case BREAK_SELECTOREXEC:
-			debugPrintf("Execute %s\n", bp->_name.c_str());
+			debugPrintf("Execute %s%s\n", bp->_name.c_str(), bpaction);
 			break;
 		case BREAK_SELECTORREAD:
-			debugPrintf("Read %s\n", bp->_name.c_str());
+			debugPrintf("Read %s%s\n", bp->_name.c_str(), bpaction);
 			break;
 		case BREAK_SELECTORWRITE:
-			debugPrintf("Write %s\n", bp->_name.c_str());
+			debugPrintf("Write %s%s\n", bp->_name.c_str(), bpaction);
 			break;
 		case BREAK_EXPORT:
 			bpdata = bp->_address;
-			debugPrintf("Execute script %d, export %d\n", bpdata >> 16, bpdata & 0xFFFF);
+			debugPrintf("Execute script %d, export %d%s\n", bpdata >> 16, bpdata & 0xFFFF, bpaction);
 			break;
 		case BREAK_ADDRESS:
-			debugPrintf("Execute address %04x:%04x\n", PRINT_REG(bp->_regAddress));
+			debugPrintf("Execute address %04x:%04x%s\n", PRINT_REG(bp->_regAddress), bpaction);
 		}
 
 		i++;
@@ -3798,6 +3813,59 @@ bool Console::cmdBreakpointDelete(int argc, const char **argv) {
 	return true;
 }
 
+bool Console::cmdBreakpointAction(int argc, const char **argv) {
+	bool usage = false;
+
+	if (argc != 3) {
+		usage = true;
+	}
+
+	Common::String arg;
+	if (argc >= 3)
+		arg = argv[2];
+
+	BreakpointAction bpaction;
+	if (arg == "break")
+		bpaction = BREAK_BREAK;
+	else if (arg == "log")
+		bpaction = BREAK_LOG;
+	else if (arg == "bt")
+		bpaction = BREAK_BACKTRACE;
+	else
+		usage = true;
+
+	if (usage) {
+		debugPrintf("Change the action for the breakpoint with the specified index.\n");
+		debugPrintf("Usage: %s <breakpoint index> break|log|bt\n", argv[0]);
+		debugPrintf("<index> * will process all breakpoints\n");
+		return true;
+	}
+
+	Common::List<Breakpoint>::iterator bp = _debugState._breakpoints.begin();
+	const Common::List<Breakpoint>::iterator end = _debugState._breakpoints.end();
+	if (strcmp(argv[1], "*") == 0) {
+		for (; bp != end; ++bp)
+			bp->_action = bpaction;
+		return true;	
+	}
+
+	const int idx = atoi(argv[1]);
+
+	// Find the breakpoint at index idx.
+	for (int i = 0; bp != end && i < idx; ++bp, ++i) {
+		// do nothing
+	}
+
+	if (bp == end) {
+		debugPrintf("Invalid breakpoint index %i\n", idx);
+		return true;
+	}
+
+	bp->_action = bpaction;
+	return true;
+}
+
+
 bool Console::cmdBreakpointMethod(int argc, const char **argv) {
 	if (argc != 2) {
 		debugPrintf("Sets a breakpoint on execution of a specified method/selector.\n");
@@ -3814,6 +3882,7 @@ bool Console::cmdBreakpointMethod(int argc, const char **argv) {
 	Breakpoint bp;
 	bp._type = BREAK_SELECTOREXEC;
 	bp._name = argv[1];
+	bp._action = BREAK_BREAK;
 
 	_debugState._breakpoints.push_back(bp);
 	_debugState._activeBreakpointTypes |= BREAK_SELECTOREXEC;
@@ -3831,6 +3900,7 @@ bool Console::cmdBreakpointRead(int argc, const char **argv) {
 	Breakpoint bp;
 	bp._type = BREAK_SELECTORREAD;
 	bp._name = argv[1];
+	bp._action = BREAK_BREAK;
 
 	_debugState._breakpoints.push_back(bp);
 	_debugState._activeBreakpointTypes |= BREAK_SELECTORREAD;
@@ -3848,6 +3918,7 @@ bool Console::cmdBreakpointWrite(int argc, const char **argv) {
 	Breakpoint bp;
 	bp._type = BREAK_SELECTORWRITE;
 	bp._name = argv[1];
+	bp._action = BREAK_BREAK;
 
 	_debugState._breakpoints.push_back(bp);
 	_debugState._activeBreakpointTypes |= BREAK_SELECTORWRITE;
@@ -3894,6 +3965,7 @@ bool Console::cmdBreakpointFunction(int argc, const char **argv) {
 	bp._type = BREAK_EXPORT;
 	// script number, export number
 	bp._address = (atoi(argv[1]) << 16 | atoi(argv[2]));
+	bp._action = BREAK_BREAK;
 
 	_debugState._breakpoints.push_back(bp);
 	_debugState._activeBreakpointTypes |= BREAK_EXPORT;
@@ -3919,6 +3991,7 @@ bool Console::cmdBreakpointAddress(int argc, const char **argv) {
 	Breakpoint bp;
 	bp._type = BREAK_ADDRESS;
 	bp._regAddress = make_reg32(addr.getSegment(), addr.getOffset());
+	bp._action = BREAK_BREAK;
 
 	_debugState._breakpoints.push_back(bp);
 	_debugState._activeBreakpointTypes |= BREAK_ADDRESS;
diff --git a/engines/sci/console.h b/engines/sci/console.h
index 1bb86b9..d0faf8a 100644
--- a/engines/sci/console.h
+++ b/engines/sci/console.h
@@ -149,6 +149,7 @@ private:
 	// Breakpoints
 	bool cmdBreakpointList(int argc, const char **argv);
 	bool cmdBreakpointDelete(int argc, const char **argv);
+	bool cmdBreakpointAction(int argc, const char **argv);
 	bool cmdBreakpointMethod(int argc, const char **argv);
 	bool cmdBreakpointRead(int argc, const char **argv);
 	bool cmdBreakpointWrite(int argc, const char **argv);
diff --git a/engines/sci/debug.h b/engines/sci/debug.h
index b8557bb..03ab594 100644
--- a/engines/sci/debug.h
+++ b/engines/sci/debug.h
@@ -46,11 +46,18 @@ enum BreakpointType {
 	BREAK_ADDRESS       = 1 << 4  // break when pc is at this address
 };
 
+enum BreakpointAction {
+	BREAK_BREAK, // break into debugger when breakpoint is triggered
+	BREAK_LOG, // log the breakpoint, and don't break into debugger
+	BREAK_BACKTRACE // show a backtrace, and don't break into debugger
+};
+
 struct Breakpoint {
 	BreakpointType _type;
 	uint32 _address;     ///< Breakpoints on exports
 	reg32_t _regAddress; ///< Breakpoints on addresses
 	Common::String _name; ///< Breakpoints on selector names
+	BreakpointAction _action;
 };
 
 enum DebugSeeking {
diff --git a/engines/sci/engine/scriptdebug.cpp b/engines/sci/engine/scriptdebug.cpp
index a439298..c5a617a 100644
--- a/engines/sci/engine/scriptdebug.cpp
+++ b/engines/sci/engine/scriptdebug.cpp
@@ -28,6 +28,7 @@
 #include "sci/engine/state.h"
 #include "sci/engine/kernel.h"
 #include "sci/engine/script.h"
+#include "sci/engine/scriptdebug.h"
 
 namespace Sci {
 
@@ -690,8 +691,12 @@ bool SciEngine::checkSelectorBreakpoint(BreakpointType breakpointType, reg_t sen
 		if (bpIter->_name == methodName ||
 		    (bpIter->_name.hasSuffix("::") && methodName.hasPrefix(bpIter->_name))) {
 			_console->debugPrintf("Break on %s (in [%04x:%04x])\n", methodName.c_str(), PRINT_REG(send_obj));
-			_debugState.debugging = true;
-			_debugState.breakpointWasHit = true;
+			if (bpIter->_action == BREAK_BREAK) {
+				_debugState.debugging = true;
+				_debugState.breakpointWasHit = true;
+			} else if (bpIter->_action == BREAK_BACKTRACE) {
+				logBacktrace();
+			}
 			return true;
 		}
 	}
@@ -706,8 +711,12 @@ bool SciEngine::checkExportBreakpoint(uint16 script, uint16 pubfunct) {
 		for (bp = _debugState._breakpoints.begin(); bp != _debugState._breakpoints.end(); ++bp) {
 			if (bp->_type == BREAK_EXPORT && bp->_address == bpaddress) {
 				_console->debugPrintf("Break on script %d, export %d\n", script, pubfunct);
-				_debugState.debugging = true;
-				_debugState.breakpointWasHit = true;
+				if (bp->_action == BREAK_BREAK) {
+					_debugState.debugging = true;
+					_debugState.breakpointWasHit = true;
+				} else if (bp->_action == BREAK_BACKTRACE) {
+					logBacktrace();
+				}
 				return true;
 			}
 		}
@@ -722,8 +731,12 @@ bool SciEngine::checkAddressBreakpoint(const reg32_t &address) {
 		for (bp = _debugState._breakpoints.begin(); bp != _debugState._breakpoints.end(); ++bp) {
 			if (bp->_type == BREAK_ADDRESS && bp->_regAddress == address) {
 				_console->debugPrintf("Break at %04x:%04x\n", PRINT_REG(address));
-				_debugState.debugging = true;
-				_debugState.breakpointWasHit = true;
+				if (bp->_action == BREAK_BREAK) {
+					_debugState.debugging = true;
+					_debugState.breakpointWasHit = true;
+				} else if (bp->_action == BREAK_BACKTRACE) {
+					logBacktrace();
+				}
 				return true;
 			}
 		}


Commit: 423ecde8e0e20d664ad8e41496bdf98cf94407da
    https://github.com/scummvm/scummvm/commit/423ecde8e0e20d664ad8e41496bdf98cf94407da
Author: Willem Jan Palenstijn (wjp at usecode.org)
Date: 2017-06-10T21:32:35+02:00

Commit Message:
SCI: Move printObject from console to scriptdebug

Changed paths:
    engines/sci/console.cpp
    engines/sci/console.h
    engines/sci/engine/scriptdebug.cpp
    engines/sci/engine/scriptdebug.h
    engines/sci/sci.cpp


diff --git a/engines/sci/console.cpp b/engines/sci/console.cpp
index 6c9be97..6efe684 100644
--- a/engines/sci/console.cpp
+++ b/engines/sci/console.cpp
@@ -4745,65 +4745,6 @@ void Console::printBitmap(reg_t reg) {
 
 #endif
 
-int Console::printObject(reg_t pos) {
-	EngineState *s = _engine->_gamestate;	// for the several defines in this function
-	const Object *obj = s->_segMan->getObject(pos);
-	const Object *var_container = obj;
-	uint i;
-
-	if (!obj) {
-		debugPrintf("[%04x:%04x]: Not an object.\n", PRINT_REG(pos));
-		return 1;
-	}
-
-	// Object header
-	debugPrintf("[%04x:%04x] %s : %3d vars, %3d methods\n", PRINT_REG(pos), s->_segMan->getObjectName(pos),
-				obj->getVarCount(), obj->getMethodCount());
-
-	if (!obj->isClass())
-		var_container = s->_segMan->getObject(obj->getSuperClassSelector());
-	debugPrintf("  -- member variables:\n");
-
-	if (getSciVersion() == SCI_VERSION_3) {
-		debugPrintf("    (----) [---] -size- = 0000:%04x (%d)\n", obj->getVarCount(), obj->getVarCount());
-		debugPrintf("    (----) [---] -classScript- = %04x:%04x (%d)\n", PRINT_REG(obj->getClassScriptSelector()), obj->getClassScriptSelector().getOffset());
-		debugPrintf("    (----) [---] -species- = %04x:%04x (%s)\n", PRINT_REG(obj->getSpeciesSelector()), s->_segMan->getObjectName(obj->getSpeciesSelector()));
-		debugPrintf("    (----) [---] -super- = %04x:%04x (%s)\n", PRINT_REG(obj->getSuperClassSelector()), s->_segMan->getObjectName(obj->getSuperClassSelector()));
-		debugPrintf("    (----) [---] -info- = %04x:%04x (%d)\n", PRINT_REG(obj->getInfoSelector()), obj->getInfoSelector().getOffset());
-	}
-
-	for (i = 0; (uint)i < obj->getVarCount(); i++) {
-		debugPrintf("    ");
-		if (var_container && i < var_container->getVarCount()) {
-			uint16 varSelector = var_container->getVarSelector(i);
-			// Times two commented out for now for easy parsing of vocab.994
-			debugPrintf("(%04x) [%03x] %s = ", i /* *2 */, varSelector, _engine->getKernel()->getSelectorName(varSelector).c_str());
-		} else
-			debugPrintf("p#%x = ", i);
-
-		reg_t val = obj->getVariable(i);
-		debugPrintf("%04x:%04x", PRINT_REG(val));
-
-		if (!val.getSegment())
-			debugPrintf(" (%d)", val.getOffset());
-
-		const Object *ref = s->_segMan->getObject(val);
-		if (ref)
-			debugPrintf(" (%s)", s->_segMan->getObjectName(val));
-
-		debugPrintf("\n");
-	}
-	debugPrintf("  -- methods:\n");
-	for (i = 0; i < obj->getMethodCount(); i++) {
-		reg_t fptr = obj->getFunction(i);
-		debugPrintf("    [%03x] %s = %04x:%04x\n", obj->getFuncSelector(i), _engine->getKernel()->getSelectorName(obj->getFuncSelector(i)).c_str(), PRINT_REG(fptr));
-	}
-	if (s->_segMan->_heap[pos.getSegment()]->getType() == SEG_TYPE_SCRIPT)
-		debugPrintf("\nOwner script: %d\n", s->_segMan->getScript(pos.getSegment())->getScriptNumber());
-
-	return 0;
-}
-
 static void printChar(byte c) {
 	if (c < 32 || c >= 127)
 		c = '.';
diff --git a/engines/sci/console.h b/engines/sci/console.h
index d0faf8a..75243e5 100644
--- a/engines/sci/console.h
+++ b/engines/sci/console.h
@@ -45,7 +45,6 @@ public:
 	void printArray(reg_t reg);
 	void printBitmap(reg_t reg);
 #endif
-	int printObject(reg_t reg);
 
 private:
 	virtual void preEnter();
diff --git a/engines/sci/engine/scriptdebug.cpp b/engines/sci/engine/scriptdebug.cpp
index c5a617a..ce1d2dd 100644
--- a/engines/sci/engine/scriptdebug.cpp
+++ b/engines/sci/engine/scriptdebug.cpp
@@ -1005,6 +1005,70 @@ void logBacktrace() {
 	}
 }
 
+bool printObject(reg_t pos) {
+	Console *con = g_sci->getSciDebugger();
+	EngineState *s = g_sci->getEngineState();
+	const Object *obj = s->_segMan->getObject(pos);
+	const Object *var_container = obj;
+	uint i;
+
+	if (!obj) {
+		con->debugPrintf("[%04x:%04x]: Not an object.\n", PRINT_REG(pos));
+		return false;
+	}
+
+	// Object header
+	con->debugPrintf("[%04x:%04x] %s : %3d vars, %3d methods\n", PRINT_REG(pos), s->_segMan->getObjectName(pos),
+	                 obj->getVarCount(), obj->getMethodCount());
+
+	if (!obj->isClass())
+		var_container = s->_segMan->getObject(obj->getSuperClassSelector());
+	con->debugPrintf("  -- member variables:\n");
+
+	if (getSciVersion() == SCI_VERSION_3) {
+		con->debugPrintf("    (----) [---] -size- = 0000:%04x (%d)\n", obj->getVarCount(), obj->getVarCount());
+		con->debugPrintf("    (----) [---] -classScript- = %04x:%04x (%d)\n", PRINT_REG(obj->getClassScriptSelector()), obj->getClassScriptSelector().getOffset());
+		con->debugPrintf("    (----) [---] -species- = %04x:%04x (%s)\n", PRINT_REG(obj->getSpeciesSelector()), s->_segMan->getObjectName(obj->getSpeciesSelector()));
+		con->debugPrintf("    (----) [---] -super- = %04x:%04x (%s)\n", PRINT_REG(obj->getSuperClassSelector()), s->_segMan->getObjectName(obj->getSuperClassSelector()));
+		con->debugPrintf("    (----) [---] -info- = %04x:%04x (%d)\n", PRINT_REG(obj->getInfoSelector()), obj->getInfoSelector().getOffset());
+	}
+
+	for (i = 0; (uint)i < obj->getVarCount(); i++) {
+		con->debugPrintf("    ");
+		if (var_container && i < var_container->getVarCount()) {
+			uint16 varSelector = var_container->getVarSelector(i);
+			// Times two commented out for now for easy parsing of vocab.994
+			con->debugPrintf("(%04x) [%03x] %s = ", i /* *2 */, varSelector, g_sci->getKernel()->getSelectorName(varSelector).c_str());
+		} else
+			con->debugPrintf("p#%x = ", i);
+
+		reg_t val = obj->getVariable(i);
+		con->debugPrintf("%04x:%04x", PRINT_REG(val));
+
+		if (!val.getSegment())
+			con->debugPrintf(" (%d)", val.getOffset());
+
+		const Object *ref = s->_segMan->getObject(val);
+		if (ref)
+			con->debugPrintf(" (%s)", s->_segMan->getObjectName(val));
+
+		con->debugPrintf("\n");
+	}
+	con->debugPrintf("  -- methods:\n");
+	for (i = 0; i < obj->getMethodCount(); i++) {
+		reg_t fptr = obj->getFunction(i);
+		con->debugPrintf("    [%03x] %s = %04x:%04x\n", obj->getFuncSelector(i), g_sci->getKernel()->getSelectorName(obj->getFuncSelector(i)).c_str(), PRINT_REG(fptr));
+	}
+
+	Script *scr = s->_segMan->getScriptIfLoaded(pos.getSegment());
+	if (scr)
+		con->debugPrintf("\nOwner script: %d\n", scr->getScriptNumber());
+
+	return true;
+}
+
+
+
 
 
 
diff --git a/engines/sci/engine/scriptdebug.h b/engines/sci/engine/scriptdebug.h
index 1d8a2ef..f893099 100644
--- a/engines/sci/engine/scriptdebug.h
+++ b/engines/sci/engine/scriptdebug.h
@@ -37,6 +37,8 @@ void logKernelCall(const KernelFunction *kernelCall, const KernelSubFunction *ke
 
 void logBacktrace();
 
+bool printObject(reg_t obj);
+
 }
 
 #endif
diff --git a/engines/sci/sci.cpp b/engines/sci/sci.cpp
index 2226cdd..c7b9bc7 100644
--- a/engines/sci/sci.cpp
+++ b/engines/sci/sci.cpp
@@ -42,6 +42,7 @@
 #include "sci/engine/script.h"	// for script_adjust_opcode_formats
 #include "sci/engine/script_patches.h"
 #include "sci/engine/selector.h"	// for SELECTOR
+#include "sci/engine/scriptdebug.h"
 
 #include "sci/sound/audio.h"
 #include "sci/sound/music.h"
@@ -637,7 +638,7 @@ void SciEngine::initStackBaseWithSelector(Selector selector) {
 
 	// Register the first element on the execution stack
 	if (!send_selector(_gamestate, _gameObjectAddress, _gameObjectAddress, _gamestate->stack_base, 2, _gamestate->stack_base)) {
-		_console->printObject(_gameObjectAddress);
+		printObject(_gameObjectAddress);
 		error("initStackBaseWithSelector: error while registering the first selector in the call stack");
 	}
 


Commit: be84cfdb59c6c2a5ac7258f78f098e9bced29cb0
    https://github.com/scummvm/scummvm/commit/be84cfdb59c6c2a5ac7258f78f098e9bced29cb0
Author: Willem Jan Palenstijn (wjp at usecode.org)
Date: 2017-06-10T21:32:35+02:00

Commit Message:
SCI: Add inspect, none breakpoint actions

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


diff --git a/engines/sci/console.cpp b/engines/sci/console.cpp
index 6efe684..eccd94e 100644
--- a/engines/sci/console.cpp
+++ b/engines/sci/console.cpp
@@ -3741,6 +3741,12 @@ bool Console::cmdBreakpointList(int argc, const char **argv) {
 		case BREAK_BACKTRACE:
 			bpaction = " (action: show backtrace)";
 			break;
+		case BREAK_INSPECT:
+			bpaction = " (action: show object)";
+			break;
+		case BREAK_NONE:
+			bpaction = " (action: ignore)";
+			break;
 		default:
 			bpaction = "";
 		}
@@ -3802,13 +3808,7 @@ bool Console::cmdBreakpointDelete(int argc, const char **argv) {
 	// Delete it
 	_debugState._breakpoints.erase(bp);
 
-	// Update EngineState::_activeBreakpointTypes.
-	int type = 0;
-	for (bp = _debugState._breakpoints.begin(); bp != end; ++bp) {
-		type |= bp->_type;
-	}
-
-	_debugState._activeBreakpointTypes = type;
+	_debugState.updateActiveBreakpointTypes();
 
 	return true;
 }
@@ -3831,13 +3831,22 @@ bool Console::cmdBreakpointAction(int argc, const char **argv) {
 		bpaction = BREAK_LOG;
 	else if (arg == "bt")
 		bpaction = BREAK_BACKTRACE;
+	else if (arg == "inspect")
+		bpaction = BREAK_INSPECT;
+	else if (arg == "none")
+		bpaction = BREAK_NONE;
 	else
 		usage = true;
 
 	if (usage) {
 		debugPrintf("Change the action for the breakpoint with the specified index.\n");
-		debugPrintf("Usage: %s <breakpoint index> break|log|bt\n", argv[0]);
+		debugPrintf("Usage: %s <breakpoint index> break|log|bt|inspect|none\n", argv[0]);
 		debugPrintf("<index> * will process all breakpoints\n");
+		debugPrintf("Actions: break  : break into debugger\n");
+		debugPrintf("         log    : log without breaking\n");
+		debugPrintf("         bt     : show backtrace without breaking\n");
+		debugPrintf("         inspect: show object (only for bpx/bpr/bpw)\n");
+		debugPrintf("         none   : ignore breakpoint\n");
 		return true;
 	}
 
@@ -3846,6 +3855,7 @@ bool Console::cmdBreakpointAction(int argc, const char **argv) {
 	if (strcmp(argv[1], "*") == 0) {
 		for (; bp != end; ++bp)
 			bp->_action = bpaction;
+		_debugState.updateActiveBreakpointTypes();
 		return true;	
 	}
 
@@ -3862,6 +3872,9 @@ bool Console::cmdBreakpointAction(int argc, const char **argv) {
 	}
 
 	bp->_action = bpaction;
+
+	_debugState.updateActiveBreakpointTypes();
+
 	return true;
 }
 
diff --git a/engines/sci/debug.h b/engines/sci/debug.h
index 03ab594..afcac19 100644
--- a/engines/sci/debug.h
+++ b/engines/sci/debug.h
@@ -47,9 +47,11 @@ enum BreakpointType {
 };
 
 enum BreakpointAction {
-	BREAK_BREAK, // break into debugger when breakpoint is triggered
-	BREAK_LOG, // log the breakpoint, and don't break into debugger
-	BREAK_BACKTRACE // show a backtrace, and don't break into debugger
+	BREAK_NONE,      // ignore breakpoint
+	BREAK_BREAK,     // break into debugger when breakpoint is triggered
+	BREAK_LOG,       // log the breakpoint, and don't break into debugger
+	BREAK_BACKTRACE, // show a backtrace, and don't break into debugger
+	BREAK_INSPECT    // show object, and don't break into debugger
 };
 
 struct Breakpoint {
@@ -81,6 +83,8 @@ struct DebugState {
 	StackPtr old_sp;
 	Common::List<Breakpoint> _breakpoints;   //< List of breakpoints
 	int _activeBreakpointTypes;  //< Bit mask specifying which types of breakpoints are active
+
+	void updateActiveBreakpointTypes();
 };
 
 // Various global variables used for debugging are declared here
diff --git a/engines/sci/engine/scriptdebug.cpp b/engines/sci/engine/scriptdebug.cpp
index ce1d2dd..277c6b0 100644
--- a/engines/sci/engine/scriptdebug.cpp
+++ b/engines/sci/engine/scriptdebug.cpp
@@ -68,6 +68,16 @@ const char *opcodeNames[] = {
 };
 #endif	// REDUCE_MEMORY_USAGE
 
+void DebugState::updateActiveBreakpointTypes() {
+	int type = 0;
+	for (Common::List<Breakpoint>::iterator bp = _breakpoints.begin(); bp != _breakpoints.end(); ++bp) {
+		if (bp->_action != BREAK_NONE)
+			type |= bp->_type;
+	}
+
+	_activeBreakpointTypes = type;
+}
+
 // Disassembles one command from the heap, returns address of next command or 0 if a ret was encountered.
 reg_t disassemble(EngineState *s, reg32_t pos, reg_t objAddr, bool printBWTag, bool printBytecode) {
 	SegmentObj *mobj = s->_segMan->getSegment(pos.getSegment(), SEG_TYPE_SCRIPT);
@@ -688,6 +698,8 @@ bool SciEngine::checkSelectorBreakpoint(BreakpointType breakpointType, reg_t sen
 	for (bpIter = _debugState._breakpoints.begin(); bpIter != _debugState._breakpoints.end(); ++bpIter) {
 		if (bpIter->_type != breakpointType)
 			continue;
+		if (bpIter->_action == BREAK_NONE)
+			continue;
 		if (bpIter->_name == methodName ||
 		    (bpIter->_name.hasSuffix("::") && methodName.hasPrefix(bpIter->_name))) {
 			_console->debugPrintf("Break on %s (in [%04x:%04x])\n", methodName.c_str(), PRINT_REG(send_obj));
@@ -696,6 +708,8 @@ bool SciEngine::checkSelectorBreakpoint(BreakpointType breakpointType, reg_t sen
 				_debugState.breakpointWasHit = true;
 			} else if (bpIter->_action == BREAK_BACKTRACE) {
 				logBacktrace();
+			} else if (bpIter->_action == BREAK_INSPECT) {
+				printObject(send_obj);
 			}
 			return true;
 		}
@@ -709,6 +723,8 @@ bool SciEngine::checkExportBreakpoint(uint16 script, uint16 pubfunct) {
 
 		Common::List<Breakpoint>::const_iterator bp;
 		for (bp = _debugState._breakpoints.begin(); bp != _debugState._breakpoints.end(); ++bp) {
+			if (bp->_action == BREAK_NONE)
+				continue;
 			if (bp->_type == BREAK_EXPORT && bp->_address == bpaddress) {
 				_console->debugPrintf("Break on script %d, export %d\n", script, pubfunct);
 				if (bp->_action == BREAK_BREAK) {
@@ -716,6 +732,8 @@ bool SciEngine::checkExportBreakpoint(uint16 script, uint16 pubfunct) {
 					_debugState.breakpointWasHit = true;
 				} else if (bp->_action == BREAK_BACKTRACE) {
 					logBacktrace();
+				} else if (bp->_action == BREAK_INSPECT) {
+					// Ignoring this mode, to make it identical to BREAK_LOG
 				}
 				return true;
 			}
@@ -729,6 +747,8 @@ bool SciEngine::checkAddressBreakpoint(const reg32_t &address) {
 	if (_debugState._activeBreakpointTypes & BREAK_ADDRESS) {
 		Common::List<Breakpoint>::const_iterator bp;
 		for (bp = _debugState._breakpoints.begin(); bp != _debugState._breakpoints.end(); ++bp) {
+			if (bp->_action == BREAK_NONE)
+				continue;
 			if (bp->_type == BREAK_ADDRESS && bp->_regAddress == address) {
 				_console->debugPrintf("Break at %04x:%04x\n", PRINT_REG(address));
 				if (bp->_action == BREAK_BREAK) {
@@ -736,6 +756,8 @@ bool SciEngine::checkAddressBreakpoint(const reg32_t &address) {
 					_debugState.breakpointWasHit = true;
 				} else if (bp->_action == BREAK_BACKTRACE) {
 					logBacktrace();
+				} else if (bp->_action == BREAK_INSPECT) {
+					// Ignoring this mode, to make it identical to BREAK_LOG
 				}
 				return true;
 			}


Commit: b3866aa3d5714e2a752a86fc2c07eb02b90ff588
    https://github.com/scummvm/scummvm/commit/b3866aa3d5714e2a752a86fc2c07eb02b90ff588
Author: Willem Jan Palenstijn (wjp at usecode.org)
Date: 2017-06-10T21:32:35+02:00

Commit Message:
SCI: Allow multiple breakpoints with same trigger but different action

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


diff --git a/engines/sci/engine/scriptdebug.cpp b/engines/sci/engine/scriptdebug.cpp
index 277c6b0..e252241 100644
--- a/engines/sci/engine/scriptdebug.cpp
+++ b/engines/sci/engine/scriptdebug.cpp
@@ -694,6 +694,8 @@ bool SciEngine::checkSelectorBreakpoint(BreakpointType breakpointType, reg_t sen
 	Common::String methodName = _gamestate->_segMan->getObjectName(send_obj);
 	methodName += "::" + getKernel()->getSelectorName(selector);
 
+	bool found = false;
+
 	Common::List<Breakpoint>::const_iterator bpIter;
 	for (bpIter = _debugState._breakpoints.begin(); bpIter != _debugState._breakpoints.end(); ++bpIter) {
 		if (bpIter->_type != breakpointType)
@@ -702,7 +704,10 @@ bool SciEngine::checkSelectorBreakpoint(BreakpointType breakpointType, reg_t sen
 			continue;
 		if (bpIter->_name == methodName ||
 		    (bpIter->_name.hasSuffix("::") && methodName.hasPrefix(bpIter->_name))) {
-			_console->debugPrintf("Break on %s (in [%04x:%04x])\n", methodName.c_str(), PRINT_REG(send_obj));
+			if (!found) // Show message once, but allow multiple actions
+				_console->debugPrintf("Break on %s (in [%04x:%04x])\n", methodName.c_str(), PRINT_REG(send_obj));
+			found = true;
+
 			if (bpIter->_action == BREAK_BREAK) {
 				_debugState.debugging = true;
 				_debugState.breakpointWasHit = true;
@@ -711,13 +716,14 @@ bool SciEngine::checkSelectorBreakpoint(BreakpointType breakpointType, reg_t sen
 			} else if (bpIter->_action == BREAK_INSPECT) {
 				printObject(send_obj);
 			}
-			return true;
 		}
 	}
-	return false;
+	return found;
 }
 
 bool SciEngine::checkExportBreakpoint(uint16 script, uint16 pubfunct) {
+	bool found = false;
+
 	if (_debugState._activeBreakpointTypes & BREAK_EXPORT) {
 		uint32 bpaddress = (script << 16 | pubfunct);
 
@@ -726,7 +732,10 @@ bool SciEngine::checkExportBreakpoint(uint16 script, uint16 pubfunct) {
 			if (bp->_action == BREAK_NONE)
 				continue;
 			if (bp->_type == BREAK_EXPORT && bp->_address == bpaddress) {
-				_console->debugPrintf("Break on script %d, export %d\n", script, pubfunct);
+				if (!found) // Show message once, but allow multiple actions
+					_console->debugPrintf("Break on script %d, export %d\n", script, pubfunct);
+				found = true;
+
 				if (bp->_action == BREAK_BREAK) {
 					_debugState.debugging = true;
 					_debugState.breakpointWasHit = true;
@@ -735,22 +744,26 @@ bool SciEngine::checkExportBreakpoint(uint16 script, uint16 pubfunct) {
 				} else if (bp->_action == BREAK_INSPECT) {
 					// Ignoring this mode, to make it identical to BREAK_LOG
 				}
-				return true;
 			}
 		}
 	}
 
-	return false;
+	return found;
 }
 
 bool SciEngine::checkAddressBreakpoint(const reg32_t &address) {
+	bool found = false;
+
 	if (_debugState._activeBreakpointTypes & BREAK_ADDRESS) {
 		Common::List<Breakpoint>::const_iterator bp;
 		for (bp = _debugState._breakpoints.begin(); bp != _debugState._breakpoints.end(); ++bp) {
 			if (bp->_action == BREAK_NONE)
 				continue;
 			if (bp->_type == BREAK_ADDRESS && bp->_regAddress == address) {
-				_console->debugPrintf("Break at %04x:%04x\n", PRINT_REG(address));
+				if (!found)
+					_console->debugPrintf("Break at %04x:%04x\n", PRINT_REG(address));
+				found = true;
+
 				if (bp->_action == BREAK_BREAK) {
 					_debugState.debugging = true;
 					_debugState.breakpointWasHit = true;
@@ -759,12 +772,11 @@ bool SciEngine::checkAddressBreakpoint(const reg32_t &address) {
 				} else if (bp->_action == BREAK_INSPECT) {
 					// Ignoring this mode, to make it identical to BREAK_LOG
 				}
-				return true;
 			}
 		}
 	}
 
-	return false;
+	return found;
 }
 
 void debugSelectorCall(reg_t send_obj, Selector selector, int argc, StackPtr argp, ObjVarRef &varp, reg_t funcp, SegManager *segMan, SelectorType selectorType) {


Commit: 4d34d586a6ae197910f716f97cbdbae11f706320
    https://github.com/scummvm/scummvm/commit/4d34d586a6ae197910f716f97cbdbae11f706320
Author: Willem Jan Palenstijn (wjp at usecode.org)
Date: 2017-06-10T21:32:35+02:00

Commit Message:
SCI: Allow setting bp action directly on creation

Changed paths:
    engines/sci/console.cpp


diff --git a/engines/sci/console.cpp b/engines/sci/console.cpp
index eccd94e..1fab99b 100644
--- a/engines/sci/console.cpp
+++ b/engines/sci/console.cpp
@@ -3813,6 +3813,22 @@ bool Console::cmdBreakpointDelete(int argc, const char **argv) {
 	return true;
 }
 
+static bool stringToBreakpointAction(Common::String str, BreakpointAction &action) {
+	if (str == "break")
+		action = BREAK_BREAK;
+	else if (str == "log")
+		action = BREAK_LOG;
+	else if (str == "bt")
+		action = BREAK_BACKTRACE;
+	else if (str == "inspect")
+		action = BREAK_INSPECT;
+	else if (str == "none")
+		action = BREAK_NONE;
+	else
+		return false;
+	return true;
+}
+
 bool Console::cmdBreakpointAction(int argc, const char **argv) {
 	bool usage = false;
 
@@ -3825,17 +3841,7 @@ bool Console::cmdBreakpointAction(int argc, const char **argv) {
 		arg = argv[2];
 
 	BreakpointAction bpaction;
-	if (arg == "break")
-		bpaction = BREAK_BREAK;
-	else if (arg == "log")
-		bpaction = BREAK_LOG;
-	else if (arg == "bt")
-		bpaction = BREAK_BACKTRACE;
-	else if (arg == "inspect")
-		bpaction = BREAK_INSPECT;
-	else if (arg == "none")
-		bpaction = BREAK_NONE;
-	else
+	if (!stringToBreakpointAction(arg, bpaction))
 		usage = true;
 
 	if (usage) {
@@ -3880,61 +3886,103 @@ bool Console::cmdBreakpointAction(int argc, const char **argv) {
 
 
 bool Console::cmdBreakpointMethod(int argc, const char **argv) {
-	if (argc != 2) {
+	if (argc < 2 || argc > 3) {
 		debugPrintf("Sets a breakpoint on execution of a specified method/selector.\n");
-		debugPrintf("Usage: %s <name>\n", argv[0]);
+		debugPrintf("Usage: %s <name> [<action>]\n", argv[0]);
 		debugPrintf("Example: %s ego::doit\n", argv[0]);
+		debugPrintf("         %s ego::doit log\n", argv[0]);
 		debugPrintf("May also be used to set a breakpoint that applies whenever an object\n");
 		debugPrintf("of a specific type is touched: %s foo::\n", argv[0]);
+		debugPrintf("See bp_action usage for possible actions.\n");
 		return true;
 	}
 
+	BreakpointAction action = BREAK_BREAK;
+	if (argc == 3) {
+		if (!stringToBreakpointAction(argv[2], action)) {
+			debugPrintf("Invalid breakpoint action %s.\n", argv[2]);
+			debugPrintf("See bp_action usage for possible actions.\n");
+			return true;
+		}
+	}
+
 	/* Note: We can set a breakpoint on a method that has not been loaded yet.
 	   Thus, we can't check whether the command argument is a valid method name.
 	   A breakpoint set on an invalid method name will just never trigger. */
 	Breakpoint bp;
 	bp._type = BREAK_SELECTOREXEC;
 	bp._name = argv[1];
-	bp._action = BREAK_BREAK;
+	bp._action = action;
 
 	_debugState._breakpoints.push_back(bp);
-	_debugState._activeBreakpointTypes |= BREAK_SELECTOREXEC;
+
+	if (action != BREAK_NONE)
+		_debugState._activeBreakpointTypes |= BREAK_SELECTOREXEC;
+
 	return true;
 }
 
 bool Console::cmdBreakpointRead(int argc, const char **argv) {
-	if (argc != 2) {
+	if (argc < 2 || argc > 3) {
 		debugPrintf("Sets a breakpoint on reading of a specified selector.\n");
-		debugPrintf("Usage: %s <name>\n", argv[0]);
+		debugPrintf("Usage: %s <name> [<action>]\n", argv[0]);
 		debugPrintf("Example: %s ego::view\n", argv[0]);
+		debugPrintf("         %s ego::view log\n", argv[0]);
+		debugPrintf("See bp_action usage for possible actions.\n");
 		return true;
 	}
 
+	BreakpointAction action = BREAK_BREAK;
+	if (argc == 3) {
+		if (!stringToBreakpointAction(argv[2], action)) {
+			debugPrintf("Invalid breakpoint action %s.\n", argv[2]);
+			debugPrintf("See bp_action usage for possible actions.\n");
+			return true;
+		}
+	}
+
 	Breakpoint bp;
 	bp._type = BREAK_SELECTORREAD;
 	bp._name = argv[1];
-	bp._action = BREAK_BREAK;
+	bp._action = action;
 
 	_debugState._breakpoints.push_back(bp);
-	_debugState._activeBreakpointTypes |= BREAK_SELECTORREAD;
+
+	if (action != BREAK_NONE)
+		_debugState._activeBreakpointTypes |= BREAK_SELECTORREAD;
+
 	return true;
 }
 
 bool Console::cmdBreakpointWrite(int argc, const char **argv) {
-	if (argc != 2) {
+	if (argc < 2 || argc > 3) {
 		debugPrintf("Sets a breakpoint on writing of a specified selector.\n");
-		debugPrintf("Usage: %s <name>\n", argv[0]);
+		debugPrintf("Usage: %s <name> [<action>]\n", argv[0]);
 		debugPrintf("Example: %s ego::view\n", argv[0]);
+		debugPrintf("         %s ego::view log\n", argv[0]);
+		debugPrintf("See bp_action usage for possible actions.\n");
 		return true;
 	}
 
+	BreakpointAction action = BREAK_BREAK;
+	if (argc == 3) {
+		if (!stringToBreakpointAction(argv[2], action)) {
+			debugPrintf("Invalid breakpoint action %s.\n", argv[2]);
+			debugPrintf("See bp_action usage for possible actions.\n");
+			return true;
+		}
+	}
+
 	Breakpoint bp;
 	bp._type = BREAK_SELECTORWRITE;
 	bp._name = argv[1];
-	bp._action = BREAK_BREAK;
+	bp._action = action;
 
 	_debugState._breakpoints.push_back(bp);
-	_debugState._activeBreakpointTypes |= BREAK_SELECTORWRITE;
+
+	if (action != BREAK_NONE)
+		_debugState._activeBreakpointTypes |= BREAK_SELECTORWRITE;
+
 	return true;
 }
 
@@ -3965,12 +4013,22 @@ bool Console::cmdBreakpointKernel(int argc, const char **argv) {
 }
 
 bool Console::cmdBreakpointFunction(int argc, const char **argv) {
-	if (argc != 3) {
+	if (argc < 3 || argc > 4) {
 		debugPrintf("Sets a breakpoint on the execution of the specified exported function.\n");
-		debugPrintf("Usage: %s <script number> <export number>\n", argv[0]);
+		debugPrintf("Usage: %s <script number> <export number> [<action>]\n", argv[0]);
+		debugPrintf("See bp_action usage for possible actions.\n");
 		return true;
 	}
 
+	BreakpointAction action = BREAK_BREAK;
+	if (argc == 4) {
+		if (!stringToBreakpointAction(argv[3], action)) {
+			debugPrintf("Invalid breakpoint action %s.\n", argv[3]);
+			debugPrintf("See bp_action usage for possible actions.\n");
+			return true;
+		}
+	}
+
 	/* Note: We can set a breakpoint on a method that has not been loaded yet.
 	   Thus, we can't check whether the command argument is a valid method name.
 	   A breakpoint set on an invalid method name will just never trigger. */
@@ -3978,7 +4036,7 @@ bool Console::cmdBreakpointFunction(int argc, const char **argv) {
 	bp._type = BREAK_EXPORT;
 	// script number, export number
 	bp._address = (atoi(argv[1]) << 16 | atoi(argv[2]));
-	bp._action = BREAK_BREAK;
+	bp._action = action;
 
 	_debugState._breakpoints.push_back(bp);
 	_debugState._activeBreakpointTypes |= BREAK_EXPORT;
@@ -3987,9 +4045,10 @@ bool Console::cmdBreakpointFunction(int argc, const char **argv) {
 }
 
 bool Console::cmdBreakpointAddress(int argc, const char **argv) {
-	if (argc != 2) {
+	if (argc < 2 || argc > 3) {
 		debugPrintf("Sets a breakpoint on the execution of the specified code address.\n");
-		debugPrintf("Usage: %s <address>\n", argv[0]);
+		debugPrintf("Usage: %s <address> [<action>]\n", argv[0]);
+		debugPrintf("See bp_action usage for possible actions.\n");
 		return true;
 	}
 
@@ -4001,10 +4060,19 @@ bool Console::cmdBreakpointAddress(int argc, const char **argv) {
 		return true;
 	}
 
+	BreakpointAction action = BREAK_BREAK;
+	if (argc == 3) {
+		if (!stringToBreakpointAction(argv[2], action)) {
+			debugPrintf("Invalid breakpoint action %s.\n", argv[2]);
+			debugPrintf("See bp_action usage for possible actions.\n");
+			return true;
+		}
+	}
+
 	Breakpoint bp;
 	bp._type = BREAK_ADDRESS;
 	bp._regAddress = make_reg32(addr.getSegment(), addr.getOffset());
-	bp._action = BREAK_BREAK;
+	bp._action = action;
 
 	_debugState._breakpoints.push_back(bp);
 	_debugState._activeBreakpointTypes |= BREAK_ADDRESS;


Commit: 0f0ecff0b8b095205a3a1c9b9a133f283d1ad39b
    https://github.com/scummvm/scummvm/commit/0f0ecff0b8b095205a3a1c9b9a133f283d1ad39b
Author: Willem Jan Palenstijn (wjp at usecode.org)
Date: 2017-06-10T21:32:35+02:00

Commit Message:
SCI: Print breakpoint info on creation

Changed paths:
    engines/sci/console.cpp
    engines/sci/console.h


diff --git a/engines/sci/console.cpp b/engines/sci/console.cpp
index 1fab99b..3840e33 100644
--- a/engines/sci/console.cpp
+++ b/engines/sci/console.cpp
@@ -3722,54 +3722,55 @@ bool Console::cmdLogKernel(int argc, const char **argv) {
 	return true;
 }
 
+void Console::printBreakpoint(int index, const Breakpoint &bp) {
+	debugPrintf("  #%i: ", index);
+	const char *bpaction;
+
+	switch (bp._action) {
+	case BREAK_LOG:
+		bpaction = " (action: log only)";
+		break;
+	case BREAK_BACKTRACE:
+		bpaction = " (action: show backtrace)";
+		break;
+	case BREAK_INSPECT:
+		bpaction = " (action: show object)";
+		break;
+	case BREAK_NONE:
+		bpaction = " (action: ignore)";
+		break;
+	default:
+		bpaction = "";
+	}
+	switch (bp._type) {
+	case BREAK_SELECTOREXEC:
+		debugPrintf("Execute %s%s\n", bp._name.c_str(), bpaction);
+		break;
+	case BREAK_SELECTORREAD:
+		debugPrintf("Read %s%s\n", bp._name.c_str(), bpaction);
+		break;
+	case BREAK_SELECTORWRITE:
+		debugPrintf("Write %s%s\n", bp._name.c_str(), bpaction);
+		break;
+	case BREAK_EXPORT: {
+		int bpdata = bp._address;
+		debugPrintf("Execute script %d, export %d%s\n", bpdata >> 16, bpdata & 0xFFFF, bpaction);
+		break;
+	}
+	case BREAK_ADDRESS:
+		debugPrintf("Execute address %04x:%04x%s\n", PRINT_REG(bp._regAddress), bpaction);
+	}
+}
+
 bool Console::cmdBreakpointList(int argc, const char **argv) {
 	int i = 0;
-	int bpdata;
 
 	debugPrintf("Breakpoint list:\n");
 
 	Common::List<Breakpoint>::const_iterator bp = _debugState._breakpoints.begin();
 	Common::List<Breakpoint>::const_iterator end = _debugState._breakpoints.end();
-	for (; bp != end; ++bp) {
-		debugPrintf("  #%i: ", i);
-		const char *bpaction;
-
-		switch (bp->_action) {
-		case BREAK_LOG:
-			bpaction = " (action: log only)";
-			break;
-		case BREAK_BACKTRACE:
-			bpaction = " (action: show backtrace)";
-			break;
-		case BREAK_INSPECT:
-			bpaction = " (action: show object)";
-			break;
-		case BREAK_NONE:
-			bpaction = " (action: ignore)";
-			break;
-		default:
-			bpaction = "";
-		}
-		switch (bp->_type) {
-		case BREAK_SELECTOREXEC:
-			debugPrintf("Execute %s%s\n", bp->_name.c_str(), bpaction);
-			break;
-		case BREAK_SELECTORREAD:
-			debugPrintf("Read %s%s\n", bp->_name.c_str(), bpaction);
-			break;
-		case BREAK_SELECTORWRITE:
-			debugPrintf("Write %s%s\n", bp->_name.c_str(), bpaction);
-			break;
-		case BREAK_EXPORT:
-			bpdata = bp->_address;
-			debugPrintf("Execute script %d, export %d%s\n", bpdata >> 16, bpdata & 0xFFFF, bpaction);
-			break;
-		case BREAK_ADDRESS:
-			debugPrintf("Execute address %04x:%04x%s\n", PRINT_REG(bp->_regAddress), bpaction);
-		}
-
-		i++;
-	}
+	for (; bp != end; ++bp)
+		printBreakpoint(i++, *bp);
 
 	if (!i)
 		debugPrintf("  No breakpoints defined.\n");
@@ -3881,6 +3882,8 @@ bool Console::cmdBreakpointAction(int argc, const char **argv) {
 
 	_debugState.updateActiveBreakpointTypes();
 
+	printBreakpoint(idx, *bp);
+
 	return true;
 }
 
@@ -3919,6 +3922,8 @@ bool Console::cmdBreakpointMethod(int argc, const char **argv) {
 	if (action != BREAK_NONE)
 		_debugState._activeBreakpointTypes |= BREAK_SELECTOREXEC;
 
+	printBreakpoint(_debugState._breakpoints.size() - 1, bp);
+
 	return true;
 }
 
@@ -3951,6 +3956,8 @@ bool Console::cmdBreakpointRead(int argc, const char **argv) {
 	if (action != BREAK_NONE)
 		_debugState._activeBreakpointTypes |= BREAK_SELECTORREAD;
 
+	printBreakpoint(_debugState._breakpoints.size() - 1, bp);
+
 	return true;
 }
 
@@ -3983,6 +3990,8 @@ bool Console::cmdBreakpointWrite(int argc, const char **argv) {
 	if (action != BREAK_NONE)
 		_debugState._activeBreakpointTypes |= BREAK_SELECTORWRITE;
 
+	printBreakpoint(_debugState._breakpoints.size() - 1, bp);
+
 	return true;
 }
 
diff --git a/engines/sci/console.h b/engines/sci/console.h
index 75243e5..0b8cd93 100644
--- a/engines/sci/console.h
+++ b/engines/sci/console.h
@@ -191,6 +191,8 @@ private:
 	 */
 	void printKernelCallsFound(int kernelFuncNum, bool showFoundScripts);
 
+	void printBreakpoint(int index, const Breakpoint &bp);
+
 	SciEngine *_engine;
 	DebugState &_debugState;
 	Common::String _videoFile;


Commit: e2e3f7c4c51e64df05b3252379bcc56f52b576dc
    https://github.com/scummvm/scummvm/commit/e2e3f7c4c51e64df05b3252379bcc56f52b576dc
Author: Willem Jan Palenstijn (wjp at usecode.org)
Date: 2017-06-10T21:32:35+02:00

Commit Message:
SCI: Move bpk/logkernel to main breakpoint infrastructure

This changes the syntax for bpk and logkernel:

Enable breakpoint on kernel call:

bpk FrameOut

Enable logging for kernel call:

bpk FrameOut log
For backward compatibility this has an alias: logkernel FrameOut

Removing a kernel call breakpoint is done with bp_del/bc now.

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


diff --git a/engines/sci/console.cpp b/engines/sci/console.cpp
index 3840e33..44d2a71 100644
--- a/engines/sci/console.cpp
+++ b/engines/sci/console.cpp
@@ -3698,27 +3698,17 @@ bool Console::cmdGo(int argc, const char **argv) {
 }
 
 bool Console::cmdLogKernel(int argc, const char **argv) {
-	if (argc < 3) {
+	if (argc != 2) {
 		debugPrintf("Logs calls to specified kernel function.\n");
-		debugPrintf("Usage: %s <kernel function/*> <on/off>\n", argv[0]);
-		debugPrintf("Example: %s StrCpy on\n", argv[0]);
+		debugPrintf("Usage: %s <kernel function/*>\n", argv[0]);
+		debugPrintf("Example: %s StrCpy\n", argv[0]);
+		debugPrintf("This is an alias for: bpk <kernel function> log\n");
 		return true;
 	}
 
-	bool logging;
-	if (strcmp(argv[2], "on") == 0)
-		logging = true;
-	else if (strcmp(argv[2], "off") == 0)
-		logging = false;
-	else {
-		debugPrintf("2nd parameter must be either on or off\n");
-		return true;
-	}
+	const char *bpk_argv[] = { "bpk", argv[1], "log" };
+	cmdBreakpointKernel(3, bpk_argv);
 
-	if (g_sci->getKernel()->debugSetFunction(argv[1], logging, -1))
-		debugPrintf("Logging %s for k%s\n", logging ? "enabled" : "disabled", argv[1]);
-	else
-		debugPrintf("Unknown kernel function %s\n", argv[1]);
 	return true;
 }
 
@@ -3759,6 +3749,9 @@ void Console::printBreakpoint(int index, const Breakpoint &bp) {
 	}
 	case BREAK_ADDRESS:
 		debugPrintf("Execute address %04x:%04x%s\n", PRINT_REG(bp._regAddress), bpaction);
+		break;
+	case BREAK_KERNEL:
+		debugPrintf("Kernel call k%s%s\n", bp._name.c_str(), bpaction);
 	}
 }
 
@@ -3996,27 +3989,68 @@ bool Console::cmdBreakpointWrite(int argc, const char **argv) {
 }
 
 bool Console::cmdBreakpointKernel(int argc, const char **argv) {
-	if (argc < 3) {
+	if (argc < 2 || argc > 3) {
 		debugPrintf("Sets a breakpoint on execution of a kernel function.\n");
-		debugPrintf("Usage: %s <name> <on/off>\n", argv[0]);
-		debugPrintf("Example: %s DrawPic on\n", argv[0]);
+		debugPrintf("Usage: %s <name> [<action>]\n", argv[0]);
+		debugPrintf("Example: %s DrawPic\n", argv[0]);
+		debugPrintf("         %s DrawPic log\n", argv[0]);
+		debugPrintf("See bp_action usage for possible actions.\n");
 		return true;
 	}
 
-	bool breakpoint;
-	if (strcmp(argv[2], "on") == 0)
-		breakpoint = true;
-	else if (strcmp(argv[2], "off") == 0)
-		breakpoint = false;
-	else {
-		debugPrintf("2nd parameter must be either on or off\n");
+	BreakpointAction action = BREAK_BREAK;
+	if (argc == 3) {
+		if (!stringToBreakpointAction(argv[2], action)) {
+			debugPrintf("Invalid breakpoint action %s.\n", argv[2]);
+			debugPrintf("See bp_action usage for possible actions.\n");
+			return true;
+		}
+	}
+
+	// Check if any kernel functions match, to catch typos
+	Common::String name = argv[1];
+	bool wildcard = name.lastChar() == '*';
+	Common::String prefix = name;
+	if (wildcard)
+		prefix.deleteLastChar();
+	bool found = false;
+	const Kernel::KernelFunctionArray &kernelFuncs = _engine->getKernel()->_kernelFuncs;
+	for (uint id = 0; id < kernelFuncs.size() && !found; id++) {
+		if (kernelFuncs[id].name) {
+			const KernelSubFunction *kernelSubCall = kernelFuncs[id].subFunctions;
+			if (!kernelSubCall) {
+				Common::String kname = kernelFuncs[id].name;
+				if (name == kname || (wildcard && kname.hasPrefix(prefix)))
+					found = true;
+			} else {
+				uint kernelSubCallCount = kernelFuncs[id].subFunctionCount;
+				for (uint subId = 0; subId < kernelSubCallCount; subId++) {
+					if (kernelSubCall->name) {
+						Common::String kname = kernelSubCall->name;
+						if (name == kname || (wildcard && kname.hasPrefix(prefix)))
+							found = true;
+					}
+					kernelSubCall++;
+				}
+			}
+		}
+	}
+	if (!found) {
+		debugPrintf("No kernel functions match %s.\n", name.c_str());
 		return true;
 	}
 
-	if (g_sci->getKernel()->debugSetFunction(argv[1], -1, breakpoint))
-		debugPrintf("Breakpoint %s for k%s\n", (breakpoint ? "enabled" : "disabled"), argv[1]);
-	else
-		debugPrintf("Unknown kernel function %s\n", argv[1]);
+	Breakpoint bp;
+	bp._type = BREAK_KERNEL;
+	bp._name = name;
+	bp._action = action;
+
+	_debugState._breakpoints.push_back(bp);
+
+	if (action != BREAK_NONE)
+		_debugState._activeBreakpointTypes |= BREAK_KERNEL;
+
+	printBreakpoint(_debugState._breakpoints.size() - 1, bp);
 
 	return true;
 }
diff --git a/engines/sci/debug.h b/engines/sci/debug.h
index afcac19..cd1d809 100644
--- a/engines/sci/debug.h
+++ b/engines/sci/debug.h
@@ -31,7 +31,7 @@ namespace Sci {
 // These types are used both as identifiers and as elements of bitfields
 enum BreakpointType {
 	/**
-	 * Break when a selector is executed. Data contains (char *) selector name
+	 * Break when a selector is executed. Data contains selector name
 	 * (in the format Object::Method)
 	 */
 	BREAK_SELECTOREXEC  = 1 << 0, // break when a function selector is executed
@@ -39,11 +39,12 @@ enum BreakpointType {
 	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. _address contains
 	 * script_no << 16 | export_no.
 	 */
 	BREAK_EXPORT        = 1 << 3,
-	BREAK_ADDRESS       = 1 << 4  // break when pc is at this address
+	BREAK_ADDRESS       = 1 << 4, // break when pc is at _regAddress
+	BREAK_KERNEL        = 1 << 5  // break on named kernel call
 };
 
 enum BreakpointAction {
diff --git a/engines/sci/engine/kernel.cpp b/engines/sci/engine/kernel.cpp
index efa10c3..539475c 100644
--- a/engines/sci/engine/kernel.cpp
+++ b/engines/sci/engine/kernel.cpp
@@ -598,7 +598,6 @@ void Kernel::mapFunctions() {
 		_kernelFuncs[id].workarounds = NULL;
 		_kernelFuncs[id].subFunctions = NULL;
 		_kernelFuncs[id].subFunctionCount = 0;
-		_kernelFuncs[id].debugLogging = false;
 		if (kernelName.empty()) {
 			// No name was given -> must be an unknown opcode
 			warning("Kernel function %x unknown", id);
@@ -715,85 +714,6 @@ void Kernel::mapFunctions() {
 	return;
 }
 
-bool Kernel::debugSetFunction(const char *kernelName, int logging, int breakpoint) {
-	if (strcmp(kernelName, "*")) {
-		for (uint id = 0; id < _kernelFuncs.size(); id++) {
-			if (_kernelFuncs[id].name) {
-				if (strcmp(kernelName, _kernelFuncs[id].name) == 0) {
-					if (_kernelFuncs[id].subFunctions) {
-						// sub-functions available and main name matched, in that case set logging of all sub-functions
-						KernelSubFunction *kernelSubCall = _kernelFuncs[id].subFunctions;
-						uint kernelSubCallCount = _kernelFuncs[id].subFunctionCount;
-						for (uint subId = 0; subId < kernelSubCallCount; subId++) {
-							if (kernelSubCall->function) {
-								if (logging != -1)
-									kernelSubCall->debugLogging = logging == 1 ? true : false;
-								if (breakpoint != -1)
-									kernelSubCall->debugBreakpoint = breakpoint == 1 ? true : false;
-							}
-							kernelSubCall++;
-						}
-						return true;
-					}
-					// function name matched, set for this one and exit
-					if (logging != -1)
-						_kernelFuncs[id].debugLogging = logging == 1 ? true : false;
-					if (breakpoint != -1)
-						_kernelFuncs[id].debugBreakpoint = breakpoint == 1 ? true : false;
-					return true;
-				} else {
-					// main name was not matched
-					if (_kernelFuncs[id].subFunctions) {
-						// Sub-Functions available
-						KernelSubFunction *kernelSubCall = _kernelFuncs[id].subFunctions;
-						uint kernelSubCallCount = _kernelFuncs[id].subFunctionCount;
-						for (uint subId = 0; subId < kernelSubCallCount; subId++) {
-							if (kernelSubCall->function) {
-								if (strcmp(kernelName, kernelSubCall->name) == 0) {
-									// sub-function name matched, set for this one and exit
-									if (logging != -1)
-										kernelSubCall->debugLogging = logging == 1 ? true : false;
-									if (breakpoint != -1)
-										kernelSubCall->debugBreakpoint = breakpoint == 1 ? true : false;
-									return true;
-								}
-							}
-							kernelSubCall++;
-						}
-					}
-				}
-			}
-		}
-		return false;
-	}
-	// Set debugLogging for all calls
-	for (uint id = 0; id < _kernelFuncs.size(); id++) {
-		if (_kernelFuncs[id].name) {
-			if (!_kernelFuncs[id].subFunctions) {
-				// No sub-functions, enable actual kernel function
-				if (logging != -1)
-					_kernelFuncs[id].debugLogging = logging == 1 ? true : false;
-				if (breakpoint != -1)
-					_kernelFuncs[id].debugBreakpoint = breakpoint == 1 ? true : false;
-			} else {
-				// Sub-Functions available, enable those too
-				KernelSubFunction *kernelSubCall = _kernelFuncs[id].subFunctions;
-				uint kernelSubCallCount = _kernelFuncs[id].subFunctionCount;
-				for (uint subId = 0; subId < kernelSubCallCount; subId++) {
-					if (kernelSubCall->function) {
-						if (logging != -1)
-							kernelSubCall->debugLogging = logging == 1 ? true : false;
-						if (breakpoint != -1)
-							kernelSubCall->debugBreakpoint = breakpoint == 1 ? true : false;
-					}
-					kernelSubCall++;
-				}
-			}
-		}
-	}
-	return true;
-}
-
 #ifdef ENABLE_SCI32
 enum {
 	kKernelEntriesSci2 = 0x8b,
diff --git a/engines/sci/engine/kernel.h b/engines/sci/engine/kernel.h
index bddfab5..d1dce37 100644
--- a/engines/sci/engine/kernel.h
+++ b/engines/sci/engine/kernel.h
@@ -143,8 +143,6 @@ struct KernelFunction {
 	const SciWorkaroundEntry *workarounds;
 	KernelSubFunction *subFunctions;
 	uint16 subFunctionCount;
-	bool debugLogging;
-	bool debugBreakpoint;
 };
 
 class Kernel {
@@ -231,11 +229,6 @@ public:
 	 */
 	void loadKernelNames(GameFeatures *features);
 
-	/**
-	 * Sets debug flags for a kernel function
-	 */
-	bool debugSetFunction(const char *kernelName, int logging, int breakpoint);
-
 private:
 	/**
 	 * Loads the kernel selector names.
diff --git a/engines/sci/engine/scriptdebug.cpp b/engines/sci/engine/scriptdebug.cpp
index e252241..9f1b536 100644
--- a/engines/sci/engine/scriptdebug.cpp
+++ b/engines/sci/engine/scriptdebug.cpp
@@ -779,6 +779,44 @@ bool SciEngine::checkAddressBreakpoint(const reg32_t &address) {
 	return found;
 }
 
+bool SciEngine::checkKernelBreakpoint(const Common::String &name) {
+	bool found = false;
+
+	if (_debugState._activeBreakpointTypes & BREAK_KERNEL) {
+
+		Common::List<Breakpoint>::const_iterator bp;
+		for (bp = _debugState._breakpoints.begin(); bp != _debugState._breakpoints.end(); ++bp) {
+			if (bp->_action == BREAK_NONE)
+				continue;
+			if (bp->_type != BREAK_KERNEL)
+				continue;
+
+			bool wildcard = bp->_name.lastChar() == '*';
+			Common::String prefix = bp->_name;
+			if (wildcard)
+				prefix.deleteLastChar();
+			if (bp->_name == name || (wildcard && name.hasPrefix(prefix))) {
+				if (bp->_action == BREAK_BREAK) {
+					if (!found)
+						_console->debugPrintf("Break on k%s\n", name.c_str());
+					_debugState.debugging = true;
+					_debugState.breakpointWasHit = true;
+				} else if (bp->_action == BREAK_BACKTRACE) {
+					if (!found)
+						_console->debugPrintf("Break on k%s\n", name.c_str());
+					logBacktrace();
+				} else if (bp->_action == BREAK_INSPECT) {
+					// Ignoring this mode, to make it identical to BREAK_LOG
+				}
+				found = true;
+			}
+		}
+	}
+
+	return found;
+}
+
+
 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);
diff --git a/engines/sci/engine/vm.cpp b/engines/sci/engine/vm.cpp
index 1e088d6..b169e0e 100644
--- a/engines/sci/engine/vm.cpp
+++ b/engines/sci/engine/vm.cpp
@@ -296,7 +296,8 @@ ExecStack *send_selector(EngineState *s, reg_t send_obj, reg_t work_obj, StackPt
 			sp = CALL_SP_CARRY; // Destroy sp, as it will be carried over
 		}
 
-		if (activeBreakpointTypes || DebugMan.isDebugChannelEnabled(kDebugLevelScripts))
+		if ((activeBreakpointTypes & (BREAK_SELECTOREXEC | BREAK_SELECTORREAD | BREAK_SELECTORWRITE))
+		     || DebugMan.isDebugChannelEnabled(kDebugLevelScripts))
 			debugSelectorCall(send_obj, selector, argc, argp, varp, funcp, s->_segMan, selectorType);
 
 		assert(argp[0].toUint16() == argc); // The first argument is argc
@@ -375,13 +376,8 @@ static void callKernelFunc(EngineState *s, int kernelCallNr, int argc) {
 		addKernelCallToExecStack(s, kernelCallNr, -1, argc, argv);
 		s->r_acc = kernelCall.function(s, argc, argv);
 
-		if (kernelCall.debugLogging)
+		if (g_sci->checkKernelBreakpoint(kernelCall.name))
 			logKernelCall(&kernelCall, NULL, s, argc, argv, s->r_acc);
-		if (kernelCall.debugBreakpoint) {
-			debugN("Break on k%s\n", kernelCall.name);
-			g_sci->_debugState.debugging = true;
-			g_sci->_debugState.breakpointWasHit = true;
-		}
 	} else {
 		// Sub-functions available, check signature and call that one directly
 		if (argc < 1)
@@ -447,13 +443,8 @@ static void callKernelFunc(EngineState *s, int kernelCallNr, int argc) {
 		addKernelCallToExecStack(s, kernelCallNr, subId, argc, argv);
 		s->r_acc = kernelSubCall.function(s, argc, argv);
 
-		if (kernelSubCall.debugLogging)
+		if (g_sci->checkKernelBreakpoint(kernelSubCall.name))
 			logKernelCall(&kernelCall, &kernelSubCall, s, argc, argv, s->r_acc);
-		if (kernelSubCall.debugBreakpoint) {
-			debugN("Break on k%s\n", kernelSubCall.name);
-			g_sci->_debugState.debugging = true;
-			g_sci->_debugState.breakpointWasHit = true;
-		}
 	}
 
 	// Remove callk stack frame again, if there's still an execution stack
diff --git a/engines/sci/sci.h b/engines/sci/sci.h
index 3139f15..8109317 100644
--- a/engines/sci/sci.h
+++ b/engines/sci/sci.h
@@ -316,6 +316,7 @@ public:
 	bool checkAddressBreakpoint(const reg32_t &address);
 
 public:
+	bool checkKernelBreakpoint(const Common::String &name);
 
 	/**
 	 * Processes a multilanguage string based on the current language settings and


Commit: 4102a7796439d0af1399e648cf3e0105332edda3
    https://github.com/scummvm/scummvm/commit/4102a7796439d0af1399e648cf3e0105332edda3
Author: Willem Jan Palenstijn (wjp at usecode.org)
Date: 2017-06-10T21:32:35+02:00

Commit Message:
SCI: Clean up breakpoint code (indentation, consistency)

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


diff --git a/engines/sci/engine/scriptdebug.cpp b/engines/sci/engine/scriptdebug.cpp
index 9f1b536..faf3411 100644
--- a/engines/sci/engine/scriptdebug.cpp
+++ b/engines/sci/engine/scriptdebug.cpp
@@ -696,24 +696,23 @@ bool SciEngine::checkSelectorBreakpoint(BreakpointType breakpointType, reg_t sen
 
 	bool found = false;
 
-	Common::List<Breakpoint>::const_iterator bpIter;
-	for (bpIter = _debugState._breakpoints.begin(); bpIter != _debugState._breakpoints.end(); ++bpIter) {
-		if (bpIter->_type != breakpointType)
+	Common::List<Breakpoint>::const_iterator bp;
+	for (bp = _debugState._breakpoints.begin(); bp != _debugState._breakpoints.end(); ++bp) {
+		if (bp->_action == BREAK_NONE || bp->_type != breakpointType)
 			continue;
-		if (bpIter->_action == BREAK_NONE)
-			continue;
-		if (bpIter->_name == methodName ||
-		    (bpIter->_name.hasSuffix("::") && methodName.hasPrefix(bpIter->_name))) {
+
+		if (bp->_name == methodName ||
+		    (bp->_name.hasSuffix("::") && methodName.hasPrefix(bp->_name))) {
 			if (!found) // Show message once, but allow multiple actions
 				_console->debugPrintf("Break on %s (in [%04x:%04x])\n", methodName.c_str(), PRINT_REG(send_obj));
 			found = true;
 
-			if (bpIter->_action == BREAK_BREAK) {
+			if (bp->_action == BREAK_BREAK) {
 				_debugState.debugging = true;
 				_debugState.breakpointWasHit = true;
-			} else if (bpIter->_action == BREAK_BACKTRACE) {
+			} else if (bp->_action == BREAK_BACKTRACE) {
 				logBacktrace();
-			} else if (bpIter->_action == BREAK_INSPECT) {
+			} else if (bp->_action == BREAK_INSPECT) {
 				printObject(send_obj);
 			}
 		}
@@ -722,28 +721,30 @@ bool SciEngine::checkSelectorBreakpoint(BreakpointType breakpointType, reg_t sen
 }
 
 bool SciEngine::checkExportBreakpoint(uint16 script, uint16 pubfunct) {
+
+	if (!(_debugState._activeBreakpointTypes & BREAK_EXPORT))
+		return false;
+
 	bool found = false;
+	uint32 bpaddress = (script << 16 | pubfunct);
 
-	if (_debugState._activeBreakpointTypes & BREAK_EXPORT) {
-		uint32 bpaddress = (script << 16 | pubfunct);
-
-		Common::List<Breakpoint>::const_iterator bp;
-		for (bp = _debugState._breakpoints.begin(); bp != _debugState._breakpoints.end(); ++bp) {
-			if (bp->_action == BREAK_NONE)
-				continue;
-			if (bp->_type == BREAK_EXPORT && bp->_address == bpaddress) {
-				if (!found) // Show message once, but allow multiple actions
-					_console->debugPrintf("Break on script %d, export %d\n", script, pubfunct);
-				found = true;
-
-				if (bp->_action == BREAK_BREAK) {
-					_debugState.debugging = true;
-					_debugState.breakpointWasHit = true;
-				} else if (bp->_action == BREAK_BACKTRACE) {
-					logBacktrace();
-				} else if (bp->_action == BREAK_INSPECT) {
-					// Ignoring this mode, to make it identical to BREAK_LOG
-				}
+	Common::List<Breakpoint>::const_iterator bp;
+	for (bp = _debugState._breakpoints.begin(); bp != _debugState._breakpoints.end(); ++bp) {
+		if (bp->_action == BREAK_NONE || bp->_type != BREAK_EXPORT)
+			continue;
+
+		if (bp->_address == bpaddress) {
+			if (!found) // Show message once, but allow multiple actions
+				_console->debugPrintf("Break on script %d, export %d\n", script, pubfunct);
+			found = true;
+
+			if (bp->_action == BREAK_BREAK) {
+				_debugState.debugging = true;
+				_debugState.breakpointWasHit = true;
+			} else if (bp->_action == BREAK_BACKTRACE) {
+				logBacktrace();
+			} else if (bp->_action == BREAK_INSPECT) {
+				// Ignoring this mode, to make it identical to BREAK_LOG
 			}
 		}
 	}
@@ -752,26 +753,28 @@ bool SciEngine::checkExportBreakpoint(uint16 script, uint16 pubfunct) {
 }
 
 bool SciEngine::checkAddressBreakpoint(const reg32_t &address) {
+	if (!(_debugState._activeBreakpointTypes & BREAK_ADDRESS))
+		return false;
+
 	bool found = false;
 
-	if (_debugState._activeBreakpointTypes & BREAK_ADDRESS) {
-		Common::List<Breakpoint>::const_iterator bp;
-		for (bp = _debugState._breakpoints.begin(); bp != _debugState._breakpoints.end(); ++bp) {
-			if (bp->_action == BREAK_NONE)
-				continue;
-			if (bp->_type == BREAK_ADDRESS && bp->_regAddress == address) {
-				if (!found)
-					_console->debugPrintf("Break at %04x:%04x\n", PRINT_REG(address));
-				found = true;
-
-				if (bp->_action == BREAK_BREAK) {
-					_debugState.debugging = true;
-					_debugState.breakpointWasHit = true;
-				} else if (bp->_action == BREAK_BACKTRACE) {
-					logBacktrace();
-				} else if (bp->_action == BREAK_INSPECT) {
-					// Ignoring this mode, to make it identical to BREAK_LOG
-				}
+	Common::List<Breakpoint>::const_iterator bp;
+	for (bp = _debugState._breakpoints.begin(); bp != _debugState._breakpoints.end(); ++bp) {
+		if (bp->_action == BREAK_NONE || bp->_type != BREAK_ADDRESS)
+			continue;
+
+		if (bp->_regAddress == address) {
+			if (!found)
+				_console->debugPrintf("Break at %04x:%04x\n", PRINT_REG(address));
+			found = true;
+
+			if (bp->_action == BREAK_BREAK) {
+				_debugState.debugging = true;
+				_debugState.breakpointWasHit = true;
+			} else if (bp->_action == BREAK_BACKTRACE) {
+				logBacktrace();
+			} else if (bp->_action == BREAK_INSPECT) {
+				// Ignoring this mode, to make it identical to BREAK_LOG
 			}
 		}
 	}
@@ -780,36 +783,34 @@ bool SciEngine::checkAddressBreakpoint(const reg32_t &address) {
 }
 
 bool SciEngine::checkKernelBreakpoint(const Common::String &name) {
+	if (!(_debugState._activeBreakpointTypes & BREAK_KERNEL))
+		return false;
+
 	bool found = false;
 
-	if (_debugState._activeBreakpointTypes & BREAK_KERNEL) {
-
-		Common::List<Breakpoint>::const_iterator bp;
-		for (bp = _debugState._breakpoints.begin(); bp != _debugState._breakpoints.end(); ++bp) {
-			if (bp->_action == BREAK_NONE)
-				continue;
-			if (bp->_type != BREAK_KERNEL)
-				continue;
-
-			bool wildcard = bp->_name.lastChar() == '*';
-			Common::String prefix = bp->_name;
-			if (wildcard)
-				prefix.deleteLastChar();
-			if (bp->_name == name || (wildcard && name.hasPrefix(prefix))) {
-				if (bp->_action == BREAK_BREAK) {
-					if (!found)
-						_console->debugPrintf("Break on k%s\n", name.c_str());
-					_debugState.debugging = true;
-					_debugState.breakpointWasHit = true;
-				} else if (bp->_action == BREAK_BACKTRACE) {
-					if (!found)
-						_console->debugPrintf("Break on k%s\n", name.c_str());
-					logBacktrace();
-				} else if (bp->_action == BREAK_INSPECT) {
-					// Ignoring this mode, to make it identical to BREAK_LOG
-				}
-				found = true;
+	Common::List<Breakpoint>::const_iterator bp;
+	for (bp = _debugState._breakpoints.begin(); bp != _debugState._breakpoints.end(); ++bp) {
+		if (bp->_action == BREAK_NONE || bp->_type != BREAK_KERNEL)
+			continue;
+
+		bool wildcard = bp->_name.lastChar() == '*';
+		Common::String prefix = bp->_name;
+		if (wildcard)
+			prefix.deleteLastChar();
+		if (bp->_name == name || (wildcard && name.hasPrefix(prefix))) {
+			if (bp->_action == BREAK_BREAK) {
+				if (!found)
+					_console->debugPrintf("Break on k%s\n", name.c_str());
+				_debugState.debugging = true;
+				_debugState.breakpointWasHit = true;
+			} else if (bp->_action == BREAK_BACKTRACE) {
+				if (!found)
+					_console->debugPrintf("Break on k%s\n", name.c_str());
+				logBacktrace();
+			} else if (bp->_action == BREAK_INSPECT) {
+				// Ignoring this mode, to make it identical to BREAK_LOG
 			}
+			found = true;
 		}
 	}
 


Commit: e7b6a257b95fd19055d0df22afcbceb614ab1899
    https://github.com/scummvm/scummvm/commit/e7b6a257b95fd19055d0df22afcbceb614ab1899
Author: Willem Jan Palenstijn (wjp at usecode.org)
Date: 2017-06-10T21:32:35+02:00

Commit Message:
SCI: Change 'none' breakpoint action to 'ignore' for consistency

Changed paths:
    engines/sci/console.cpp


diff --git a/engines/sci/console.cpp b/engines/sci/console.cpp
index 44d2a71..701f08b 100644
--- a/engines/sci/console.cpp
+++ b/engines/sci/console.cpp
@@ -3816,7 +3816,7 @@ static bool stringToBreakpointAction(Common::String str, BreakpointAction &actio
 		action = BREAK_BACKTRACE;
 	else if (str == "inspect")
 		action = BREAK_INSPECT;
-	else if (str == "none")
+	else if (str == "ignore")
 		action = BREAK_NONE;
 	else
 		return false;
@@ -3846,7 +3846,7 @@ bool Console::cmdBreakpointAction(int argc, const char **argv) {
 		debugPrintf("         log    : log without breaking\n");
 		debugPrintf("         bt     : show backtrace without breaking\n");
 		debugPrintf("         inspect: show object (only for bpx/bpr/bpw)\n");
-		debugPrintf("         none   : ignore breakpoint\n");
+		debugPrintf("         ignore : ignore breakpoint\n");
 		return true;
 	}
 


Commit: c7d631cb6697a4c8c6d9cc0e32caba79f9f7b922
    https://github.com/scummvm/scummvm/commit/c7d631cb6697a4c8c6d9cc0e32caba79f9f7b922
Author: Willem Jan Palenstijn (wjp at usecode.org)
Date: 2017-06-10T21:32:35+02:00

Commit Message:
SCI: Expand kernel breakpoint pattern matching for negative matches

See matchKernelBreakpointPattern() for samples. The main envisioned use is

DoSound*,!DoSoundUpdateCues

to match all DoSound sub-functions except DoSoundUpdateCues.

Changed paths:
    engines/sci/console.cpp
    engines/sci/engine/scriptdebug.cpp
    engines/sci/engine/scriptdebug.h


diff --git a/engines/sci/console.cpp b/engines/sci/console.cpp
index 701f08b..4508854 100644
--- a/engines/sci/console.cpp
+++ b/engines/sci/console.cpp
@@ -3993,6 +3993,9 @@ bool Console::cmdBreakpointKernel(int argc, const char **argv) {
 		debugPrintf("Sets a breakpoint on execution of a kernel function.\n");
 		debugPrintf("Usage: %s <name> [<action>]\n", argv[0]);
 		debugPrintf("Example: %s DrawPic\n", argv[0]);
+		debugPrintf("         %s DoSoundPlay,DoSoundStop\n", argv[0]);
+		debugPrintf("         %s DoSound*\n", argv[0]);
+		debugPrintf("         %s DoSound*,!DoSoundUpdateCues\n", argv[0]);
 		debugPrintf("         %s DrawPic log\n", argv[0]);
 		debugPrintf("See bp_action usage for possible actions.\n");
 		return true;
@@ -4008,11 +4011,7 @@ bool Console::cmdBreakpointKernel(int argc, const char **argv) {
 	}
 
 	// Check if any kernel functions match, to catch typos
-	Common::String name = argv[1];
-	bool wildcard = name.lastChar() == '*';
-	Common::String prefix = name;
-	if (wildcard)
-		prefix.deleteLastChar();
+	Common::String pattern = argv[1];
 	bool found = false;
 	const Kernel::KernelFunctionArray &kernelFuncs = _engine->getKernel()->_kernelFuncs;
 	for (uint id = 0; id < kernelFuncs.size() && !found; id++) {
@@ -4020,14 +4019,14 @@ bool Console::cmdBreakpointKernel(int argc, const char **argv) {
 			const KernelSubFunction *kernelSubCall = kernelFuncs[id].subFunctions;
 			if (!kernelSubCall) {
 				Common::String kname = kernelFuncs[id].name;
-				if (name == kname || (wildcard && kname.hasPrefix(prefix)))
+				if (matchKernelBreakpointPattern(pattern, kname))
 					found = true;
 			} else {
 				uint kernelSubCallCount = kernelFuncs[id].subFunctionCount;
 				for (uint subId = 0; subId < kernelSubCallCount; subId++) {
 					if (kernelSubCall->name) {
 						Common::String kname = kernelSubCall->name;
-						if (name == kname || (wildcard && kname.hasPrefix(prefix)))
+						if (matchKernelBreakpointPattern(pattern, kname))
 							found = true;
 					}
 					kernelSubCall++;
@@ -4036,13 +4035,13 @@ bool Console::cmdBreakpointKernel(int argc, const char **argv) {
 		}
 	}
 	if (!found) {
-		debugPrintf("No kernel functions match %s.\n", name.c_str());
+		debugPrintf("No kernel functions match %s.\n", pattern.c_str());
 		return true;
 	}
 
 	Breakpoint bp;
 	bp._type = BREAK_KERNEL;
-	bp._name = name;
+	bp._name = pattern;
 	bp._action = action;
 
 	_debugState._breakpoints.push_back(bp);
diff --git a/engines/sci/engine/scriptdebug.cpp b/engines/sci/engine/scriptdebug.cpp
index faf3411..a15725e 100644
--- a/engines/sci/engine/scriptdebug.cpp
+++ b/engines/sci/engine/scriptdebug.cpp
@@ -30,6 +30,8 @@
 #include "sci/engine/script.h"
 #include "sci/engine/scriptdebug.h"
 
+#include "common/algorithm.h"
+
 namespace Sci {
 
 //#define VM_DEBUG_SEND
@@ -782,6 +784,49 @@ bool SciEngine::checkAddressBreakpoint(const reg32_t &address) {
 	return found;
 }
 
+bool matchKernelBreakpointPattern(const Common::String &pattern, const Common::String &name) {
+	// Pattern:
+	// A comma-separated list of atoms.
+	// An atom is a (possibly empty) word, optionally with a ! prefix (for
+	// a negative-match), and/or a * suffix (for a prefix-match).
+
+	// The last matching atom in the pattern takes effect.
+
+	// Examples:
+	// FrameOut : matches only FrameOut
+	// * : matches everything
+	// *,!FrameOut : matches everything except FrameOut
+	// InitBresen,DoBresen : matches InitBresen and DoBresen
+	// DoSound*,!DoSoundUpdateCues : matches all DoSound sub-functions except
+	//                               DoSoundUpdateCues
+
+	bool result = false;
+
+	Common::String::const_iterator i = pattern.begin();
+	while (i != pattern.end()) {
+		Common::String::const_iterator next = Common::find(i, pattern.end(), ',');
+		bool negative = *i == '!';
+
+		if (negative)
+			i++;
+
+		Common::String atom(i, next - i);
+
+		bool wildcard = atom.lastChar() == '*';
+		if (wildcard)
+			atom.deleteLastChar();
+
+		if ((!wildcard && atom == name) || (wildcard && name.hasPrefix(atom)))
+			result = !negative;
+
+		i = next;
+		if (i != pattern.end())
+			++i; // skip comma
+	}
+
+	return result;
+}
+
 bool SciEngine::checkKernelBreakpoint(const Common::String &name) {
 	if (!(_debugState._activeBreakpointTypes & BREAK_KERNEL))
 		return false;
@@ -793,11 +838,7 @@ bool SciEngine::checkKernelBreakpoint(const Common::String &name) {
 		if (bp->_action == BREAK_NONE || bp->_type != BREAK_KERNEL)
 			continue;
 
-		bool wildcard = bp->_name.lastChar() == '*';
-		Common::String prefix = bp->_name;
-		if (wildcard)
-			prefix.deleteLastChar();
-		if (bp->_name == name || (wildcard && name.hasPrefix(prefix))) {
+		if (matchKernelBreakpointPattern(bp->_name, name)) {
 			if (bp->_action == BREAK_BREAK) {
 				if (!found)
 					_console->debugPrintf("Break on k%s\n", name.c_str());
diff --git a/engines/sci/engine/scriptdebug.h b/engines/sci/engine/scriptdebug.h
index f893099..5e927ef 100644
--- a/engines/sci/engine/scriptdebug.h
+++ b/engines/sci/engine/scriptdebug.h
@@ -39,6 +39,8 @@ void logBacktrace();
 
 bool printObject(reg_t obj);
 
+bool matchKernelBreakpointPattern(const Common::String &pattern, const Common::String &name);
+
 }
 
 #endif





More information about the Scummvm-git-logs mailing list