exits with a segfault instead of vpo

[chocolate-import]

The following bug was originally reported on Sourceforge by gammafactorial, 2008-09-29 22:26:47:

chocolate-doom -file shdwpit.wad -warp 01 exits with a segmentation fault instead of the visplane overflow message like the exe

[jmtd]

WAD in question: http://www.doomworld.com/idgames/levels/doom2/deathmatch/Ports/s-u/shdwpit

[fabiangreffrath]

This happens because R_CheckPlane() does not have a check if MAXVISPLANES is reached before making a new one and R_FindPlane() only checks for (lastvisplane - visplanes == MAXVISPLANES), not >=.

[mfrancis95]

You can add a NULL check to the visplane in R_CheckPlane and error out early, and this seems to catch it, but I don't think this is the most ideal way of solving the issue.

diff --git a/src/doom/r_plane.c b/src/doom/r_plane.c
index 16dde4c2..db882e57 100644
--- a/src/doom/r_plane.c
+++ b/src/doom/r_plane.c
@@ -266,7 +266,11 @@ R_CheckPlane
     int		unionl;
     int		unionh;
     int		x;
-	
+    if (pl == NULL)
+    {
+        I_Error ("R_DrawPlanes: visplane overflow (%" PRIiPTR ")",
+         lastvisplane - visplanes);
+    }
     if (start < pl->minx)
     {
 	intrl = pl->minx;

[fabiangreffrath]

A pointer pointing past the visplanes[] array isn't necessarily a NULL pointer.

--- a/src/doom/r_plane.c
+++ b/src/doom/r_plane.c
@@ -307,6 +307,9 @@ R_CheckPlane
     lastvisplane->picnum = pl->picnum;
     lastvisplane->lightlevel = pl->lightlevel;
     
+    if (lastvisplane - visplanes == MAXVISPLANES)
+       I_Error ("R_CheckPlane: no more visplanes");
+
     pl = lastvisplane++;
     pl->minx = start;
     pl->maxx = stop;

[mfrancis95]

So @fabiangreffrath could that be merged in? Is there anything else that would need to be done?

[fabiangreffrath]

I expressed some objections against your approach, but have none against mine. 😉 Could you confirm that it works as expected?

[AXDOOMER]

Is something else than the VPO wrong with this map? It won't even run in Doom+. It locks up with the music playing, either there's a missing player 1 start position or there's more than 1024 visplanes. I don't have tools to check.

[AXDOOMER]

@fabiangreffrath I implemented your fix in my source port and it works as expected.

AXDOOMER/doom-vanille@56ccf3a