Fix unreliable behavior when special vars are readonly or unset (#27)

src/cmd/ksh93/data/variables.c:
 - Running 'unset .sh.lineno' creates a memory fault, so fix that
   by giving it the NV_NOFREE attribute. This crash was happening
   because ${.sh.lineno} is an integer that cannot be freed from
   memory with free(3).

src/cmd/ksh93/sh/init.c:
 - Tell _nv_unset to ignore NV_RDONLY when $RANDOM and $LINENO are
   restored from the subshell scope. This is required to fully
   restore the original state of these variables after a virtual
   subshell finishes.

src/cmd/ksh93/bltins/typeset.c,
src/cmd/ksh93/sh/subshell.c:
 - Disabled some optimizations for two instances of 'sh_assignok' to
   fix 'readonly' in virtual subshells and '(unset .sh.level)' in
   nested functions. This fixes the following variables when
   '(readonly $varname); enum varname=' is run:

   $_
   ${.sh.name}
   ${.sh.subscript}
   ${.sh.level}

   The optimization in question prevents sh_assignok from saving the
   original state of these variables by making the sh_assignok call
   a no-op. Ksh needs the original state of a variable for it to be
   properly restored after a virtual subshell has run, otherwise ksh
   will simply carry over any new flags (being NV_RDONLY in this case)
   from the subshell into the main shell.

src/cmd/ksh93/tests/variables.sh:
 - Add regression tests from Martijn Dekker for setting special
   variables as readonly in virtual subshells and for unsetting
   special variables in general.

Fixes #4

NEWS

@@ -3,6 +3,21 @@ For full details, see the git log at: https://github.com/ksh93/ksh
 
 Any uppercase BUG_* names are modernish shell bug IDs.
 
+2020-06-20:
+
+- Fixed a bug that caused setting the following variables as readonly in
+  a virtual subshell to affect the environment outside of the subshell:
+  $_
+  ${.sh.name}
+  ${.sh.subscript}
+  ${.sh.level}
+  $RANDOM
+  $LINENO
+
+- Fixed two bugs that caused `unset .sh.lineno` to always produce a memory
+  fault and `(unset .sh.level)` to memory fault when run in nested
+  functions.
+
 2020-06-18:
 
 - A two decade old bug that caused 'whence -a' to base the path of

src/cmd/ksh93/bltins/typeset.c

@@ -783,7 +783,7 @@ static int     setall(char **argv,register int flag,Dt_t *troot,struct tdata *tp
 			if (tp->aflag && (tp->argnum>0 || (curflag!=newflag)))
 			{
 				if(shp->subshell)
-					sh_assignok(np,1);
+					sh_assignok(np,3);
 				if(troot!=shp->var_tree)
 					nv_setattr(np,newflag&~NV_ASSIGN);
 				else
@@ -1281,7 +1281,7 @@ static int unall(int argc, char **argv, register Dt_t *troot, Shell_t* shp)
 						 * Variables with internal trap/discipline functions (LC_*, LINENO, etc.) need to be
 						 * cloned, as moving them will remove the discipline function.
 						 */
-						np=sh_assignok(np,1);
+						np=sh_assignok(np,2);
 					}
 					else
 						np=sh_assignok(np,0);

src/cmd/ksh93/data/variables.c

@@ -95,7 +95,7 @@ const struct shtable2 shtab_variables[] =
 	".sh.fun",	0,				(char*)0,
 	".sh.subshell",	NV_INTEGER|NV_SHORT|NV_NOFREE,	(char*)0,
 	".sh.level",	0,				(char*)0,
-	".sh.lineno",	NV_INTEGER,			(char*)0,
+	".sh.lineno",	NV_INTEGER|NV_NOFREE,		(char*)0,
 	".sh.stats",	0,				(char*)0,
 	".sh.math",	0,				(char*)0,
 	".sh.pool",	0,				(char*)0,

src/cmd/ksh93/include/version.h

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

src/cmd/ksh93/sh/init.c

@@ -638,7 +638,7 @@ static void put_rand(register Namval_t* np,const char *val,int flags,Namfun_t *f
 		fp = nv_stack(np, NIL(Namfun_t*));
 		if(fp && !fp->nofree)
 			free((void*)fp);
-		_nv_unset(np,0);
+		_nv_unset(np,NV_RDONLY);
 		return;
 	}
 	if(flags&NV_INTEGER)
@@ -696,7 +696,7 @@ static void put_lineno(Namval_t* np,const char *val,int flags,Namfun_t *fp)
 		fp = nv_stack(np, NIL(Namfun_t*));
 		if(fp && !fp->nofree)
 			free((void*)fp);
-		_nv_unset(np,0);
+		_nv_unset(np,NV_RDONLY);
 		return;
 	}
 	if(flags&NV_INTEGER)

src/cmd/ksh93/sh/subshell.c

@@ -235,8 +235,15 @@ int nv_subsaved(register Namval_t *np)
 
 /*
  * This routine will make a copy of the given node in the
- * layer created by the most recent subshell_fork if the
- * node hasn't already been copied
+ * layer created by the most recent virtual subshell if the
+ * node hasn't already been copied.
+ *
+ * add == 0:    Move the node pointer from the parent shell to the current virtual subshell.
+ * add == 1:    Create a copy of the node pointer in the current virtual subshell.
+ * add == 2:    This will create a copy of the node pointer like 1, but it will disable the
+ *              optimization for ${.sh.level}.
+ * add == 3:    This is like 1, but it will never skip the following variables:
+ *              ${.sh.level}, $_, ${.sh.subscript} and ${.sh.name}.
  */
 Namval_t *sh_assignok(register Namval_t *np,int add)
 {
@@ -252,7 +259,7 @@ Namval_t *sh_assignok(register Namval_t *np,int add)
 	if(sp->subshare)
 		return(np);
 	/* don't bother with this */
-	if(!sp->shpwd || np==SH_LEVELNOD || np==L_ARGNOD || np==SH_SUBSCRNOD || np==SH_NAMENOD)
+	if(!sp->shpwd || (add != 3 && ((add != 2 && np==SH_LEVELNOD) || np==L_ARGNOD || np==SH_SUBSCRNOD || np==SH_NAMENOD)))
 		return(np);
 	if((ap=nv_arrayptr(np)) && (mp=nv_opensub(np)))
 	{

src/cmd/ksh93/tests/variables.sh

@@ -795,5 +795,148 @@ expect=1
 actual=$(env SHLVL="2#11+x[\$(env echo Exploited vuln CVE-2019-14868 >&2)0]" "$SHELL" -c 'echo $SHLVL' 2>&1)
 [[ $actual == $expect ]] || err_exit "expression allowed on env var import (expected '$expect', got '$actual')"
 
+# ======
+# Check unset and cleanup/restore behavior of special variables.
+
+# Keep the list in sync (minus ".sh") with shtab_variables[] in src/cmd/ksh93/data/variables.c
+# Note: as long as changing $PATH forks a virtual subshell, "PATH" should also be excluded below.
+set -- \
+	"PS1" \
+	"PS2" \
+	"IFS" \
+	"PWD" \
+	"HOME" \
+	"MAIL" \
+	"REPLY" \
+	"SHELL" \
+	"EDITOR" \
+	"MAILCHECK" \
+	"RANDOM" \
+	"ENV" \
+	"HISTFILE" \
+	"HISTSIZE" \
+	"HISTEDIT" \
+	"HISTCMD" \
+	"FCEDIT" \
+	"CDPATH" \
+	"MAILPATH" \
+	"PS3" \
+	"OLDPWD" \
+	"VISUAL" \
+	"COLUMNS" \
+	"LINES" \
+	"PPID" \
+	"_" \
+	"TMOUT" \
+	"SECONDS" \
+	"LINENO" \
+	"OPTARG" \
+	"OPTIND" \
+	"PS4" \
+	"FPATH" \
+	"LANG" \
+	"LC_ALL" \
+	"LC_COLLATE" \
+	"LC_CTYPE" \
+	"LC_MESSAGES" \
+	"LC_NUMERIC" \
+	"FIGNORE" \
+	"KSH_VERSION" \
+	"JOBMAX" \
+	".sh.edchar" \
+	".sh.edcol" \
+	".sh.edtext" \
+	".sh.edmode" \
+	".sh.name" \
+	".sh.subscript" \
+	".sh.value" \
+	".sh.version" \
+	".sh.dollar" \
+	".sh.match" \
+	".sh.command" \
+	".sh.file" \
+	".sh.fun" \
+	".sh.lineno" \
+	".sh.subshell" \
+	".sh.level" \
+	".sh.stats" \
+	".sh.math" \
+	".sh.pool" \
+	"SHLVL" \
+	"CSWIDTH"
+
+# ... unset
+$SHELL -c '
+	errors=0
+	unset -v "$@" || let errors++
+	for var
+	do	if	[[ $var != "_" ]] &&	# only makes sense that $_ is immediately set again
+			{ [[ -v $var ]] || eval "[[ -n \${$var+s} ]]"; }
+		then	echo "	$0: special variable $var still set" >&2
+			let errors++
+		elif	eval "[[ -n \${$var} ]]"
+		then	echo "	$0: special variable $var has value, though unset" >&2
+			let errors++
+		fi
+	done
+	exit $((errors + 1))	# a possible erroneous asynchronous fork would cause exit status 0
+' unset_test "$@"
+e=$?
+((e == 1)) || err_exit "Failure in unsetting one or more special variables (exit status $e)"
+
+# ... unset in virtual subshell inside of nested function
+$SHELL -c '
+	errors=0
+	fun1()
+	{
+		fun2()
+		{
+			(
+				unset -v "$@" || let errors++
+				for var
+				do	if	[[ $var != "_" ]] &&	# only makes sense that $_ is immediately set again
+						{ [[ -v $var ]] || eval "[[ -n \${$var+s} ]]"; }
+					then	echo "	$0: special variable $var still set" >&2
+						let errors++
+					elif	eval "[[ -n \${$var} ]]"
+					then	echo "	$0: special variable $var has value, though unset" >&2
+						let errors++
+					fi
+				done
+				exit $errors
+			) || errors=$?
+		}
+		fun2 "$@"
+	}
+	fun1 "$@"
+	exit $((errors + 1))	# a possible erroneous asynchronous fork would cause exit status 0
+' unset_subsh_fun_test "$@"
+e=$?
+((e == 1)) || err_exit "Unset of special variable(s) in a virtual subshell within a nested function fails (exit status $e)"
+
+# ... readonly in subshell
+$SHELL -c '
+	errors=0
+	(
+		readonly "$@"
+		for var
+		do	if	(eval "$var=") 2>/dev/null
+			then	echo "	$0: special variable $var not made readonly in subshell" >&2
+				let errors++
+			fi
+		done
+		exit $errors
+	) || errors=$?
+	for var
+	do	if	! (eval "$var=")
+		then	echo "	$0: special variable $var still readonly outside subshell" >&2
+			let errors++
+		fi
+	done
+	exit $((errors + 1))	# a possible erroneous asynchronous fork would cause exit status 0
+' readonly_test "$@"
+e=$?
+((e == 1)) || err_exit "Failure in making one or more special variables readonly in a subshell (exit status $e)"
+
 # ======
 exit $((Errors<125?Errors:125))