Re: Complicated segfault regression

[Peter Stephenson <p.w.stephenson@xxxxxxxxxxxx>]

On Fri, 3 Jul 2015 23:11:59 +0100
Peter Stephenson <p.w.stephenson@xxxxxxxxxxxx> wrote:
> n->pop has no dcoumentation whatsoever, but the calculation associated
> with it in 5.0.7 is to do with how much state->pc gets incremented for
> each case pattern set, not coutning the string.  Why?  No idea.  The
> calculation has changed, and now looks like the following.

I've come to the conclusion this has always been wrong, or at least
confusingly inaccurate.  I think it sort-of worked because the
comparison landed in the right target area, just not at the right point.

Anyway, if the code is supposed to read like the following, which still
passes the new test, I now understand it better.  (And if it had read
like this before, which should always have worked, I wouldn't have had
to fiddle with it at all...)

Martijn's test is probably the best overall for this.

n->pop isn't really anything to do with the structure of the case block,
it's just saying something like "we've got to the end of this chunk, so
it's time to go back up the parse stack to find the next piece of code".
So this just uses the standard logic for "the end of this chunk" instead
of recalculating it wrongly from the point we happen to have reached.

pws

diff --git a/Src/text.c b/Src/text.c
index d63141b..cf73004 100644
--- a/Src/text.c
+++ b/Src/text.c
@@ -681,7 +681,7 @@ gettext2(Estate state)
 	case WC_CASE:
 	    if (!s) {
 		Wordcode end = state->pc + WC_CASE_SKIP(code);
-		wordcode nalts, ialts;
+		wordcode ialts;
 
 		taddstr("case ");
 		taddstr(ecgetstr(state, EC_NODUP, NULL));
@@ -695,6 +695,7 @@ gettext2(Estate state)
 		    taddstr("esac");
 		    stack = 1;
 		} else {
+		    Wordcode prev_pc;
 		    tindent++;
 		    if (tnewlins)
 			taddnl(0);
@@ -702,7 +703,8 @@ gettext2(Estate state)
 			taddchr(' ');
 		    taddstr("(");
 		    code = *state->pc++;
-		    nalts = ialts = *state->pc++;
+		    prev_pc = state->pc++;
+		    ialts = *prev_pc;
 		    while (ialts--) {
 			taddstr(ecgetstr(state, EC_NODUP, NULL));
 			state->pc++;
@@ -713,11 +715,11 @@ gettext2(Estate state)
 		    tindent++;
 		    n = tpush(code, 0);
 		    n->u._case.end = end;
-		    n->pop = (state->pc - 2 - nalts + WC_CASE_SKIP(code)
-			      >= end);
+		    n->pop = (prev_pc + WC_CASE_SKIP(code) >= end);
 		}
 	    } else if (state->pc < s->u._case.end) {
-		wordcode nalts, ialts;
+		Wordcode prev_pc;
+		wordcode ialts;
 		dec_tindent();
 		switch (WC_CASE_TYPE(code)) {
 		case WC_CASE_OR:
@@ -738,7 +740,8 @@ gettext2(Estate state)
 		    taddchr(' ');
 		taddstr("(");
 		code = *state->pc++;
-		nalts = ialts = *state->pc++;
+		prev_pc = state->pc++;
+		ialts = *prev_pc;
 		while (ialts--) {
 		    taddstr(ecgetstr(state, EC_NODUP, NULL));
 		    state->pc++;
@@ -748,7 +751,7 @@ gettext2(Estate state)
 		taddstr(") ");
 		tindent++;
 		s->code = code;
-		s->pop = ((state->pc - 2 - nalts + WC_CASE_SKIP(code)) >=
+		s->pop = (prev_pc + WC_CASE_SKIP(code) >=
 			  s->u._case.end);
 	    } else {
 		dec_tindent();