random sigsegv in thz2 turret room

crash report: srb2win.RPT

EIP in the crash report points to p_mobj.c, line 2596:

					// Don't explode on the sky!
>					if (!(mo->eflags & MFE_VERTICALFLIP)
					&& mo->subsector->sector->floorpic == skyflatnum
					&& mo->subsector->sector->floorheight == mo->floorz)
						P_RemoveMobj(mo);

This is from the code handling MF_MISSILE objects in P_ZMovement.

This crash line doesn't make a lot of sense to me at the moment, since mo being invalid would surely have crashed earlier in the function. Maybe gdb is giving me slightly the wrong thing, however.

What if mo is valid, but mo->subsector->sector is invalid?
(The two lines under the if are part of the same condition, so the RPT might point to the starting line instead of the exact line? 🤷)

'tis possible.

Well, converting our debug file into a text file and making sense of the ASM, I actually find that it's trying to dereference mo->subsector when it actually has a value of NULL:

[ebx = mo]

C:\users\[snip]\srb2\src/p_mobj.c:2597
  4e3e50:	8b 53 60             	mov    0x60(%ebx),%edx [edx = mo + 0x60 (mo->subsector)]
  4e3e53:	8b 0d d4 52 18 01    	mov    0x11852d4,%ecx [ecx = skyflatnum]
> 4e3e59:	8b 12                	mov    (%edx),%edx [dereference mo->subsector]

So you were sort of on the right track, just that it's actually that the missile object doesn't have a subsector for some reason.

Looking at the code surrounding this, I have a hunch that this crash has to do with #561.

My guess is that, somehow, an object was removed via P_RemoveMobj, and its subsector value was set to NULL via that function. However, the object was not removed properly, and as such P_MobjWasRemoved still returns false. When it finally reaches this code, it tries to access a subsector value that does not exist, and crashes.

As for why this would happen, I have no idea, but the evidence seems to suggest this course of action. No other function in the game aside from P_RemoveMobj sets an object's subsector value to NULL. Plus, the fact that #561 even happens in the first place suggests that something is definitely getting past the game's radar somehow.

mentioned in merge request !1590 (merged)

mentioned in commit 6cf4f79b

closed