From 8680ae708b8dff9c52f23e8b47f06320f30b8c95 Mon Sep 17 00:00:00 2001 From: Kaz Kylheku Date: Thu, 21 Nov 2013 22:07:55 -0800 Subject: Nasty bug fixed: @(accept) from inside a @(collect) was found not to propagate bindings. The culprit? The bindings_coll variable in the v_collect function being indeterminate by the well-documented and understood action of setjmp. Marking the bindings_coll variable volatile instantly fixed it. I reviewed this code to find any other instance of this oversight. * match.c (v_skip, v_collect): Mark some local variable volatile: precisely those which are used after possibly returning via an unwind, and which might have been modified since setting up the unwind block. --- ChangeLog | 14 ++++++++++++++ match.c | 12 ++++++------ 2 files changed, 20 insertions(+), 6 deletions(-) diff --git a/ChangeLog b/ChangeLog index 9b990984..fab42277 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,17 @@ +2013-11-21 Kaz Kylheku + + Nasty bug fixed: @(accept) from inside a @(collect) was found not to + propagate bindings. The culprit? The bindings_coll variable in the + v_collect function being indeterminate by the well-documented and + understood action of setjmp. Marking the bindings_coll variable + volatile instantly fixed it. I reviewed this code to find any other + instance of this oversight. + + * match.c (v_skip, v_collect): Mark some local variable volatile: + precisely those which are used after possibly returning via an + unwind, and which might have been modified since setting up + the unwind block. + 2013-11-05 Kaz Kylheku * genvim.txr: Handle symbols whose C names end with _star_s, diff --git a/match.c b/match.c index d51feffe..5316d278 100644 --- a/match.c +++ b/match.c @@ -1941,8 +1941,8 @@ static val v_skip(match_files_ctx *c) cnum cmax = fixnump(max) ? c_num(max) : 0; cnum cmin = fixnump(min) ? c_num(min) : 0; val greedy = eq(max, greedy_k); - val last_good_result = nil; - val last_good_line = num(0); + volatile val last_good_result = nil; + volatile val last_good_line = num(0); { cnum reps_max = 0, reps_min = 0; @@ -2638,8 +2638,8 @@ static val v_collect(match_files_ctx *c) val coll_spec = second(first_spec); val until_last_spec = third(first_spec); val args = fourth(first_spec); - val bindings_coll = nil; - val last_bindings = nil; + volatile val bindings_coll = nil; + volatile val last_bindings = nil; val max = txeval(specline, getplist(args, maxgap_k), c->bindings); val min = txeval(specline, getplist(args, mingap_k), c->bindings); val gap = txeval(specline, getplist(args, gap_k), c->bindings); @@ -2648,7 +2648,7 @@ static val v_collect(match_files_ctx *c) val maxtimes = txeval(specline, getplist(args, maxtimes_k), c->bindings); val lines = txeval(specline, getplist(args, lines_k), c->bindings); val have_vars; - val vars = getplist_f(args, vars_k, &have_vars); + volatile val vars = getplist_f(args, vars_k, &have_vars); cnum cmax = fixnump(gap) ? c_num(gap) : (fixnump(max) ? c_num(max) : 0); cnum cmin = fixnump(gap) ? c_num(gap) : (fixnump(min) ? c_num(min) : 0); cnum mincounter = cmin, maxcounter = 0; @@ -2656,7 +2656,7 @@ static val v_collect(match_files_ctx *c) : (fixnump(maxtimes) ? c_num(maxtimes) : 0); cnum ctimin = fixnump(times) ? c_num(times) : (fixnump(mintimes) ? c_num(mintimes) : 0); - cnum timescounter = 0, linescounter = 0; + volatile cnum timescounter = 0, linescounter = 0; cnum ctimes = fixnump(times) ? c_num(times) : 0; cnum clines = fixnump(lines) ? c_num(lines) : 0; val iter; -- cgit v1.2.3