Fix (hopefully) the problem reported on current-users by Patrick Welche.
we had incorrect usage of setstackmark()/popstackmark() There was an ancient idiom (imported from CSRG in 1993) where code can do: setstackmark(&smark); loop until whatever condition { /* do lots of code */ popstackmark(&smark); } popstackmark(&smark); The 1st (inner) popstackmark() resets the stack, conserving memory, The 2nd one is needed just in case the "whatever condition" was never true, and the first one was never executed. This is (was) safe as all popstackmark() did was reset the stack. That could be done over and over again with no harm. That is, until 2000 when a fix from FreeBSD for another problem was imported. That connected all the stack marks as a list (so they can be located). That caused the problem, as the idiom was not changed, now there is this list of marks, and popstackmark() was removing an entry. It rarely (never?) caused any problems as the idiom was rarely used (the shell used to do loops like above, mostly, without the inner popstackmark()). Further, the stack mark list is only ever used when a memory block is realloc'd. That is, until last weekend - with the recent set of changes. Part of that copied code from FreeBSD introduced the idiom above into more functions - functions used much more, and with a greater possibility of stack marks being set on blocks that are realloc'd and so cause the problem. In the FreeBSD code, they changed the idiom, and always do a setstackmark() immediately after the inner popstackmark(). But not for reasons related to a list of stack marks, as in the intervening period, FreeBSD deleted that, but for another reason. We do not have their issue, and I did not believe that their updated idiom was needed (I did some analysis of exactly this issue - just missed the important part!), and just continued using the old one. Hence Patrick's core dump.... The solution used here is to split popstackmark() into 2 halves, popstackmark() continues to do what it has (recently) done, but is now implemented as a call of (a new func) rststackmark() which does all the original work of popstackmark - but not removing the entry from the stack mark list (which remains in popstackmark()). Then in the idiom above, the inner popstackmark() turns into a call of rststackmark() so the stack is reset, but the stack mark list is unchanged. Tail recursion elimination makes this essentially free.
This commit is contained in:
parent
4a99e7bfda
commit
70696c0161
|
@ -1,4 +1,4 @@
|
|||
/* $NetBSD: eval.c,v 1.159 2018/08/19 23:50:27 kre Exp $ */
|
||||
/* $NetBSD: eval.c,v 1.160 2018/08/22 20:08:54 kre Exp $ */
|
||||
|
||||
/*-
|
||||
* Copyright (c) 1993
|
||||
|
@ -37,7 +37,7 @@
|
|||
#if 0
|
||||
static char sccsid[] = "@(#)eval.c 8.9 (Berkeley) 6/8/95";
|
||||
#else
|
||||
__RCSID("$NetBSD: eval.c,v 1.159 2018/08/19 23:50:27 kre Exp $");
|
||||
__RCSID("$NetBSD: eval.c,v 1.160 2018/08/22 20:08:54 kre Exp $");
|
||||
#endif
|
||||
#endif /* not lint */
|
||||
|
||||
|
@ -234,7 +234,7 @@ evalstring(char *s, int flag)
|
|||
evaltree(n, flag);
|
||||
any = 1;
|
||||
}
|
||||
popstackmark(&smark);
|
||||
rststackmark(&smark);
|
||||
}
|
||||
popfile();
|
||||
popstackmark(&smark);
|
||||
|
@ -362,7 +362,7 @@ evaltree(union node *n, int flags)
|
|||
break;
|
||||
}
|
||||
n = next;
|
||||
popstackmark(&smark);
|
||||
rststackmark(&smark);
|
||||
} while(n != NULL);
|
||||
out1:
|
||||
popstackmark(&smark);
|
||||
|
|
|
@ -1,4 +1,4 @@
|
|||
/* $NetBSD: main.c,v 1.75 2018/08/19 23:50:27 kre Exp $ */
|
||||
/* $NetBSD: main.c,v 1.76 2018/08/22 20:08:54 kre Exp $ */
|
||||
|
||||
/*-
|
||||
* Copyright (c) 1991, 1993
|
||||
|
@ -42,7 +42,7 @@ __COPYRIGHT("@(#) Copyright (c) 1991, 1993\
|
|||
#if 0
|
||||
static char sccsid[] = "@(#)main.c 8.7 (Berkeley) 7/19/95";
|
||||
#else
|
||||
__RCSID("$NetBSD: main.c,v 1.75 2018/08/19 23:50:27 kre Exp $");
|
||||
__RCSID("$NetBSD: main.c,v 1.76 2018/08/22 20:08:54 kre Exp $");
|
||||
#endif
|
||||
#endif /* not lint */
|
||||
|
||||
|
@ -294,8 +294,7 @@ cmdloop(int top)
|
|||
numeof = 0;
|
||||
evaltree(n, 0);
|
||||
}
|
||||
popstackmark(&smark);
|
||||
setstackmark(&smark);
|
||||
rststackmark(&smark);
|
||||
|
||||
/*
|
||||
* Any SKIP* can occur here! SKIP(FUNC|BREAK|CONT) occur when
|
||||
|
|
|
@ -1,4 +1,4 @@
|
|||
/* $NetBSD: memalloc.c,v 1.31 2018/07/22 20:37:57 kre Exp $ */
|
||||
/* $NetBSD: memalloc.c,v 1.32 2018/08/22 20:08:54 kre Exp $ */
|
||||
|
||||
/*-
|
||||
* Copyright (c) 1991, 1993
|
||||
|
@ -37,7 +37,7 @@
|
|||
#if 0
|
||||
static char sccsid[] = "@(#)memalloc.c 8.3 (Berkeley) 5/4/95";
|
||||
#else
|
||||
__RCSID("$NetBSD: memalloc.c,v 1.31 2018/07/22 20:37:57 kre Exp $");
|
||||
__RCSID("$NetBSD: memalloc.c,v 1.32 2018/08/22 20:08:54 kre Exp $");
|
||||
#endif
|
||||
#endif /* not lint */
|
||||
|
||||
|
@ -160,7 +160,7 @@ stunalloc(pointer p)
|
|||
}
|
||||
|
||||
|
||||
|
||||
/* save the current status of the sh stack */
|
||||
void
|
||||
setstackmark(struct stackmark *mark)
|
||||
{
|
||||
|
@ -172,15 +172,23 @@ setstackmark(struct stackmark *mark)
|
|||
markp = mark;
|
||||
}
|
||||
|
||||
|
||||
/* reset the stack mark, and remove it from the list of marks */
|
||||
void
|
||||
popstackmark(struct stackmark *mark)
|
||||
{
|
||||
markp = mark->marknext; /* delete mark from the list */
|
||||
rststackmark(mark); /* and reset stack */
|
||||
}
|
||||
|
||||
/* reset the shell stack to its state recorded in the stack mark */
|
||||
void
|
||||
rststackmark(struct stackmark *mark)
|
||||
{
|
||||
struct stack_block *sp;
|
||||
|
||||
INTOFF;
|
||||
markp = mark->marknext;
|
||||
while (stackp != mark->stackp) {
|
||||
/* delete any recently allocated mem blocks */
|
||||
sp = stackp;
|
||||
stackp = sp->prev;
|
||||
ckfree(sp);
|
||||
|
|
|
@ -1,4 +1,4 @@
|
|||
/* $NetBSD: memalloc.h,v 1.17 2017/06/17 07:22:12 kre Exp $ */
|
||||
/* $NetBSD: memalloc.h,v 1.18 2018/08/22 20:08:54 kre Exp $ */
|
||||
|
||||
/*-
|
||||
* Copyright (c) 1991, 1993
|
||||
|
@ -55,6 +55,7 @@ pointer stalloc(int);
|
|||
void stunalloc(pointer);
|
||||
void setstackmark(struct stackmark *);
|
||||
void popstackmark(struct stackmark *);
|
||||
void rststackmark(struct stackmark *);
|
||||
void growstackblock(void);
|
||||
void grabstackblock(int);
|
||||
char *growstackstr(void);
|
||||
|
|
Loading…
Reference in New Issue