[?-3.7 and 3.8] square1.pk3 tntcrate VM abort

[Edward-san]

When you 'use' the tnt crate, you will pick it up, and then with 'attack' you'll drop it. Since some time (maybe since VM?), the drop will result in this vm abort error:

(vm_jit 1 version)

Code: Select all

VM execution aborted:  Weapon.A_Lower called without valid caller. Weapon expected
Called from PlayerPawn.FireWeapon at gzdoom.pk3:zscript/shared/player.txt, line 357
Called from PlayerPawn.CheckWeaponFire at gzdoom.pk3:zscript/shared/player.txt, line 425
Called from PlayerPawn.TickPSprites at gzdoom.pk3:zscript/shared/player.txt, line 513
Called from PlayerPawn.PlayerThink at gzdoom.pk3:zscript/shared/player.txt, line 1642
Called from state CrateWeapon.18
Called from PlayerInfo.SetPSprite [Native]

(vm_jit 0 version, appears a bit different)

Code: Select all

VM execution aborted:  Weapon.A_Lower called without valid caller. Weapon expected
Called from state CrateWeapon.18
Called from PlayerInfo.SetPSprite [Native]
Called from PlayerPawn.FireWeapon at gzdoom.pk3:zscript/shared/player.txt, line 291
Called from PlayerPawn.CheckWeaponFire at gzdoom.pk3:zscript/shared/player.txt, line 359
Called from PlayerPawn.TickPSprites at gzdoom.pk3:zscript/shared/player.txt, line 447
Called from PlayerPawn.PlayerThink at gzdoom.pk3:zscript/shared/player.txt, line 1449

To test this fast, you can use 'summon tntcrate' on the first map.

Note: I could test +vm_jit 0 only on 3.7 branch because of an assertion error on the master branch (reported here).

[Graf Zahl]

This is actually a mod bug that just didn't get reported before 3.7.

What happens here is that the mod takes away the weapon and then calls action functions referring to the weapon. This has plain and simply been undefined behavior and this just got caught in it when the error condition was actually started to be checked for.

IIRC this would outright crash the JIT code if left unchecked.

[Xaser]

Fun timing -- I just fixed this in Square last night.

But yeah, those A_Lower's should have never been there. Just something that slipped through the cracks, I guess.

Square's a bit overdue for a patch for modern GZDooms anyway. Not sure what the ETA is just yet but I'm giving it a playthrough to see if anything else seems wonky -- so far so good now that the crates are taken care of.

[Graf Zahl]

Thanks, I wouldn't have liked having to disable this check because this can really cause bad crashes if the function being called actually does something with the weapon. A_Lower only changes the sprite's coordinates but with the JIT the rules are a bit different than before and things that went through undiscovered would just crash it.

[Xaser]

No worries -- a VM abort is definitely the right thing to do here. Now I can catch the mistake when I inevitably make it again.