From: nethack.rankin Date: Tue, 5 Nov 2013 00:57:56 +0000 (+0000) Subject: fix #H3039 - panic() when trying to drop destroyed items X-Git-Tag: MOVE2GIT~2 X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=e8e291b018f9d56acb1cc18d7204697512137109;p=nethack fix #H3039 - panic() when trying to drop destroyed items From a bug report, dropping a lit (burning) potion of oil while levitating can produce an explosion which can destroy inventory. If in the process of dropping multiple items, the ones after the oil might be gone, resulting in use of stale pointers and possibly triggering an "extract_nobj: object lost" panic or even a crash. While testing my fix, I discovered that being killed by an exploding potion of oil could produce an "object_is_local" panic if bones are saved (and reproduced with unmodified 3.4.3). --- diff --git a/doc/fixes35.0 b/doc/fixes35.0 index 8aef6e126..f3e1636c7 100644 --- a/doc/fixes35.0 +++ b/doc/fixes35.0 @@ -850,6 +850,10 @@ engraving feedback about partial text when weapon became too dull to finish impossible() might display inaccurate feedback after updating paniclog fix crash which occurred if hero was teleported onto a sink while busy putting on or taking off levitation boots +fix "object lost" panic (or even crash) when dropping multiple items while + levitating and a lit potion of oil explodes and destroys some inventory +fix "object_is_local" panic when saving bones after hero is killed by explosion + produced by dropped or thrown lit potion of oil Platform- and/or Interface-Specific Fixes diff --git a/include/extern.h b/include/extern.h index 2dd1e3e31..13600247d 100644 --- a/include/extern.h +++ b/include/extern.h @@ -701,6 +701,7 @@ E long FDECL(rndexp, (BOOLEAN_P)); E void FDECL(explode, (int,int,int,int,CHAR_P,int)); E long FDECL(scatter, (int, int, int, unsigned int, struct obj *)); E void FDECL(splatter_burning_oil, (int, int)); +E void FDECL(explode_oil, (struct obj *,int,int)); /* ### extralev.c ### */ @@ -2685,6 +2686,8 @@ E struct obj *FDECL(which_armor, (struct monst *,long)); E void FDECL(mon_break_armor, (struct monst *,BOOLEAN_P)); E void FDECL(bypass_obj, (struct obj *)); E void NDECL(clear_bypasses); +E void FDECL(bypass_objlist, (struct obj *,BOOLEAN_P)); +E struct obj *FDECL(nxt_unbypassed_obj, (struct obj *)); E int FDECL(racial_exception, (struct monst *, struct obj *)); /* ### write.c ### */ diff --git a/src/do.c b/src/do.c index 643a0de1d..fa09f7d53 100644 --- a/src/do.c +++ b/src/do.c @@ -705,18 +705,51 @@ int retry; } if (drop_everything) { - for(otmp = invent; otmp; otmp = otmp2) { - otmp2 = otmp->nobj; + /* + * Dropping a burning potion of oil while levitating can cause + * an explosion which might destroy some of hero's inventory, + * so the old code + * for (otmp = invent; otmp; otmp = otmp2) { + * otmp2 = otmp->nobj; + * n_dropped += drop(otmp); + * } + * was unreliable and could lead to an "object lost" panic. + * + * Use the bypass bit to mark items already processed (hence + * not droppable) and rescan inventory until no unbypassed + * items remain. + */ + bypass_objlist(invent, FALSE); /* clear bypass bit for invent */ + while ((otmp = nxt_unbypassed_obj(invent)) != 0) n_dropped += drop(otmp); - } + /* we might not have dropped everything (worn armor, welded weapon, + cursed loadstones), so reset any remaining inventory to normal */ + bypass_objlist(invent, FALSE); } else { /* should coordinate with perm invent, maybe not show worn items */ n = query_objlist("What would you like to drop?", invent, USE_INVLET|INVORDER_SORT, &pick_list, PICK_ANY, all_categories ? allow_all : allow_category); if (n > 0) { + /* + * picklist[] contains a set of pointers into inventory, but + * as soon as something gets dropped, they might become stale + * (see the drop_everything code above for an explanation). + * Just checking to see whether one is still in the invent + * chain is not sufficient validation since destroyed items + * will be freed and items we've split here might have already + * reused that memory and put the same pointer value back into + * invent. Ditto for using invlet to validate. So we start + * by setting bypass on all of invent, then check each pointer + * to verify that it is in invent and has that bit set. + */ + bypass_objlist(invent, TRUE); for (i = 0; i < n; i++) { otmp = pick_list[i].item.a_obj; + for (otmp2 = invent; otmp2; otmp2 = otmp2->nobj) + if (otmp2 == otmp) break; + if (!otmp2 || !otmp2->bypass) continue; + /* found next selected invent item */ cnt = pick_list[i].count; if (cnt < otmp->quan) { if (welded(otmp)) { @@ -735,6 +768,7 @@ int retry; } n_dropped += drop(otmp); } + bypass_objlist(invent, FALSE); /* reset invent to normal */ free((genericptr_t) pick_list); } } diff --git a/src/dothrow.c b/src/dothrow.c index f1461df6e..9ebd06c9a 100644 --- a/src/dothrow.c +++ b/src/dothrow.c @@ -1699,7 +1699,7 @@ boolean from_invent; case POT_WATER: /* really, all potions */ obj->in_use = 1; /* in case it's fatal */ if (obj->otyp == POT_OIL && obj->lamplit) { - splatter_burning_oil(x,y); + explode_oil(obj, x, y); } else if (distu(x,y) <= 2) { if (!breathless(youmonst.data) || haseyes(youmonst.data)) { if (obj->otyp != POT_WATER) { diff --git a/src/explode.c b/src/explode.c index c17748420..a2416dd75 100644 --- a/src/explode.c +++ b/src/explode.c @@ -642,4 +642,16 @@ splatter_burning_oil(x, y) explode(x, y, ZT_SPELL_O_FIRE, d(4,4), BURNING_OIL, EXPL_FIERY); } +/* lit potion of oil is exploding; extinguish it as a light source before + possibly killing the hero and attempting to save bones */ +void +explode_oil(obj, x, y) +struct obj *obj; +int x, y; +{ + if (!obj->lamplit) impossible("exploding unlit oil"); + end_burn(obj, TRUE); + splatter_burning_oil(x, y); +} + /*explode.c*/ diff --git a/src/invent.c b/src/invent.c index 8973a7468..b51151a38 100644 --- a/src/invent.c +++ b/src/invent.c @@ -1490,7 +1490,7 @@ register int allflag, mx; register const char *olets, *word; /* olets is an Obj Class char array */ register int FDECL((*fn),(OBJ_P)), FDECL((*ckfn),(OBJ_P)); { - struct obj *otmp, *otmp2, *otmpo; + struct obj *otmp, *otmpo; register char sym, ilet; register int cnt = 0, dud = 0, tmp; boolean takeoff, nodot, ident, take_out, put_in, first, ininv; @@ -1512,9 +1512,21 @@ nextclass: ilet = 'a'-1; if (*objchn && (*objchn)->oclass == COIN_CLASS) ilet--; /* extra iteration */ - for (otmp = *objchn; otmp; otmp = otmp2) { - if(ilet == 'z') ilet = 'A'; else ilet++; - otmp2 = otmp->nobj; + /* + * Multiple Drop can change the invent chain while it operates + * (dropping a burning potion of oil while levitating creates + * an explosion which can destroy inventory items), so simple + * list traversal + * for (otmp = *objchn; otmp; otmp = otmp2) { + * otmp2 = otmp->nobj; + * ... + * } + * is inadeqate here. Use each object's bypass bit to keep + * track of which list elements have already been processed. + */ + bypass_objlist(*objchn, FALSE); /* clear chain's bypass bits */ + while ((otmp = nxt_unbypassed_obj(*objchn)) != 0) { + if (ilet == 'z') ilet = 'A'; else ilet++; if (olets && *olets && otmp->oclass != *olets) continue; if (takeoff && !is_worn(otmp)) continue; if (ident && !not_fully_identified(otmp)) continue; @@ -1589,6 +1601,7 @@ nextclass: if(!takeoff && (dud || cnt)) pline("That was all."); else if(!dud && !cnt) pline("No applicable objects."); ret: + bypass_objlist(*objchn, FALSE); return(cnt); } diff --git a/src/potion.c b/src/potion.c index 42124f288..ede26f139 100644 --- a/src/potion.c +++ b/src/potion.c @@ -1206,7 +1206,7 @@ boolean your_fault; switch (obj->otyp) { case POT_OIL: if (obj->lamplit) - splatter_burning_oil(u.ux, u.uy); + explode_oil(obj, u.ux, u.uy); break; case POT_POLYMORPH: You_feel("a little %s.", Hallucination ? "normal" : "strange"); @@ -1356,7 +1356,7 @@ boolean your_fault; break; case POT_OIL: if (obj->lamplit) - splatter_burning_oil(mon->mx, mon->my); + explode_oil(obj, mon->mx, mon->my); break; case POT_ACID: if (!resists_acid(mon) && !resist(mon, POTION_CLASS, 0, NOTELL)) { diff --git a/src/worn.c b/src/worn.c index e669d05f8..f750bf410 100644 --- a/src/worn.c +++ b/src/worn.c @@ -673,6 +673,35 @@ struct obj *obj; context.bypasses = TRUE; } +/* set or clear the bypass bit in a list of objects */ +void +bypass_objlist(objchain, on) +struct obj *objchain; +boolean on; /* TRUE => set, FALSE => clear */ +{ + if (on && objchain) context.bypasses = TRUE; + while (objchain) { + objchain->bypass = on ? 1 : 0; + objchain = objchain->nobj; + } +} + +/* return the first object without its bypass bit set; set that bit + before returning so that successive calls will find further objects */ +struct obj * +nxt_unbypassed_obj(objchain) +struct obj *objchain; +{ + while (objchain) { + if (!objchain->bypass) { + bypass_obj(objchain); + break; + } + objchain = objchain->nobj; + } + return objchain; +} + void mon_break_armor(mon, polyspot) struct monst *mon; diff --git a/src/zap.c b/src/zap.c index df7d3faac..e265b898d 100644 --- a/src/zap.c +++ b/src/zap.c @@ -1710,6 +1710,9 @@ struct obj *obj, *otmp; * meat; prevent those contents from being hit. * retouch_equipment() - bypass flag is used to track which * items have been handled (bhito isn't involved). + * menu_drop(), askchain() - inventory traversal where multiple + * Drop can alter the invent chain while traversal + * is in progress (bhito isn't involved). * * The bypass bit on all objects is reset each turn, whenever * context.bypasses is set.