OfficerSpy / TF2-MvM-Defender-TFBots

TFBots that can play Mann vs Machine
GNU General Public License v3.0
9 stars 2 forks source link

Crashes in `CTFBotSpySap_Update` #2

Closed OfficerSpy closed 5 months ago

OfficerSpy commented 5 months ago

Happens on Windows and Linux, no idea why...

https://crash.limetech.org/nbhhpzqyi7t3 https://crash.limetech.org/2oci3px27v4r

OfficerSpy commented 5 months ago

Also add that the bot doesn't move during this behavior either, weird...

caxanga334 commented 5 months ago

Limited investigation since I would need to duplicate your server and attach a debugger to it and also due to the limited access to the crash dump itself.

The function the caused the crash (cell_t CHalfLife2::EntityToBCompatRef(CBaseEntity *pEntity)), the first thing it does is check if the passed entity is NULL. So it's probably not a NULL pointer exception crash.

The data being passed is probably garbage, so somewhere in that stack trace the CBaseEntity pointer is becoming garbage data.

You also mentioned bots not moving, IsHindrance is used by the NextBots to determine if a given entity should block the bot's path. If a valid entity is returned then the bot will stop and wait for said entity to get out of the way (think of a door that is moving).

IsHindrance is also mentioned on the stacktrace, so what I think is happening, the bot is trying to move, PathFollower will call IsHindrance, the entity pointer passed to it is becoming garbage on the way, garbage data reaches SourceMod and crashes.

OfficerSpy commented 5 months ago

As far as I see, the crash isn't entirely consistent. Whenever a player builds near the spy, the bot just stands still, and a crash happens sometimes. However, spawning a sentry in via rtd crashes every time.

I wanted to believe it's a problem with the building, but it works fine for pathing in another behavior other bots use.

An IsHindrance callback is used in the spy's prior action, though it functions without errors. Neither of the callbacks are currently making use of the entity parameter either, but it still crashes regardless and blames function CTFBotSpySap_Update.

The crash reports state this:

caxanga334 commented 5 months ago

That value is very interesting, looks like the Actions extension is not handling this correctly.

#define IS_ANY_HINDRANCE_POSSIBLE ( (CBaseEntity*)0xFFFFFFFF ) from here.

Passing that value to SourceMod will bypass the NULL check because NULL is 0 and 0xFFFFFFFF is -1 on 32 bits, triggering the crash.

OfficerSpy commented 5 months ago

Ah, and this would end up being checked for in PathFollower::FindBlocker which happens whenever the path updates. I take it using any IsHindrance hook/callback would be susceptible to crashes then?

caxanga334 commented 5 months ago

Probably best to avoid using it until it's fixed. Note that the crash is caused by the Actions extension, you should probably file an issue on it's repo. It's passing an invalid CBaseEntity pointer to SourceMod.

The fix should be a simple check for the 0xFFFFFFFF value.

CBaseNPC is fine.

OfficerSpy commented 5 months ago

Fixed in the newest build of Actions (3.8.8).

As for why the bot doesn't move, apparently the buildings don't have m_lastNavArea updated upon being built, even though I thought they all did upon spawning.