From 3ce02acbc537449347ca6c6d03a9ce9a095bcc32 Mon Sep 17 00:00:00 2001 From: PatR Date: Mon, 20 Mar 2023 17:05:02 -0700 Subject: [PATCH] fix #K3888 - object lost panic cased by lava Report was for spell-casting monster using the destroy armor spell on hero's levitation boots while hero was floating over lava. The boots became unworn but still in inventory, hero dropped into lava, the boots happened to be an inventory item which got burned up, then the call stack unwound back to the destroy armor routine which tried to finish by deleting them but they were already gone by then. Could also happen for black dragon breath, hero reading scroll of destroy armor, or overenchanting the boots with scroll of enchant armor, so not so unlikely that nobody would be expected to notice. Initially I couldn't reproduce the object lost panic. It only happens if the memory used by the boots is cleared or clobbered during first time it's freed, otherwise second free doesn't notice any problem. The 'wornarm_destroyed()' portion of this commit is sufficient to fix this. The other bits are things I tried before figuring out how to reproduce it, plus zeroing out any object passed to dealloc_obj(). --- doc/fixes3-7-0.txt | 6 +++- src/do_wear.c | 68 ++++++++++++++++++++++++++++++++++------------ src/mkobj.c | 6 ++++ src/trap.c | 9 +++++- 4 files changed, 70 insertions(+), 19 deletions(-) diff --git a/doc/fixes3-7-0.txt b/doc/fixes3-7-0.txt index a6ad5219c..47dd71311 100644 --- a/doc/fixes3-7-0.txt +++ b/doc/fixes3-7-0.txt @@ -1121,7 +1121,11 @@ feedback if a named, shape-shifted vampire reverted to original shape rather adjust archeologist and valkyrie starting intrinsics once per game if receiving killing blow from near-full hp, leave 1 hp spell of knock can knock back small monsters -protection from shape changers prevents the Wizard from mimicking monsters +protection from shape changers now prevents the Wizard from mimicking monsters +having worn levitation boots removed and destroyed (scroll, monster spell, + dragon breath) and floating down into lava can destroy them twice; + yielded "object lost" panic if program had been built with a debugging + malloc implementation which overwrites the contents of freed memory Fixes to 3.7.0-x Problems that Were Exposed Via git Repository diff --git a/src/do_wear.c b/src/do_wear.c index f95f99b2d..22bbe3721 100644 --- a/src/do_wear.c +++ b/src/do_wear.c @@ -38,6 +38,7 @@ static int select_off(struct obj *); static struct obj *do_takeoff(void); static int take_off(void); static int menu_remarm(int); +static void wornarm_destroyed(struct obj *); static void count_worn_stuff(struct obj **, boolean); static int armor_or_accessory_off(struct obj *); static int accessory_or_armor_on(struct obj *); @@ -239,7 +240,8 @@ Boots_off(void) case WATER_WALKING_BOOTS: /* check for lava since fireproofed boots make it viable */ if ((is_pool(u.ux, u.uy) || is_lava(u.ux, u.uy)) - && !Levitation && !Flying && !is_clinger(gy.youmonst.data) + && !Levitation && !Flying + && !(is_clinger(gy.youmonst.data) && has_ceiling(&u.uz)) && !gc.context.takeoff.cancelled_don /* avoid recursive call to lava_effects() */ && !iflags.in_lava_effects) { @@ -258,7 +260,10 @@ Boots_off(void) case LEVITATION_BOOTS: if (!oldprop && !HLevitation && !(BLevitation & FROMOUTSIDE) && !gc.context.takeoff.cancelled_don) { - (void) float_down(0L, 0L); + /* lava_effects() sets in_lava_effects and calls Boots_off() + so hero is already in midst of floating down */ + if (!iflags.in_lava_effects) + (void) float_down(0L, 0L); makeknown(otyp); } else { float_vs_flight(); /* maybe toggle (BFlying & I_SPECIAL) */ @@ -2933,9 +2938,45 @@ menu_remarm(int retry) return 0; } +/* take off the specific worn object and if it still exists after that, + destroy it (taking off the item might already destroy it by dunking + hero into lava) */ +static void +wornarm_destroyed(struct obj *wornarm) +{ + struct obj *invobj; + unsigned wornoid = wornarm->o_id; + + if (wornarm == uarmc) + (void) Cloak_off(); + else if (wornarm == uarm) + (void) Armor_off(); + else if (wornarm == uarmu) + (void) Shirt_off(); + else if (wornarm == uarmh) + (void) Helmet_off(); + else if (wornarm == uarmg) + (void) Gloves_off(); + else if (wornarm == uarmf) + (void) Boots_off(); + else if (wornarm == uarms) + (void) Shield_off(); + + /* 'wornarm' might be destroyed as a side-effect of xxx_off() so + using carried() to check wornarm->where==OBJ_INVENT is not viable; + scan invent instead; if already freed it shouldn't be possible to + have re-used the stale memory for a new item yet but verify o_id + just in case */ + for (invobj = gi.invent; invobj; invobj = invobj->nobj) + if (invobj == wornarm && invobj->o_id == wornoid) { + useup(wornarm); + break; + } +} + /* hit by destroy armor scroll/black dragon breath/monster spell */ int -destroy_arm(register struct obj *atmp) +destroy_arm(struct obj *atmp) { struct obj *otmp; /* @@ -2957,8 +2998,7 @@ destroy_arm(register struct obj *atmp) urgent_pline("Your %s crumbles and turns to dust!", /* cloak/robe/apron/smock (ID'd apron)/wrapping */ cloak_simple_name(uarmc)); - (void) Cloak_off(); - useup(otmp); + wornarm_destroyed(uarmc); } else if (DESTROY_ARM(uarm)) { const char *suit = suit_simple_name(uarm); @@ -2972,41 +3012,35 @@ destroy_arm(register struct obj *atmp) /* suit might be "dragon scales" so vtense() is needed */ suit, vtense(suit, "turn"), vtense(suit, "fall"), surface(u.ux, u.uy)); - (void) Armor_gone(); - useup(otmp); + wornarm_destroyed(uarm); } else if (DESTROY_ARM(uarmu)) { if (donning(otmp)) cancel_don(); urgent_pline("Your %s crumbles into tiny threads and falls apart!", shirt_simple_name(uarmu)); /* always "shirt" */ - (void) Shirt_off(); - useup(otmp); + wornarm_destroyed(uarmu); } else if (DESTROY_ARM(uarmh)) { if (donning(otmp)) cancel_don(); urgent_pline("Your %s turns to dust and is blown away!", helm_simple_name(uarmh)); /* "helm" or "hat" */ - (void) Helmet_off(); - useup(otmp); + wornarm_destroyed(uarmh); } else if (DESTROY_ARM(uarmg)) { if (donning(otmp)) cancel_don(); urgent_pline("Your %s vanish!", gloves_simple_name(uarmg)); - (void) Gloves_off(); - useup(otmp); + wornarm_destroyed(uarmg); selftouch("You"); } else if (DESTROY_ARM(uarmf)) { if (donning(otmp)) cancel_don(); urgent_pline("Your %s disintegrate!", boots_simple_name(uarmf)); - (void) Boots_off(); - useup(otmp); + wornarm_destroyed(uarmf); } else if (DESTROY_ARM(uarms)) { if (donning(otmp)) cancel_don(); urgent_pline("Your %s crumbles away!", shield_simple_name(uarms)); - (void) Shield_off(); - useup(otmp); + wornarm_destroyed(uarms); } else { return 0; /* could not destroy anything */ } diff --git a/src/mkobj.c b/src/mkobj.c index 87f36dc15..3bcdb26b9 100644 --- a/src/mkobj.c +++ b/src/mkobj.c @@ -2635,6 +2635,12 @@ dealloc_obj(struct obj *obj) obj->where = OBJ_LUAFREE; return; } +#ifdef DEBUG + /* clobber out of date information contained in the about-to-become + stale memory; do this before 'free()' in case a debugging malloc + implementation overwrites the memory with something else */ + *obj = cg.zeroobj; +#endif free((genericptr_t) obj); } diff --git a/src/trap.c b/src/trap.c index ef385a3cf..698cdd50b 100644 --- a/src/trap.c +++ b/src/trap.c @@ -3607,7 +3607,7 @@ float_down( if (is_pool(u.ux, u.uy) && !Wwalking && !Swimming && !u.uinwater) no_msg = drown(); - if (is_lava(u.ux, u.uy)) { + if (is_lava(u.ux, u.uy) && !iflags.in_lava_effects) { (void) lava_effects(); no_msg = TRUE; } @@ -6050,6 +6050,9 @@ unconscious(void) static const char lava_killer[] = "molten lava"; +/* hero enters pool of molten lava; returns True if hero is killed and + then life-saved (with teleport to safe spot), False for other survival; + no return at all if hero dies and isn't life-saved */ boolean lava_effects(void) { @@ -6057,6 +6060,10 @@ lava_effects(void) int dmg = d(6, 6); /* only applicable for water walking */ boolean usurvive, boil_away; + if (iflags.in_lava_effects) { + debugpline0("Skipping recursive lava_effects()."); + return FALSE; + } feel_newsym(u.ux, u.uy); /* in case Blind, map the lava here */ burn_away_slime(); if (likes_lava(gy.youmonst.data))