From 45012dbd6ac4247285d207e073cdade7370d40ea Mon Sep 17 00:00:00 2001 From: toaster Date: Mon, 17 Jul 2023 13:51:48 +0100 Subject: [PATCH 1/2] Make sure MT_LOOPCENTERPOINT doesn't have an invalid stack-allocated spawnpoint pointer Fixes connecting to a server that's currently on a map with a loop. Also tidies P_SpawnItemRow, P_SpawnItemCircle to reduce the likelihood of this happening again, and possible crash with Lua-shortcircuited loop spawning --- src/p_mobj.c | 28 ++++++++++++++++++---------- 1 file changed, 18 insertions(+), 10 deletions(-) diff --git a/src/p_mobj.c b/src/p_mobj.c index ae19e7844..d927090fb 100644 --- a/src/p_mobj.c +++ b/src/p_mobj.c @@ -13788,8 +13788,15 @@ static void P_SpawnItemRow(mapthing_t *mthing, mobjtype_t *itemtypes, UINT8 numi y + FixedMul(length, FINESINE(fineangle)), z, MT_LOOPCENTERPOINT); - if (!P_MobjWasRemoved(loopanchor)) - Obj_LinkLoopAnchor(loopanchor, loopcenter, mthing->args[0]); + if (P_MobjWasRemoved(loopanchor)) + { + // No recovery. + return; + } + + loopanchor->spawnpoint = NULL; + + Obj_LinkLoopAnchor(loopanchor, loopcenter, mthing->args[0]); } for (r = 0; r < numitems; r++) @@ -13809,15 +13816,15 @@ static void P_SpawnItemRow(mapthing_t *mthing, mobjtype_t *itemtypes, UINT8 numi if (!inclusive) mobj = P_SpawnMobjFromMapThing(&dummything, x, y, z, itemtype); - if (!mobj) + if (P_MobjWasRemoved(mobj)) continue; - if (isloopend) - { - Obj_InitLoopEndpoint(mobj, loopanchor); - } - mobj->spawnpoint = NULL; + + if (!isloopend) + continue; + + Obj_InitLoopEndpoint(mobj, loopanchor); } } @@ -13875,11 +13882,12 @@ static void P_SpawnItemCircle(mapthing_t *mthing, mobjtype_t *itemtypes, UINT8 n mobj = P_SpawnMobjFromMapThing(&dummything, x + v[0], y + v[1], z + v[2], itemtype); - if (!mobj) + if (P_MobjWasRemoved(mobj)) continue; - mobj->z -= mobj->height/2; mobj->spawnpoint = NULL; + + mobj->z -= mobj->height/2; } } From f8922c83c893a1da3583caec8023104fab5be77b Mon Sep 17 00:00:00 2001 From: toaster Date: Mon, 17 Jul 2023 13:53:13 +0100 Subject: [PATCH 2/2] MD2_TID: Don't pass a macro directly to P_SetThingTID This isn't currently the cause of any issue, but found this future footgun while researching the previous commit. Sometimes functions are turned into macros, which could potentially result in multiple save->p digestions if this was eventually turned into one. --- src/p_saveg.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/p_saveg.c b/src/p_saveg.c index 885b7b5c0..655e915ff 100644 --- a/src/p_saveg.c +++ b/src/p_saveg.c @@ -4074,7 +4074,10 @@ static thinker_t* LoadMobjThinker(savebuffer_t *save, actionf_p1 thinker) if (diff2 & MD2_RENDERFLAGS) mobj->renderflags = READUINT32(save->p); if (diff2 & MD2_TID) - P_SetThingTID(mobj, READINT16(save->p)); + { + INT16 tid = READINT16(save->p); + P_SetThingTID(mobj, tid); + } if (diff2 & MD2_SPRITESCALE) { mobj->spritexscale = READFIXED(save->p);