From 664deaea597307bb5b055897384ae06a63424183 Mon Sep 17 00:00:00 2001 From: Stephen Clavering Date: Thu, 12 Apr 2018 21:15:17 +0100 Subject: [PATCH] Fix two bugs in Range's solver_reasoning_recursion(). Firstly, it tries to use newstate after freeing it. I haven't come up with any way of actually triggering that bug. I suppose you'd need a grid that required recursion to solve, which the generator of course does not normally create. Secondly, it stores M_BLACK or M_WHITE in the grid before doing the recursion, whereas it should store BLACK or WHITE. I believe that since it tries M_BLACK first, which is zero, and since it's trying it on an undecided square (also zero), this leads to infinite recursion, and an eventual crash once you run out of stack space. You can (sometimes) trigger this by asking for a hint on a grid where you've already made a mistake. e.g. on the puzzle desc below there's a "9" at (13,7). If you place a black tile to its immediate left and right - (12,7) and (14,7) - then press 'h', you get a beachball and then crash (on macOS at least - I presume other OSes are similar). 15x15:i5g4d16a7a7c3b3b11f11_15d3p8b7_8b4h18c4d13b4i7a16k9a9i4b2d10c14h11b10_10b17p6d8_7f9b8b11c10a2a5d5g7i --- range.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/range.c b/range.c index 41c6a35..58d6e48 100644 --- a/range.c +++ b/range.c @@ -613,15 +613,16 @@ static move *solver_reasoning_recursion(game_state *state, /* FIXME: add enum alias for smallest and largest (or N) */ for (colour = M_BLACK; colour <= M_WHITE; ++colour) { newstate = dup_game(state); - newstate->grid[cell] = colour; + newstate->grid[cell] = colour == M_BLACK ? BLACK : WHITE; recursive_result = do_solve(newstate, nclues, clues, buf, DIFF_RECURSION); - free_game(newstate); if (recursive_result == NULL) { + free_game(newstate); solver_makemove(r, c, M_BLACK + M_WHITE - colour, state, &buf); return buf; } for (i = 0; i < n && newstate->grid[i] != EMPTY; ++i); + free_game(newstate); if (i == n) return buf; } }