From a48b4aa8ba5e6b06a8d1d25153f8d07f3549ee10 Mon Sep 17 00:00:00 2001 From: PatR Date: Tue, 8 Sep 2020 03:03:03 -0700 Subject: [PATCH] fix #K1963 - warning after placing worm tail Report described this as a panic triggered by the sanity_check option, but that's because it was running under the fuzzer, which escalates any impossible() to panic(), rather than because nethack panicked. I couldn't find anything wrong--which doesn't mean that there isn't something wrong--with place_worm_tail_randomly() and random_dir(). They use xchar for map coordinates which should be fine as long as no negative values are generated and I couldn't discover any such. The suggested fix of changing xchar to int might indicate a compiler bug (although the odds of that are low). The bogus coordinate of -15000 in the report suggests that typedef short int schar; (which changes xchar too) is being used in the configuration but I don't recall having any problems attributable to that. This switches from xchar to int as a side-effect of replacing the offending code entirely. The new code might produce an 'ny' of -1 before goodpos() rejects it, so xchar would be inappropriate now. The old code is commented out via #if 0 _after_ changing it from xchar to int. This also adds an extra sanity_check for worm tails, unrelated to the current bug. I'm not aware of any instance where it fails. EXTRA_SANITY_CHECKS needs to be defined for it to do anything. --- doc/fixes37.0 | 4 ++- include/extern.h | 3 +- src/mon.c | 6 ++-- src/worm.c | 88 +++++++++++++++++++++++++++++++++++++++--------- 4 files changed, 82 insertions(+), 19 deletions(-) diff --git a/doc/fixes37.0 b/doc/fixes37.0 index 3364e09c4..94944bd90 100644 --- a/doc/fixes37.0 +++ b/doc/fixes37.0 @@ -1,4 +1,4 @@ -NHDT-Branch: NetHack-3.7 $:$NHDT-Revision: 1.298 $ $NHDT-Date: 1599434249 2020/09/06 23:17:29 $ +NHDT-Branch: NetHack-3.7 $:$NHDT-Revision: 1.299 $ $NHDT-Date: 1599559379 2020/09/08 10:02:59 $ General Fixes and Modified Features ----------------------------------- @@ -328,6 +328,8 @@ the fix to make worm visibility checks work as intended forced the coordinates using 'O' to try to change 'symset' was a no-op; 'roguesymset' worked change default for lit attribute in special level des.terrain directives to 'unchanged' instead of 'unlit' +replace worm tail placement code that reportedly led to a sanity_check warning + [no actual code problem found; might be compiler bug for 'xchar'] curses: 'msg_window' option wasn't functional for curses unless the binary also included tty support diff --git a/include/extern.h b/include/extern.h index 437431035..c1f2ae298 100644 --- a/include/extern.h +++ b/include/extern.h @@ -1,4 +1,4 @@ -/* NetHack 3.7 extern.h $NHDT-Date: 1597069374 2020/08/10 14:22:54 $ $NHDT-Branch: NetHack-3.7 $:$NHDT-Revision: 1.855 $ */ +/* NetHack 3.7 extern.h $NHDT-Date: 1599559379 2020/09/08 10:02:59 $ $NHDT-Branch: NetHack-3.7 $:$NHDT-Revision: 1.856 $ */ /* Copyright (c) Steve Creps, 1988. */ /* NetHack may be freely redistributed. See license for details. */ @@ -3078,6 +3078,7 @@ E void FDECL(save_worm, (NHFILE *)); E void FDECL(rest_worm, (NHFILE *)); E void FDECL(place_wsegs, (struct monst *, struct monst *)); E void FDECL(sanity_check_worm, (struct monst *)); +E void NDECL(wormno_sanity_check); E void FDECL(remove_worm, (struct monst *)); E void FDECL(place_worm_tail_randomly, (struct monst *, XCHAR_P, XCHAR_P)); E int FDECL(size_wseg, (struct monst *)); diff --git a/src/mon.c b/src/mon.c index 83e28ebbc..2df04ef29 100644 --- a/src/mon.c +++ b/src/mon.c @@ -1,4 +1,4 @@ -/* NetHack 3.7 mon.c $NHDT-Date: 1599330921 2020/09/05 18:35:21 $ $NHDT-Branch: NetHack-3.7 $:$NHDT-Revision: 1.345 $ */ +/* NetHack 3.7 mon.c $NHDT-Date: 1599559379 2020/09/08 10:02:59 $ $NHDT-Branch: NetHack-3.7 $:$NHDT-Revision: 1.346 $ */ /* Copyright (c) Stichting Mathematisch Centrum, Amsterdam, 1985. */ /*-Copyright (c) Derek S. Ray, 2015. */ /* NetHack may be freely redistributed. See license for details. */ @@ -150,6 +150,8 @@ mon_sanity_check() for (mtmp = g.migrating_mons; mtmp; mtmp = mtmp->nmon) { sanity_check_single_mon(mtmp, FALSE, "migr"); } + + wormno_sanity_check(); /* test for bogus worm tail */ } /* Would monster be OK with poison gas? */ @@ -1820,7 +1822,7 @@ struct monst *mtmp, *mtmp2; if (mtmp != u.usteed) /* don't place steed onto the map */ place_monster(mtmp2, mtmp2->mx, mtmp2->my); if (mtmp2->wormno) /* update level.monsters[wseg->wx][wseg->wy] */ - place_wsegs(mtmp2, NULL); /* locations to mtmp2 not mtmp. */ + place_wsegs(mtmp2, mtmp); /* locations to mtmp2 not mtmp. */ if (emits_light(mtmp2->data)) { /* since this is so rare, we don't have any `mon_move_light_source' */ new_light_source(mtmp2->mx, mtmp2->my, emits_light(mtmp2->data), diff --git a/src/worm.c b/src/worm.c index c32c8c928..9027f925b 100644 --- a/src/worm.c +++ b/src/worm.c @@ -1,4 +1,4 @@ -/* NetHack 3.7 worm.c $NHDT-Date: 1596841504 2020/08/07 23:05:04 $ $NHDT-Branch: NetHack-3.7 $:$NHDT-Revision: 1.47 $ */ +/* NetHack 3.7 worm.c $NHDT-Date: 1599559380 2020/09/08 10:03:00 $ $NHDT-Branch: NetHack-3.7 $:$NHDT-Revision: 1.48 $ */ /* Copyright (c) Stichting Mathematisch Centrum, Amsterdam, 1985. */ /*-Copyright (c) Robert Patrick Rankin, 2009. */ /* NetHack may be freely redistributed. See license for details. */ @@ -16,7 +16,9 @@ struct wseg { static void FDECL(toss_wsegs, (struct wseg *, BOOLEAN_P)); static void FDECL(shrink_worm, (int)); -static void FDECL(random_dir, (XCHAR_P, XCHAR_P, xchar *, xchar *)); +#if 0 +static void FDECL(random_dir, (int, int, int *, int *)); +#endif static struct wseg *FDECL(create_worm_tail, (int)); /* Description of long worm implementation. @@ -680,6 +682,31 @@ struct monst *worm; } } +/* called from mon_sanity_check(mon.c) */ +void +wormno_sanity_check() +{ +#ifdef EXTRA_SANITY_CHECKS + struct wseg *seg; + int wh = 0, wt = 0; + + /* checking tail management, not a particular monster; since wormno==0 + means 'not a worm', wheads[0] and wtails[0] should always be empty; + note: if erroneously non-Null, tail segment count will include the + extra segment for the worm's head that isn't shown on the map */ + for (seg = wheads[0]; seg; seg = seg->nseg) + ++wh; + for (seg = wtails[0]; seg; seg = seg->nseg) + ++wt; + if (wh || wt) { + impossible( + "phantom worm tail #0 [head=%s, %d segment%s; tail=%s, %d segment%s]", + fmt_ptr(wheads[0]), wh, plur(wh), + fmt_ptr(wtails[0]), wt, plur(wt)); + } +#endif /* EXTRA_SANITY_CHECKS */ +} + /* * remove_worm() * @@ -721,7 +748,7 @@ xchar x, y; int wnum = worm->wormno; struct wseg *curr = wtails[wnum]; struct wseg *new_tail; - xchar ox = x, oy = y; + int ox = x, oy = y; if (wnum && (!wtails[wnum] || !wheads[wnum])) { impossible("place_worm_tail_randomly: wormno is set without a tail!"); @@ -729,10 +756,14 @@ xchar x, y; } if (wtails[wnum] == wheads[wnum]) { /* single segment, co-located with worm so nothing to place */ - if (curr->wx != worm->mx || curr->wy != worm->my) + if (curr->wx != worm->mx || curr->wy != worm->my) { impossible( "place_worm_tail_randomly: tail segement at <%d,%d>, worm at <%d,%d>", curr->wx, curr->wy, worm->mx, worm->my); + if (m_at(curr->wx, curr->wy) == worm) + remove_monster(curr->wx, curr->wy); + curr->wx = worm->mx, curr->wy = worm->my; + } return; } /* remove head segment from map in case we end up calling toss_wsegs(); @@ -747,30 +778,56 @@ xchar x, y; new_tail->wy = y; while (curr) { - xchar nx, ny; - int tryct = 0; + int nx = 0, ny = 0; +#if 0 /* old code */ + int trycnt = 0; - /* pick a random direction from x, y and search for goodpos() */ + /* pick a random direction from x, y and test for goodpos() */ do { random_dir(ox, oy, &nx, &ny); - } while (!goodpos(nx, ny, worm, 0) && (tryct++ < 50)); + } while (!goodpos(nx, ny, worm, 0) && ++tryct <= 50); - if (tryct < 50) { + if (tryct <= 50) +#else /* new code */ + int i, j, k, dirs[8]; + + /* instead of picking a random direction up to 50 times, try each + of the eight directions at most once after shuffling their order */ + for (i = 0; i < 8; ++i) + dirs[i] = i; + for (i = 8; i > 0; --i) { + j = rn2(i); + k = dirs[j]; + dirs[j] = dirs[i - 1]; + dirs[i - 1] = k; + } + for (i = 0; i < 8; ++i) { + nx = ox + xdir[dirs[i]]; + ny = oy + ydir[dirs[i]]; + if (goodpos(nx, ny, worm, 0)) /* includes an isok() check */ + break; + } + + if (i < 8) +#endif + { place_worm_seg(worm, nx, ny); - curr->wx = ox = nx; - curr->wy = oy = ny; + curr->wx = (xchar) (ox = nx); + curr->wy = (xchar) (oy = ny); wtails[wnum] = curr; curr = curr->nseg; wtails[wnum]->nseg = new_tail; new_tail = wtails[wnum]; newsym(nx, ny); - } else { /* Oops. Truncate because there was */ - toss_wsegs(curr, FALSE); /* no place for the rest of it */ + } else { + /* Oops. Truncate because there is no place for rest of it. */ + toss_wsegs(curr, FALSE); curr = (struct wseg *) 0; } } } +#if 0 /* * Given a coordinate x, y. * return in *nx, *ny, the coordinates of one of the <= 8 squares adjoining. @@ -781,8 +838,8 @@ xchar x, y; static void random_dir(x, y, nx, ny) -xchar x, y; -xchar *nx, *ny; +int x, y; +int *nx, *ny; { *nx = x + (x > 1 /* extreme left ? */ ? (x < COLNO - 1 /* extreme right ? */ @@ -802,6 +859,7 @@ xchar *nx, *ny; : -1) /* bottom, use -1 */ : 1); /* top, use +1 */ } +#endif /* for size_monst(cmd.c) to support #stats */ int