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.
This commit is contained in:
PatR
2020-09-08 03:03:03 -07:00
parent f20a6bb491
commit a48b4aa8ba
4 changed files with 82 additions and 19 deletions
+3 -1
View File
@@ -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 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 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 change default for lit attribute in special level des.terrain directives to
'unchanged' instead of 'unlit' '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 curses: 'msg_window' option wasn't functional for curses unless the binary
also included tty support also included tty support
+2 -1
View File
@@ -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. */ /* Copyright (c) Steve Creps, 1988. */
/* NetHack may be freely redistributed. See license for details. */ /* 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(rest_worm, (NHFILE *));
E void FDECL(place_wsegs, (struct monst *, struct monst *)); E void FDECL(place_wsegs, (struct monst *, struct monst *));
E void FDECL(sanity_check_worm, (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(remove_worm, (struct monst *));
E void FDECL(place_worm_tail_randomly, (struct monst *, XCHAR_P, XCHAR_P)); E void FDECL(place_worm_tail_randomly, (struct monst *, XCHAR_P, XCHAR_P));
E int FDECL(size_wseg, (struct monst *)); E int FDECL(size_wseg, (struct monst *));
+4 -2
View File
@@ -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) Stichting Mathematisch Centrum, Amsterdam, 1985. */
/*-Copyright (c) Derek S. Ray, 2015. */ /*-Copyright (c) Derek S. Ray, 2015. */
/* NetHack may be freely redistributed. See license for details. */ /* 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) { for (mtmp = g.migrating_mons; mtmp; mtmp = mtmp->nmon) {
sanity_check_single_mon(mtmp, FALSE, "migr"); sanity_check_single_mon(mtmp, FALSE, "migr");
} }
wormno_sanity_check(); /* test for bogus worm tail */
} }
/* Would monster be OK with poison gas? */ /* 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 */ if (mtmp != u.usteed) /* don't place steed onto the map */
place_monster(mtmp2, mtmp2->mx, mtmp2->my); place_monster(mtmp2, mtmp2->mx, mtmp2->my);
if (mtmp2->wormno) /* update level.monsters[wseg->wx][wseg->wy] */ 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)) { if (emits_light(mtmp2->data)) {
/* since this is so rare, we don't have any `mon_move_light_source' */ /* 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), new_light_source(mtmp2->mx, mtmp2->my, emits_light(mtmp2->data),
+73 -15
View File
@@ -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) Stichting Mathematisch Centrum, Amsterdam, 1985. */
/*-Copyright (c) Robert Patrick Rankin, 2009. */ /*-Copyright (c) Robert Patrick Rankin, 2009. */
/* NetHack may be freely redistributed. See license for details. */ /* 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(toss_wsegs, (struct wseg *, BOOLEAN_P));
static void FDECL(shrink_worm, (int)); 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)); static struct wseg *FDECL(create_worm_tail, (int));
/* Description of long worm implementation. /* 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() * remove_worm()
* *
@@ -721,7 +748,7 @@ xchar x, y;
int wnum = worm->wormno; int wnum = worm->wormno;
struct wseg *curr = wtails[wnum]; struct wseg *curr = wtails[wnum];
struct wseg *new_tail; struct wseg *new_tail;
xchar ox = x, oy = y; int ox = x, oy = y;
if (wnum && (!wtails[wnum] || !wheads[wnum])) { if (wnum && (!wtails[wnum] || !wheads[wnum])) {
impossible("place_worm_tail_randomly: wormno is set without a tail!"); impossible("place_worm_tail_randomly: wormno is set without a tail!");
@@ -729,10 +756,14 @@ xchar x, y;
} }
if (wtails[wnum] == wheads[wnum]) { if (wtails[wnum] == wheads[wnum]) {
/* single segment, co-located with worm so nothing to place */ /* 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( impossible(
"place_worm_tail_randomly: tail segement at <%d,%d>, worm at <%d,%d>", "place_worm_tail_randomly: tail segement at <%d,%d>, worm at <%d,%d>",
curr->wx, curr->wy, worm->mx, worm->my); 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; return;
} }
/* remove head segment from map in case we end up calling toss_wsegs(); /* remove head segment from map in case we end up calling toss_wsegs();
@@ -747,30 +778,56 @@ xchar x, y;
new_tail->wy = y; new_tail->wy = y;
while (curr) { while (curr) {
xchar nx, ny; int nx = 0, ny = 0;
int tryct = 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 { do {
random_dir(ox, oy, &nx, &ny); 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); place_worm_seg(worm, nx, ny);
curr->wx = ox = nx; curr->wx = (xchar) (ox = nx);
curr->wy = oy = ny; curr->wy = (xchar) (oy = ny);
wtails[wnum] = curr; wtails[wnum] = curr;
curr = curr->nseg; curr = curr->nseg;
wtails[wnum]->nseg = new_tail; wtails[wnum]->nseg = new_tail;
new_tail = wtails[wnum]; new_tail = wtails[wnum];
newsym(nx, ny); newsym(nx, ny);
} else { /* Oops. Truncate because there was */ } else {
toss_wsegs(curr, FALSE); /* no place for the rest of it */ /* Oops. Truncate because there is no place for rest of it. */
toss_wsegs(curr, FALSE);
curr = (struct wseg *) 0; curr = (struct wseg *) 0;
} }
} }
} }
#if 0
/* /*
* Given a coordinate x, y. * Given a coordinate x, y.
* return in *nx, *ny, the coordinates of one of the <= 8 squares adjoining. * return in *nx, *ny, the coordinates of one of the <= 8 squares adjoining.
@@ -781,8 +838,8 @@ xchar x, y;
static static
void void
random_dir(x, y, nx, ny) random_dir(x, y, nx, ny)
xchar x, y; int x, y;
xchar *nx, *ny; int *nx, *ny;
{ {
*nx = x + (x > 1 /* extreme left ? */ *nx = x + (x > 1 /* extreme left ? */
? (x < COLNO - 1 /* extreme right ? */ ? (x < COLNO - 1 /* extreme right ? */
@@ -802,6 +859,7 @@ xchar *nx, *ny;
: -1) /* bottom, use -1 */ : -1) /* bottom, use -1 */
: 1); /* top, use +1 */ : 1); /* top, use +1 */
} }
#endif
/* for size_monst(cmd.c) to support #stats */ /* for size_monst(cmd.c) to support #stats */
int int