Skip to content

Commit

Permalink
SCI: Improve kernel subfunction logging
Browse files Browse the repository at this point in the history
ExecStack now stores the kernel call number as well as the subfunction.
This allows kStub and backtraces to log the actual subfunction called.

The kernel call number in ExecStack used to be stored in the
debugSelector field. It now has its own field, to avoid confusion.
  • Loading branch information
wjp committed Jul 2, 2016
1 parent 9f2fff4 commit 08f1727
Show file tree
Hide file tree
Showing 6 changed files with 50 additions and 17 deletions.
5 changes: 4 additions & 1 deletion engines/sci/console.cpp
Expand Up @@ -3094,7 +3094,10 @@ bool Console::cmdBacktrace(int argc, const char **argv) {
break;

case EXEC_STACK_TYPE_KERNEL: // Kernel function
debugPrintf(" %x:[%x] k%s(", i, call.debugOrigin, _engine->getKernel()->getKernelName(call.debugSelector).c_str());
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:
Expand Down
14 changes: 14 additions & 0 deletions engines/sci/engine/kernel.cpp
Expand Up @@ -91,6 +91,20 @@ const Common::String &Kernel::getKernelName(uint number) const {
return _kernelNames[number];
}

Common::String Kernel::getKernelName(uint number, uint subFunction) const {
// FIXME: The following check is a temporary workaround for an issue
// leading to crashes when using the debugger's backtrace command.
if (number >= _kernelFuncs.size())
return _invalid;
const KernelFunction &kernelCall = _kernelFuncs[number];

if (subFunction >= kernelCall.subFunctionCount)
return _invalid;

return kernelCall.subFunctions[subFunction].name;
}


int Kernel::findKernelFuncPos(Common::String kernelFuncName) {
for (uint32 i = 0; i < _kernelNames.size(); i++)
if (_kernelNames[i] == kernelFuncName)
Expand Down
1 change: 1 addition & 0 deletions engines/sci/engine/kernel.h
Expand Up @@ -158,6 +158,7 @@ class Kernel {

uint getKernelNamesSize() const;
const Common::String &getKernelName(uint number) const;
Common::String getKernelName(uint number, uint subFunction) const;

/**
* Determines the selector ID of a selector by its name.
Expand Down
20 changes: 15 additions & 5 deletions engines/sci/engine/kmisc.cpp
Expand Up @@ -605,18 +605,28 @@ reg_t kEmpty(EngineState *s, int argc, reg_t *argv) {
reg_t kStub(EngineState *s, int argc, reg_t *argv) {
Kernel *kernel = g_sci->getKernel();
int kernelCallNr = -1;
int kernelSubCallNr = -1;

Common::List<ExecStack>::const_iterator callIterator = s->_executionStack.end();
if (callIterator != s->_executionStack.begin()) {
callIterator--;
ExecStack lastCall = *callIterator;
kernelCallNr = lastCall.debugSelector;
kernelCallNr = lastCall.debugKernelFunction;
kernelSubCallNr = lastCall.debugKernelSubFunction;
}

Common::String warningMsg = "Dummy function k" + kernel->getKernelName(kernelCallNr) +
Common::String::format("[%x]", kernelCallNr) +
" invoked. Params: " +
Common::String::format("%d", argc) + " (";
Common::String warningMsg;
if (kernelSubCallNr == -1) {
warningMsg = "Dummy function k" + kernel->getKernelName(kernelCallNr) +
Common::String::format("[%x]", kernelCallNr);
} else {
warningMsg = "Dummy function k" + kernel->getKernelName(kernelCallNr, kernelSubCallNr) +
Common::String::format("[%x:%x]", kernelCallNr, kernelSubCallNr);

}

warningMsg += " invoked. Params: " +
Common::String::format("%d", argc) + " (";

for (int i = 0; i < argc; i++) {
warningMsg += Common::String::format("%04x:%04x", PRINT_REG(argv[i]));
Expand Down
14 changes: 7 additions & 7 deletions engines/sci/engine/vm.cpp
Expand Up @@ -240,7 +240,7 @@ ExecStack *execute_method(EngineState *s, uint16 script, uint16 pubfunct, StackP
g_sci->checkExportBreakpoint(script, pubfunct);

ExecStack xstack(calling_obj, calling_obj, sp, argc, argp,
seg, make_reg32(seg, exportAddr), -1, pubfunct, -1,
seg, make_reg32(seg, exportAddr), -1, -1, -1, pubfunct, -1,
s->_executionStack.size() - 1, EXEC_STACK_TYPE_CALL);
s->_executionStack.push_back(xstack);
return &(s->_executionStack.back());
Expand Down Expand Up @@ -313,7 +313,7 @@ ExecStack *send_selector(EngineState *s, reg_t send_obj, reg_t work_obj, StackPt
debugSelectorCall(send_obj, selector, argc, argp, varp, funcp, s->_segMan, selectorType);

ExecStack xstack(work_obj, send_obj, curSP, argc, argp,
0xFFFF, curFP, selector, -1, -1,
0xFFFF, curFP, selector, -1, -1, -1, -1,
origin, stackType);

if (selectorType == kSelectorVariable)
Expand All @@ -335,12 +335,12 @@ ExecStack *send_selector(EngineState *s, reg_t send_obj, reg_t work_obj, StackPt
return s->_executionStack.empty() ? NULL : &(s->_executionStack.back());
}

static void addKernelCallToExecStack(EngineState *s, int kernelCallNr, int argc, reg_t *argv) {
static void addKernelCallToExecStack(EngineState *s, int kernelCallNr, int kernelSubCallNr, int argc, reg_t *argv) {
// Add stack frame to indicate we're executing a callk.
// This is useful in debugger backtraces if this
// kernel function calls a script itself.
ExecStack xstack(NULL_REG, NULL_REG, NULL, argc, argv - 1, 0xFFFF, make_reg32(0, 0),
kernelCallNr, -1, -1, s->_executionStack.size() - 1, EXEC_STACK_TYPE_KERNEL);
-1, kernelCallNr, kernelSubCallNr, -1, -1, s->_executionStack.size() - 1, EXEC_STACK_TYPE_KERNEL);
s->_executionStack.push_back(xstack);
}

Expand Down Expand Up @@ -386,7 +386,7 @@ static void callKernelFunc(EngineState *s, int kernelCallNr, int argc) {

// Call kernel function
if (!kernelCall.subFunctionCount) {
addKernelCallToExecStack(s, kernelCallNr, argc, argv);
addKernelCallToExecStack(s, kernelCallNr, -1, argc, argv);
s->r_acc = kernelCall.function(s, argc, argv);

if (kernelCall.debugLogging)
Expand Down Expand Up @@ -444,7 +444,7 @@ static void callKernelFunc(EngineState *s, int kernelCallNr, int argc) {
}
if (!kernelSubCall.function)
error("[VM] k%s: subfunction ID %d requested, but not available", kernelCall.name, subId);
addKernelCallToExecStack(s, kernelCallNr, argc, argv);
addKernelCallToExecStack(s, kernelCallNr, subId, argc, argv);
s->r_acc = kernelSubCall.function(s, argc, argv);

if (kernelSubCall.debugLogging)
Expand Down Expand Up @@ -841,7 +841,7 @@ void run_vm(EngineState *s) {
ExecStack xstack(s->xs->objp, s->xs->objp, s->xs->sp,
(call_base->requireUint16()) + s->r_rest, call_base,
s->xs->local_segment, make_reg32(s->xs->addr.pc.getSegment(), localCallOffset),
NULL_SELECTOR, -1, localCallOffset, s->_executionStack.size() - 1,
NULL_SELECTOR, -1, -1, -1, localCallOffset, s->_executionStack.size() - 1,
EXEC_STACK_TYPE_CALL);

s->_executionStack.push_back(xstack);
Expand Down
13 changes: 9 additions & 4 deletions engines/sci/engine/vm.h
Expand Up @@ -93,16 +93,19 @@ struct ExecStack {

SegmentId local_segment; // local variables etc

Selector debugSelector; // The selector which was used to call or -1 if not applicable
int debugExportId; // The exportId which was called or -1 if not applicable
int debugLocalCallOffset; // Local call offset or -1 if not applicable
int debugOrigin; // The stack frame position the call was made from, or -1 if it was the initial call
Selector debugSelector; // The selector which was used to call or -1 if not applicable
int debugExportId; // The exportId which was called or -1 if not applicable
int debugLocalCallOffset; // Local call offset or -1 if not applicable
int debugOrigin; // The stack frame position the call was made from, or -1 if it was the initial call
int debugKernelFunction; // The kernel function called, or -1 if not applicable
int debugKernelSubFunction; // The kernel subfunction called, or -1 if not applicable
ExecStackType type;

reg_t* getVarPointer(SegManager *segMan) const;

ExecStack(reg_t objp_, reg_t sendp_, StackPtr sp_, int argc_, StackPtr argp_,
SegmentId localsSegment_, reg32_t pc_, Selector debugSelector_,
int debugKernelFunction_, int debugKernelSubFunction_,
int debugExportId_, int debugLocalCallOffset_, int debugOrigin_,
ExecStackType type_) {
objp = objp_;
Expand All @@ -118,6 +121,8 @@ struct ExecStack {
else
local_segment = pc_.getSegment();
debugSelector = debugSelector_;
debugKernelFunction = debugKernelFunction_;
debugKernelSubFunction = debugKernelSubFunction_;
debugExportId = debugExportId_;
debugLocalCallOffset = debugLocalCallOffset_;
debugOrigin = debugOrigin_;
Expand Down

0 comments on commit 08f1727

Please sign in to comment.