[Fixed] Crash on Gore [Software Renderer]

Bugs that have been investigated and resolved somehow.

Moderator: GZDoom Developers

Crash on Gore [Software Renderer]

Postby Rachael » Sun Oct 02, 2016 11:11 am

I know I shouldn't be playing OpenGL specific mods using the software renderer - but I have my reasons.

Needless to say, the titlemap on Gore crashes in the latest ZDoom builds. It does not crash in ZDoom 2.8.1.

It's reliable about 5-10 seconds in from viewing the titlemap for this crash to occur.

Unfortunately, I can't give too much more details about what's going on here, because even loading it in the debugger I am not sure what.

What appears to happen is in r_drawt.cpp, line 531, *dest is pointing to either a null pointer, or to an object that's been freed. All the other variables seem to be fine.

Screenshot
User avatar
Rachael
Webmaster
 
Joined: 13 Jan 2004
Discord: Rachael#3767
Twitch ID: madamerachelle
Github ID: madame-rachelle

Re: Crash on Gore [Software Renderer]

Postby Graf Zahl » Sun Oct 02, 2016 12:04 pm

Well, I have no idea, this is too deep in the software renderer.
User avatar
Graf Zahl
Lead GZDoom Developer
Lead GZDoom Developer
 
Joined: 19 Jul 2003
Location: Germany

Re: Crash on Gore [Software Renderer]

Postby dpJudas » Sun Oct 02, 2016 3:32 pm

This function draws a column to the screen at (sx,yl) to (sx,yh). You could add some asserts at the top of the function like this:

assert(sx >= 0 && sx < SCREENWIDTH); // check x range
assert(yh < yl ||(yl >= 0 && yl < SCREENHEIGHT)); // check y start
assert(yh < yl ||(yh >= 0 && yh < SCREENHEIGHT)); // check y end

One or more of those are probably wrong due to some calculation in R_DrawVisSprite.
dpJudas
 
 
 
Joined: 28 May 2016

Re: Crash on Gore [Software Renderer]

Postby Graf Zahl » Sun Oct 02, 2016 3:52 pm

One of the y's was way beyond that range, somewhere around 30000. But this is for Randi.
User avatar
Graf Zahl
Lead GZDoom Developer
Lead GZDoom Developer
 
Joined: 19 Jul 2003
Location: Germany

Re: Crash on Gore [Software Renderer]

Postby Rachael » Sun Oct 02, 2016 3:58 pm

There's nothing to see with Y's that far off screen. I think it should be safe to put in sanity checks on these functions that determines the column position and length, and if both are off screen, to simply return the function and do nothing. I'm going to try that and see what happens.
User avatar
Rachael
Webmaster
 
Joined: 13 Jan 2004
Discord: Rachael#3767
Twitch ID: madamerachelle
Github ID: madame-rachelle

Re: Crash on Gore [Software Renderer]

Postby Graf Zahl » Sun Oct 02, 2016 4:23 pm

We don't even know what the problem is to decide whether to patch the symptoms rather than the cause. It's never a good idea to just add a few range checks to hide a bad calculation elsewhere.
User avatar
Graf Zahl
Lead GZDoom Developer
Lead GZDoom Developer
 
Joined: 19 Jul 2003
Location: Germany

Re: Crash on Gore [Software Renderer]

Postby Rachael » Sun Oct 02, 2016 4:38 pm

Fair enough then - as far as code submissions go, I will not make any to try to fix this.

I could be wrong, but here's what I have guessed is happening:

This exception is being thrown at a translucent drawer (the add drawer, specifically) - in order to work it has to read from the screen. (or at least the buffer that it is currently using)

For some odd reason, it's also writing (before the crash) to the screen at coordinates clearly out of range without throwing an exception.

Now - why it's off screen in the first place - well - simple - the sprites are really big, are being shown up-close, and naturally have huge Y clip-ends which the masking function theoretically is calling the translucent drawer to handle.

I know it's a bit of a gamble to go off of a theory from someone who has probably a week's experience playing with the software renderer, but I'm going to at least play around with it even if nothing comes of it to see if I can nail this one down.
User avatar
Rachael
Webmaster
 
Joined: 13 Jan 2004
Discord: Rachael#3767
Twitch ID: madamerachelle
Github ID: madame-rachelle

Re: Crash on Gore [Software Renderer]

Postby dpJudas » Sun Oct 02, 2016 10:48 pm

This part of the code is not trivial to read because it defers the calculations in buffers and uses callbacks to do it. Roughly, the algorithm is as follows:

1. R_DrawVisSprite is called to draw a sprite. Sets up light, blend mode, scaling. At this point the X position has already been calculated elsewhere.
2. Loops through vis->x1 to vis->x2 to draw the sprite. For four columns at a time it:
3. Calls rt_initcols. This resets the working buffer it uses to eventually pass in yl/yh values to rt_addclamp1col.
4. Calls R_DrawMaskedColumnHoriz four times to add a column at a time to the working buffers.
4a. In R_DrawMaskedColumnHoriz it calculates the unclipped position of the sprite for that column (search for "calculate unclipped screen coordinates for post").
4b. Then it clips it vertically according to the min/max values in mfloorclip and mceilingclip. One possibility is that the mfloorclip or mceilingclip values are out of bounds.
4c. Then it clips it based on the texture 'span post'. This code I can barely read, so that gives it a 200% extra chance of being buggy. Especially as there is a double involved there, so its been patched since the floating point change. Or it might be perfectly fine but could still fail if it gets invalid span post info from the texture.
4c. Now it calls hcolfunc_pre to tell rt_drawt functions to copy the data to the working buffers.
4d. Last it inverts the yl/yh ranges if the sprite is flipped.
5. Calls rt_draw4cols to copy the result to the screen. Here it does some sorting to draw things in pairs of four whenever the columns align. Here it calls rt_addclamp1 for one of them that don't and the program crashes.

To summarize, the candidates for this crash are: A) mfloorclip + mceilingclip, B) R_DrawMaskedColumnHoriz's "if (dc_yl <= dc_yh)" stuff juggling of dc_yl and dc_yh, C) FTexture::GetColumn returning invalid span data, D) rt_draw4cols having a bug in its sorting. Of those four, B is the only one doing double stuff, so my bet would be to examine that one first. If it is mfloorclip and mceilingclip it gets really really nasty as those are set elsewhere far far away from this part of the code. You really don't want it to be A. :)
dpJudas
 
 
 
Joined: 28 May 2016

Re: Crash on Gore [Software Renderer]

Postby Graf Zahl » Mon Oct 03, 2016 2:10 am

dpJudas wrote:If it is mfloorclip and mceilingclip it gets really really nasty as those are set elsewhere far far away from this part of the code. You really don't want it to be A. :)


... and now take a guess why I absolutely refuse to work with this code. This design has blocked me several times to implement some new feature because I was unable to figure out where to set the necessary info.
User avatar
Graf Zahl
Lead GZDoom Developer
Lead GZDoom Developer
 
Joined: 19 Jul 2003
Location: Germany

Re: Crash on Gore [Software Renderer]

Postby dpJudas » Mon Oct 03, 2016 3:06 am

Hehe, I can't really blame you. I actually tried several times to see if I could refactor the swrender code a little bit. All those branches failed. My last attempt was to rewrite the whole thing (r_swrenderer2.cpp/h in the QZDoom repository). I got as far as having the main rendering working well enough to render most of vanilla Doom/Doom2, but adding fake floors and 3d floors made me conclude the task was just too big.

At this point the only thing I think can really be done to this part of the codebase is to put it all in a single class and just accept its variable usage is too convoluted to split it any further. A shame tho, because I really wanted to throw multiple cores at the bsp walking part.
dpJudas
 
 
 
Joined: 28 May 2016

Re: Crash on Gore [Software Renderer]

Postby Graf Zahl » Mon Oct 03, 2016 4:03 am

That's only one of the problems. Another one is if you cannot track down where a variable is set for a certain task, there's no chance to add a new feature depending on it. This was the reason why I never managed to add per-tier colorizing for walls.
User avatar
Graf Zahl
Lead GZDoom Developer
Lead GZDoom Developer
 
Joined: 19 Jul 2003
Location: Germany

Re: Crash on Gore [Software Renderer]

Postby dpJudas » Mon Oct 03, 2016 5:23 am

The primary reason I want it in a class is actually to deal with that situation. As long as the codebase uses globals that aren't even declared in headers, it becomes impossible to see which variables effectively act as function arguments and where the variables even came from. Once the variables are members of a class, it gets much easier to attack them one variable at a time. Especially if you can also see the full set of function names at a glance by looking in the header file.
dpJudas
 
 
 
Joined: 28 May 2016

Re: Crash on Gore [Software Renderer]

Postby Gez » Mon Oct 03, 2016 5:28 am

Graf Zahl wrote:per-tier colorizing for walls.

Do you mean Doom64-style color gradients?
Gez
 
 
 
Joined: 06 Jul 2007

Re: Crash on Gore [Software Renderer]

Postby Graf Zahl » Mon Oct 03, 2016 6:01 am

What I meant was a color equivalent to Transfer_Light and Transfer_WallLight and a per-tier version of Transfer_WallLight. The reason I never managed to do that was the sad fact that the global variable state is such a total mess.
User avatar
Graf Zahl
Lead GZDoom Developer
Lead GZDoom Developer
 
Joined: 19 Jul 2003
Location: Germany

Re: Crash on Gore [Software Renderer]

Postby dpJudas » Mon Oct 03, 2016 8:33 am

There's barely any global variables in the software renderer: https://github.com/dpjudas/dpDoom/blob/swrenderclass/src/r_swrenderer.h :D
dpJudas
 
 
 
Joined: 28 May 2016

Next

Return to Closed Bugs

Who is online

Users browsing this forum: No registered users and 1 guest