Fix leak and crash upon defining functions in subshells

A memory leak occurred upon leaving a virtual subshell if a
function was defined within it. If this was done more than 32766
(= 2^15-2 = the 'short' max value - 1) times, the shell crashed.
Discussion and reproducer: https://github.com/ksh93/ksh/issues/114

src/cmd/ksh93/sh/subshell.c: table_unset():
- A subshell-defined function was never freed because a broken
  check for autoloaded functions (which must not be freed[*]). It
  looked for an initial '/' in the canonical path of the script
  file that defined the function, but that path is also stored for
  regular functions. Now use a check that executes nv_search() in
  fpathdict, the same method used in _nv_unset() in name.c for a
  regular function unset.

src/cmd/ksh93/bltins/misc.c: b_dot_cmd():
- Fix an additional memory leak introduced in bd88cc7f, that caused
  POSIX functions (which are run with b_dot_cmd() like dot scripts)
  to leak extra. This fix avoids both the crash fixed there and the
  memory leak by introducing a 'tofree' variable remembering the
  filename to free. Thanks to Johnothan King for the patch.

src/lib/libast/include/stk.h,
src/lib/libast/misc/stk.c,
src/lib/libast/man/stk.3,
src/lib/libast/man/stak.3:
- Make the stack more resilient by extending the stack reference
  counter 'stkref' from (signed) short to unsigned int. On modern
  systems with 32-bit ints, this extends the maximum number of
  elements on a stack from 2^15-1==32767 to 2^32-1==4294967295.
  The ref counter can never be negative, so there is no reason for
  signedness. sizeof(int) is defined as the size of a single CPU
  word, so this should not affect performance at all.
     On a 16-bit system (not that ksh still compiles there), this
  doubles the max number of entries to 2^16-1=65535.

src/cmd/ksh93/tests/leaks.sh:
- Add leak regression tests for ksh functions, POSIX functions, dot
  scripts run with '.', and dot scripts run with 'source'.

src/cmd/ksh93/tests/path.sh:
- Add an output builtin with a redirect to an autoloaded function
  so that a crash[*] is triggered if the check for an autoloaded
  function is ever removed from table_unset(), as was done in ksh
  93v- (which crashed).

[*] Freeing autoloaded functions after leaving a virtual subshell
    causes a crashing bug: https://github.com/att/ast/issues/803

Co-authored-by: Johnothan King <johnothanking@protonmail.com>
Fixes: https://github.com/ksh93/ksh/issues/114

NEWS

@@ -3,6 +3,11 @@ For full details, see the git log at: https://github.com/ksh93/ksh
 
 Any uppercase BUG_* names are modernish shell bug IDs.
 
+2020-08-13:
+
+- Fixed memory leaks and a crashing bug that occurred when defining and
+  running functions in subshells.
+
 2020-08-11:
 
 - Fixed an intermittent crash upon running a large number of subshells.

src/cmd/ksh93/bltins/misc.c

@@ -219,7 +219,7 @@ int    b_dot_cmd(register int n,char *argv[],Shbltin_t *context)
 	register int jmpval;
 	register Shell_t *shp = context->shp;
 	struct sh_scoped savst, *prevscope = shp->st.self;
-	char *filename=0, *buffer=0;
+	char *filename=0, *buffer=0, *tofree;
 	int	fd;
 	struct dolnod   *saveargfor;
 	volatile struct dolnod   *argsave=0;
@@ -282,8 +282,9 @@ int    b_dot_cmd(register int n,char *argv[],Shbltin_t *context)
 	shp->st.self = &savst;
 	shp->topscope = (Shscope_t*)shp->st.self;
 	prevscope->save_tree = shp->var_tree;
+	tofree = shp->st.filename;
 	if(np)
-		shp->st.filename = np->nvalue.rp->fname ? strdup(np->nvalue.rp->fname) : 0;
+		shp->st.filename = np->nvalue.rp->fname;
 	nv_putval(SH_PATHNAMENOD, shp->st.filename ,NV_NOFREE);
 	shp->posix_fun = 0;
 	if(np || argv[1])
@@ -307,7 +308,7 @@ int    b_dot_cmd(register int n,char *argv[],Shbltin_t *context)
 	if(buffer)
 		free(buffer);
 	if(!np)
-		free((void*)shp->st.filename);
+		free(tofree);
 	shp->dot_depth--;
 	if((np || argv[1]) && jmpval!=SH_JMPSCRIPT)
 		sh_argreset(shp,(struct dolnod*)argsave,saveargfor);

src/cmd/ksh93/include/version.h

@@ -17,4 +17,4 @@
 *                  David Korn <dgk@research.att.com>                   *
 *                                                                      *
 ***********************************************************************/
-#define SH_RELEASE	"93u+m 2020-08-11"
+#define SH_RELEASE	"93u+m 2020-08-13"

src/cmd/ksh93/sh/subshell.c

@@ -397,7 +397,11 @@ Dt_t *sh_subfuntree(int create)
 	return(sp->shp->fun_tree);
 }
 
-static void table_unset(register Dt_t *root,int fun)
+/*
+ * Remove and free a subshell table at *root after leaving a virtual subshell.
+ * Pass 'fun' as nonzero when removing a subshell's shell functions.
+ */
+static void table_unset(Shell_t *shp,register Dt_t *root,int fun)
 {
 	register Namval_t *np,*nq;
 	int flag;
@@ -405,7 +409,9 @@ static void table_unset(register Dt_t *root,int fun)
 	{
 		nq = (Namval_t*)dtnext(root,np);
 		flag=0;
-		if(fun && np->nvalue.rp && np->nvalue.rp->fname && *np->nvalue.rp->fname=='/')
+		/* Check for autoloaded function; it must not be freed. */
+		if(fun && np->nvalue.rp && np->nvalue.rp->fname && shp->fpathdict
+		&& nv_search(np->nvalue.rp->fname,shp->fpathdict,0))
 		{
 			np->nvalue.rp->fdict = 0;
 			flag = NV_NOFREE;
@@ -693,7 +699,7 @@ Sfio_t *sh_subshell(Shell_t *shp,Shnode_t *t, volatile int flags, int comsub)
 		if(sp->sfun)
 		{
 			shp->fun_tree = dtview(sp->sfun,0);
-			table_unset(sp->sfun,1);
+			table_unset(shp,sp->sfun,1);
 			dtclose(sp->sfun);
 		}
 		n = shp->st.trapmax-savst.trapmax;

src/cmd/ksh93/tests/leaks.sh

@@ -60,11 +60,14 @@ before=0 after=0 i=0 u=0
 # Number of iterations for each test
 N=512
 
-# Check results. Add a tolerance of N/4 bytes to avoid false leaks.
+# Check results. Add a tolerance of 2 bytes per iteration. Some tests appear to have tiny leaks, but they
+# must be vmalloc artefacts; they may increase with the number of iterations, then suddenly go away when the
+# number of iterations is large enough (e.g. 10000) and don't return when increasing iterations further.
+#
 # The function has 'err_exit' in the name so that shtests counts each call as at test.
 function err_exit_if_leak
 {
-	if	((after > before + N / 4))
+	if	((after > before + 2 * N))
 	then	err\_exit "$1" "$2 (leaked $((after - before)) bytes after $N iterations)"
 	fi
 }
@@ -142,5 +145,47 @@ done >/dev/null
 after=$(getmem)
 err_exit_if_leak 'memory leak on PATH reset before subshell PATH search'
 
+# ======
+# Defining a function in a virtual subshell
+# https://github.com/ksh93/ksh/issues/114
+
+unset -f foo
+before=$(getmem)
+for ((i=0; i < N; i++))
+do	(function foo { :; }; foo)
+done
+after=$(getmem)
+err_exit_if_leak 'ksh function defined in virtual subshell'
+typeset -f foo >/dev/null && err_exit 'ksh function leaks out of subshell'
+
+unset -f foo
+before=$(getmem)
+for ((i=0; i < N; i++))
+do	(foo() { :; }; foo)
+done
+after=$(getmem)
+err_exit_if_leak 'POSIX function defined in virtual subshell'
+typeset -f foo >/dev/null && err_exit 'POSIX function leaks out of subshell'
+
+# ======
+# Sourcing a dot script in a virtual subshell
+
+echo 'echo "$@"' > $tmp/dot.sh
+before=$(getmem)
+for ((i=0; i < N; i++))
+do	(. "$tmp/dot.sh" dot one two three >/dev/null)
+done
+after=$(getmem)
+err_exit_if_leak 'script dotted in virtual subshell'
+
+echo 'echo "$@"' > $tmp/dot.sh
+before=$(getmem)
+for ((i=0; i < N; i++))
+do	(source "$tmp/dot.sh" source four five six >/dev/null)
+done
+after=$(getmem)
+err_exit_if_leak 'script sourced in virtual subshell'
+
 # ======
 exit $((Errors<125?Errors:125))
+[A

src/cmd/ksh93/tests/path.sh

@@ -36,7 +36,7 @@ type /xxxxxx > out1 2> out2
 [[ -s out2 ]] || err_exit 'type should write on stderr for not found case'
 mkdir dir1 dir2
 cat  > dir1/foobar << '+++'
-foobar() { print foobar1;}
+foobar() { print foobar1 >foobar1.txt; cat <foobar1.txt;}
 function dir1 { print dir1;}
 +++
 cat  > dir2/foobar << '+++'

src/lib/libast/include/stk.h

@@ -65,7 +65,7 @@ extern Sfio_t		_Stk_data;
 extern Stk_t*		stkopen(int);
 extern Stk_t*		stkinstall(Stk_t*, char*(*)(int));
 extern int		stkclose(Stk_t*);
-extern int		stklink(Stk_t*);
+extern unsigned int	stklink(Stk_t*);
 extern char*		stkalloc(Stk_t*, size_t);
 extern char*		stkcopy(Stk_t*, const char*);
 extern char*		stkset(Stk_t*, char*, size_t);

src/lib/libast/man/stak.3

@@ -12,7 +12,7 @@
 Stak_t *stakcreate(int \fIflags\fP);
 Stak_t *stakinstall(Stak_t *\fIstack\fP, char *(\fIoverflow\fP)(int));
 int stakdelete(Stak_t *\fIstack\fP);
-void staklink(Stak_t *\fIstack\fP)
+unsigned int staklink(Stak_t *\fIstack\fP)
 
 char *stakalloc(unsigned \fIsize\fP);
 char *stakcopy(const char *\fIstring\fP);
@@ -62,7 +62,7 @@ If successful,
 count is 1.
 Otherwise, \f5stakcreate\fP() returns a null pointer.
 The \f5staklink\fP() function increases the reference count for the
-given \fIstack\fP.
+given \fIstack\fP and returns the increased count.
 The \f5stakinstall\fP() function
 makes the specified \fIstack\fP the active stack and returns a pointer
 to the previous active stack.

src/lib/libast/man/stk.3

@@ -12,7 +12,7 @@
 Stk_t *stkopen(int \fIflags\fP);
 Stk_t *stkinstall(Stk_t *\fIstack\fP, char *(\fIoverflow\fP)(int));
 int stkclose(Stk_t *\fIstack\fP);
-void stklink(Stk_t *\fIstack\fP)
+unsigned int stklink(Stk_t *\fIstack\fP)
 
 char *stkalloc(Stk_t *\fIstack\fP, unsigned \fIsize\fP);
 char *stkcopy(Stk_t *\fIstack\fP, const char *\fIstring\fP);
@@ -63,7 +63,7 @@ If successful,
 count is 1.
 Otherwise, \f5stkopen\fP() returns a null pointer.
 The \f5stklink\fP() function increases the reference count for the
-given \fIstack\fP.
+given \fIstack\fP and returns the increased count.
 The \f5stkinstall\fP() function
 makes the specified \fIstack\fP the active stack and returns a pointer
 to the previous active stack.

src/lib/libast/misc/stk.c

@@ -76,7 +76,7 @@ struct frame
 struct stk
 {
 	_stk_overflow_	stkoverflow;	/* called when malloc fails */
-	short		stkref;	/* reference count; */
+	unsigned int	stkref;		/* reference count */
 	short		stkflags;	/* stack attributes */
 	char		*stkbase;	/* beginning of current stack frame */
 	char		*stkend;	/* end of current stack frame */
@@ -152,7 +152,7 @@ static int stkexcept(register Sfio_t *stream, int type, void* val, Sfdisc_t* dp)
 			register struct stk *sp = stream2stk(stream); 
 			register char *cp = sp->stkbase;
 			register struct frame *fp;
-			if(--sp->stkref<=0)
+			if(--sp->stkref == 0)
 			{
 				increment(delete);
 				if(stream==stkstd)
@@ -294,7 +294,7 @@ Sfio_t *stkinstall(Sfio_t *stream, _stk_overflow_ oflow)
 /*
  * increase the reference count on the given <stack>
  */
-int stklink(register Sfio_t* stream)
+unsigned int stklink(register Sfio_t* stream)
 {
 	register struct stk *sp = stream2stk(stream);
 	return(sp->stkref++);