From a2bcee60c0f78fb0c219aca9cb9ab2841e352ffa Mon Sep 17 00:00:00 2001 From: AJ Martinez Date: Fri, 2 Feb 2024 19:18:50 -0700 Subject: [PATCH 01/23] Validate PvPTouchDamage (crash fix) --- src/k_collide.cpp | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/k_collide.cpp b/src/k_collide.cpp index 5969e179e..c7d41237f 100644 --- a/src/k_collide.cpp +++ b/src/k_collide.cpp @@ -1042,6 +1042,10 @@ boolean K_PvPTouchDamage(mobj_t *t1, mobj_t *t2) return false; } + // What the fuck is calling this with stale refs? Whatever, validation's cheap. + if (P_MobjWasRemoved(t1) || P_MobjWasRemoved(t2) || !t1->player || !t2->player) + return false; + // Clash instead of damage if both parties have any of these conditions auto canClash = [](mobj_t *t1, mobj_t *t2) { From 924d46d1024b89247431a002942d9e39c4f93f27 Mon Sep 17 00:00:00 2001 From: James R Date: Fri, 2 Feb 2024 22:03:41 -0800 Subject: [PATCH 02/23] K_DrawDraftCombiring: clamp inputs --- src/k_kart.c | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/src/k_kart.c b/src/k_kart.c index 6740e07e0..858762384 100644 --- a/src/k_kart.c +++ b/src/k_kart.c @@ -1255,7 +1255,15 @@ static void K_DrawDraftCombiring(player_t *player, mobj_t *victim, fixed_t curdi } else { - c = FixedMul((CHAOTIXBANDCOLORS - 1)<> FRACBITS; + fixed_t num = curdist - minimumdist; + fixed_t den = maxdist - minimumdist; + if (den < 1) + den = 1; + if (num < 0) + num = 0; + if (num > den) + num = den; + c = FixedMul((CHAOTIXBANDCOLORS - 1)<> FRACBITS; } stepx = (victim->x - player->mo->x) / CHAOTIXBANDLEN; From 93ff380730755bdad704d702465e47e59aec0e3c Mon Sep 17 00:00:00 2001 From: James R Date: Sat, 3 Feb 2024 00:53:06 -0800 Subject: [PATCH 03/23] Clear g_dc each frame I was running into a memcpy of overlapping memory regions in R_RenderMaskedSegRange. - This is because of the reallocation of lightlists, which uses Z_Frame_Alloc. - The memory pool that Z_Frame_Alloc draws from is cleared each frame. - g_dc was not cleared though, so when the lightlists were reallocated, it'd try to copy from invalidated pointers. - Access to invalid pointers within the memory pool does not cause a segfault directly (because the memory pool is allocated once). However, a memcpy involving such an invalid pointer leads to overlap, which may cause memory corruption. --- src/d_main.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/src/d_main.cpp b/src/d_main.cpp index 4bd5caee2..7dee5488f 100644 --- a/src/d_main.cpp +++ b/src/d_main.cpp @@ -856,6 +856,7 @@ void D_SRB2Loop(void) precise_t enterprecise = I_GetPreciseTime(); precise_t finishprecise = enterprecise; + g_dc = {}; Z_Frame_Reset(); { From efd01708565a78c6fe51c31e9f3147ea18260719 Mon Sep 17 00:00:00 2001 From: James R Date: Sat, 3 Feb 2024 00:58:27 -0800 Subject: [PATCH 04/23] R_DrawMaskedColumn: set texheight to column length - In R_DrawColumnTemplate, texheight is used to switch between a non-PO2 rasterizer and a more efficient PO2 rasterizer. - There is bounds checking on the non-PO2 version (in the form of sourcelength) but not on the PO2 version. - texheight was set to the sprite patch height, which may be taller than the column (sourcelength), leading to a read out of bounds. --- src/r_things.cpp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/r_things.cpp b/src/r_things.cpp index 3c4385f29..239ab99a8 100644 --- a/src/r_things.cpp +++ b/src/r_things.cpp @@ -689,6 +689,7 @@ void R_DrawMaskedColumn(drawcolumndata_t* dc, column_t *column, column_t *bright { dc->source = (UINT8 *)column + 3; dc->sourcelength = column->length; + dc->texheight = column->length; if (brightmap != NULL) { dc->brightmap = (UINT8 *)brightmap + 3; @@ -775,6 +776,7 @@ void R_DrawFlippedMaskedColumn(drawcolumndata_t* dc, column_t *column, column_t { dc->source = static_cast(ZZ_Alloc(column->length)); dc->sourcelength = column->length; + dc->texheight = column->length; for (s = (UINT8 *)column+2+column->length, d = dc->source; d < dc->source+column->length; --s) *d++ = *s; From 057001a66f24baa682ab367433322ff2cca5c0bd Mon Sep 17 00:00:00 2001 From: James R Date: Sat, 3 Feb 2024 01:47:08 -0800 Subject: [PATCH 05/23] DirectorInfo::update_positions: fix player index out of bounds if all players are spectating --- src/k_director.cpp | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/k_director.cpp b/src/k_director.cpp index 308b30bca..b50d4a11b 100644 --- a/src/k_director.cpp +++ b/src/k_director.cpp @@ -212,6 +212,12 @@ private: } } + if (playerstat[0].sorted == -1) + { + maxdist = -1; + return; + } + maxdist = P_ScaleFromMap(players[playerstat[0].sorted].distancetofinish, FRACUNIT); } From 9bb6031a4e90b5e28aa9a625b562b97166fca846 Mon Sep 17 00:00:00 2001 From: James R Date: Sat, 3 Feb 2024 18:38:51 -0800 Subject: [PATCH 06/23] R_StoreWallRange: do not render textures with holes - Holey textures are ones where the column height may not match the texture height. - R_DrawColumn cannot cope with this directly and it may lead to a read out bounds. - Transparency would not render for true wall textures anyway since these are not masked midtextures, so just don't render the texture in this case. --- src/r_segs.cpp | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/src/r_segs.cpp b/src/r_segs.cpp index ac98b78e1..b3fb1db0f 100644 --- a/src/r_segs.cpp +++ b/src/r_segs.cpp @@ -2049,11 +2049,17 @@ void R_StoreWallRange(INT32 start, INT32 stop) ceilingbackslide = FixedMul(backsector->c_slope->zdelta, FINECOSINE((lineangle-backsector->c_slope->xydirection)>>ANGLETOFINESHIFT)); } + auto get_flat_tex = [](INT32 texnum) + { + texnum = R_GetTextureNum(texnum); + return textures[texnum]->holes ? 0 : texnum; // R_DrawWallColumn cannot render holey textures + }; + if (!backsector) { fixed_t texheight; // single sided line - midtexture = R_GetTextureNum(sidedef->midtexture); + midtexture = get_flat_tex(sidedef->midtexture); midbrightmapped = R_TextureHasBrightmap(midtexture); midremap = wantremap && R_TextureCanRemap(sidedef->midtexture); texheight = textureheight[midtexture]; @@ -2246,7 +2252,7 @@ void R_StoreWallRange(INT32 start, INT32 stop) { fixed_t texheight; // top texture - toptexture = R_GetTextureNum(sidedef->toptexture); + toptexture = get_flat_tex(sidedef->toptexture); topbrightmapped = R_TextureHasBrightmap(toptexture); topremap = wantremap && R_TextureCanRemap(sidedef->toptexture); texheight = textureheight[toptexture]; @@ -2276,7 +2282,7 @@ void R_StoreWallRange(INT32 start, INT32 stop) && (worldlow > worldbottom || worldlowslope > worldbottomslope)) // Only if VISIBLE!!! { // bottom texture - bottomtexture = R_GetTextureNum(sidedef->bottomtexture); + bottomtexture = get_flat_tex(sidedef->bottomtexture); bottombrightmapped = R_TextureHasBrightmap(bottomtexture); bottomremap = wantremap && R_TextureCanRemap(sidedef->bottomtexture); From d2a22a69608484735b3b436e92f868fef28fa5a4 Mon Sep 17 00:00:00 2001 From: James R Date: Sun, 4 Feb 2024 00:59:54 -0800 Subject: [PATCH 07/23] R_DrawMaskedColumn: do not draw zero length column - Column would be zero length if there are no visible pixels in it. - Trying to draw such a column results in a negative heightmask in R_DrawColumnTemplate and a probable read out of bounds. --- src/r_things.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/r_things.cpp b/src/r_things.cpp index 239ab99a8..a9a13182f 100644 --- a/src/r_things.cpp +++ b/src/r_things.cpp @@ -685,7 +685,7 @@ void R_DrawMaskedColumn(drawcolumndata_t* dc, column_t *column, column_t *bright if (dc->yh >= baseclip && baseclip != -1) dc->yh = baseclip; - if (dc->yl <= dc->yh && dc->yh > 0) + if (dc->yl <= dc->yh && dc->yh > 0 && column->length != 0) { dc->source = (UINT8 *)column + 3; dc->sourcelength = column->length; @@ -772,7 +772,7 @@ void R_DrawFlippedMaskedColumn(drawcolumndata_t* dc, column_t *column, column_t if (dc->yh >= vid.height) // dc_yl must be < vid.height, so reduces number of checks in tight loop dc->yh = vid.height - 1; - if (dc->yl <= dc->yh && dc->yh > 0) + if (dc->yl <= dc->yh && dc->yh > 0 && column->length != 0) { dc->source = static_cast(ZZ_Alloc(column->length)); dc->sourcelength = column->length; From f9b66ad96970adf971e331ff34bf70980c195e82 Mon Sep 17 00:00:00 2001 From: James R Date: Sun, 4 Feb 2024 01:28:21 -0800 Subject: [PATCH 08/23] R_RenderMaskedSegLoop, R_RenderThickSideRange: set dc.colormap above first FOF How lightlists work: - Each FOF casts a shadow beneath it. - Draw the column above each FOF with colormap set from the previous FOF. - Then set colormap from the current FOF, so the next FOF is bathed in the current FOF's shadow. What broke: - Colormap was not set when drawing the column above the first FOF. This commit: - Before iterating lightlists, set colormap to base sector lighting. - De-duplicate some code by using lambdas. --- src/r_segs.cpp | 152 ++++++++++++++++++++++++++----------------------- 1 file changed, 82 insertions(+), 70 deletions(-) diff --git a/src/r_segs.cpp b/src/r_segs.cpp index b3fb1db0f..f27875f6f 100644 --- a/src/r_segs.cpp +++ b/src/r_segs.cpp @@ -396,13 +396,10 @@ static void R_RenderMaskedSegLoop(drawcolumndata_t* dc, drawseg_t *drawseg, INT3 bmCol = (column_t *)((UINT8 *)R_GetBrightmapColumn(texnum, maskedtexturecol[dc->x]) - 3); } - for (i = 0; i < dc->numlights; i++) + auto set_light_vars = [&](INT32 i) { rlight = &dc->lightlist[i]; - if ((rlight->flags & FOF_NOSHADE)) - continue; - lightnum = R_AdjustLightLevel(rlight->lightnum); if (lightnum < 0) @@ -421,20 +418,38 @@ static void R_RenderMaskedSegLoop(drawcolumndata_t* dc, drawseg_t *drawseg, INT3 rlight->rcolormap = rlight->extra_colormap->colormap + (xwalllights[pindex] - colormaps); else rlight->rcolormap = xwalllights[pindex]; + }; + + auto set_colormap_below_light = [&] + { + dc->colormap = rlight->rcolormap; + dc->lightmap = xwalllights[pindex]; + dc->fullbright = colormaps; + if (remap && !(ldef->flags & ML_TFERLINE)) + { + dc->colormap += COLORMAP_REMAPOFFSET; + dc->fullbright += COLORMAP_REMAPOFFSET; + } + }; + + // Use the base sector's light level above the first FOF. + // You can imagine it as the sky casting its light on top of the highest FOF. + set_light_vars(0); + set_colormap_below_light(); + + for (i = 0; i < dc->numlights; i++) + { + if ((dc->lightlist[i].flags & FOF_NOSHADE)) + continue; + + set_light_vars(i); height = rlight->height; rlight->height += rlight->heightstep; if (height <= windowtop) { - dc->colormap = rlight->rcolormap; - dc->lightmap = xwalllights[pindex]; - dc->fullbright = colormaps; - if (remap && !(ldef->flags & ML_TFERLINE)) - { - dc->colormap += COLORMAP_REMAPOFFSET; - dc->fullbright += COLORMAP_REMAPOFFSET; - } + set_colormap_below_light(); continue; } @@ -453,14 +468,7 @@ static void R_RenderMaskedSegLoop(drawcolumndata_t* dc, drawseg_t *drawseg, INT3 } colfunc_2s(dc, col, bmCol, -1); windowtop = windowbottom + 1; - dc->colormap = rlight->rcolormap; - dc->lightmap = xwalllights[pindex]; - dc->fullbright = colormaps; - if (remap && !(ldef->flags & ML_TFERLINE)) - { - dc->colormap += COLORMAP_REMAPOFFSET; - dc->fullbright += COLORMAP_REMAPOFFSET; - } + set_colormap_below_light(); } windowbottom = realbot; if (windowtop < windowbottom) @@ -1158,42 +1166,64 @@ void R_RenderThickSideRange(drawseg_t *ds, INT32 x1, INT32 x2, ffloor_t *pfloor) fixed_t bheight = 0; INT32 solid = 0; + auto set_light_vars = [&](INT32 i) + { + rlight = &dc->lightlist[i]; + + lightnum = R_AdjustLightLevel(rlight->lightnum); + + if (lightnum < 0) + xwalllights = scalelight[0]; + else if (lightnum >= LIGHTLEVELS) + xwalllights = scalelight[LIGHTLEVELS-1]; + else + xwalllights = scalelight[lightnum]; + + pindex = FixedMul(spryscale, LIGHTRESOLUTIONFIX)>>LIGHTSCALESHIFT; + + if (pindex >= MAXLIGHTSCALE) + pindex = MAXLIGHTSCALE-1; + + if (pfloor->fofflags & FOF_FOG) + { + if (pfloor->master->frontsector->extra_colormap) + rlight->rcolormap = pfloor->master->frontsector->extra_colormap->colormap + (xwalllights[pindex] - colormaps); + else + rlight->rcolormap = xwalllights[pindex]; + } + else + { + if (rlight->extra_colormap) + rlight->rcolormap = rlight->extra_colormap->colormap + (xwalllights[pindex] - colormaps); + else + rlight->rcolormap = xwalllights[pindex]; + } + }; + + auto set_colormap_below_light = [&] + { + dc->colormap = rlight->rcolormap; + dc->lightmap = xwalllights[pindex]; + dc->fullbright = colormaps; + if (remap && !(curline->linedef->flags & ML_TFERLINE)) + { + dc->colormap += COLORMAP_REMAPOFFSET; + dc->fullbright += COLORMAP_REMAPOFFSET; + } + }; + + // Use the base sector's light level above the first FOF. + // You can imagine it as the sky casting its light on top of the highest FOF. + set_light_vars(0); + set_colormap_below_light(); + for (i = 0; i < dc->numlights; i++) { // Check if the current light effects the colormap/lightlevel rlight = &dc->lightlist[i]; xwalllights = NULL; if (!(dc->lightlist[i].flags & FOF_NOSHADE)) - { - lightnum = R_AdjustLightLevel(rlight->lightnum); - - if (lightnum < 0) - xwalllights = scalelight[0]; - else if (lightnum >= LIGHTLEVELS) - xwalllights = scalelight[LIGHTLEVELS-1]; - else - xwalllights = scalelight[lightnum]; - - pindex = FixedMul(spryscale, LIGHTRESOLUTIONFIX)>>LIGHTSCALESHIFT; - - if (pindex >= MAXLIGHTSCALE) - pindex = MAXLIGHTSCALE-1; - - if (pfloor->fofflags & FOF_FOG) - { - if (pfloor->master->frontsector->extra_colormap) - rlight->rcolormap = pfloor->master->frontsector->extra_colormap->colormap + (xwalllights[pindex] - colormaps); - else - rlight->rcolormap = xwalllights[pindex]; - } - else - { - if (rlight->extra_colormap) - rlight->rcolormap = rlight->extra_colormap->colormap + (xwalllights[pindex] - colormaps); - else - rlight->rcolormap = xwalllights[pindex]; - } - } + set_light_vars(i); solid = 0; // don't carry over solid-cutting flag from the previous light @@ -1227,16 +1257,7 @@ void R_RenderThickSideRange(drawseg_t *ds, INT32 x1, INT32 x2, ffloor_t *pfloor) if (height <= windowtop) { if (xwalllights) - { - dc->colormap = rlight->rcolormap; - dc->lightmap = xwalllights[pindex]; - dc->fullbright = colormaps; - if (remap && !(curline->linedef->flags & ML_TFERLINE)) - { - dc->colormap += COLORMAP_REMAPOFFSET; - dc->fullbright += COLORMAP_REMAPOFFSET; - } - } + set_colormap_below_light(); if (solid && windowtop < bheight) windowtop = bheight; continue; @@ -1264,16 +1285,7 @@ void R_RenderThickSideRange(drawseg_t *ds, INT32 x1, INT32 x2, ffloor_t *pfloor) else windowtop = windowbottom + 1; if (xwalllights) - { - dc->colormap = rlight->rcolormap; - dc->lightmap = xwalllights[pindex]; - dc->fullbright = colormaps; - if (remap && !(curline->linedef->flags & ML_TFERLINE)) - { - dc->colormap += COLORMAP_REMAPOFFSET; - dc->fullbright += COLORMAP_REMAPOFFSET; - } - } + set_colormap_below_light(); } windowbottom = sprbotscreen; // draw the texture, if there is any space left From 4a7d2504b0666e4d78936d347eeb794454175897 Mon Sep 17 00:00:00 2001 From: James R Date: Sun, 4 Feb 2024 04:51:45 -0800 Subject: [PATCH 09/23] srb2::hwr2::PaletteManager: disable lighttables upload - This code uploads encoremap to the GPU as a texture. - It assumes encoremap is 256 * 32 bytes, but in reality encoremap is only 256 bytes. - The textures go completely unused, so I simply commented out the code altogether. --- src/hwr2/resource_management.cpp | 6 ++++++ src/hwr2/resource_management.hpp | 4 ++++ 2 files changed, 10 insertions(+) diff --git a/src/hwr2/resource_management.cpp b/src/hwr2/resource_management.cpp index b16c366d0..7d1fb37e2 100644 --- a/src/hwr2/resource_management.cpp +++ b/src/hwr2/resource_management.cpp @@ -32,6 +32,7 @@ void PaletteManager::update(Rhi& rhi, Handle ctx) palette_ = rhi.create_texture({TextureFormat::kRGBA, kPaletteSize, 1, TextureWrapMode::kClamp, TextureWrapMode::kClamp}); } +#if 0 if (!lighttable_) { lighttable_ = rhi.create_texture({TextureFormat::kLuminance, kPaletteSize, kLighttableRows, TextureWrapMode::kClamp, TextureWrapMode::kClamp}); @@ -41,6 +42,7 @@ void PaletteManager::update(Rhi& rhi, Handle ctx) { encore_lighttable_ = rhi.create_texture({TextureFormat::kLuminance, kPaletteSize, kLighttableRows, TextureWrapMode::kClamp, TextureWrapMode::kClamp}); } +#endif if (!default_colormap_) { @@ -57,6 +59,7 @@ void PaletteManager::update(Rhi& rhi, Handle ctx) rhi.update_texture(ctx, palette_, {0, 0, kPaletteSize, 1}, PixelFormat::kRGBA8, tcb::as_bytes(tcb::span(palette_32))); } +#if 0 // Lighttables { if (colormaps != nullptr) @@ -65,12 +68,15 @@ void PaletteManager::update(Rhi& rhi, Handle ctx) rhi.update_texture(ctx, lighttable_, {0, 0, kPaletteSize, kLighttableRows}, PixelFormat::kR8, colormap_bytes); } + // FIXME: This is broken, encoremap should not be used directly. + // Instead, use colormaps + COLORMAP_REMAPOFFSET. See R_ReInitColormaps. if (encoremap != nullptr) { tcb::span encoremap_bytes = tcb::as_bytes(tcb::span(encoremap, kPaletteSize * kLighttableRows)); rhi.update_texture(ctx, encore_lighttable_, {0, 0, kPaletteSize, kLighttableRows}, PixelFormat::kR8, encoremap_bytes); } } +#endif // Default colormap { diff --git a/src/hwr2/resource_management.hpp b/src/hwr2/resource_management.hpp index 8c9adffc0..153b0b1d8 100644 --- a/src/hwr2/resource_management.hpp +++ b/src/hwr2/resource_management.hpp @@ -20,8 +20,10 @@ namespace srb2::hwr2 class PaletteManager { rhi::Handle palette_; +#if 0 rhi::Handle lighttable_; rhi::Handle encore_lighttable_; +#endif rhi::Handle default_colormap_; std::unordered_map> colormaps_; @@ -36,8 +38,10 @@ public: PaletteManager& operator=(PaletteManager&&); rhi::Handle palette() const noexcept { return palette_; } +#if 0 rhi::Handle lighttable() const noexcept { return lighttable_; } rhi::Handle encore_lighttable() const noexcept { return encore_lighttable_; } +#endif rhi::Handle default_colormap() const noexcept { return default_colormap_; } void update(rhi::Rhi& rhi, rhi::Handle ctx); From 6f616265c4230962df655a069cb3ea7661ec0075 Mon Sep 17 00:00:00 2001 From: AJ Martinez Date: Tue, 6 Feb 2024 01:47:10 -0700 Subject: [PATCH 10/23] Don't try to orbit things that aren't there --- src/k_kart.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/k_kart.c b/src/k_kart.c index 858762384..8c4060216 100644 --- a/src/k_kart.c +++ b/src/k_kart.c @@ -7243,7 +7243,7 @@ void K_RepairOrbitChain(mobj_t *orbit) } // Then recount to make sure item amount is correct - if (orbit->target && orbit->target->player) + if (orbit->target && orbit->target->player && !P_MobjWasRemoved(orbit->target)) { INT32 num = 0; From 45f043a1c41a3a84a7b089e90e5f1ef19484b91d Mon Sep 17 00:00:00 2001 From: AJ Martinez Date: Tue, 6 Feb 2024 06:00:59 -0700 Subject: [PATCH 11/23] Don't attempt itemamount correction after orbitals self-destruct --- src/k_kart.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/k_kart.c b/src/k_kart.c index 8c4060216..ffd9cbeef 100644 --- a/src/k_kart.c +++ b/src/k_kart.c @@ -7260,7 +7260,7 @@ void K_RepairOrbitChain(mobj_t *orbit) prev->movedir = num; } - if (orbit->target->player->itemamount != num) + if (!P_MobjWasRemoved(orbit->target) && orbit->target->player->itemamount != num) orbit->target->player->itemamount = num; } } From 4efaed36901e6a9b3674199db667bcaf30583513 Mon Sep 17 00:00:00 2001 From: AJ Martinez Date: Tue, 6 Feb 2024 06:02:17 -0700 Subject: [PATCH 12/23] Fix sound falloff calculation overflow --- src/s_sound.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/s_sound.c b/src/s_sound.c index 93a0ecff9..78d5f618b 100644 --- a/src/s_sound.c +++ b/src/s_sound.c @@ -986,7 +986,8 @@ boolean S_AdjustSoundParams(const mobj_t *listener, const mobj_t *source, INT32 if (sfxinfo->pitch & SF_OUTSIDESOUND) // Rain special case { - fixed_t x, y, yl, yh, xl, xh, newdist; + INT64 x, y, yl, yh, xl, xh; + fixed_t newdist; if (R_PointInSubsector(listensource.x, listensource.y)->sector->ceilingpic == skyflatnum) approx_dist = 0; From 1dad6be6a4fe255f1dff17a45198a8e471b0b043 Mon Sep 17 00:00:00 2001 From: AJ Martinez Date: Wed, 7 Feb 2024 15:05:49 -0700 Subject: [PATCH 13/23] Fix stale target in A_SSMineExplode (crash) --- src/p_enemy.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/p_enemy.c b/src/p_enemy.c index 092a115eb..b92fbed73 100644 --- a/src/p_enemy.c +++ b/src/p_enemy.c @@ -12139,7 +12139,12 @@ void A_SSMineExplode(mobj_t *actor) return; delay = K_MineExplodeAttack(actor, (3*actor->info->painchance)>>1, (boolean)locvar1); - K_SpawnMineExplosion(actor, (actor->target && actor->target->player) ? actor->target->player->skincolor : SKINCOLOR_KETCHUP, delay); + + skincolornum_t color = SKINCOLOR_KETCHUP; + if (!P_MobjWasRemoved(actor->target) && actor->target->player) + color = actor->target->player->skincolor; + + K_SpawnMineExplosion(actor, color, delay); } void A_SSMineFlash(mobj_t *actor) From 6b980cb1c7eb7a4a56436a29b713e3e2caa15f10 Mon Sep 17 00:00:00 2001 From: AJ Martinez Date: Wed, 7 Feb 2024 15:09:26 -0700 Subject: [PATCH 14/23] Fix stale reference in P_KillMobj (crash) --- src/p_inter.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/p_inter.c b/src/p_inter.c index 3f9a80e8b..eefa20233 100644 --- a/src/p_inter.c +++ b/src/p_inter.c @@ -1643,7 +1643,7 @@ void P_KillMobj(mobj_t *target, mobj_t *inflictor, mobj_t *source, UINT8 damaget // SRB2kart // I wish I knew a better way to do this - if (target->target && target->target->player && target->target->player->mo) + if (!P_MobjWasRemoved(target->target) && target->target->player && !P_MobjWasRemoved(target->target->player->mo)) { if ((target->target->player->itemflags & IF_EGGMANOUT) && target->type == MT_EGGMANITEM_SHIELD) target->target->player->itemflags &= ~IF_EGGMANOUT; From 290b97500af4789ef026f51d90de1db042ab36e9 Mon Sep 17 00:00:00 2001 From: AJ Martinez Date: Wed, 7 Feb 2024 20:21:32 -0700 Subject: [PATCH 15/23] Fix crashes when P_DamageMobj was naively passed a removed source --- src/p_inter.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/p_inter.c b/src/p_inter.c index eefa20233..b19e03aa1 100644 --- a/src/p_inter.c +++ b/src/p_inter.c @@ -2698,7 +2698,7 @@ boolean P_DamageMobj(mobj_t *target, mobj_t *inflictor, mobj_t *source, INT32 da if (damagetype != DMG_SPECTATOR && target->player && target->player->spectator) return false; - if (source && source->player && source->player->spectator) + if (!P_MobjWasRemoved(source) && source->player && source->player->spectator) return false; if (((damagetype & DMG_TYPEMASK) == DMG_STING) From af363e575a47d7af64b5d22d7a109cf04e03a9f5 Mon Sep 17 00:00:00 2001 From: James R Date: Wed, 7 Feb 2024 22:09:32 -0800 Subject: [PATCH 16/23] K_UpdateDistanceFromFinishLine: fix memory leak with backwards pathfinding --- src/k_kart.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/k_kart.c b/src/k_kart.c index ffd9cbeef..5d1220867 100644 --- a/src/k_kart.c +++ b/src/k_kart.c @@ -9770,6 +9770,7 @@ void K_UpdateDistanceFromFinishLine(player_t *const player) adddist = (UINT32)disttowaypoint; } */ + Z_Free(pathBackwards.array); } /* else From 5c0bebbeba4e16a18f1a1fa7df82ea0b08aa3480 Mon Sep 17 00:00:00 2001 From: SteelT Date: Thu, 8 Feb 2024 01:25:13 -0500 Subject: [PATCH 17/23] discord.c: Fix removeRequest->discriminator not being freed in DRPC_RemoveRequest --- src/discord.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/discord.c b/src/discord.c index 861069bd4..494eac52e 100644 --- a/src/discord.c +++ b/src/discord.c @@ -301,6 +301,7 @@ void DRPC_RemoveRequest(discordRequest_t *removeRequest) } Z_Free(removeRequest->username); + Z_Free(removeRequest->discriminator); Z_Free(removeRequest->userID); Z_Free(removeRequest); } From cbb7c281c8e5bfb559369d5aabe84781c5ecac28 Mon Sep 17 00:00:00 2001 From: SteelT Date: Thu, 8 Feb 2024 01:26:07 -0500 Subject: [PATCH 18/23] discord.c: Fix joinSecret memory leak --- src/discord.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/discord.c b/src/discord.c index 494eac52e..12d2998cc 100644 --- a/src/discord.c +++ b/src/discord.c @@ -413,6 +413,7 @@ void DRPC_UpdatePresence(void) #endif boolean joinSecretSet = false; + char *clientJoinSecret = NULL; DiscordRichPresence discordPresence; memset(&discordPresence, 0, sizeof(discordPresence)); @@ -448,7 +449,8 @@ void DRPC_UpdatePresence(void) // Grab the host's IP for joining. if ((join = DRPC_GetServerIP()) != NULL) { - discordPresence.joinSecret = DRPC_XORIPString(join); + clientJoinSecret = DRPC_XORIPString(join); + discordPresence.joinSecret = clientJoinSecret; joinSecretSet = true; } else @@ -651,6 +653,7 @@ void DRPC_UpdatePresence(void) } Discord_UpdatePresence(&discordPresence); + free(clientJoinSecret); } #endif // HAVE_DISCORDRPC From b6241adbe9cb60f08f86206ca33c3e9dfdc0721b Mon Sep 17 00:00:00 2001 From: James R Date: Thu, 8 Feb 2024 04:11:19 -0800 Subject: [PATCH 19/23] Use PU_LEVEL for texture caching - Some maps may contain very large or very many (animated) textures - Texture sets are not typically shared between maps, so each texture allocation may go unused for a long time after the map ends - Keeping these PU_STATIC leads to significant memory usage over the program duration --- src/r_textures.c | 20 +++++++++----------- 1 file changed, 9 insertions(+), 11 deletions(-) diff --git a/src/r_textures.c b/src/r_textures.c index dc8d9f12a..971b63ab9 100644 --- a/src/r_textures.c +++ b/src/r_textures.c @@ -255,7 +255,7 @@ static UINT8 *R_AllocateTextureBlock(size_t blocksize, UINT8 **user) { texturememory += blocksize; - return Z_Malloc(blocksize, PU_CACHE, user); + return Z_Malloc(blocksize, PU_LEVEL, user); } static UINT8 *R_AllocateDummyTextureBlock(size_t width, UINT8 **user) @@ -351,7 +351,7 @@ UINT8 *R_GenerateTexture(size_t texnum) return block; } - pdata = W_CacheLumpNumPwad(wadnum, lumpnum, PU_CACHE); + pdata = W_CacheLumpNumPwad(wadnum, lumpnum, PU_LEVEL); realpatch = (softwarepatch_t *)pdata; #ifndef NO_PNG_LUMPS @@ -392,7 +392,7 @@ UINT8 *R_GenerateTexture(size_t texnum) texture->holes = true; texture->flip = patch->flip; blocksize = lumplength; - block = Z_Calloc(blocksize, PU_STATIC, // will change tag at end of this function + block = Z_Calloc(blocksize, PU_LEVEL, // will change tag at end of this function &texturecache[texnum]); M_Memcpy(block, realpatch, blocksize); texturememory += blocksize; @@ -423,7 +423,7 @@ UINT8 *R_GenerateTexture(size_t texnum) texture->flip = 0; blocksize = (texture->width * 4) + (texture->width * texture->height); texturememory += blocksize; - block = Z_Malloc(blocksize+1, PU_STATIC, &texturecache[texnum]); + block = Z_Malloc(blocksize+1, PU_LEVEL, &texturecache[texnum]); memset(block, TRANSPARENTPIXEL, blocksize+1); // Transparency hack @@ -446,7 +446,7 @@ UINT8 *R_GenerateTexture(size_t texnum) wadnum = patch->wad; lumpnum = patch->lump; - pdata = W_CacheLumpNumPwad(wadnum, lumpnum, PU_CACHE); + pdata = W_CacheLumpNumPwad(wadnum, lumpnum, PU_LEVEL); lumplength = W_LumpLengthPwad(wadnum, lumpnum); realpatch = (softwarepatch_t *)pdata; dealloc = true; @@ -515,8 +515,6 @@ UINT8 *R_GenerateTexture(size_t texnum) } done: - // Now that the texture has been built in column cache, it is purgable from zone memory. - Z_ChangeTag(block, PU_CACHE); return blocktex; } @@ -535,7 +533,7 @@ UINT8 *R_GenerateTextureAsFlat(size_t texnum) if (!texture->flat) { // Well, let's do it now, then. - texture->flat = Z_Malloc(size, PU_STATIC, NULL); + Z_Malloc(size, PU_LEVEL, &texture->flat); // Picture_TextureToFlat handles everything for us. converted = (UINT8 *)Picture_TextureToFlat(texnum); @@ -895,7 +893,7 @@ UINT8 *R_GetBrightmapColumn(fixed_t tex, INT32 col) void *R_GetFlat(lumpnum_t flatlumpnum) { - return W_CacheLumpNum(flatlumpnum, PU_CACHE); + return W_CacheLumpNum(flatlumpnum, PU_LEVEL); } // @@ -941,7 +939,7 @@ void *R_GetLevelFlat(drawspandata_t* ds, levelflat_t *levelflat) { INT32 pngwidth, pngheight; - levelflat->picture = Picture_PNGConvert(W_CacheLumpNum(levelflat->u.flat.lumpnum, PU_CACHE), PICFMT_FLAT, &pngwidth, &pngheight, NULL, NULL, W_LumpLength(levelflat->u.flat.lumpnum), NULL, 0); + levelflat->picture = Picture_PNGConvert(W_CacheLumpNum(levelflat->u.flat.lumpnum, PU_LEVEL), PICFMT_FLAT, &pngwidth, &pngheight, NULL, NULL, W_LumpLength(levelflat->u.flat.lumpnum), NULL, 0); levelflat->width = (UINT16)pngwidth; levelflat->height = (UINT16)pngheight; @@ -954,7 +952,7 @@ void *R_GetLevelFlat(drawspandata_t* ds, levelflat_t *levelflat) { UINT8 *converted; size_t size; - softwarepatch_t *patch = W_CacheLumpNum(levelflat->u.flat.lumpnum, PU_CACHE); + softwarepatch_t *patch = W_CacheLumpNum(levelflat->u.flat.lumpnum, PU_LEVEL); levelflat->width = ds->flatwidth = SHORT(patch->width); levelflat->height = ds->flatheight = SHORT(patch->height); From 78a727e9cbad9a479d7d4c74bc1245e2462b049c Mon Sep 17 00:00:00 2001 From: AJ Martinez Date: Thu, 8 Feb 2024 05:18:54 -0700 Subject: [PATCH 20/23] Guard against more unsafe accesses to P_DamageMobj source --- src/p_inter.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/p_inter.c b/src/p_inter.c index b19e03aa1..e31874134 100644 --- a/src/p_inter.c +++ b/src/p_inter.c @@ -2698,7 +2698,11 @@ boolean P_DamageMobj(mobj_t *target, mobj_t *inflictor, mobj_t *source, INT32 da if (damagetype != DMG_SPECTATOR && target->player && target->player->spectator) return false; - if (!P_MobjWasRemoved(source) && source->player && source->player->spectator) + // source is checked without a removal guard in so many places that it's genuinely less work to do it here. + if (source && P_MobjWasRemoved(source)) + source = NULL; + + if (source && source->player && source->player->spectator) return false; if (((damagetype & DMG_TYPEMASK) == DMG_STING) From 56e710266cbbb6d05538bb5509f0d47084455145 Mon Sep 17 00:00:00 2001 From: James R Date: Thu, 8 Feb 2024 18:16:39 -0800 Subject: [PATCH 21/23] M_TokenizerOpen: pass size in as argument instead of calling strlen implicitly This function is used for parsing TEXTMAP and the data is not NUL-terminated. --- src/doomdef.h | 2 +- src/k_rank.cpp | 2 +- src/m_misc.cpp | 4 ++-- src/p_setup.cpp | 2 +- 4 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/doomdef.h b/src/doomdef.h index 21e8bdb4f..14c879506 100644 --- a/src/doomdef.h +++ b/src/doomdef.h @@ -500,7 +500,7 @@ char *M_GetToken(const char *inputString); void M_UnGetToken(void); UINT32 M_GetTokenPos(void); -void M_TokenizerOpen(const char *inputString); +void M_TokenizerOpen(const char *inputString, size_t inputLength); void M_TokenizerClose(void); const char *M_TokenizerRead(UINT32 i); UINT32 M_TokenizerGetEndPos(void); diff --git a/src/k_rank.cpp b/src/k_rank.cpp index 702a55fe8..2c62c39cb 100644 --- a/src/k_rank.cpp +++ b/src/k_rank.cpp @@ -199,7 +199,7 @@ static boolean RankCapsules_LoadMapData(const virtres_t *virt) if (g_rankCapsules_udmf) // Count how many entries for each type we got in textmap. { virtlump_t *textmap = vres_Find(virt, "TEXTMAP"); - M_TokenizerOpen((char *)textmap->data); + M_TokenizerOpen((char *)textmap->data, textmap->size); if (!RankCapsules_TextmapCount(textmap->size)) { M_TokenizerClose(); diff --git a/src/m_misc.cpp b/src/m_misc.cpp index 7c993ca29..bda4b49f2 100644 --- a/src/m_misc.cpp +++ b/src/m_misc.cpp @@ -2276,7 +2276,7 @@ static UINT32 tokenizerInputLength = 0; static UINT8 tokenizerInComment = 0; // 0 = not in comment, 1 = // Single-line, 2 = /* Multi-line */ static boolean tokenizerIsString = false; // did we strip quotes from this token? -void M_TokenizerOpen(const char *inputString) +void M_TokenizerOpen(const char *inputString, size_t inputLength) { size_t i; @@ -2286,7 +2286,7 @@ void M_TokenizerOpen(const char *inputString) tokenCapacity[i] = 1024; tokenizerToken[i] = (char*)Z_Malloc(tokenCapacity[i] * sizeof(char), PU_STATIC, NULL); } - tokenizerInputLength = strlen(tokenizerInput); + tokenizerInputLength = inputLength; } void M_TokenizerClose(void) diff --git a/src/p_setup.cpp b/src/p_setup.cpp index 8ed2d00ca..6a660bb60 100644 --- a/src/p_setup.cpp +++ b/src/p_setup.cpp @@ -3459,7 +3459,7 @@ static boolean P_LoadMapData(const virtres_t *virt) if (udmf) // Count how many entries for each type we got in textmap. { virtlump_t *textmap = vres_Find(virt, "TEXTMAP"); - M_TokenizerOpen((char *)textmap->data); + M_TokenizerOpen((char *)textmap->data, textmap->size); if (!TextmapCount(textmap->size)) { M_TokenizerClose(); From 42146c5ea286f672b187c859222efbdb11546214 Mon Sep 17 00:00:00 2001 From: James R Date: Thu, 8 Feb 2024 18:37:01 -0800 Subject: [PATCH 22/23] R_AddPrecipitationSprites: fix use of th after freed R_ProjectPrecipitationSprite may free th --- src/r_things.cpp | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/r_things.cpp b/src/r_things.cpp index a9a13182f..3c2e9576f 100644 --- a/src/r_things.cpp +++ b/src/r_things.cpp @@ -2843,7 +2843,7 @@ void R_AddPrecipitationSprites(void) const fixed_t drawdist = cv_drawdist_precip.value * mapobjectscale; INT32 xl, xh, yl, yh, bx, by; - precipmobj_t *th; + precipmobj_t *th, *next; // no, no infinite draw distance for precipitation. this option at zero is supposed to turn it off if (drawdist == 0) @@ -2863,8 +2863,11 @@ void R_AddPrecipitationSprites(void) { for (by = yl; by <= yh; by++) { - for (th = precipblocklinks[(by * bmapwidth) + bx]; th; th = th->bnext) + for (th = precipblocklinks[(by * bmapwidth) + bx]; th; th = next) { + // Store this beforehand because R_ProjectPrecipitionSprite may free th (see P_PrecipThinker) + next = th->bnext; + if (R_PrecipThingVisible(th)) { R_ProjectPrecipitationSprite(th); From 1f2e57c060c841a00536229e2e47576a57abc4d3 Mon Sep 17 00:00:00 2001 From: AJ Martinez Date: Fri, 9 Feb 2024 12:31:34 -0700 Subject: [PATCH 23/23] K_RepairOrbitChain: Don't naively call P_MobjWasRemoved on NULL --- src/k_kart.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/k_kart.c b/src/k_kart.c index 5d1220867..aef487e21 100644 --- a/src/k_kart.c +++ b/src/k_kart.c @@ -7260,7 +7260,7 @@ void K_RepairOrbitChain(mobj_t *orbit) prev->movedir = num; } - if (!P_MobjWasRemoved(orbit->target) && orbit->target->player->itemamount != num) + if (orbit->target && !P_MobjWasRemoved(orbit->target) && orbit->target->player->itemamount != num) orbit->target->player->itemamount = num; } }