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).

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

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 ### */

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);
 	}
     }

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) {

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*/

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);
 }
 

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)) {

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;

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.