[Scummvm-git-logs] scummvm master -> 23c6a5182e3aa46d168a1c75e244df5c51603d69
bluegr
bluegr at gmail.com
Sun Mar 15 23:17:08 UTC 2020
This automated email contains information about 1 new commit which have been
pushed to the 'scummvm' repo located at https://github.com/scummvm/scummvm .
Summary:
23c6a5182e SCI: Added new VM hook mechanism, allowing new instructions
Commit: 23c6a5182e3aa46d168a1c75e244df5c51603d69
https://github.com/scummvm/scummvm/commit/23c6a5182e3aa46d168a1c75e244df5c51603d69
Author: Zvika Haramaty (haramaty.zvika at gmail.com)
Date: 2020-03-16T01:17:03+02:00
Commit Message:
SCI: Added new VM hook mechanism, allowing new instructions
The current patch mechanism allows only replacing intructions in current
scripts, but not adding new instructions out of nowhere.
Thus, issue like #9646 is about to be closed as WONTFIX.
This new mechanism adds a hook on vm.cpp, before executing opcodes, and
if required, executs a new code.
It solves one of the issues of #9646, the others can be solved as well using that mechanism.
Changed paths:
A engines/sci/engine/vm_hooks.cpp
A engines/sci/engine/vm_hooks.h
engines/sci/engine/vm.cpp
engines/sci/module.mk
diff --git a/engines/sci/engine/vm.cpp b/engines/sci/engine/vm.cpp
index e8808a715c..b7259d6140 100644
--- a/engines/sci/engine/vm.cpp
+++ b/engines/sci/engine/vm.cpp
@@ -37,6 +37,7 @@
#include "sci/engine/gc.h"
#include "sci/engine/workarounds.h"
#include "sci/engine/scriptdebug.h"
+#include "sci/engine/vm_hooks.h"
namespace Sci {
@@ -577,6 +578,8 @@ void run_vm(EngineState *s) {
StackPtr s_temp; // Temporary stack pointer
int16 opparams[4]; // opcode parameters
+ VmHooks vmHooks;
+
s->r_rest = 0; // &rest adjusts the parameter count by this value
// Current execution data:
s->xs = &(s->_executionStack.back());
@@ -602,6 +605,8 @@ void run_vm(EngineState *s) {
#endif
while (1) {
+ vmHooks.vm_hook_before_exec(s);
+
int var_type; // See description below
int var_number;
@@ -659,7 +664,12 @@ void run_vm(EngineState *s) {
// Get opcode
byte extOpcode;
- s->xs->addr.pc.incOffset(readPMachineInstruction(scr->getBuf(s->xs->addr.pc.getOffset()), extOpcode, opparams));
+ if (!vmHooks.isActive())
+ s->xs->addr.pc.incOffset(readPMachineInstruction(scr->getBuf(s->xs->addr.pc.getOffset()), extOpcode, opparams));
+ else {
+ int offset = readPMachineInstruction(vmHooks.data(), extOpcode, opparams);
+ vmHooks.advance(offset);
+ }
const byte opcode = extOpcode >> 1;
//debug("%s: %d, %d, %d, %d, acc = %04x:%04x, script %d, local script %d", opcodeNames[opcode], opparams[0], opparams[1], opparams[2], opparams[3], PRINT_REG(s->r_acc), scr->getScriptNumber(), local_script->getScriptNumber());
@@ -794,30 +804,44 @@ void run_vm(EngineState *s) {
case op_bt: // 0x17 (23)
// Branch relative if true
- if (s->r_acc.getOffset() || s->r_acc.getSegment())
- s->xs->addr.pc.incOffset(opparams[0]);
+ if (!vmHooks.isActive()) {
+ if (s->r_acc.getOffset() || s->r_acc.getSegment())
+ s->xs->addr.pc.incOffset(opparams[0]);
- if (s->xs->addr.pc.getOffset() >= local_script->getScriptSize())
- error("[VM] op_bt: request to jump past the end of script %d (offset %d, script is %d bytes)",
- local_script->getScriptNumber(), s->xs->addr.pc.getOffset(), local_script->getScriptSize());
+ if (s->xs->addr.pc.getOffset() >= local_script->getScriptSize())
+ error("[VM] op_bt: request to jump past the end of script %d (offset %d, script is %d bytes)",
+ local_script->getScriptNumber(), s->xs->addr.pc.getOffset(), local_script->getScriptSize());
+ } else {
+ if (s->r_acc.getOffset() || s->r_acc.getSegment())
+ vmHooks.advance(opparams[0]);
+ }
break;
case op_bnt: // 0x18 (24)
// Branch relative if not true
- if (!(s->r_acc.getOffset() || s->r_acc.getSegment()))
- s->xs->addr.pc.incOffset(opparams[0]);
+ if (!vmHooks.isActive()) {
+ if (!(s->r_acc.getOffset() || s->r_acc.getSegment()))
+ s->xs->addr.pc.incOffset(opparams[0]);
- if (s->xs->addr.pc.getOffset() >= local_script->getScriptSize())
- error("[VM] op_bnt: request to jump past the end of script %d (offset %d, script is %d bytes)",
- local_script->getScriptNumber(), s->xs->addr.pc.getOffset(), local_script->getScriptSize());
+ if (s->xs->addr.pc.getOffset() >= local_script->getScriptSize())
+ error("[VM] op_bnt: request to jump past the end of script %d (offset %d, script is %d bytes)",
+ local_script->getScriptNumber(), s->xs->addr.pc.getOffset(), local_script->getScriptSize());
+ } else {
+ if (!(s->r_acc.getOffset() || s->r_acc.getSegment()))
+ vmHooks.advance(opparams[0]);
+ }
break;
case op_jmp: // 0x19 (25)
- s->xs->addr.pc.incOffset(opparams[0]);
+ if (!vmHooks.isActive()) {
+ s->xs->addr.pc.incOffset(opparams[0]);
- if (s->xs->addr.pc.getOffset() >= local_script->getScriptSize())
- error("[VM] op_jmp: request to jump past the end of script %d (offset %d, script is %d bytes)",
- local_script->getScriptNumber(), s->xs->addr.pc.getOffset(), local_script->getScriptSize());
+ if (s->xs->addr.pc.getOffset() >= local_script->getScriptSize())
+ error("[VM] op_jmp: request to jump past the end of script %d (offset %d, script is %d bytes)",
+ local_script->getScriptNumber(), s->xs->addr.pc.getOffset(), local_script->getScriptSize());
+ } else {
+ vmHooks.advance(opparams[0]);
+ }
break;
case op_ldi: // 0x1a (26)
diff --git a/engines/sci/engine/vm_hooks.cpp b/engines/sci/engine/vm_hooks.cpp
new file mode 100644
index 0000000000..d598d9a9a1
--- /dev/null
+++ b/engines/sci/engine/vm_hooks.cpp
@@ -0,0 +1,165 @@
+/* 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.
+ *
+ */
+
+#include "common/hashmap.h"
+#include "common/array.h"
+#include "sci/engine/vm_hooks.h"
+#include "sci/engine/vm.h"
+#include "sci/engine/state.h"
+#include "sci/engine/kernel.h"
+#include "sci/engine/scriptdebug.h"
+
+namespace Sci {
+
+/*****************************************************************************************************************
+ *
+ * This mechanism allows inserting new instructions, and not only replace existing code.
+ * Note that when using hooks, the regular PC is frozen, and doesn't advance.
+ * Therefore, 'jmp', 'bt' and 'bnt' are used to locally move around inside the patch.
+ * call* opcodes can be used - but they should be last executed opcode, in order to successfully transfer control.
+ *
+ ******************************************************************************************************************/
+
+
+// solves the issue described at #9646:
+// "
+// When in room 58, and type "run", the hero will fall and his HP will decrease by 1 point. This can be repeated, but will never cause the hero to die.
+// When typing "run" the ego will be assigned with the script egoRuns.
+// egoRuns::changeState calls proc0_36 in script 0 which is deducing damage from the hero's HP.
+// This procedure returns TRUE if the hero is still alive, but the return value is never observed in egoRuns.
+// "
+// we solve that by calling the hook before executing the opcode following proc0_36 call
+// and check the return value. if the hero should die, we kill him
+
+const byte qfg1_die_after_running_on_ice[] = {
+ // if shouldn't die, jump to end
+ 0x2f, 22, // bt +22
+
+ // should die - done according to the code at main.sc, proc0_29:
+ // (proc0_1 0 59 80 {Death from Overwork} 82 800 1 4)
+ 0x39, 0x08, // pushi 8 -- num of parameters
+ 0x39, 0x00, // pushi 0
+ 0x39, 59, // pushi 59
+ 0x39, 0, // pushi 0 -- modified, not using {Death from Overwork}
+ 0x36, // push
+ 0x39, 82, // pushi 82
+ 0x38, 32, 3, // push 800
+ 0x39, 1, // pushi 1
+ 0x39, 4, // pushi 4
+ 0x47, 0x00, 0x01, 0x10 // calle proc0_1
+};
+
+const byte del_me[] = {
+ 38 // illegal opcode
+};
+
+/** Write here all games hooks
+ * From this we'll build _hooksMap, which contains only relevant hooks to current game
+ * The match is performed according to PC, script number, opcode (only opcode name, as seen in ScummVM debugger),
+ * and either:
+ * - objName and selector (and then externID is -1)
+ * - external function ID (and then selector is "")
+ * = in that case, if objName == "" it will be ignored, otherwise, it will be also used to match
+ */
+static const GeneralHookEntry allGamesHooks[] = {
+ // GID, script, PC.offset, objName, selector, externID, opcode, hook array
+ {GID_QFG1, {58, 0x144d}, {"egoRuns", "changeState", -1 , "push0", HOOKARRAY(qfg1_die_after_running_on_ice)}}
+};
+
+
+VmHooks::VmHooks() {
+ // build _hooksMap
+ for (uint i = 0; i < ARRAYSIZE(allGamesHooks); i++) {
+ if (allGamesHooks[i].gameId == g_sci->getGameId())
+ _hooksMap.setVal(allGamesHooks[i].key, allGamesHooks[i].entry);
+ }
+
+ _lastPc = NULL_REG;
+ _location = 0;
+}
+
+uint64 HookHashKey::hash() {
+ return ((uint64)scriptNumber << 32) + offset;
+}
+
+
+// returns true if entry is matching to current state
+bool hook_exec_match(Sci::EngineState *s, HookEntry entry) {
+ Script *scr = s->_segMan->getScript(s->xs->addr.pc.getSegment());
+ const char *objName = s->_segMan->getObjectName(s->xs->objp);
+ Common::String selector;
+ if (s->xs->debugSelector != -1)
+ selector = g_sci->getKernel()->getSelectorName(s->xs->debugSelector);
+ byte opcode = (scr->getBuf(s->xs->addr.pc.getOffset())[0]) >> 1;
+
+ bool objMatch;
+ if (entry.exportId != -1 && strcmp(entry.objName, "") == 0)
+ objMatch = true;
+ else
+ objMatch = strcmp(objName, entry.objName) == 0;
+
+ return objMatch && selector == entry.selector &&
+ s->xs->debugExportId == entry.exportId && strcmp(entry.opcodeName, opcodeNames[opcode]) == 0;
+}
+
+
+void VmHooks::vm_hook_before_exec(Sci::EngineState *s) {
+ Script *scr = s->_segMan->getScript(s->xs->addr.pc.getSegment());
+ int scriptNumber = scr->getScriptNumber();
+ HookHashKey key = { scriptNumber, s->xs->addr.pc.getOffset() };
+ if (_lastPc != s->xs->addr.pc && _hooksMap.contains(key)) {
+ _lastPc = s->xs->addr.pc;
+ HookEntry entry = _hooksMap[key];
+ if (hook_exec_match(s, entry)) {
+ debugC(kDebugLevelPatcher, "vm_hook: patching script: %d, PC: %04x:%04x, obj: %s, selector: %s, extern: %d, opcode: %s", scriptNumber, PRINT_REG(s->xs->addr.pc), entry.objName, entry.selector.c_str(), entry.exportId, entry.opcodeName);
+ Common::Array<byte> buffer(entry.patch, entry.patchSize);
+
+ _hookScriptData = buffer;
+ } else {
+ debugC(kDebugLevelPatcher, "vm_hook: failed to match! script: %d, PC: %04x:%04x, obj: %s, selector: %s, extern: %d, opcode: %s", scriptNumber, PRINT_REG(s->xs->addr.pc), entry.objName, entry.selector.c_str(), entry.exportId, entry.opcodeName);
+ }
+ }
+}
+
+byte *VmHooks::data() {
+ return _hookScriptData.data() + _location;
+}
+
+bool VmHooks::isActive() {
+ return !_hookScriptData.empty();
+}
+
+void VmHooks::advance(int offset) {
+ int newLocation = _location + offset;
+ if (newLocation < 0)
+ error("VmHooks: requested to change offset before start of patch");
+ else if ((uint)newLocation > _hookScriptData.size())
+ error("VmHooks: requested to change offset after end of patch");
+ else if ((uint)newLocation == _hookScriptData.size()) {
+ _hookScriptData.clear();
+ _location = 0;
+ } else
+ _location = newLocation;
+}
+
+
+} // End of namespace Sci
diff --git a/engines/sci/engine/vm_hooks.h b/engines/sci/engine/vm_hooks.h
new file mode 100644
index 0000000000..5c5e699d53
--- /dev/null
+++ b/engines/sci/engine/vm_hooks.h
@@ -0,0 +1,103 @@
+/* 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_VM_HOOKS_H
+#define SCI_ENGINE_VM_HOOKS_H
+
+#include "sci/engine/vm.h"
+
+namespace Sci {
+
+
+#define ARRAYSIZE(x) ((int)(sizeof(x) / sizeof(x[0])))
+#define HOOKARRAY(x) x, ARRAYSIZE(x)
+
+/** _hooksMap keys are built from script number and PC's offset */
+struct HookHashKey {
+ int scriptNumber;
+ uint32 offset;
+
+ uint64 hash();
+
+ bool operator==(const HookHashKey &other) const {
+ return scriptNumber == other.scriptNumber && offset == other.offset;
+ }
+
+
+};
+
+/** _hooksMap value entry */
+struct HookEntry {
+ /** These are used to make sure that the PC is indeed the requested place */
+ const char *objName;
+ Common::String selector;
+ int exportId;
+ const char *opcodeName;
+
+ /** If all the previous match, patch */
+ const byte *patch;
+ uint patchSize;
+};
+
+/** Used for allGamesHooks - from it we build the specific _hooksMap */
+struct GeneralHookEntry {
+ SciGameId gameId;
+ HookHashKey key;
+ HookEntry entry;
+};
+
+/** Hash key equality function */
+struct HookHash : public Common::UnaryFunction<HookHashKey, uint64> {
+ uint64 operator()(HookHashKey val) const { return val.hash(); }
+};
+
+/** VM Hook mechanism */
+class VmHooks {
+public:
+ VmHooks();
+
+ /** Called just before executing opcode, to check if there is a requried hook */
+ void vm_hook_before_exec(Sci::EngineState *s);
+
+ byte *data();
+
+ bool isActive();
+
+ void advance(int offset);
+
+private:
+ /** Hash map of all game's hooks */
+ Common::HashMap<HookHashKey, HookEntry, HookHash> _hooksMap;
+
+ Common::Array<byte> _hookScriptData;
+
+ /** Used to avoid double patching in row */
+ reg_t _lastPc;
+
+ /** Location inside patch */
+ int _location;
+};
+
+
+} // End of namespace Sci
+
+#endif // SCI_ENGINE_VM_HOOKS_H
diff --git a/engines/sci/module.mk b/engines/sci/module.mk
index 8004131621..232c27da2e 100644
--- a/engines/sci/module.mk
+++ b/engines/sci/module.mk
@@ -41,6 +41,7 @@ MODULE_OBJS := \
engine/state.o \
engine/static_selectors.o \
engine/vm.o \
+ engine/vm_hooks.o \
engine/vm_types.o \
engine/workarounds.o \
graphics/animate.o \
More information about the Scummvm-git-logs
mailing list