struct Player {
...
Player *mSuppressionPrev, *mSuppressionNext; // 0x7d4, 0x7d8
...
};
When the SensorManager is created, it makes a dummy player which is the root of a doubly linked list. When Player::setSensorSupression is called, Player::UnlinkFromSuppressionList(004398C8) is called to unlink the player from the list (if necessary), and then SensorManager::addPlayerToSuppressionList(004398C8) is called to link the player to the list if Player::setSensorSupression was called with a positive value.
Player::UnlinkFromSuppressionList:
http://www.team5150.com/~andrew/sensorcrash2.png
Unfortunately, Player::UnlinkFromSuppressionList is also called by Player::Kill and internally by Player::onRemove when the actual Player object is garbage collected, aka 2 more times after the player object has already been removed from the list. You should notice that mSuppressionPrev and mSuppressionNext are NOT null'd out when the player object is unlinked.
The crash happens like so:
- dummy{prev=null, next=null}
[ A activates his sensor jammer pack ]
- A{prev=null,next=dummy} -> dummy{prev=A, next=null}
[ B activates his sensor jammer pack ]
- A{prev=null,next=B} -> B{prev=A,next=dummy} -> dummy{prev=B, next=null}
[ A deactivates his pack ]
- B{prev=null,next=dummy} -> dummy{prev=B, next=null}, !!!A{prev=null,next=B}!!!
[ B dies, his object is garbage collected, and the memory freed before A dies or reactivates his pack. Player::UnlinkFromSuppressionList is called 3 times for B, but nothing happens because B only points at dummy, which still exists ]
[ A dies, Player::UnlinkFromSuppressionList tries to write to A->next->prev aka B->prev and crashes ]
Borland C++ was great at producing redundant code so it's possible to recode the function to null out 7d4/7d8:
http://www.team5150.com/~andrew/sensorcrash2_patch.png
Patch:
http://www.team5150.com/~andrew/tribes. ... ion.lpatch
You might want to hit CTRL-A in Olly (analyze code) so you can see the program structure. It's also helpful to explain how to reproduce a crash since an EIP and access violation usually don't give enough context unless the crash is trivial to debug.
It only took me half an hour or so to find out what was wrong, but I have much of Tribes labeled in Olly and know how most of it works. The major "breakthrough" was realizing the spot at where it crashed looked like updating pointers in a doubly linked list. After that it was easy to realize it was unhooking the player from the list, that the function was called twice after it was already unlinked (I already had Player::kill and Player::onRemove labeled), and that it might dereference a pointer which no longer exists. I could have set hardware breakpoints on the offsets at 7d4/7d8 in the player object to see what other functions were using them, but it wasn't necessary.
