From 02259dd1d06a4db14357a043f98fa1e83ca86e29 Mon Sep 17 00:00:00 2001 From: nhmall Date: Mon, 9 Dec 2024 15:52:29 -0500 Subject: [PATCH] light source troubleshooting I don't think this solves the recent light source reports, but it changes a couple of things in an attempt to get more information. 1. Having gy.youmonst.m_id field always be zero makes it tough to distinguish it from uninitialized memory, or a random memory value. This changes the m_id for the hero's gy.youmonst.m_id to always hold the identifier 1, instead of 0. 2. write_ls was taking the stashed pointer in the light source, and using it to immediately extract the m_id field and search for that m_id. This changes the approach slightly, to actually try and locate the stashed pointer itself in one of the monster chains. Only if the monster pointer is located, do we dereference it to obtain the m_id field. 3. For the interim, mark the saved ls with another set bit when there has been a failure to locate the monst. At this time, no code is acting on that bit, but it can be seen in a debug session. Hopefully, the next report will provide enough information to understand the scenario a little better. --- include/hack.h | 5 ++-- src/allmain.c | 2 +- src/light.c | 79 +++++++++++++++++++++++++++++++++++++++++++------- src/mkobj.c | 2 +- src/polyself.c | 7 +++-- 5 files changed, 77 insertions(+), 18 deletions(-) diff --git a/include/hack.h b/include/hack.h index 40b4d4ada..09e41407e 100644 --- a/include/hack.h +++ b/include/hack.h @@ -1256,11 +1256,12 @@ typedef uint32_t mmflags_nht; /* makemon MM_ flags */ /* flag for suppressing perm_invent update when name gets assigned */ #define ONAME_SKIP_INVUPD 0x0200U /* don't call update_inventory() */ -/* Flags to control find_mid() */ +/* Flags to control find_mid() and whereis_mon() */ #define FM_FMON 0x01 /* search the fmon chain */ #define FM_MIGRATE 0x02 /* search the migrating monster chain */ #define FM_MYDOGS 0x04 /* search gm.mydogs */ -#define FM_EVERYWHERE (FM_FMON | FM_MIGRATE | FM_MYDOGS) +#define FM_YOU 0x08 /* check for gy.youmonst */ +#define FM_EVERYWHERE (FM_YOU | FM_FMON | FM_MIGRATE | FM_MYDOGS) /* Flags to control pick_[race,role,gend,align] routines in role.c */ #define PICK_RANDOM 0 diff --git a/src/allmain.c b/src/allmain.c index c10446417..852904683 100644 --- a/src/allmain.c +++ b/src/allmain.c @@ -731,7 +731,7 @@ newgame(void) /* make sure welcome messages are given before noticing monsters */ notice_mon_off(); disp.botlx = TRUE; - svc.context.ident = 1; + svc.context.ident = 2; /* id 1 is reserved for gy.youmonst */ svc.context.warnlevel = 1; svc.context.next_attrib_check = 600L; /* arbitrary first setting */ svc.context.tribute.enabled = TRUE; /* turn on 3.6 tributes */ diff --git a/src/light.c b/src/light.c index 5ab517177..6675e0d86 100644 --- a/src/light.c +++ b/src/light.c @@ -13,8 +13,8 @@ * Light sources are "things" that have a physical position and range. * They have a type, which gives us information about them. Currently * they are only attached to objects and monsters. Note well: the - * polymorphed-player handling assumes that both gy.youmonst.m_id and - * gy.youmonst.mx will always remain 0. + * polymorphed-player handling assumes that gy.youmonst.m_id will + * always remain 1 and gy.youmonst.mx will always remain 0. * * Light sources, like timers, either follow game play (RANGE_GLOBAL) or * stay on a level (RANGE_LEVEL). Light sources are unique by their @@ -38,8 +38,9 @@ */ /* flags */ -#define LSF_SHOW 0x1 /* display the light source */ -#define LSF_NEEDS_FIXUP 0x2 /* need oid fixup */ +#define LSF_SHOW 0x1 /* display the light source */ +#define LSF_NEEDS_FIXUP 0x2 /* need oid fixup */ +#define LSF_IS_PROBLEMATIC 0x4 /* impossible situation encountered */ staticfn light_source *new_light_core(coordxy, coordxy, int, int, anything *) NONNULLPTRS; @@ -47,6 +48,7 @@ staticfn void delete_ls(light_source *); staticfn void discard_flashes(void); staticfn void write_ls(NHFILE *, light_source *); staticfn int maybe_write_ls(NHFILE *, int, boolean); +staticfn unsigned whereis_mon(struct monst *, unsigned); /* imported from vision.c, for small circles */ extern const coordxy circle_data[]; @@ -374,7 +376,7 @@ find_mid(unsigned nid, unsigned fmflags) { struct monst *mtmp; - if (!nid) + if ((fmflags & FM_YOU) && nid == 1) return &gy.youmonst; if (fmflags & FM_FMON) for (mtmp = fmon; mtmp; mtmp = mtmp->nmon) @@ -391,6 +393,28 @@ find_mid(unsigned nid, unsigned fmflags) return (struct monst *) 0; } +staticfn unsigned +whereis_mon(struct monst *mon, unsigned fmflags) +{ + struct monst *mtmp; + + if ((fmflags & FM_YOU) && mon == &gy.youmonst) + return FM_YOU; + if (fmflags & FM_FMON) + for (mtmp = fmon; mtmp; mtmp = mtmp->nmon) + if (mtmp == mon) + return FM_FMON; + if (fmflags & FM_MIGRATE) + for (mtmp = gm.migrating_mons; mtmp; mtmp = mtmp->nmon) + if (mtmp == mon) + return FM_MIGRATE; + if (fmflags & FM_MYDOGS) + for (mtmp = gm.mydogs; mtmp; mtmp = mtmp->nmon) + if (mtmp == mon) + return FM_MYDOGS; + return 0; +} + /* Save all light sources of the given range. */ void save_light_sources(NHFILE *nhfp, int range) @@ -624,22 +648,55 @@ write_ls(NHFILE *nhfp, light_source *ls) otmp = ls->id.a_obj; ls->id = cg.zeroany; ls->id.a_uint = otmp->o_id; - if (find_oid((unsigned) ls->id.a_uint) != otmp) + if (find_oid((unsigned) ls->id.a_uint) != otmp) { impossible("write_ls: can't find obj #%u!", ls->id.a_uint); + ls->flags |= LSF_IS_PROBLEMATIC; + } } else { /* ls->type == LS_MONSTER */ + unsigned monloc = 0; + mtmp = (struct monst *) ls->id.a_monst; - ls->id = cg.zeroany; - ls->id.a_uint = mtmp->m_id; - if (find_mid((unsigned) ls->id.a_uint, FM_EVERYWHERE) != mtmp) - impossible("write_ls: can't find mon #%u!", - ls->id.a_uint); + + /* The monster pointer has been stashed in the light source + * for a while and while there is code meant to clean-up the + * light source aspects if a monster goes away, there have + * been some reports of light source issues, such as when + * going to the planes. + * + * Verify that the stashed monst pointer is still present + * in one of the monster chains before pulling subfield + * values such as m_id from it, to avoid any attempt to + * pull random m_id value from (now) freed memory. + * + * find_mid() disregards a DEADMONSTER, but whereis_mon() + * does not. */ + + if ((monloc = whereis_mon(mtmp, FM_EVERYWHERE)) != 0) { + ls->id = cg.zeroany; + ls->id.a_uint = mtmp->m_id; + if (find_mid((unsigned) ls->id.a_uint, monloc) != mtmp) { + impossible("write_ls: can't find mon #%u%s!", + DEADMONSTER(mtmp) ? " because it's dead" + : "", + ls->id.a_uint); + ls->flags |= LSF_IS_PROBLEMATIC; + } + } else { + impossible( + "write_ls: stashed monst ptr not in any chain"); + ls->flags |= LSF_IS_PROBLEMATIC; + } + } + if (ls->flags & LSF_IS_PROBLEMATIC) { + /* TODO: cleanup this ls, or skip writing it */ } ls->flags |= LSF_NEEDS_FIXUP; if (nhfp->structlevel) bwrite(nhfp->fd, (genericptr_t) ls, sizeof(light_source)); ls->id = arg_save; ls->flags &= ~LSF_NEEDS_FIXUP; + ls->flags &= ~LSF_IS_PROBLEMATIC; } } else { impossible("write_ls: bad type (%d)", ls->type); diff --git a/src/mkobj.c b/src/mkobj.c index 8a9be1e0d..79e088cff 100644 --- a/src/mkobj.c +++ b/src/mkobj.c @@ -522,7 +522,7 @@ next_ident(void) uses 16-bit 'int'), just live with that and hope no o_id conflicts between objects or m_id conflicts between monsters arise */ if (!svc.context.ident) - svc.context.ident = rnd(2); + svc.context.ident = rnd(2) + 1; /* id 1 is reserved */ return res; } diff --git a/src/polyself.c b/src/polyself.c index 38f9b01ea..623bf1976 100644 --- a/src/polyself.c +++ b/src/polyself.c @@ -5,9 +5,9 @@ /* * Polymorph self routine. * - * Note: the light source handling code assumes that both gy.youmonst.m_id - * and gy.youmonst.mx will always remain 0 when it handles the case of the - * player polymorphed into a light-emitting monster. + * Note: the light source handling code assumes that gy.youmonst.m_id + * always remains 1 and gy.youmonst.mx will always remain 0 when it handles + * the case of the player polymorphed into a light-emitting monster. * * Transformation sequences: * /-> polymon poly into monster form @@ -41,6 +41,7 @@ set_uasmon(void) boolean was_vampshifter = valid_vampshiftform(gy.youmonst.cham, u.umonnum); set_mon_data(&gy.youmonst, mdat); + gy.youmonst.m_id = 1; if (Protection_from_shape_changers) gy.youmonst.cham = NON_PM;