From 541436b5a8750272755940c7b5c5c3bb469043a8 Mon Sep 17 00:00:00 2001 From: bones_was_here Date: Tue, 7 May 2024 11:23:16 +1000 Subject: [PATCH] PVS: dynamically allocate all FatPVS buffers Some were dynamic, others weren't... Fixes invisible entities and monsters ignoring the player on big/complex maps, and undefined behaviour (reading past the end of the buffer). Fixes https://gitlab.com/xonotic/darkplaces/-/issues/322 Adds Mem_Size() for retrieving the size of an allocation. Simplifies some checks (we can check the pointer now so we dont need to store byte counts just to see if there's any data). Signed-off-by: bones_was_here --- clvm_cmds.c | 7 +++---- gl_rmain.c | 17 ++++------------- gl_rsurf.c | 2 +- model_brush.c | 20 ++++++++++++++------ model_brush.h | 2 +- model_shared.c | 2 +- qdefs.h | 4 ++-- render.h | 4 +--- server.h | 3 +-- snd_main.c | 11 +---------- sv_ents.c | 7 ++++--- sv_send.c | 2 +- svvm_cmds.c | 17 ++++++++--------- zone.h | 7 +++++++ 14 files changed, 49 insertions(+), 56 deletions(-) diff --git a/clvm_cmds.c b/clvm_cmds.c index 99f907d4..5ba1b3e5 100644 --- a/clvm_cmds.c +++ b/clvm_cmds.c @@ -4527,8 +4527,7 @@ static void VM_CL_checkpvs (prvm_prog_t *prog) #if 1 unsigned char *pvs; #else - int fatpvsbytes; - unsigned char fatpvs[MAX_MAP_LEAFS/8]; + unsigned char *fatpvs = NULL; #endif VM_SAFEPARMCOUNT(2, VM_CL_checkpvs); @@ -4568,8 +4567,8 @@ static void VM_CL_checkpvs (prvm_prog_t *prog) PRVM_G_FLOAT(OFS_RETURN) = 3; return; } - fatpvsbytes = cl.worldmodel->brush.FatPVS(cl.worldmodel, viewpos, 8, fatpvs, sizeof(fatpvs), false); - if(!fatpvsbytes) + cl.worldmodel->brush.FatPVS(cl.worldmodel, viewpos, 8, &fatpvs, cls.levelmempool, false); + if(!fatpvs) { // viewpos isn't in any PVS... darn PRVM_G_FLOAT(OFS_RETURN) = 2; diff --git a/gl_rmain.c b/gl_rmain.c index 26c90911..7113caca 100644 --- a/gl_rmain.c +++ b/gl_rmain.c @@ -3011,8 +3011,6 @@ static void R_Main_FreeViewCache(void) static void R_Main_ResizeViewCache(void) { int numentities = r_refdef.scene.numentities; - int numclusters = r_refdef.scene.worldmodel ? r_refdef.scene.worldmodel->brush.num_pvsclusters : 1; - int numclusterbytes = r_refdef.scene.worldmodel ? r_refdef.scene.worldmodel->brush.num_pvsclusterbytes : 1; int numleafs = r_refdef.scene.worldmodel ? r_refdef.scene.worldmodel->brush.num_leafs : 1; int numsurfaces = r_refdef.scene.worldmodel ? r_refdef.scene.worldmodel->num_surfaces : 1; if (r_refdef.viewcache.maxentities < numentities) @@ -3022,14 +3020,7 @@ static void R_Main_ResizeViewCache(void) Mem_Free(r_refdef.viewcache.entityvisible); r_refdef.viewcache.entityvisible = (unsigned char *)Mem_Alloc(r_main_mempool, r_refdef.viewcache.maxentities); } - if (r_refdef.viewcache.world_numclusters != numclusters) - { - r_refdef.viewcache.world_numclusters = numclusters; - r_refdef.viewcache.world_numclusterbytes = numclusterbytes; - if (r_refdef.viewcache.world_pvsbits) - Mem_Free(r_refdef.viewcache.world_pvsbits); - r_refdef.viewcache.world_pvsbits = (unsigned char *)Mem_Alloc(r_main_mempool, r_refdef.viewcache.world_numclusterbytes); - } + // bones_was_here: r_refdef.viewcache.world_pvsbits was (re)allocated here, now done in Mod_BSP_FatPVS() if (r_refdef.viewcache.world_numleafs != numleafs) { r_refdef.viewcache.world_numleafs = numleafs; @@ -4754,7 +4745,7 @@ void R_Water_AddWaterPlane(msurface_t *surface, int entno) if (p->materialflags & (MATERIALFLAG_WATERSHADER | MATERIALFLAG_REFRACTION | MATERIALFLAG_REFLECTION) && r_refdef.scene.worldmodel && r_refdef.scene.worldmodel->brush.FatPVS && r_refdef.scene.worldmodel->brush.PointInLeaf && r_refdef.scene.worldmodel->brush.PointInLeaf(r_refdef.scene.worldmodel, center)->clusterindex >= 0) { - r_refdef.scene.worldmodel->brush.FatPVS(r_refdef.scene.worldmodel, center, 2, p->pvsbits, sizeof(p->pvsbits), p->pvsvalid); + r_refdef.scene.worldmodel->brush.FatPVS(r_refdef.scene.worldmodel, center, 2, &p->pvsbits, r_main_mempool, p->pvsvalid); p->pvsvalid = true; } } @@ -4915,7 +4906,7 @@ static void R_Water_ProcessPlanes(int fbo, rtexture_t *depthtexture, rtexture_t if(r_refdef.scene.worldmodel && r_refdef.scene.worldmodel->brush.FatPVS) { r_refdef.view.usecustompvs = true; - r_refdef.scene.worldmodel->brush.FatPVS(r_refdef.scene.worldmodel, visorigin, 2, r_refdef.viewcache.world_pvsbits, (r_refdef.viewcache.world_numclusters+7)>>3, false); + r_refdef.scene.worldmodel->brush.FatPVS(r_refdef.scene.worldmodel, visorigin, 2, &r_refdef.viewcache.world_pvsbits, r_main_mempool, false); } } @@ -4970,7 +4961,7 @@ static void R_Water_ProcessPlanes(int fbo, rtexture_t *depthtexture, rtexture_t if(p->camera_entity && r_refdef.scene.worldmodel && r_refdef.scene.worldmodel->brush.FatPVS) { r_refdef.view.usecustompvs = true; - r_refdef.scene.worldmodel->brush.FatPVS(r_refdef.scene.worldmodel, visorigin, 2, r_refdef.viewcache.world_pvsbits, (r_refdef.viewcache.world_numclusters+7)>>3, false); + r_refdef.scene.worldmodel->brush.FatPVS(r_refdef.scene.worldmodel, visorigin, 2, &r_refdef.viewcache.world_pvsbits, r_main_mempool, false); } // camera needs no clipplane diff --git a/gl_rsurf.c b/gl_rsurf.c index a68daf5b..d9f0c461 100644 --- a/gl_rsurf.c +++ b/gl_rsurf.c @@ -475,7 +475,7 @@ void R_View_WorldVisibility(qbool forcenovis) viewleaf = model->brush.PointInLeaf ? model->brush.PointInLeaf(model, r_refdef.view.origin) : NULL; // if possible fetch the visible cluster bits if (!r_lockpvs.integer && model->brush.FatPVS) - model->brush.FatPVS(model, r_refdef.view.origin, 2, r_refdef.viewcache.world_pvsbits, (r_refdef.viewcache.world_numclusters+7)>>3, false); + model->brush.FatPVS(model, r_refdef.view.origin, 2, &r_refdef.viewcache.world_pvsbits, r_main_mempool, false); // if floating around in the void (no pvs data available, and no // portals available), simply use all on-screen leafs. diff --git a/model_brush.c b/model_brush.c index 35d12f00..a69a7eed 100644 --- a/model_brush.c +++ b/model_brush.c @@ -3822,18 +3822,26 @@ static void Mod_BSP_FatPVS_RecursiveBSPNode(model_t *model, const vec3_t org, ve //Calculates a PVS that is the inclusive or of all leafs within radius pixels //of the given point. -static int Mod_BSP_FatPVS(model_t *model, const vec3_t org, vec_t radius, unsigned char *pvsbuffer, int pvsbufferlength, qbool merge) +static size_t Mod_BSP_FatPVS(model_t *model, const vec3_t org, vec_t radius, unsigned char **pvsbuffer, mempool_t *pool, qbool merge) { - int bytes = model->brush.num_pvsclusterbytes; - bytes = min(bytes, pvsbufferlength); + size_t bytes = model->brush.num_pvsclusterbytes; + + if (!*pvsbuffer || bytes != Mem_Size(*pvsbuffer)) + { +// Con_Printf("^4FatPVS: allocating a%s ^4buffer in pool %s, old size %zu new size %zu\n", *pvsbuffer == NULL ? " ^5NEW" : "", pool->name, *pvsbuffer != NULL ? Mem_Size(*pvsbuffer) : 0, bytes); + if (*pvsbuffer) + Mem_Free(*pvsbuffer); // don't reuse stale data when the worldmodel changes + *pvsbuffer = Mem_AllocType(pool, unsigned char, bytes); + } + if (r_novis.integer || r_trippy.integer || !model->brush.num_pvsclusters || !Mod_BSP_GetPVS(model, org)) { - memset(pvsbuffer, 0xFF, bytes); + memset(*pvsbuffer, 0xFF, bytes); return bytes; } if (!merge) - memset(pvsbuffer, 0, bytes); - Mod_BSP_FatPVS_RecursiveBSPNode(model, org, radius, pvsbuffer, bytes, model->brush.data_nodes + model->brushq1.hulls[0].firstclipnode); + memset(*pvsbuffer, 0, bytes); + Mod_BSP_FatPVS_RecursiveBSPNode(model, org, radius, *pvsbuffer, bytes, model->brush.data_nodes + model->brushq1.hulls[0].firstclipnode); return bytes; } diff --git a/model_brush.h b/model_brush.h index bec06c30..dae4a3cf 100644 --- a/model_brush.h +++ b/model_brush.h @@ -330,7 +330,7 @@ typedef struct model_brush_s int (*SuperContentsFromNativeContents)(int nativecontents); int (*NativeContentsFromSuperContents)(int supercontents); unsigned char *(*GetPVS)(struct model_s *model, const vec3_t p); - int (*FatPVS)(struct model_s *model, const vec3_t org, vec_t radius, unsigned char *pvsbuffer, int pvsbufferlength, qbool merge); + size_t (*FatPVS)(struct model_s *model, const vec3_t org, vec_t radius, unsigned char **pvsbuffer, mempool_t *pool, qbool merge); int (*BoxTouchingPVS)(struct model_s *model, const unsigned char *pvs, const vec3_t mins, const vec3_t maxs); int (*BoxTouchingLeafPVS)(struct model_s *model, const unsigned char *pvs, const vec3_t mins, const vec3_t maxs); int (*BoxTouchingVisibleLeafs)(struct model_s *model, const unsigned char *visibleleafs, const vec3_t mins, const vec3_t maxs); diff --git a/model_shared.c b/model_shared.c index d6cea915..adf8a867 100644 --- a/model_shared.c +++ b/model_shared.c @@ -1044,7 +1044,7 @@ void Mod_ShadowMesh_AddMesh(shadowmesh_t *mesh, const float *vertex3f, int numtr for (i = 0;i < numtris;i++) { - if ((mesh->numtriangles * 3 + 2) * sizeof(int) + 1 >= ((memheader_t *)((unsigned char *)mesh->element3i - sizeof(memheader_t)))->size) + if ((mesh->numtriangles * 3 + 2) * sizeof(int) + 1 >= Mem_Size(mesh->element3i)) { // FIXME: we didn't allocate enough space for all the tris, see R_Mod_CompileShadowMap Con_Print(CON_WARN "Mod_ShadowMesh_AddMesh: insufficient memory allocated!\n"); diff --git a/qdefs.h b/qdefs.h index 92770227..78f668d8 100644 --- a/qdefs.h +++ b/qdefs.h @@ -70,7 +70,7 @@ #define MAX_PARTICLETEXTURES 256 #define MAXCLVIDEOS 1 #define MAX_DYNAMIC_TEXTURE_COUNT 2 -#define MAX_MAP_LEAFS 8192 +//#define MAX_MAP_LEAFS 8192 #define MAXTRACKS 256 #define MAX_DYNAMIC_CHANNELS 64 @@ -137,7 +137,7 @@ #define MAX_PARTICLETEXTURES 256 ///< maximum number of unique particle textures in the particle font #define MAXCLVIDEOS 65 ///< maximum number of video streams being played back at once (1 is reserved for the playvideo command) #define MAX_DYNAMIC_TEXTURE_COUNT 64 ///< maximum number of dynamic textures (web browsers, playvideo, etc) -#define MAX_MAP_LEAFS 65536 ///< maximum number of BSP leafs in world (8192 in Quake) +//#define MAX_MAP_LEAFS 65536 ///< maximum number of BSP leafs in world (8192 in Quake), now dynamically allocated #define MAXTRACKS 256 ///< max CD track index // 0 to NUM_AMBIENTS - 1 = water, etc diff --git a/render.h b/render.h index fe21b600..a2234a04 100644 --- a/render.h +++ b/render.h @@ -326,8 +326,6 @@ typedef struct r_refdef_viewcache_s { // updated by gl_main_newmap() int maxentities; - int world_numclusters; - int world_numclusterbytes; int world_numleafs; int world_numsurfaces; @@ -856,7 +854,7 @@ typedef struct r_waterstate_waterplane_s r_rendertarget_t *rt_camera; // MATERIALFLAG_CAMERA mplane_t plane; int materialflags; // combined flags of all water surfaces on this plane - unsigned char pvsbits[(MAX_MAP_LEAFS+7)>>3]; // FIXME: buffer overflow on huge maps + unsigned char *pvsbits; qbool pvsvalid; int camera_entity; vec3_t mins, maxs; diff --git a/server.h b/server.h index 1ad3158e..66caeeff 100644 --- a/server.h +++ b/server.h @@ -152,8 +152,7 @@ typedef struct server_s sizebuf_t *writeentitiestoclient_msg; vec3_t writeentitiestoclient_eyes[MAX_CLIENTNETWORKEYES]; int writeentitiestoclient_numeyes; - int writeentitiestoclient_pvsbytes; - unsigned char writeentitiestoclient_pvs[MAX_MAP_LEAFS/8]; + unsigned char *writeentitiestoclient_pvs; const entity_state_t *writeentitiestoclient_sendstates[MAX_EDICTS]; unsigned short writeentitiestoclient_csqcsendstates[MAX_EDICTS]; diff --git a/snd_main.c b/snd_main.c index 03d7e33e..781d3d67 100644 --- a/snd_main.c +++ b/snd_main.c @@ -2136,16 +2136,7 @@ void S_Update(const matrix4x4_t *listenermatrix) Matrix4x4_Invert_Simple(&listener_basematrix, listenermatrix); Matrix4x4_OriginFromMatrix(listenermatrix, listener_origin); if (cl.worldmodel && cl.worldmodel->brush.FatPVS && cl.worldmodel->brush.num_pvsclusterbytes && cl.worldmodel->brush.PointInLeaf) - { - if(cl.worldmodel->brush.num_pvsclusterbytes != listener_pvsbytes) - { - if(listener_pvs) - Mem_Free(listener_pvs); - listener_pvsbytes = cl.worldmodel->brush.num_pvsclusterbytes; - listener_pvs = (unsigned char *) Mem_Alloc(snd_mempool, listener_pvsbytes); - } - cl.worldmodel->brush.FatPVS(cl.worldmodel, listener_origin, 2, listener_pvs, listener_pvsbytes, 0); - } + listener_pvsbytes = cl.worldmodel->brush.FatPVS(cl.worldmodel, listener_origin, 2, &listener_pvs, snd_mempool, 0); else { if(listener_pvs) diff --git a/sv_ents.c b/sv_ents.c index 895601fa..7d82a480 100644 --- a/sv_ents.c +++ b/sv_ents.c @@ -361,10 +361,11 @@ void SV_WriteEntitiesToClient(client_t *client, prvm_edict_t *clent, sizebuf_t * sv.writeentitiestoclient_cliententitynumber = PRVM_EDICT_TO_PROG(clent); // LadyHavoc: for comparison purposes camera = PRVM_EDICT_NUM( client->clientcamera ); VectorAdd(PRVM_serveredictvector(camera, origin), PRVM_serveredictvector(clent, view_ofs), eye); - sv.writeentitiestoclient_pvsbytes = 0; // get the PVS values for the eye location, later FatPVS calls will merge if (sv.worldmodel && sv.worldmodel->brush.FatPVS) - sv.writeentitiestoclient_pvsbytes = sv.worldmodel->brush.FatPVS(sv.worldmodel, eye, 8, sv.writeentitiestoclient_pvs, sizeof(sv.writeentitiestoclient_pvs), sv.writeentitiestoclient_pvsbytes != 0); + sv.worldmodel->brush.FatPVS(sv.worldmodel, eye, 8, &sv.writeentitiestoclient_pvs, sv_mempool, false); + else + sv.writeentitiestoclient_pvs = NULL; // add the eye to a list for SV_CanSeeBox tests VectorCopy(eye, sv.writeentitiestoclient_eyes[sv.writeentitiestoclient_numeyes]); @@ -390,7 +391,7 @@ void SV_WriteEntitiesToClient(client_t *client, prvm_edict_t *clent, sizebuf_t * // build PVS from the new eyes if (sv.worldmodel && sv.worldmodel->brush.FatPVS) for(i = 1; i < sv.writeentitiestoclient_numeyes; ++i) - sv.writeentitiestoclient_pvsbytes = sv.worldmodel->brush.FatPVS(sv.worldmodel, sv.writeentitiestoclient_eyes[i], 8, sv.writeentitiestoclient_pvs, sizeof(sv.writeentitiestoclient_pvs), sv.writeentitiestoclient_pvsbytes != 0); + sv.worldmodel->brush.FatPVS(sv.worldmodel, sv.writeentitiestoclient_eyes[i], 8, &sv.writeentitiestoclient_pvs, sv_mempool, sv.writeentitiestoclient_pvs != NULL); sv.sententitiesmark++; diff --git a/sv_send.c b/sv_send.c index ae2d9b9d..80466451 100644 --- a/sv_send.c +++ b/sv_send.c @@ -920,7 +920,7 @@ void SV_MarkWriteEntityStateToClient(entity_state_t *s, client_t *client) ed = PRVM_EDICT_NUM(s->number); // if not touching a visible leaf - if (sv_cullentities_pvs.integer && !r_novis.integer && !r_trippy.integer && sv.writeentitiestoclient_pvsbytes) + if (sv_cullentities_pvs.integer && !r_novis.integer && !r_trippy.integer && sv.writeentitiestoclient_pvs) { if (ed->priv.server->pvs_numclusters < 0) { diff --git a/svvm_cmds.c b/svvm_cmds.c index 4ad07559..770f1abe 100644 --- a/svvm_cmds.c +++ b/svvm_cmds.c @@ -781,8 +781,7 @@ static void VM_SV_tracetoss(prvm_prog_t *prog) //============================================================================ -static int checkpvsbytes; -static unsigned char checkpvs[MAX_MAP_LEAFS/8]; +static unsigned char *checkpvs; static int VM_SV_newcheckclient(prvm_prog_t *prog, int check) { @@ -816,9 +815,10 @@ static int VM_SV_newcheckclient(prvm_prog_t *prog, int check) // get the PVS for the entity VectorAdd(PRVM_serveredictvector(ent, origin), PRVM_serveredictvector(ent, view_ofs), org); - checkpvsbytes = 0; if (sv.worldmodel && sv.worldmodel->brush.FatPVS) - checkpvsbytes = sv.worldmodel->brush.FatPVS(sv.worldmodel, org, 0, checkpvs, sizeof(checkpvs), false); + sv.worldmodel->brush.FatPVS(sv.worldmodel, org, 0, &checkpvs, sv_mempool, false); + else + checkpvs = NULL; return i; } @@ -864,7 +864,7 @@ static void VM_SV_checkclient(prvm_prog_t *prog) // if current entity can't possibly see the check entity, return 0 self = PRVM_PROG_TO_EDICT(PRVM_serverglobaledict(self)); VectorAdd(PRVM_serveredictvector(self, origin), PRVM_serveredictvector(self, view_ofs), view); - if (sv.worldmodel && checkpvsbytes && !sv.worldmodel->brush.BoxTouchingPVS(sv.worldmodel, checkpvs, view, view)) + if (sv.worldmodel && checkpvs && !sv.worldmodel->brush.BoxTouchingPVS(sv.worldmodel, checkpvs, view, view)) { c_notvis++; VM_RETURN_EDICT(prog->edicts); @@ -895,8 +895,7 @@ static void VM_SV_checkpvs(prvm_prog_t *prog) #if 1 unsigned char *pvs; #else - int fatpvsbytes; - unsigned char fatpvs[MAX_MAP_LEAFS/8]; + unsigned char *fatpvs = NULL; #endif VM_SAFEPARMCOUNT(2, VM_SV_checkpvs); @@ -935,8 +934,8 @@ static void VM_SV_checkpvs(prvm_prog_t *prog) PRVM_G_FLOAT(OFS_RETURN) = 3; return; } - fatpvsbytes = sv.worldmodel->brush.FatPVS(sv.worldmodel, viewpos, 8, fatpvs, sizeof(fatpvs), false); - if(!fatpvsbytes) + sv.worldmodel->brush.FatPVS(sv.worldmodel, viewpos, 8, &fatpvs, sv_mempool, false); + if(!fatpvs) { // viewpos isn't in any PVS... darn PRVM_G_FLOAT(OFS_RETURN) = 2; diff --git a/zone.h b/zone.h index 2225ec7c..b0dedc9d 100644 --- a/zone.h +++ b/zone.h @@ -117,6 +117,13 @@ qbool Mem_IsAllocated(mempool_t *pool, const void *data); char *_Mem_strdup(mempool_t *pool, const char *s, const char *filename, int fileline); +/// Returns the current size of an allocation +// not a macro so that it doesn't allow the size to be changed. +static inline size_t Mem_Size(void *data) +{ + return ((memheader_t *)((unsigned char *)data - sizeof(memheader_t)))->size; +} + typedef struct memexpandablearray_array_s { unsigned char *data; -- 2.39.2