Q2BSP - Out of Bounds - Compiler or Engine Bug?

[jitspoe]

Ok, there are a couple issues here. One is definitely an engine bug.

In gl_model.c in the Mod_LoadLeafs function,

Code: Select all

      out->firstmarksurface = loadmodel->marksurfaces +
         LittleShort(in->firstleafface);


The in->firstleafface is supposed to be unsigned. Large/complex maps will likely result in a crash as that value will turn negative. I've fixed that by casting to an the LittleShort result to an unsigned short, but now I'm running into another suspicious case.

Sometimes in->firstleafface is equal to loadmodel->nummarksurfaces. That means out->firstmarksurface is actually out of bounds. I'm trying to figure out why this is sometimes the case. Is it a bug in the compiler? Should this be a special case? What should I do with these?

[jitspoe]

Just did some more investigation. In writebsp.c's EmitLeaf:

Code: Select all

   leaf_p->firstleafface = numleaffaces;

   for (p = node->portals ; p ; p = p->next[s])   
   {
      s = (p->nodes[1] == node);
      f = p->face[s];
      if (!f)
         continue;   // not a visible portal

      EmitMarkFace (leaf_p, f);
   }
   
   leaf_p->numleaffaces = numleaffaces - leaf_p->firstleafface;

Looks like this is a non-issue. It should only be out of bounds when numleaffaces is 0.

The signed short bit is a real issue, though, so anybody using a Q2 engine should probably fix that to support larger maps.

[Jay Dolan]

Hehe, I found and fixed that bug about 4 years ago. This map (not surprisingly) is what blew it up:



Nasty little crash, it was.

[Knightmare]

Just for reference, Mod_LoadLeafs should now be:

Code: Select all

void Mod_LoadLeafs (lump_t *l)
{
   dleaf_t    *in;
   mleaf_t    *out;
   int         i, j, count, p;
   glpoly_t   *poly;

   in = (void *)(mod_base + l->fileofs);
   if (l->filelen % sizeof(*in))
      VID_Error (ERR_DROP, "MOD_LoadBmodel: funny lump size in %s",loadmodel->name);
   count = l->filelen / sizeof(*in);
   out = Hunk_Alloc (count*sizeof(*out));   

   loadmodel->leafs = out;
   loadmodel->numleafs = count;

   for (i=0; i<count; i++, in++, out++)
   {
      for (j=0; j<3; j++)
      {
         out->minmaxs[j] = LittleShort (in->mins[j]);
         out->minmaxs[3+j] = LittleShort (in->maxs[j]);
      }

      p = LittleLong(in->contents);
      out->contents = p;

      out->cluster = LittleShort(in->cluster);
      out->area = LittleShort(in->area);

      out->firstmarksurface = loadmodel->marksurfaces +
         (unsigned short)LittleShort(in->firstleafface);   // Knightmare- make sure this doesn't turn negative!
      out->nummarksurfaces = LittleShort(in->numleaffaces);
   }   
}


BTW, that image isn't loading for me, Jay.

[Jay Dolan]

Weird! Postimg.org must randomly move images from server to server. Updated the URL; maybe it'll stick for a while.