diff --git a/doc/fixes37.0 b/doc/fixes37.0 index 0fc0401a4..4794c86ad 100644 --- a/doc/fixes37.0 +++ b/doc/fixes37.0 @@ -1,4 +1,4 @@ -NHDT-Branch: NetHack-3.7 $:$NHDT-Revision: 1.369 $ $NHDT-Date: 1606781767 2020/12/01 00:16:07 $ +NHDT-Branch: NetHack-3.7 $:$NHDT-Revision: 1.370 $ $NHDT-Date: 1606919254 2020/12/02 14:27:34 $ General Fixes and Modified Features ----------------------------------- @@ -316,6 +316,8 @@ autodescribe when moving the cursor was erroneously honoring MSGTYPE=stop and potentially delivering sounds reduce the number of "seeXYZ" commands by renaming some: #seenv -> #wizseenv, #seespells -> #showspells, and #seetrap -> #showtrap +when saving while punished or game ends while punished, handling for ball and + chain might access freed memory with unpredictable consequences Fixes to 3.7.0-x Problems that Were Exposed Via git Repository diff --git a/include/decl.h b/include/decl.h index def2db1cb..6cc5f75bb 100644 --- a/include/decl.h +++ b/include/decl.h @@ -1,4 +1,4 @@ -/* NetHack 3.7 decl.h $NHDT-Date: 1606765210 2020/11/30 19:40:10 $ $NHDT-Branch: NetHack-3.7 $:$NHDT-Revision: 1.246 $ */ +/* NetHack 3.7 decl.h $NHDT-Date: 1606919254 2020/12/02 14:27:34 $ $NHDT-Branch: NetHack-3.7 $:$NHDT-Revision: 1.247 $ */ /* Copyright (c) Stichting Mathematisch Centrum, Amsterdam, 1985. */ /*-Copyright (c) Michael Allison, 2007. */ /* NetHack may be freely redistributed. See license for details. */ @@ -1103,6 +1103,8 @@ struct instance_globals { boolean havestate; unsigned ustuck_id; /* need to preserve during save */ unsigned usteed_id; /* need to preserve during save */ + struct obj *looseball; /* track uball during save and... */ + struct obj *loosechain; /* track uchain since saving might free it */ /* shk.c */ /* auto-response flag for/from "sell foo?" 'a' => 'y', 'q' => 'n' */ diff --git a/include/extern.h b/include/extern.h index 3be154e70..64df72baa 100644 --- a/include/extern.h +++ b/include/extern.h @@ -1,4 +1,4 @@ -/* NetHack 3.7 extern.h $NHDT-Date: 1606343573 2020/11/25 22:32:53 $ $NHDT-Branch: NetHack-3.7 $:$NHDT-Revision: 1.882 $ */ +/* NetHack 3.7 extern.h $NHDT-Date: 1606919254 2020/12/02 14:27:34 $ $NHDT-Branch: NetHack-3.7 $:$NHDT-Revision: 1.886 $ */ /* Copyright (c) Steve Creps, 1988. */ /* NetHack may be freely redistributed. See license for details. */ @@ -3122,6 +3122,7 @@ E void FDECL(flip_worm_segs_horizontal, (struct monst *, int, int)); E void FDECL(setworn, (struct obj *, long)); E void FDECL(setnotworn, (struct obj *)); +E void NDECL(allunworn); E struct obj *FDECL(wearmask_to_obj, (long)); E long FDECL(wearslot, (struct obj *)); E void FDECL(mon_set_minvis, (struct monst *)); diff --git a/src/decl.c b/src/decl.c index 54d38ac6d..09e6d608a 100644 --- a/src/decl.c +++ b/src/decl.c @@ -1,4 +1,4 @@ -/* NetHack 3.7 decl.c $NHDT-Date: 1600468453 2020/09/18 22:34:13 $ $NHDT-Branch: NetHack-3.7 $:$NHDT-Revision: 1.218 $ */ +/* NetHack 3.7 decl.c $NHDT-Date: 1606919256 2020/12/02 14:27:36 $ $NHDT-Branch: NetHack-3.7 $:$NHDT-Revision: 1.221 $ */ /* Copyright (c) Stichting Mathematisch Centrum, Amsterdam, 1985. */ /*-Copyright (c) Michael Allison, 2009. */ /* NetHack may be freely redistributed. See license for details. */ @@ -611,6 +611,8 @@ const struct instance_globals g_init = { TRUE, /* havestate*/ 0, /* ustuck_id */ 0, /* usteed_id */ + (struct obj *) 0, /* looseball */ + (struct obj *) 0, /* loosechain */ /* shk.c */ 'a', /* sell_response */ diff --git a/src/display.c b/src/display.c index 1c4707b04..1e0ac89e7 100644 --- a/src/display.c +++ b/src/display.c @@ -1,4 +1,4 @@ -/* NetHack 3.7 display.c $NHDT-Date: 1597700875 2020/08/17 21:47:55 $ $NHDT-Branch: NetHack-3.7 $:$NHDT-Revision: 1.136 $ */ +/* NetHack 3.7 display.c $NHDT-Date: 1606919261 2020/12/02 14:27:41 $ $NHDT-Branch: NetHack-3.7 $:$NHDT-Revision: 1.139 $ */ /* Copyright (c) Dean Luick, with acknowledgements to Kevin Darcy */ /* and Dave Cohrs, 1990. */ /* NetHack may be freely redistributed. See license for details. */ @@ -1691,6 +1691,10 @@ int cursor_on_u; static int delay_flushing = 0; register int x, y; + /* 3.7: don't update map, status, or perm_invent during save/restore */ + if (g.program_state.saving || g.program_state.restoring) + return; + if (cursor_on_u == -1) delay_flushing = !delay_flushing; if (delay_flushing) diff --git a/src/save.c b/src/save.c index 586ae177d..c8f1865da 100644 --- a/src/save.c +++ b/src/save.c @@ -1,4 +1,4 @@ -/* NetHack 3.7 save.c $NHDT-Date: 1596498207 2020/08/03 23:43:27 $ $NHDT-Branch: NetHack-3.7 $:$NHDT-Revision: 1.160 $ */ +/* NetHack 3.7 save.c $NHDT-Date: 1606919257 2020/12/02 14:27:37 $ $NHDT-Branch: NetHack-3.7 $:$NHDT-Revision: 1.163 $ */ /* Copyright (c) Stichting Mathematisch Centrum, Amsterdam, 1985. */ /*-Copyright (c) Michael Allison, 2009. */ /* NetHack may be freely redistributed. See license for details. */ @@ -29,7 +29,7 @@ static void FDECL(saveobj, (NHFILE *,struct obj *)); static void FDECL(savemon, (NHFILE *,struct monst *)); static void FDECL(savelevl, (NHFILE *,BOOLEAN_P)); static void FDECL(save_stairs, (NHFILE *)); -static void FDECL(saveobjchn, (NHFILE *,struct obj *)); +static void FDECL(saveobjchn, (NHFILE *,struct obj **)); static void FDECL(savemonchn, (NHFILE *,struct monst *)); static void FDECL(savetrapchn, (NHFILE *,struct trap *)); static void FDECL(savegamestate, (NHFILE *)); @@ -86,7 +86,9 @@ dosave0() d_level uz_save; char whynot[BUFSZ]; NHFILE *nhfp, *onhfp; + int res = 0; + g.program_state.saving++; /* inhibit status and perm_invent updates */ /* we may get here via hangup signal, in which case we want to fix up a few of things before saving so that they won't be restored in an improper state; these will be no-ops for normal save sequence */ @@ -99,9 +101,9 @@ dosave0() u.uburied = 1, iflags.save_uburied = 0; if (!g.program_state.something_worth_saving || !g.SAVEF[0]) - return 0; - fq_save = fqname(g.SAVEF, SAVEPREFIX, 1); /* level files take 0 */ + goto done; + fq_save = fqname(g.SAVEF, SAVEPREFIX, 1); /* level files take 0 */ #ifndef NO_SIGNAL #if defined(UNIX) || defined(VMS) sethanguphandler((void FDECL((*), (int) )) SIG_IGN); @@ -118,7 +120,7 @@ dosave0() There("seems to be an old save file."); if (yn("Overwrite the old file?") == 'n') { nh_compress(fq_save); - return 0; + goto done; } } } @@ -129,7 +131,7 @@ dosave0() if (!nhfp) { HUP pline("Cannot open save file."); (void) delete_savefile(); /* ab@unido */ - return 0; + goto done; } vision_recalc(2); /* shut down vision to prevent problems @@ -158,6 +160,15 @@ dosave0() store_plname_in_file(nhfp); g.ustuck_id = (u.ustuck ? u.ustuck->m_id : 0); g.usteed_id = (u.usteed ? u.usteed->m_id : 0); + /* savelev() might save uball and uchain, releasing their memory if + FREEING, so we need to check their status now; if hero is swallowed, + uball and uchain will persist beyond saving map floor and inventory + so these copies of their pointers will be valid and savegamestate() + will know to save them separately (from floor and invent); when not + swallowed, uchain will be stale by then, and uball will be too if + ball is on the floor rather than carried */ + g.looseball = BALL_IN_MON ? uball : 0; + g.loosechain = CHAIN_IN_MON ? uchain : 0; savelev(nhfp, ledger_no(&u.uz)); savegamestate(nhfp); @@ -197,7 +208,7 @@ dosave0() (void) delete_savefile(); HUP Strcpy(g.killer.name, whynot); HUP done(TRICKED); - return 0; + goto done; } minit(); /* ZEROCOMP */ getlev(onhfp, g.hackpid, ltmp); @@ -217,7 +228,11 @@ dosave0() nh_compress(fq_save); /* this should probably come sooner... */ g.program_state.something_worth_saving = 0; - return 1; + res = 1; + + done: + g.program_state.saving--; + return res; } static void @@ -225,7 +240,10 @@ savegamestate(nhfp) NHFILE *nhfp; { unsigned long uid; - struct obj * bc_objs = (struct obj *)0; + struct obj *bc_objs = (struct obj *)0; + + if (!g.program_state.saving) + impossible("savegamestate called when not saving or changing levels?"); uid = (unsigned long) getuid(); if (nhfp->structlevel) { @@ -251,27 +269,31 @@ NHFILE *nhfp; save_timers(nhfp, RANGE_GLOBAL); save_light_sources(nhfp, RANGE_GLOBAL); - saveobjchn(nhfp, g.invent); + /* when FREEING, deletes objects in invent and sets invent to Null; + pointers into invent (uwep, uarmg, uamul, &c) are set to Null too */ + saveobjchn(nhfp, &g.invent); - /* save ball and chain if they are currently dangling free (i.e. not on - floor or in inventory) */ - if (CHAIN_IN_MON) { - uchain->nobj = bc_objs; - bc_objs = uchain; + /* save ball and chain if they are currently dangling free (i.e. not + on floor or in inventory); 'looseball' and 'loosechain' have been + set up in caller because ball and chain will be gone by now if on + floor, or ball gone if carried; caveat: uball and uchain pointers + will be non-Null but stale (point to freed memory) in those cases */ + bc_objs = (struct obj *) 0; + if (g.loosechain) { + g.loosechain->nobj = bc_objs; /* uchain */ + bc_objs = g.loosechain; } - if (BALL_IN_MON) { - uball->nobj = bc_objs; - bc_objs = uball; + if (g.looseball) { + g.looseball->nobj = bc_objs; + bc_objs = g.looseball; } - saveobjchn(nhfp, bc_objs); + saveobjchn(nhfp, &bc_objs); /* frees objs in list, sets bc_objs to Null */ + g.looseball = g.loosechain = (struct obj *) 0; - saveobjchn(nhfp, g.migrating_objs); + saveobjchn(nhfp, &g.migrating_objs); /* frees objs and sets to Null */ savemonchn(nhfp, g.migrating_mons); - if (release_data(nhfp)) { - g.invent = 0; - g.migrating_objs = 0; - g.migrating_mons = 0; - } + if (release_data(nhfp)) + g.migrating_mons = (struct monst *) 0; if (nhfp->structlevel) bwrite(nhfp->fd, (genericptr_t) g.mvitals, sizeof g.mvitals); save_dungeon(nhfp, (boolean) !!perform_bwrite(nhfp), @@ -329,6 +351,7 @@ savestateinlock() char whynot[BUFSZ]; NHFILE *nhfp; + g.program_state.saving++; /* inhibit status and perm_invent updates */ /* When checkpointing is on, the full state needs to be written * on each checkpoint. When checkpointing is off, only the pid * needs to be in the level.0 file, so it does not need to be @@ -356,16 +379,21 @@ savestateinlock() if (g.hackpid != hpid) { Sprintf(whynot, "Level #0 pid (%d) doesn't match ours (%d)!", hpid, g.hackpid); - pline1(whynot); - Strcpy(g.killer.name, whynot); - done(TRICKED); + goto giveup; } close_nhfile(nhfp); nhfp = create_levelfile(0, whynot); if (!nhfp) { pline1(whynot); + giveup: Strcpy(g.killer.name, whynot); + /* done(TRICKED) will return when running in wizard mode; + clear the display-update-suppression flag before rather + than after so that screen updating behaves normally; + game data shouldn't be inconsistent yet, unlike it would + become midway through saving */ + g.program_state.saving--; done(TRICKED); return; } @@ -384,10 +412,13 @@ savestateinlock() g.ustuck_id = (u.ustuck ? u.ustuck->m_id : 0); g.usteed_id = (u.usteed ? u.usteed->m_id : 0); + g.looseball = BALL_IN_MON ? uball : 0; + g.loosechain = CHAIN_IN_MON ? uchain : 0; savegamestate(nhfp); } close_nhfile(nhfp); } + g.program_state.saving--; g.havestate = flags.ins_chkpt; } #endif @@ -401,6 +432,7 @@ xchar lev; short tlev; #endif + g.program_state.saving++; /* even if current mode is FREEING */ /* * Level file contents: * version info (handled by caller); @@ -454,7 +486,8 @@ xchar lev; if (nhfp->mode == FREEING) /* see above */ goto skip_lots; - savelevl(nhfp, (boolean) ((sfsaveinfo.sfi1 & SFI1_RLECOMP) == SFI1_RLECOMP)); + savelevl(nhfp, + (boolean) ((sfsaveinfo.sfi1 & SFI1_RLECOMP) == SFI1_RLECOMP)); if (nhfp->structlevel) { bwrite(nhfp->fd, (genericptr_t) g.lastseentyp, sizeof g.lastseentyp); bwrite(nhfp->fd, (genericptr_t) &g.monstermoves, sizeof g.monstermoves); @@ -475,9 +508,9 @@ xchar lev; savemonchn(nhfp, fmon); save_worm(nhfp); /* save worm information */ savetrapchn(nhfp, g.ftrap); - saveobjchn(nhfp, fobj); - saveobjchn(nhfp, g.level.buriedobjlist); - saveobjchn(nhfp, g.billobjs); + saveobjchn(nhfp, &fobj); + saveobjchn(nhfp, &g.level.buriedobjlist); + saveobjchn(nhfp, &g.billobjs); if (release_data(nhfp)) { int x,y; /* TODO: maybe use clear_level_structures() */ @@ -503,6 +536,7 @@ xchar lev; if (nhfp->structlevel) bflush(nhfp->fd); } + g.program_state.saving--; } static void @@ -695,11 +729,13 @@ NHFILE *nhfp; } static void -saveobjchn(nhfp, otmp) +saveobjchn(nhfp, obj_p) NHFILE *nhfp; -register struct obj *otmp; +struct obj **obj_p; { - register struct obj *otmp2; + register struct obj *otmp = *obj_p; + struct obj *otmp2; + boolean is_invent = (otmp && otmp == g.invent); int minusone = -1; while (otmp) { @@ -708,7 +744,7 @@ register struct obj *otmp; saveobj(nhfp, otmp); } if (Has_contents(otmp)) - saveobjchn(nhfp, otmp->cobj); + saveobjchn(nhfp, &otmp->cobj); if (release_data(nhfp)) { /* * If these are on the floor, the discarding could be @@ -733,6 +769,10 @@ register struct obj *otmp; otmp->cobj = NULL; /* contents handled above */ otmp->timed = 0; /* not timed any more */ otmp->lamplit = 0; /* caller handled lights */ + otmp->leashmon = 0; /* mon->mleashed could still be set; + * unfortunately, we can't clear that + * until after the monster is saved */ + otmp->owornmask = 0L; /* no longer care */ dealloc_obj(otmp); } otmp = otmp2; @@ -741,6 +781,11 @@ register struct obj *otmp; if (nhfp->structlevel) bwrite(nhfp->fd, (genericptr_t) &minusone, sizeof (int)); } + if (release_data(nhfp)) { + if (is_invent) + allunworn(); /* clear uwep, uarm, uball, &c pointers */ + *obj_p = (struct obj *) 0; + } } static void @@ -825,7 +870,7 @@ register struct monst *mtmp; savemon(nhfp, mtmp); } if (mtmp->minvent) - saveobjchn(nhfp, mtmp->minvent); + saveobjchn(nhfp, &mtmp->minvent); if (release_data(nhfp)) { if (mtmp == g.context.polearm.hitmon) { g.context.polearm.m_id = mtmp->m_id; @@ -1033,7 +1078,7 @@ freedynamicdata() tmp_at(DISP_FREEMEM, 0); /* temporary display effects */ #ifdef FREE_ALL_MEMORY #define free_current_level() savelev(&tnhfp, -1) -#define freeobjchn(X) (saveobjchn(&tnhfp, X), X = 0) +#define freeobjchn(X) (saveobjchn(&tnhfp, &X), X = 0) #define freemonchn(X) (savemonchn(&tnhfp, X), X = 0) #define freefruitchn() savefruitchn(&tnhfp) #define freenames() savenames(&tnhfp) diff --git a/src/worn.c b/src/worn.c index ad52e2474..f5c4c3a1d 100644 --- a/src/worn.c +++ b/src/worn.c @@ -1,4 +1,4 @@ -/* NetHack 3.7 worn.c $NHDT-Date: 1596498231 2020/08/03 23:43:51 $ $NHDT-Branch: NetHack-3.7 $:$NHDT-Revision: 1.67 $ */ +/* NetHack 3.7 worn.c $NHDT-Date: 1606919259 2020/12/02 14:27:39 $ $NHDT-Branch: NetHack-3.7 $:$NHDT-Revision: 1.70 $ */ /* Copyright (c) Stichting Mathematisch Centrum, Amsterdam, 1985. */ /*-Copyright (c) Robert Patrick Rankin, 2013. */ /* NetHack may be freely redistributed. See license for details. */ @@ -131,7 +131,7 @@ register struct obj *obj; is pending (via 'A' command for multiple items) */ cancel_doff(obj, wp->w_mask); - *(wp->w_obj) = 0; + *(wp->w_obj) = (struct obj *) 0; p = objects[obj->otyp].oc_oprop; u.uprops[p].extrinsic = u.uprops[p].extrinsic & ~wp->w_mask; obj->owornmask &= ~wp->w_mask; @@ -145,6 +145,24 @@ register struct obj *obj; update_inventory(); } +/* called when saving with FREEING flag set has just discarded inventory */ +void +allunworn() +{ + const struct worn *wp; + + u.twoweap = 0; /* uwep and uswapwep are going away */ + /* remove stale pointers; called after the objects have been freed + (without first being unworn) while saving invent during game save; + note: uball and uchain might not be freed yet but we clear them + here anyway (savegamestate() and its callers deal with them) */ + for (wp = worn; wp->w_mask; wp++) { + /* object is already gone so we don't/can't update is owornmask */ + *(wp->w_obj) = (struct obj *) 0; + } +} + + /* return item worn in slot indiciated by wornmask; needed by poly_obj() */ struct obj * wearmask_to_obj(wornmask)