SCI: Improve kernel subfunction logging

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.

6 files changed, 50 insertions, 17 deletions

diff --git a/engines/sci/console.cpp b/engines/sci/console.cpp
index 27ac4fac49..2c74fe4b0c 100644
--- a/engines/sci/console.cpp
+++ b/engines/sci/console.cpp
@@ -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:
diff --git a/engines/sci/engine/kernel.cpp b/engines/sci/engine/kernel.cpp
index 796dea2382..4370e6de04 100644
--- a/engines/sci/engine/kernel.cpp
+++ b/engines/sci/engine/kernel.cpp
@@ -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)
diff --git a/engines/sci/engine/kernel.h b/engines/sci/engine/kernel.h
index 1202982986..d95e228045 100644
--- a/engines/sci/engine/kernel.h
+++ b/engines/sci/engine/kernel.h
@@ -158,6 +158,7 @@ public:
 
 	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.
diff --git a/engines/sci/engine/kmisc.cpp b/engines/sci/engine/kmisc.cpp
index 0a5f26476f..065625f85f 100644
--- a/engines/sci/engine/kmisc.cpp
+++ b/engines/sci/engine/kmisc.cpp
@@ -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]));
diff --git a/engines/sci/engine/vm.cpp b/engines/sci/engine/vm.cpp
index 64e6c045db..7e708efc8b 100644
--- a/engines/sci/engine/vm.cpp
+++ b/engines/sci/engine/vm.cpp
@@ -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());
@@ -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)
@@ -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);
 }
 
@@ -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)
@@ -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)
@@ -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);
diff --git a/engines/sci/engine/vm.h b/engines/sci/engine/vm.h
index 514bf58b64..dc789b6201 100644
--- a/engines/sci/engine/vm.h
+++ b/engines/sci/engine/vm.h
@@ -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_;
@@ -118,6 +121,8 @@ struct ExecStack {
 		else
 			local_segment = pc_.getSegment();
 		debugSelector = debugSelector_;
+		debugKernelFunction = debugKernelFunction_;
+		debugKernelSubFunction = debugKernelSubFunction_;
 		debugExportId = debugExportId_;
 		debugLocalCallOffset = debugLocalCallOffset_;
 		debugOrigin = debugOrigin_;