[net.games.rogue] Patch to Rogue5.3 binary fixes Nymph-Save-Game bug

chuck@dartvax.UUCP (08/21/84)

<Are we doing the impossible yet?>

This article describes the symptoms of a bug in rogue 5.3, portions of the 
rogue code which seem to be in error and how they cause this bug, and
how to patch your rogue binary so that this bug no longer exists.

Every now and then you may find yourself getting attacked by a Nymph.  
When the Nymph hits you, it attempts to take away a group of objects
(for example "2 scrolls called manic laugh").  Whenever the Nymph tries
to steal a group which contains more than 2 objects (note that "10 +0,+0
shurikens" counts as only one object) things seem to start going wrong.
For starters, the objects still seem to be in your pack.  Then the game
seems to save itself.  And every time the game is restarted, it saves
itself again after a few moves.  Finally, if you do succeed in making it
down to a new level, the contents of your pack are screwed up.

How can we explain all this?  Well, it's surprisingly easy.  I've been
doing some heavy hacking of the rogue binary under ADB and I decided to
have a go at this bug to see if I could figure out what was wrong.  There
appear to be two ways that an object can be removed from your pack.  Either 
you can remove a single object by dropping, quaffing, throwing, etc; or a
Nymph can remove a whole group of objects by stealing it.  All of this is
done inside a routine called "_leave_pack".  

Let me describe how I interpret the _leave_pack routine based upon my
looking at the binary for the routine.  The routine takes three arguments
and returns a pointer.  The first argument is a pointer
to a data structure describing the group that is being removed.  The
second argument tells the number of items that are to be removed from
that group.  The third argument is a boolean flag that tells whether or
not we want to completely remove the group.  The pointer returned points 
to a data structure describing the items which were removed.

Normally this routine is called to remove a single item from the pack.
Whenever we are asked to remove something from a group containing only
a single item, we simply unlink that group from the linked list which
makes up the pack and we return the passed pointer to our caller.  This
works fine.  If there is more than one object in the group being removed
but the calling routine only wants to remove one item from the group, 
then a copy of the group is made and given a count of 1, the count of the 
original group is decremented, and a pointer to the copy is returned.
If the caller wants to remove everything in a group, they will pass
a non-zero value as the third argument and we will simply unlink that
group and return a pointer to the group.  

Note what happens, however, when the caller asks to remove zero items
from a group.  In this case, we simply return the passed pointer to the
caller.  A seemingly innocuous thing to do.  Unfortunately, when the Nymph
steals an item, it does "_leave_pack (ptr, 0, 0)".  If there is only
one item in the group being stolen, then that is handled early on and
we successfully remove the item from the pack.  However, if there is
more than one item in the group, then NOTHING HAPPENS!!!  That is, the
object remains linked into the pack list so some routines think that the
storage is allocated.  Meanwhile, the Nymph attacker thinks it just
took the storage away from the pack so it frees the storage.  My, what
an inconsistant state we are in now!

Well, now all our symptoms are easily explained.  If we ask for an
inventory, we see the items still in our pack.  But with memory in
such an inconsistent state, it is only a matter of time before we
get a memory fault or buss error or some such.  Whenever rogue gets
a fatal error, it traps the error and calls a routine to save the file.
Thus, the file gets saved.  When the game is restarted, the saved
information is still trashed, and pretty soon we will get another
fatal error, causing the game to save again.  Finally, when we go
down a level, rogue begins deallocating and allocating all kinds of
storage.  In particular, it reallocates the piece of storage stolen
by the Nymph.  So when we go down a level and do an inventory, we
are looking at a really random linked list.

So how do we fix this?  I suspect that the programmers of Rogue
(if I may be so bold as to 2nd guess them) meant for the Nymph routine 
to do "_leave_pack (ptr, 0, 1)".  This should work.  Note that
"_leave_pack (ptr, 1, 0)" will not work because the Nymph attack
assumes that the complete group has been removed from the pack.

The code which controls attacking monsters exists in a subroutine
called "_attack".  Partway through this subroutine there is a case
statement.  If we follow the branch taken by case "N", we will eventually
come to a call of _leave_pack.  This call should be adjusted appropriately.
On our Vax 11/750, the call occurs at _attack+0x4ec more or less.  The code
looks like:
    _attack+0x4ec: pushl $0
                   pushl $0
                   pushl r7
                   calls $3,_leave_pack

I have patched this to:
    _attack+0x4ec: pushl $1
                   pushl $0
                   pushl r7
                   calls $3,_leave_pack

I hope this works for you, and I also hope this patch is not yet
common knowledge.

    Chuck Simmons at Dartvax

tims@mako.UUCP (Tim Stoehr) (08/24/84)

[]
I put the nypmph-bug-fix-patch in and it seems to work.  But when the nymph
takes 3 scrolls of scare monster, or whatever, she takes them ALL.  I don't
think that is what was intended, though I must agree it is preferable to
getting nymphed-buggered.