[Prboom-notes] r4502 - branches/prboom-plus-24/prboom2/src

[<web...@pr...>]

Author: entryway
Date: Mon Jan 23 08:30:04 2017
New Revision: 4502

Log:
Seg Fault Crash for OpenGL with -O2 compiler optimizations enabled

fixed by dmooter

I'm attaching a fix for this bug. :) I hope you implement it in the next release! This bug is really annoying, and the fix is simple.

I'm running the PRBoom-Plus 2.5.1.5 from Ubuntu 16's 64-bit repositories. Some maps segfault when using Open GL. Easy way to reproduce is to run Hell to Pay (HELL2PAY.WAD) and just let the opening intro animation run its course.

Crash only happens when -O2 compiler optimizations enabled. I think these are the culprits: -falign-loops -falign-labels -fexpensive-optimizations -fipa-sra -ftree-switch-conversion -ftree-pre -ftree-builtin-call-dce -ftree-vrp -fipa-ra.

Root cause is in R_Subsector(). Pointer floorplane is assigned to the address of dummyfloorplane, which is in the local call stack. Because dummyfloorplane was declared inside a nested if, it runs out of scope outside that if statement. Compiler optimizations appear to then reuse that stackframe space to allocate the next stackframe when passing dummyfloorplane into gld_AddPlane(). This results in floorplane being a dangling pointerd. Ditto ceilingplane and dummyceilingplane.

Solution is to move the declarations of dummyfloorplane and dummyceilingplane to the top of the function so that they do not go out of scope while floorplane and ceilingplane are still in scope pointing to them.

floorplane and ceilingplane are globals, so they'd have problems if these pointers were used after R_Subsector() unwinds, but I haven't had a problem with this so am assuming they aren't used in that manner.

Modified:
   branches/prboom-plus-24/prboom2/src/r_bsp.c

Modified: branches/prboom-plus-24/prboom2/src/r_bsp.c
==============================================================================
--- branches/prboom-plus-24/prboom2/src/r_bsp.c	Sat Jan 14 20:07:13 2017	(r4501)
+++ branches/prboom-plus-24/prboom2/src/r_bsp.c	Mon Jan 23 08:30:04 2017	(r4502)
@@ -601,6 +601,14 @@
   sector_t    tempsec;              // killough 3/7/98: deep water hack
   int         floorlightlevel;      // killough 3/16/98: set floor lightlevel
   int         ceilinglightlevel;    // killough 4/11/98
+  #ifdef GL_DOOM
+  // dmooter 1/16/2017 Move from being declared next to its use several lines lower.
+  // Needs to remain in scope to the end of the function so its stack memory is recycled,
+  // compiler optimizations will make the floorplane pointer a dangling pointer
+  // when passed into gld_AddPlane().
+  visplane_t dummyfloorplane;
+  visplane_t dummyceilingplane;
+  #endif
 
 #ifdef RANGECHECK
   if (num>=numsubsectors)
@@ -658,8 +666,6 @@
       // check if the sector is faked
       if (!gl_use_stencil && frontsector == sub->sector)
       {
-        visplane_t dummyfloorplane;
-        visplane_t dummyceilingplane;
         sector_t *tmpsec;
 
         // if the sector has bottomtextures, then the floorheight will be set to the