diff options
author | Willem Jan Palenstijn | 2017-05-27 20:26:57 +0200 |
---|---|---|
committer | Willem Jan Palenstijn | 2017-06-10 21:32:35 +0200 |
commit | e2e3f7c4c51e64df05b3252379bcc56f52b576dc (patch) | |
tree | 7ba3bb19a5309402e5fa0c218bffe4f4f7ced252 | |
parent | 0f0ecff0b8b095205a3a1c9b9a133f283d1ad39b (diff) | |
download | scummvm-rg350-e2e3f7c4c51e64df05b3252379bcc56f52b576dc.tar.gz scummvm-rg350-e2e3f7c4c51e64df05b3252379bcc56f52b576dc.tar.bz2 scummvm-rg350-e2e3f7c4c51e64df05b3252379bcc56f52b576dc.zip |
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.
-rw-r--r-- | engines/sci/console.cpp | 94 | ||||
-rw-r--r-- | engines/sci/debug.h | 7 | ||||
-rw-r--r-- | engines/sci/engine/kernel.cpp | 80 | ||||
-rw-r--r-- | engines/sci/engine/kernel.h | 7 | ||||
-rw-r--r-- | engines/sci/engine/scriptdebug.cpp | 38 | ||||
-rw-r--r-- | engines/sci/engine/vm.cpp | 17 | ||||
-rw-r--r-- | engines/sci/sci.h | 1 |
7 files changed, 111 insertions, 133 deletions
diff --git a/engines/sci/console.cpp b/engines/sci/console.cpp index 3840e33548..44d2a71683 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 afcac19f04..cd1d8095f9 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 efa10c337c..539475c464 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 bddfab531f..d1dce37e48 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 e2522414e9..9f1b536fec 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 1e088d6d7c..b169e0ec11 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 3139f1583b..8109317cdf 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 |