[Scummvm-cvs-logs] scummvm master -> 02acf2e6ee8b3dca41090433ab811655e41fd8b6
wjp
wjp at usecode.org
Sun Jul 3 13:00:08 CEST 2016
This automated email contains information about 6 new commits which have been
pushed to the 'scummvm' repo located at https://github.com/scummvm/scummvm .
Summary:
9f2fff4f66 SCI: Remove unneeded copy
08f1727b08 SCI: Improve kernel subfunction logging
7f12638763 SCI: Remove unclear &rest handling
d643cb651f SCI: Remove unexpected side effect from ExecStack constructor
4b72a42da9 SCI: Remove presumably long-outdated FIXME
02acf2e6ee Merge pull request #741 from wjp/sci-call
Commit: 9f2fff4f6614cd2ab11cf0d236ac553319e94702
https://github.com/scummvm/scummvm/commit/9f2fff4f6614cd2ab11cf0d236ac553319e94702
Author: Willem Jan Palenstijn (wjp at usecode.org)
Date: 2016-07-02T21:25:53+02:00
Commit Message:
SCI: Remove unneeded copy
Changed paths:
engines/sci/engine/workarounds.cpp
diff --git a/engines/sci/engine/workarounds.cpp b/engines/sci/engine/workarounds.cpp
index 073bb93..2a2540b 100644
--- a/engines/sci/engine/workarounds.cpp
+++ b/engines/sci/engine/workarounds.cpp
@@ -780,7 +780,7 @@ SciWorkaroundSolution trackOriginAndFindWorkaround(int index, const SciWorkaroun
Common::List<ExecStack>::const_iterator callIterator = state->_executionStack.end();
while (callIterator != state->_executionStack.begin()) {
callIterator--;
- ExecStack loopCall = *callIterator;
+ const ExecStack &loopCall = *callIterator;
if ((loopCall.debugSelector != -1) || (loopCall.debugExportId != -1)) {
lastCall->debugSelector = loopCall.debugSelector;
lastCall->debugExportId = loopCall.debugExportId;
Commit: 08f1727b0813c9e20a2462402f58c4578c5902c8
https://github.com/scummvm/scummvm/commit/08f1727b0813c9e20a2462402f58c4578c5902c8
Author: Willem Jan Palenstijn (wjp at usecode.org)
Date: 2016-07-02T21:25:53+02:00
Commit Message:
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.
Changed paths:
engines/sci/console.cpp
engines/sci/engine/kernel.cpp
engines/sci/engine/kernel.h
engines/sci/engine/kmisc.cpp
engines/sci/engine/vm.cpp
engines/sci/engine/vm.h
diff --git a/engines/sci/console.cpp b/engines/sci/console.cpp
index 27ac4fa..2c74fe4 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 796dea2..4370e6d 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 1202982..d95e228 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 0a5f264..065625f 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 64e6c04..7e708ef 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 514bf58..dc789b6 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_;
Commit: 7f12638763e9732bc75aab4823c43ebdf8a8c2be
https://github.com/scummvm/scummvm/commit/7f12638763e9732bc75aab4823c43ebdf8a8c2be
Author: Willem Jan Palenstijn (wjp at usecode.org)
Date: 2016-07-02T21:25:53+02:00
Commit Message:
SCI: Remove unclear &rest handling
Modifying a value above the stack pointer doesn't seem to make much
sense. This was added in FreeSCI back in 2002 in a pair of commits that
did not make clear what the purpose of this was. My guess is that it
attempted to adjust argc, but failed. This wouldn't have been noticed
since argc was always set correctly by make_exec_stack_entry (which is
now the ExecStack constructor).
Changed paths:
engines/sci/engine/vm.cpp
diff --git a/engines/sci/engine/vm.cpp b/engines/sci/engine/vm.cpp
index 7e708ef..273492c 100644
--- a/engines/sci/engine/vm.cpp
+++ b/engines/sci/engine/vm.cpp
@@ -834,7 +834,6 @@ void run_vm(EngineState *s) {
int argc = (opparams[1] >> 1) // Given as offset, but we need count
+ 1 + s->r_rest;
StackPtr call_base = s->xs->sp - argc;
- s->xs->sp[1].incOffset(s->r_rest);
uint32 localCallOffset = s->xs->addr.pc.getOffset() + opparams[0];
Commit: d643cb651f5661a80c7d7ea493ff397cf0bef3fd
https://github.com/scummvm/scummvm/commit/d643cb651f5661a80c7d7ea493ff397cf0bef3fd
Author: Willem Jan Palenstijn (wjp at usecode.org)
Date: 2016-07-02T21:25:53+02:00
Commit Message:
SCI: Remove unexpected side effect from ExecStack constructor
The ExecStack constructor set argp[0] to argc before. This is now moved
to the caller, to make this action more explicit.
Changed paths:
engines/sci/engine/vm.cpp
engines/sci/engine/vm.h
diff --git a/engines/sci/engine/vm.cpp b/engines/sci/engine/vm.cpp
index 273492c..3e12084 100644
--- a/engines/sci/engine/vm.cpp
+++ b/engines/sci/engine/vm.cpp
@@ -239,6 +239,7 @@ ExecStack *execute_method(EngineState *s, uint16 script, uint16 pubfunct, StackP
// Check if a breakpoint is set on this method
g_sci->checkExportBreakpoint(script, pubfunct);
+ assert(argp[0].toUint16() == argc); // The first argument is argc
ExecStack xstack(calling_obj, calling_obj, sp, argc, argp,
seg, make_reg32(seg, exportAddr), -1, -1, -1, pubfunct, -1,
s->_executionStack.size() - 1, EXEC_STACK_TYPE_CALL);
@@ -312,6 +313,7 @@ ExecStack *send_selector(EngineState *s, reg_t send_obj, reg_t work_obj, StackPt
if (activeBreakpointTypes || DebugMan.isDebugChannelEnabled(kDebugLevelScripts))
debugSelectorCall(send_obj, selector, argc, argp, varp, funcp, s->_segMan, selectorType);
+ assert(argp[0].toUint16() == argc); // The first argument is argc
ExecStack xstack(work_obj, send_obj, curSP, argc, argp,
0xFFFF, curFP, selector, -1, -1, -1, -1,
origin, stackType);
@@ -386,6 +388,7 @@ static void callKernelFunc(EngineState *s, int kernelCallNr, int argc) {
// Call kernel function
if (!kernelCall.subFunctionCount) {
+ argv[-1] = make_reg(0, argc); // The first argument is argc
addKernelCallToExecStack(s, kernelCallNr, -1, argc, argv);
s->r_acc = kernelCall.function(s, argc, argv);
@@ -444,6 +447,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);
+ argv[-1] = make_reg(0, argc); // The first argument is argc
addKernelCallToExecStack(s, kernelCallNr, subId, argc, argv);
s->r_acc = kernelSubCall.function(s, argc, argv);
@@ -837,8 +841,10 @@ void run_vm(EngineState *s) {
uint32 localCallOffset = s->xs->addr.pc.getOffset() + opparams[0];
+ int final_argc = (call_base->requireUint16()) + s->r_rest;
+ call_base[0] = make_reg(0, final_argc); // The first argument is argc
ExecStack xstack(s->xs->objp, s->xs->objp, s->xs->sp,
- (call_base->requireUint16()) + s->r_rest, call_base,
+ final_argc, call_base,
s->xs->local_segment, make_reg32(s->xs->addr.pc.getSegment(), localCallOffset),
NULL_SELECTOR, -1, -1, -1, localCallOffset, s->_executionStack.size() - 1,
EXEC_STACK_TYPE_CALL);
diff --git a/engines/sci/engine/vm.h b/engines/sci/engine/vm.h
index dc789b6..c41060d 100644
--- a/engines/sci/engine/vm.h
+++ b/engines/sci/engine/vm.h
@@ -115,7 +115,6 @@ struct ExecStack {
fp = sp = sp_;
argc = argc_;
variables_argp = argp_;
- *variables_argp = make_reg(0, argc); // The first argument is argc
if (localsSegment_ != 0xFFFF)
local_segment = localsSegment_;
else
Commit: 4b72a42da93da5560ecde38010d328196ff727fd
https://github.com/scummvm/scummvm/commit/4b72a42da93da5560ecde38010d328196ff727fd
Author: Willem Jan Palenstijn (wjp at usecode.org)
Date: 2016-07-02T21:31:29+02:00
Commit Message:
SCI: Remove presumably long-outdated FIXME
Changed paths:
engines/sci/engine/kernel.cpp
diff --git a/engines/sci/engine/kernel.cpp b/engines/sci/engine/kernel.cpp
index 4370e6d..2fc338b 100644
--- a/engines/sci/engine/kernel.cpp
+++ b/engines/sci/engine/kernel.cpp
@@ -84,23 +84,15 @@ uint Kernel::getKernelNamesSize() const {
}
const Common::String &Kernel::getKernelName(uint number) const {
- // FIXME: The following check is a temporary workaround for an issue
- // leading to crashes when using the debugger's backtrace command.
- if (number >= _kernelNames.size())
- return _invalid;
+ assert(number < _kernelFuncs.size());
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;
+ assert(number < _kernelFuncs.size());
const KernelFunction &kernelCall = _kernelFuncs[number];
- if (subFunction >= kernelCall.subFunctionCount)
- return _invalid;
-
+ assert(subFunction < kernelCall.subFunctionCount);
return kernelCall.subFunctions[subFunction].name;
}
Commit: 02acf2e6ee8b3dca41090433ab811655e41fd8b6
https://github.com/scummvm/scummvm/commit/02acf2e6ee8b3dca41090433ab811655e41fd8b6
Author: Willem Jan Palenstijn (wjp at usecode.org)
Date: 2016-07-03T12:59:59+02:00
Commit Message:
Merge pull request #741 from wjp/sci-call
SCI: Clean up some aspects of call handling
Changed paths:
engines/sci/console.cpp
engines/sci/engine/kernel.cpp
engines/sci/engine/kernel.h
engines/sci/engine/kmisc.cpp
engines/sci/engine/vm.cpp
engines/sci/engine/vm.h
engines/sci/engine/workarounds.cpp
More information about the Scummvm-git-logs
mailing list