[Scummvm-git-logs] scummvm master -> f36f3887916e90a559954180acd48f2839eaf544
dreammaster
noreply at scummvm.org
Sat Feb 24 20:05:29 UTC 2024
This automated email contains information about 3 new commits which have been
pushed to the 'scummvm' repo located at https://github.com/scummvm/scummvm .
Summary:
a6929c4882 M4: Attempt to workaround double free() issue
41d87f2178 M4: Cleanup of dead machine code, deleting of dead machines between scenes
f36f388791 M4: Added console command to set boonsville time for triggering events
Commit: a6929c4882cb74e56124d96fd7909e961b3fc8f6
https://github.com/scummvm/scummvm/commit/a6929c4882cb74e56124d96fd7909e961b3fc8f6
Author: antoniou79 (a.antoniou79 at gmail.com)
Date: 2024-02-24T12:05:22-08:00
Commit Message:
M4: Attempt to workaround double free() issue
In some cases the engine may access unallocate space or do a double free_mem on an "machine" object
This can happen when a script "machine"'s step triggers a callback that can result to killing the machine itself (via terminateMachineAndNull()) and free its allocated space via a different pointer, but the code in the machine's step continues to try and access the machine's now unallocated space with its original pointer.
This can be observed by running valgrind with scummvm on Linux. Otherwise crashes may occur in Linux and mingw-w64 release mode builds.
An example case is when Wilbur runs on the hamster wheel on the bottom level of the gerbil case (room602).
The proposed solution here is a workaround, based on valgrind's output, and probably does a poor job of cleaning "machine" memory that's no longer used.
More knowledge on the engine's workings is required for a proper and more efficient fix.
The workaround skips full deallocation of a machine object (in the shutdownMachine()), until the ws_KillMachines() is called. The machID field value of 0xdeaddead is used to skip the machine (which remains in the chain). A more efficient fix could do a full deallocation of a machine object earlier (and remove it from the chain like the previous code did), thus avoiding cluttering of the machine's chain.
Changed paths:
engines/m4/wscript/ws_machine.cpp
diff --git a/engines/m4/wscript/ws_machine.cpp b/engines/m4/wscript/ws_machine.cpp
index e3dc82ca777..52ab1b2e649 100644
--- a/engines/m4/wscript/ws_machine.cpp
+++ b/engines/m4/wscript/ws_machine.cpp
@@ -179,7 +179,6 @@ static void op_ON_END_SEQ(machine *m, int32 *pcOffset) {
if (!_GWS(myArg1)) {
ws_Error(m, ERR_MACH, 0x0260, "on_seq_end() failed.");
}
-
ws_OnEndSeqRequest(m->myAnim8, *pcOffset, *_GWS(myArg1) >> 14);
*pcOffset += (int32)*_GWS(myArg1) >> 14;
}
@@ -456,7 +455,6 @@ static bool op_REPLY_MSG(machine *m, int32 *pcOffset) {
}
static bool op_SYSTEM_MSG(machine *m, int32 *pcOffset) {
-
if (!_GWS(myArg1)) {
ws_Error(m, ERR_MACH, 0x0263, "functionality: send to 'C' callback function with msg arg1");
}
@@ -684,6 +682,10 @@ void ws_RefreshWoodscriptBuffer(Buffer *cleanBackground, int16 *depth_table,
static void cancelAllEngineReqs(machine *m) {
globalMsgReq *myGMsg, *tempGMsg;
+ if (m->machID == 0xdeaddead) {
+ return;
+ }
+
//---- CANCEL CRUNCHER REQS
if (m->myAnim8) {
ws_CancelOnEndSeq(m->myAnim8);
@@ -713,6 +715,10 @@ static void cancelAllEngineReqs(machine *m) {
static void shutdownMachine(machine *m) {
+ if (m->machID == 0xdeaddead) {
+ return;
+ }
+
dbg_RemoveWSMach(m);
if (m->myAnim8) {
@@ -731,26 +737,26 @@ static void shutdownMachine(machine *m) {
// Clear any existing walk path
DisposePath(m->walkPath);
- // If there is no previous machine, the next machine becomes the first one
- if (m->prev) {
- m->prev->next = m->next;
- } else {
- _GWS(firstMachine) = m->next;
- }
-
- if (m->next) {
- m->next->prev = m->prev;
- }
-
m->machID = 0xdeaddead;
if (m->machName) {
+ m->machName[0] = '\0';
mem_free((void *)m->machName);
+ m->machName = nullptr;
}
-
- mem_free((void *)m);
}
+static machine *getValidNext(machine *currMachine) {
+ machine *iterMachine = currMachine;
+ if (iterMachine) {
+ while ((iterMachine = iterMachine->next) != nullptr) {
+ if (iterMachine->machID != 0xdeaddead) {
+ return iterMachine;
+ }
+ }
+ }
+ return nullptr;
+}
void terminateMachinesByHash(uint32 machHash) {
machine *curr, *next;
@@ -795,7 +801,7 @@ bool verifyMachineExists(machine *m) {
// Loop through the active machine list, looking for m
tempM = _GWS(firstMachine);
while (tempM && (tempM != m)) {
- tempM = tempM->next;
+ tempM = getValidNext(tempM);
}
// If the end of the list was reached, and m was not found, false
@@ -815,10 +821,13 @@ int32 ws_KillMachines() {
// Deallocate all machines
myMachine = _GWS(firstMachine);
while (myMachine) {
+ // get any next Machine here, not validNext
_GWS(firstMachine) = _GWS(firstMachine)->next;
- clear_msg_list(myMachine);
- clear_persistent_msg_list(myMachine);
+ if (myMachine->machID != 0xdeaddead) {
+ cancelAllEngineReqs(myMachine);
+ shutdownMachine(myMachine);
+ }
mem_free((void *)myMachine);
myBytes += sizeof(machine);
@@ -1221,7 +1230,7 @@ void sendWSMessage(uint32 msgHash, frac16 msgValue, machine *recvM,
while (currMachine && more_to_send) {
// Set nextXM up in case this machine is deleted during the ws_StepWhile
// nextXM will be maintained by ShutDownMachine()
- _GWS(nextXM) = currMachine->next;
+ _GWS(nextXM) = getValidNext(currMachine);
// Have we got a machine of the specified hash
if (currMachine->myHash == _GWS(myGlobalMessages)->machHash) {
Commit: 41d87f2178dc72c3d221b40320310f6a70cf9159
https://github.com/scummvm/scummvm/commit/41d87f2178dc72c3d221b40320310f6a70cf9159
Author: Paul Gilbert (dreammaster at scummvm.org)
Date: 2024-02-24T12:05:22-08:00
Commit Message:
M4: Cleanup of dead machine code, deleting of dead machines between scenes
Changed paths:
engines/m4/core/rooms.cpp
engines/m4/wscript/ws_machine.cpp
engines/m4/wscript/ws_machine.h
diff --git a/engines/m4/core/rooms.cpp b/engines/m4/core/rooms.cpp
index 57259187880..0b58db3a8e5 100644
--- a/engines/m4/core/rooms.cpp
+++ b/engines/m4/core/rooms.cpp
@@ -213,6 +213,8 @@ void Sections::m4EndScene() {
conv_unload(conv_get_handle());
+ ws_KillDeadMachines();
+
//-------------------- DUMP ASSETS AND MINI-ENGINES ------------------
// Note machines should always be cleared before anything else
ClearWSAssets(_WS_ASSET_MACH, 0, 255);
diff --git a/engines/m4/wscript/ws_machine.cpp b/engines/m4/wscript/ws_machine.cpp
index 52ab1b2e649..581eeebebd4 100644
--- a/engines/m4/wscript/ws_machine.cpp
+++ b/engines/m4/wscript/ws_machine.cpp
@@ -682,7 +682,7 @@ void ws_RefreshWoodscriptBuffer(Buffer *cleanBackground, int16 *depth_table,
static void cancelAllEngineReqs(machine *m) {
globalMsgReq *myGMsg, *tempGMsg;
- if (m->machID == 0xdeaddead) {
+ if (m->machID == DEAD_MACHINE_ID) {
return;
}
@@ -715,7 +715,7 @@ static void cancelAllEngineReqs(machine *m) {
static void shutdownMachine(machine *m) {
- if (m->machID == 0xdeaddead) {
+ if (m->machID == DEAD_MACHINE_ID) {
return;
}
@@ -737,7 +737,7 @@ static void shutdownMachine(machine *m) {
// Clear any existing walk path
DisposePath(m->walkPath);
- m->machID = 0xdeaddead;
+ m->machID = DEAD_MACHINE_ID;
if (m->machName) {
m->machName[0] = '\0';
@@ -750,7 +750,7 @@ static machine *getValidNext(machine *currMachine) {
machine *iterMachine = currMachine;
if (iterMachine) {
while ((iterMachine = iterMachine->next) != nullptr) {
- if (iterMachine->machID != 0xdeaddead) {
+ if (iterMachine->machID != DEAD_MACHINE_ID) {
return iterMachine;
}
}
@@ -824,7 +824,7 @@ int32 ws_KillMachines() {
// get any next Machine here, not validNext
_GWS(firstMachine) = _GWS(firstMachine)->next;
- if (myMachine->machID != 0xdeaddead) {
+ if (myMachine->machID != DEAD_MACHINE_ID) {
cancelAllEngineReqs(myMachine);
shutdownMachine(myMachine);
}
@@ -845,6 +845,23 @@ int32 ws_KillMachines() {
return myBytes;
}
+void ws_KillDeadMachines() {
+ machine *myMachine;
+ machine **priorNext = &_GWS(firstMachine);
+
+ // Deallocate all machines that are dead
+ while ((myMachine = *priorNext) != nullptr) {
+ if (myMachine->machID == DEAD_MACHINE_ID) {
+ // Shutdown the dead machine, and unlink it from the machine chain
+ *priorNext = myMachine->next;
+ mem_free(myMachine);
+
+ } else {
+ // Valid machine, skip over
+ priorNext = &myMachine->next;
+ }
+ }
+}
// This is the proc designed to evaluate the instructions of the state machine
@@ -981,7 +998,7 @@ machine *TriggerMachineByHash(int32 myHash, Anim8 *parentAnim8, int32 dataHash,
// Initialize the identification fields
_GWS(machineIDCount)++;
- if (_GWS(machineIDCount) == 0xdeaddead) {
+ if (_GWS(machineIDCount) == DEAD_MACHINE_ID) {
_GWS(machineIDCount)++;
}
diff --git a/engines/m4/wscript/ws_machine.h b/engines/m4/wscript/ws_machine.h
index 1625ad6ce7d..fa3af060785 100644
--- a/engines/m4/wscript/ws_machine.h
+++ b/engines/m4/wscript/ws_machine.h
@@ -29,6 +29,8 @@
namespace M4 {
+#define DEAD_MACHINE_ID 0xdeaddead
+
enum {
NOSEPICK = 0,
STARTWALK = 1,
@@ -193,6 +195,7 @@ void terminateMachinesByHash(uint32 machHash);
void terminateMachineAndNull(machine *&m);
bool verifyMachineExists(machine *m);
int32 ws_KillMachines();
+void ws_KillDeadMachines();
void ws_StepWhile(machine *m, int32 pcOffset, int32 pcCount);
void IntoTheState(machine *m);
machine *TriggerMachineByHash(int32 myHash, Anim8 *parentAnim8, int32 dataHash, int32 dataRow, MessageCB CintrMsg, bool debug, const char *machName);
Commit: f36f3887916e90a559954180acd48f2839eaf544
https://github.com/scummvm/scummvm/commit/f36f3887916e90a559954180acd48f2839eaf544
Author: Paul Gilbert (dreammaster at scummvm.org)
Date: 2024-02-24T12:05:22-08:00
Commit Message:
M4: Added console command to set boonsville time for triggering events
Changed paths:
engines/m4/burger/console.cpp
engines/m4/burger/console.h
diff --git a/engines/m4/burger/console.cpp b/engines/m4/burger/console.cpp
index 0bc749c75f9..2414946a794 100644
--- a/engines/m4/burger/console.cpp
+++ b/engines/m4/burger/console.cpp
@@ -29,6 +29,7 @@ namespace Burger {
Console::Console() : M4::Console() {
registerCmd("global", WRAP_METHOD(Console, cmdGlobal));
registerCmd("test", WRAP_METHOD(Console, cmdTest));
+ registerCmd("time", WRAP_METHOD(Console, cmdTime));
}
bool Console::cmdGlobal(int argc, const char **argv) {
@@ -58,5 +59,17 @@ bool Console::cmdTest(int argc, const char **argv) {
return true;
}
+bool Console::cmdTime(int argc, const char **argv) {
+ if (argc == 2) {
+ int newTime = atol(argv[1]);
+ _G(flags).set_boonsville_time(newTime - 1);
+ return false;
+
+ } else {
+ debugPrintf("Current time is %d\n", _G(flags)[kBoonsvilleTime]);
+ return true;
+ }
+}
+
} // End of namespace Burger
} // End of namespace M4
diff --git a/engines/m4/burger/console.h b/engines/m4/burger/console.h
index 69a8c9aecc3..f5b99893fdf 100644
--- a/engines/m4/burger/console.h
+++ b/engines/m4/burger/console.h
@@ -32,6 +32,7 @@ class Console : public M4::Console {
private:
bool cmdTest(int argc, const char **argv);
bool cmdGlobal(int argc, const char **argv);
+ bool cmdTime(int argc, const char **argv);
public:
Console();
More information about the Scummvm-git-logs
mailing list