Extended limits crash bug!

[mh]

Wow, this one was obscure.

Fitz, RMQ, QuakeSpasm (not checked the latest source though) and DirectQ all have it. Bengt Jardrup's doesn't. I assume that DP doesn't either.

OK, you're going to need a really really big map with lots and lots of entities. One where the initial server message is larger than 8192 bytes.

Got one? Good.

Now check this out (in CL_ParseServerInfo):

Code: Select all

   for (i=1 ; i<nummodels ; i++)
   {
      cl.model_precache[i] = Mod_ForName (model_precache[i], false);
      if (cl.model_precache[i] == NULL)
      {
         Con_Printf("Model %s not found\n", model_precache[i]);
         return;
      }
      CL_KeepaliveMessage ();
   }

With me still? Let's go to CL_KeepaliveMessage then:

Code: Select all

   byte      olddata[8192];

   if (sv.active)
      return;      // no need if server is local
   if (cls.demoplayback)
      return;

// read messages from server, should just be nops
   old = net_message;
   memcpy (olddata, net_message.data, net_message.cursize);

BOOM!

One thing I like doing is keeping a 1MB "scratch buffer" hanging around for any temporary allocations - ones that are usually only needed within the space of a single function. Perfect for something like this (saves on stack space too).

[Baker]

Thanks for info. I probably have inherited that as well.

[qbism]

This discovery may greatly reduce gnashing of teeth. Searching for "];"

[r00k]

sync olddata with NET_MAXMESSAGE...

I assume this could also transpire in CL_GetMessage for demos too?

[metlslime]

Code: Select all

   if (sv.active)
      return;      // no need if server is local


This explains why I didn't catch it in my testing -- only tried with a local server.

[mh]

To be honest no maps really exist - yet - that can push it over the limit either, and in one I was testing it crashed only in DirectQ (which uses dynamic allocation and memory protection everywhere) so presumably it overflows somewhere safe elsewhere.

Still doesn't mean that it shouldn't be fixed though.

I don't think just increasing buffer sizes is wise here; this is allocated on the stack so it's 64k of stack space that you better be sure that you have. Either dynamically allocate it or create a completely new sizebuf_t.

[szo]

I don't think just increasing buffer sizes is wise here; this is allocated on the stack so it's 64k of stack space that you better be sure that you have. Either dynamically allocate it or create a completely new sizebuf_t.

Doing it static, like

Code: Select all

-   byte      olddata[8192];
+   static byte   olddata[NET_MAXMESSAGE];

... should do the trick, no?

[mh]

It would work but one shouldn't make a habit of it. There are already a lot of large static arrays scattered throughout the Quake code.

[metlslime]

To be honest no maps really exist - yet - that can push it over the limit...

I thought there was one; i'll check my notes at home (i once wrote down a list of all limit-breaking maps and which limits each one exceeded.)

[mh]

To be honest no maps really exist - yet - that can push it over the limit...

I thought there was one; i'll check my notes at home (i once wrote down a list of all limit-breaking maps and which limits each one exceeded.)

Any chance of sharing that list if you find it? I've my own selection of test maps (warpc, ne_tower, RMQ stuff, etc) but something more comprehensive would be great (and useful for other folks too). I'd even owe you a beer.

[metlslime]

This list is current up to the release of Fort Driant in 2009. Since then I'm sure a few more limit-breaking maps have been released. Also, the last few columns refer to values that can change over the course of the map; I simply recorded whether they are excessive right at the beginning of the level.

Code: Select all

                |           mark    clip            vis     signon                  light   static  static                                  vis     
map             |   faces   surfs   nodes   nodes   leafs   buf     models  sounds  maps    sounds  ents    efrags  gltex   edicts  packet  edicts 
----------------|-----------------------------------------------------------------------------------------------------------------------------------
std limit       |   32767   32767   32767   32767   8192    7998    255     255     64      116     128     640     1024    600     1024    256     
----------------|-----------------------------------------------------------------------------------------------------------------------------------
masque          |   .       .       .       .       .       x       x       .       .       x       .       .       .       x       x       .       
warpb           |   .       x       x       .       .       x       x       .       x       .       .       .       .       x       .       .       
warpc           |   x       x       x       .       .       x       x       .       x       x       x       .       .       x       .       .       
warpd           |   x       x       x       .       .       x       x       .       x       .       .       .       .       x       .       .       
sickbase        |   x       x       x       .       .       .       x       .       x       .       .       .       .       .       .       .       
bod             |   x       x       x       .       .       .       .       .       x       .       .       .       .       .       .       .       
omlabx          |   .       x       x       .       .       .       .       .       x       .       .       .       .       .       .       .       
digs04          |   x       x       x       .       x       .       x       .       .       .       .       .       .       .       .       .       
768_negke       |   x       x       x       .       x       .       x       x       .       .       .       x       .       x       .       .       
nsoe2           |   .       .       .       .       .       .       .       x       .       .       .       .       .       .       .       .       
nsoe3           |   .       .       x       .       .       x       .       x       x       .       .       .       x       x       x       x       
nsoe5           |   .       .       .       .       .       .       .       x       .       .       .       .       .       .       .       .       
nsoe6           |   .       .       .       .       .       x       .       .       .       .       .       .       .       x       .       .       
fort-driant     |   x       x       x       .       x       .       x       .       .       .       .       .       .       x       .       .       

[metlslime]

hmm, looking back at this list the "gltex" limit is almost meaningless because, for example, fitzquake uses gltextures for everything but glquake doesn't put certain classes of textures into that array (e.g. lightmaps) So you'll hit the limit at different times for different engines.

[mh]

efrags and static entities are connected too, as each static entity will generate 1 or more efrags. You can't really bump static entities without also bumping efrags.

I believe software Quake just uses efrags for everything so that might be relevant for some people.

numnodes + numleafs can't exceed 65536; there's another. ID Quake just assumes ~32k each, but you could have 60k nodes and 4k leafs (your BSP would be hellishly weird I'd expect, but it's possible and the format allows it).

Lightmap limits really assume that the lightmap size is always 128x128; bumping it to at least 256x256 can give better performance.

Still great to have for reference and useful for testing an engine with extended limits. Nice one.

[metlslime]

Lightmap limits really assume that the lightmap size is always 128x128; bumping it to at least 256x256 can give better performance.

True, and i guess about 1/3rd of these numbers are implementation-specific, so if your engine is sufficiently re-written they don't mean anything. The ones that are consistent across engines are the ones that are inherent to the bsp format, network protocol, or quakec VM.

[szo]

I fixed the reported problem in quakespasm svn at rev. 440, here:
http://quakespasm.svn.sourceforge.net/v ... vision=440
... it will be integrated in the next release (whenever it happens).
If there is a better fix and/or there are more places that need attending to, I'd like to hear.