TR2X bug: Rare crash in Barkhang Monastery

[aredfan]

Dev snapshot: 0.2-227-g6e863b2

---

The cause or steps to reproduce this crash is TBD. Hopefully this issue will provide a clue to help figure out a fix.

The waypoints shown below mark the 3 places I experienced a crash. In all cases, it was after Lara collected a Prayer Wheel in room 45.

[rr-]

I can't reproduce this – would appreciate more detailed testing instructions. CC-ing @lahm86 as he's been of great help tracking down the root cause of the previous crash.

[lahm86]

I'll gladly take a look at this tomorrow. IIRC there is an untriggerable enemy in that area with "consumed" pickups, perhaps that's a lead.

[lahm86]

I'm struggling to reproduce this too, or find any lead. I don't suppose you have a save from when you played the level, just prior to this area maybe? And what order did you do the level in? Just thinking it may depend on which items are active elsewhere in the level.

[aredfan]

I'm struggling to reproduce this too, or find any lead. I don't suppose you have a save from when you played the level, just prior to this area maybe? And what order did you do the level in? Just thinking it may depend on which items are active elsewhere in the level.

Yep, I got 3 save files prior to this crash: savegame.zip. Crash 3 was after savegame.2 when I was trying to reproduce it, but I had little luck getting the game to crash since then.

This is the first prayer wheel I collected. I have not used the key yet to access the main hall with the buddhist statue. If it helps, I also retraced my steps through the level to the best of my recollection, on trview: Route.zip

Also no monks died before this point.

[lahm86]

Thanks for the detailed information. I've still not managed to reproduce it unfortunately, so I'm not sure at all what the cause could be. There is nothing abnormal going on in this area from what I can tell, all items that are active are "running" normally, and effects seem to be OK.

I'll continue trying to recreate it though.

[aredfan]

Thank you @lahm86, I appreciate you looking into this issue.

The good news is I've managed to get crash 4 to happen on video. One notable difference is this crash happened before the room got flipped. At the end of the video, I was holding down action so Lara can stay locked onto the goon.

Video: https://drive.google.com/drive/folders/1dttvc2gIHps7h6duhKQKc1SQ6Gm8nA2z?usp=sharing

[lahm86]

Thanks. Still no luck at this end 😄
If you have time, could you try the attached build? Nothing has changed, it just has lots of logging. Maybe between us, if we keep trying, we can get it to crash again, and then the log file should hopefully give us more of a hint. The log file will be pretty big if you have the game running for a while, but it should compress down nicely for GH.

TR2X.zip

[aredfan]

No problem at all. I must be lucky because I've managed to trigger a crash on 2nd try after adding your build.

Log: TR2X.zip

It went roughly like this:

1. Loaded savegame.6
2. Collected the prayer wheel
3. Collected the dropped small medi pack
4. Pulled a crate
5. Made a save in savegame.7
6. Loaded savegame.6 again
7. Took out a flare (I think)
8. Collected the prayer wheel

I'm actually not sure what was happening when the crash occurred after step 8, I think Lara was still in the fire room.

Saves 6 & 7, for good measure: savegame.zip

[lahm86]

Thanks. So the good news is I've managed to get it to crash as well, and it matches up with your crash, in that this is the final debug message before the crash.

game/game.c 139 Game_Control (item #177, type 48)->control

So that points to it being something in this enemy's control that is at fault. He is triggered when the prayer wheel is collected.

But equally, this doesn't explain the crash you had before picking up the prayer wheel, so I'm thinking it's more a general thing in the mercenary1 control. That code isn't decompiled yet, but the Creature logic is, so I'm wondering if the fix @rr- provided yesterday for #138 could also have affected the mercenary control. I've not managed to get it to crash yet on the latest dev build, but of course it's not consistent to reproduce anyway so that's not a complete indicator.

I'm also wondering if this is potentially an OG bug in the mercenary control, but I imagine that, although it's quite difficult to reproduce, someone in the community would have noticed it before. I know Barkhang can get buggy with traps and doors freezing, but I don't think I've seen the crash before in OG.

[lahm86]

Some progress (it is crashing on latest dev build after all), Room_GetDoor I believe is where it's crashing, and you can see here 1280 is the room number at that point (followed by z and x sector). That's clearly incorrect. The value in the log for Room_GetDoor is the sector's FD index, but that's obviously incorrect too because it's being passed incorrect data.

Will continue trying to backtrace to see where this is coming from.

[lahm86]

I can't find a lead as to what's corrupting the room number. Everything in Creature_Animate checks out as far as I can tell: I traced every possible call to Room_GetSector from within that, and the room number remains valid. So something must be happening in the call before it, which will be the Merc1 control routine. We may need to come back to this once that's decompiled.

[aredfan]

It took me a while to to trigger a crash, this time it happened before collecting the prayer wheel.

Dev snapshot: 0.2-236-g613af1c

TR2X.log

[rr-]

WTF why there's no stack trace :D
@aredfan can you send the DLL and DMP as well? Thanks

[aredfan]

@rr- Of course 🤞

TR2X.zip

[rr-]

Right, these are still release builds… That explains the missing stack trace. I'll change the dev snapshot to produce debug symbols from now on 🤦‍♀️

[rr-]

@aredfan the latest dev snapshot now contains debug symbols and should have the new stack trace capture working. The asset is significantly larger as it's no longer compressed with UPX unlike release builds.

[walkawayy]

I can't reproduce it. I was going to just leave the game alt-tabbed until it crashed while I did other things, but my game pauses on minimize. Maybe aredfan will get lucky with this debug build with more log information.

[lahm86]

I think I've found something in LOS_Check. If start and target have identical x and z values, target->room_num is never set via LOS_CheckX/LOS_CheckZ. Having identical values here is massively rare (as indicated by how difficult this is to reproduce). But when it comes to monks/mercenaries, as they don't have collision with each other, it's more likely for them to end up in the same position while fighting, although still rare as it relies on their moods as well.

Judging by TR1X, target's room_num is never initialised, so it could well be garbage and hence it's later getting passed to Room_GetSector, and so Room_GetDoor is crashing because it's getting some random memory as a sector.
https://github.com/LostArtefacts/TR1X/blob/develop/src/game/creature.c#L676

My guess is that in OG, the garbage room number turns out to be within the valid room range by chance, so the OG GetFloor call doesn't bomb out.

You can simulate a crash by setting x/z to match on start and target in LOS_Check.

game/game.c 139 Game_Control (item #177, type 48, room 107)->control
game/los.c 270 LOS_Check start 64000 -6400 23900 107
game/los.c 271 LOS_Check target 64000 -6656 23900 1280
game/los.c 284 LOS_Check target room 1280
game/room.c 108 Room_GetSector 64000 -6656 23900 1280
game/room.c 135 Room_GetSector 1280 0 1
game/room.c 749 Room_GetDoor 35591
libtrx/log_windows.c 91 Log_CrashHandler == CRASH REPORT ==

Here is a proposed patch, but I don't really like having to set NO_ROOM here. The mercenary/monk calls to this function aren't decompiled yet though, so perhaps this could be a stopgap? I imagine there is a similar function to this yet to be decompiled, ideally it would be set there.

diff --git a/src/game/los.c b/src/game/los.c
index bd338a0..1b0799f 100644
--- a/src/game/los.c
+++ b/src/game/los.c
@@ -263,6 +263,10 @@ int32_t __cdecl LOS_Check(
     int32_t los1;
     int32_t los2;
 
+    if (target->room_num < 0 || target->room_num >= g_RoomCount) {
+        target->room_num = NO_ROOM;
+    }
+
     if (ABS(target->z - start->z) > ABS(target->x - start->x)) {
         los1 = LOS_CheckX(start, target);
         los2 = LOS_CheckZ(start, target);
@@ -271,7 +275,7 @@ int32_t __cdecl LOS_Check(
         los2 = LOS_CheckX(start, target);
     }
 
-    if (!los2) {
+    if (!los2 || target->room_num == NO_ROOM) {
         return 0;
     }

[walkawayy]

Nice find. I don't think I've ever seen a reported crash like this for the OG games though. Even though it's rare, you'd think there would be reports over the years. Is there some added safety check or change that would caused us to be getting this?

[lahm86]

My only assumption is that it could be a difference on the stack because of changes in TR2X elsewhere? The room number is consistently 1280 when it's crashing (for this particular scenario), so I can only guess in OG the value is within the room range.

[walkawayy]

@lahm86 based on the recently decompiled functions, I think you're theory is exactly right. The only odd thing is this seemingly never happening in OG TR2 for some reason. But anyway, Creature_CanTargetEnemy, Gun_FireWeapon, Diver_Control, and FinalLevelCounter_FindBestBoss all call LOS_Check with an uninitialized room number.

It's probable we may also need to decompile Gun_FindTargetPoint to see what happens in that function as Gun_TargetInfo and Gun_GetNewTarget call LOS_Check after and could pass in an uninitialized room number. @rr-

IMO, instead of initializing to NO_ROOM, maybe it should just be initialized to whatever the GAME_VECTOR start's room number is?