Page 1 of 1

[BETA7] Q - Quake stuff random crashes

PostPosted: Wed Jun 12, 2019 2:46 am
by DabbingSquidward
Playing this mod in GZDoom 4.1.3 crashes randomly, almost always in the first level. I made sure that autoload's disabled this time. Not sure if mod fault or engine bug.
Link to the mod here: viewtopic.php?f=43&t=54907
Crash report is attached.

Re: [BETA7] Q - Quake stuff random crashes

PostPosted: Wed Jun 12, 2019 4:10 am
by Rachael
Is this a self compiled version? My debug symbols do not work with this.

We can only diagnose crash dumps from official builds. If you compiled it yourself, then you have access to a debugger and you can attach it yourself - and when it crashes you should be able to copy the stack trace and paste it here.

Also - if I am to guess, you're using the software renderer, here.

Re: [BETA7] Q - Quake stuff random crashes

PostPosted: Wed Jun 12, 2019 4:13 am
by DabbingSquidward
I use the official Windows x64 build from the News page and the OpenGL renderer, weird.
Anyway, Dark-Assassin, the author of the mod, just replied and linked me newer dev versions of his mod, should be more stable.

Re: [BETA7] Q - Quake stuff random crashes

PostPosted: Wed Jun 12, 2019 4:15 am
by Rachael
We're still going to need the crash-causing version since the overall goal is to reduce or hopefully eliminate any opportunities GZDoom has to crash (even if we'll never actually reach that point).

Re: [BETA7] Q - Quake stuff random crashes

PostPosted: Wed Jun 12, 2019 4:50 am
by Rachael
I managed to reproduce a crash with model interpolation, where it seems like there are two of the same frame being interpolated. Not sure exactly what is going on in this code or I'd have fixed it myself.

Spoiler: Stack Trace

Spoiler: Active Variables

The issue here is that in models.cpp line 261, nextState is null and its null struct is being directly referenced.

Re: [BETA7] Q - Quake stuff random crashes

PostPosted: Wed Jun 12, 2019 5:18 am
by Graf Zahl
The problem is quite obvious:
Code: Select allExpand view
            // [BB] Workaround for actors that use the same frame twice in a row.
            // Most of the standard Doom monsters do this in their see state.
            if ((smf->flags & MDL_INTERPOLATEDOUBLEDFRAMES))
            {
               const FState *prevState = curState - 1;
               if ((curState->sprite == prevState->sprite) && (curState->Frame == prevState->Frame))
               {
                  inter /= 2.;
                  inter += 0.5;
               }
               if ((curState->sprite == nextState->sprite) && (curState->Frame == nextState->Frame))
               {
                  inter /= 2.;
                  nextState = nextState->GetNextState();
}


This hack to work around double frames in monster walking animations needs to advance but unlike the outer code does not check if it gets a valid nextState.
Adding a null check to the 'if' should be enough.

Re: [BETA7] Q - Quake stuff random crashes

PostPosted: Wed Jun 12, 2019 5:32 am
by Rachael
That isn't the block where it was crashing, though. Do you want me to add one both in that block and in the if statement where it actually crashes?

(Doing so, by my tests, seems to fix the issue)

(And boy, this is a pretty gross hack... I don't like how it defines the prevState)

Re: [BETA7] Q - Quake stuff random crashes

PostPosted: Wed Jun 12, 2019 5:49 am
by Graf Zahl
By all accounts that code needs to be considered broken. The prevState thing definitely needs a few more sanity checks...

Re: [BETA7] Q - Quake stuff random crashes

PostPosted: Wed Jun 12, 2019 8:06 am
by Rachael
prevState needs to track the actual previous state, not assume that it's "curState - 1". If I do it, it's going to be when the actor's thinker tic execution advances the actor's state, and it'll be saved to the actor itself and this code can call that up.

At any rate, do you want me to commit the nullptr checks, for now?

Also, do you want me to move this thread to "On Hold" so that a proper rewrite can be addressed later?

Re: [BETA7] Q - Quake stuff random crashes

PostPosted: Wed Jun 12, 2019 8:44 am
by Graf Zahl
Yes, please commit the checks, they already help.
For the prevState, it may need a bit more thinking. The code as it is is just naive and careless.

Re: [BETA7] Q - Quake stuff random crashes

PostPosted: Wed Jun 12, 2019 9:22 am
by Rachael
Alright, done, but I'll leave this topic alone for you to decide what to do with it concerning the prevState stuff.