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 <bones_was_here@xonotic.au>

diff --git a/clvm_cmds.c b/clvm_cmds.c
index 99f907d492400e1e5e891fd8ae8a6567dc6c6a2c..5ba1b3e53564fbb8fb40c52aa3bfde7351b5fa20 100644 (file)
--- 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 26c90911d26ba016dd676a6a4ba83fe995ab06cc..7113cacad923e20daea780f7f83572272b44dcbb 100644 (file)
--- 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 a68daf5b68e539e032441a0d715e48d3977e0a5d..d9f0c461837d96c7e6278f31a0c77e4e6a357ffa 100644 (file)
--- 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 35d12f002acaa0a62e61ea20d1c1a16c102ccb0b..a69a7eed152137bc7e331883b19affc9f3b549be 100644 (file)
--- 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 bec06c30e43e73e6bac9aba7dca49ee00aa0746c..dae4a3cfdeb6a01993466086f734bb30ed4239e7 100644 (file)
--- 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 d6cea915396455ffce1b181e5ed6a0f99018af31..adf8a867cb77b8b769536aab1529f137832124ef 100644 (file)
--- 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 927702270e55c03ecec57d4c174bdd9f9010848e..78f668d899fc895823eaf91b2cfb8fec2094fc9e 100644 (file)
--- 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 fe21b6000a1e266cb987ae4dc3ddaf7c440664e2..a2234a048e09de4e096a7dc07eecb158d1d93fb5 100644 (file)
--- 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 1ad3158eb7a33b79e8ab045c9c4bb975c631f8e3..66caeeff8f3d6144cd1939bc1cd001637a21361a 100644 (file)
--- 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 03d7e33e76ae59c9892810a11809bb2532dac5b3..781d3d675e00b3146e63cb3428e61a109392df44 100644 (file)
--- 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 895601fa77a63178316c6139f788f8e2a47d7112..7d82a48091de71ff437f50b384c204b2ae1b09a5 100644 (file)
--- 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 ae2d9b9df38271742c26bf0bd829ef22d5a178d7..80466451d9d34c00009c59d51af334d954a620d6 100644 (file)
--- 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 4ad07559b7473ba25fbf1fe0007c88c3ee19795a..770f1abededce1e8f91b465b3187c0ad606f49cd 100644 (file)
--- 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 2225ec7c7f47ea9275a9ad72b2f85a2d14dbfeb8..b0dedc9dc2126ced5e04efc2513294e778462c21 100644 (file)
--- 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;