[Fixed] player_t::GetPSprite crashes if ReadyWeapon is null

Bugs that have been investigated and resolved somehow.

Moderator: GZDoom Developers

player_t::GetPSprite crashes if ReadyWeapon is null

Postby phantombeta » Sun Jan 26, 2020 5:23 pm

Exactly what it says in the thread title. Someone on the ZDoom Discord (Jekyll Grim Payne) was having issues with this. Taking the ReadyWeapon away and setting ReadyWeapon to null was making his mod crash.
This is caused by the "newcaller" pointer in GetPSprite being set to null, which it seems to not like. (Specifically, one of the weapons in the mod was calling SetPSprite from DoEffect even when not selected)

No idea what would be the best course of action here. To me, it seems like this may have to cause a VM abort. I hope I'm wrong and it can be made to work with a null ReadyWeapon, though, as the ReadyWeapon isn't even the only thing that can create PSprites now anyways.

Steps to reproduce:
  • Start a new game with the attached example PK3.
  • Give yourself "Test" in the console.
Attachments
GZDoom_GetPSpriteCrash.pk3
(356 Bytes) Downloaded 20 times
User avatar
phantombeta
In the meadow of sinful thoughts, every flower's a perfect one
Global Moderator
 
Joined: 02 May 2013
Location: Brazil, South America, Earth, Orion-Cygnus Arm, Milky Way
Discord: phantombeta#2461
Twitch ID: phantombeta_
Github ID: Doom2fan
Operating System: Windows 10/8.1/8 64-bit
OS Test Version: No (Using Stable Public Version)
Graphics Processor: nVidia with Vulkan support

Re: player_t::GetPSprite crashes if ReadyWeapon is null

Postby Rachael » Sun Jan 26, 2020 7:49 pm

It's been a while since I worked with psprite code but if I remember correctly, too many things depend on ReadyWeapon not being null for some reason. I'm not sure what the best course of action here would be, either, but my opinion is that it would be best to force the code to accept a null ReadyWeapon without errors (or in operations where one absolutely must be present, contingency operations for when it isn't).
User avatar
Rachael
Webmaster
 
Joined: 13 Jan 2004
Discord: Rachael#3767
Twitch ID: madamerachelle
Github ID: madame-rachelle
Operating System: Windows 10/8.1/8 64-bit
OS Test Version: No (Using Stable Public Version)
Graphics Processor: nVidia with Vulkan support

Re: player_t::GetPSprite crashes if ReadyWeapon is null

Postby Graf Zahl » Mon Jan 27, 2020 1:05 am

Well, when I see code like this

Code: Select allExpand view
   if (layer >= PSP_TARGETCENTER)
   {
      if (mo != nullptr)
      {
         newcaller = mo->FindInventory(NAME_PowerTargeter, true);
      }
   }
   else if (layer == PSP_STRIFEHANDS)
   {
      newcaller = mo;
   }
   else
   {
      newcaller = ReadyWeapon;
   }

   assert(newcaller != nullptr);


I automatically get suspicious. This function was written under the assumption that it can never fail(TM) and logically nearly all calls to it actually don't check if it returns something valid, even in this special case where it simply cannot return something other than NULL. GetPSprite gets called in roughly 30 places - not a single one of them performs validation on the result. The function was "designed to succeed" even though it cannot give that guaerantee.
User avatar
Graf Zahl
Lead GZDoom+Raze Developer
Lead GZDoom+Raze Developer
 
Joined: 19 Jul 2003
Location: Germany

Re: player_t::GetPSprite crashes if ReadyWeapon is null

Postby Rachael » Mon Jan 27, 2020 4:37 am

Yikes, so I remembered correctly...
User avatar
Rachael
Webmaster
 
Joined: 13 Jan 2004
Discord: Rachael#3767
Twitch ID: madamerachelle
Github ID: madame-rachelle
Operating System: Windows 10/8.1/8 64-bit
OS Test Version: No (Using Stable Public Version)
Graphics Processor: nVidia with Vulkan support

Re: player_t::GetPSprite crashes if ReadyWeapon is null

Postby Major Cooke » Mon Jan 27, 2020 9:04 am

Isn't that why FindPSprite was made?
User avatar
Major Cooke
d = klabs(wall[wall[wall[sector[dasect].wallptr].point2].point2].x-s->x)...
Global Moderator
 
Joined: 28 Jan 2007

Re: player_t::GetPSprite crashes if ReadyWeapon is null

Postby Graf Zahl » Mon Jan 27, 2020 10:18 am

Not really. FindPSprite only checks if something already exists, not whether it can exist.
User avatar
Graf Zahl
Lead GZDoom+Raze Developer
Lead GZDoom+Raze Developer
 
Joined: 19 Jul 2003
Location: Germany

Re: player_t::GetPSprite crashes if ReadyWeapon is null

Postby _mental_ » Sat Mar 14, 2020 5:04 am

It was fixed in a638cfb + 4201c4f.
_mental_
 
 
 
Joined: 07 Aug 2011


Return to Closed Bugs

Who is online

Users browsing this forum: No registered users and 0 guests