Replace the hash alias with a proper builtin

This commit replaces the old hash alias with a proper builtin.
I based this builtin off of the code alias uses for handling
`alias -t --`, but with the hack for `--` removed as it has
no use in the new builtin. `alias -t --` will no longer work,
that hack is now gone.

While I was testing this builtin, I found a bug with hash tables
in non-forking subshells. If the hash table of a non-forking
subshell is changed, the parent shell's hash table is also changed.
As an example, running `(hash -r)` was resetting the parent shell's
hash table. The workaround is to force the subshell to fork if the
hash table will be changed.

src/cmd/ksh93/bltins/typeset.c:
 - Move the code for hash out of the alias builtin into a dedicated
   hash builtin. `alias -t --` is no longer supported.

src/cmd/ksh93/data/aliases.c:
 - Remove the old alias for hash from the table of predefined aliases.

src/cmd/ksh93/data/builtins.c:
 - Fix the broken entry for the hash builtin and add a man page for
   the new builtin.

src/cmd/ksh93/sh.1:
 - Replace the entry for the hash alias with a more detailed entry
   for the hash builtin.

src/cmd/ksh93/sh/name.c:
 - Force non-forking subshells to fork if the PATH is being reset
   to workaround a bug with the hash tree.

src/cmd/ksh93/tests/alias.sh:
 - Add a regression test for resetting a hash table, then adding
   a utility to the refreshed hash table.

src/cmd/ksh93/tests/subshell.sh:
 - Add regression tests for changing the hash table in subshells.

(cherry picked from commit d8428a833afe9270b61745ba3d6df355fe1d5499)

NEWS

@@ -4,6 +4,16 @@ For full details, see the git log at:
 
 Any uppercase BUG_* names are modernish shell bug IDs.
 
+2020-06-10:
+
+- The 'hash' utility is now a regular builtin instead of an alias to
+  'alias -t --'. The functionality of the old command has been removed
+  from the alias builtin.
+
+- Changing the hash table in a subshell will no longer affect the parent
+  shell's hash table. This fix applies to the hash utility and when the
+  PATH is reset manually.
+
 2020-06-09:
 
 - The 'unalias' builtin will now return a non-zero status if it tries

TODO

@@ -20,11 +20,6 @@ Fix build system:
 ______
 Fix or remove broken or misguided default aliases:
 
-- Make a proper POSIX 'hash' builtin command and remove the builtin alias
-  hash='alias -t --', which, when removed, exposes a b0rken 'hash' builtin.
-  To work with this alias, the 'alias' builtin parses the '-r' for 'hash'
-  *after* '--' ('alias -t -- -r'), which is utterly bogus and violates POSIX.
-
 - Make a proper builtin out of the redirect='command exec' alias. It should
   really only parse redirections. Currently, if an unwitting user notices this
   alias and tries out something like 'redirect ls >file', it does 'exec ls

src/cmd/ksh93/bltins/typeset.c

@@ -24,6 +24,7 @@
  * typeset [options]  [arg...]
  * alias [-ptx] [arg...]
  * unalias [arg...]
+ * hash [-r] [utility...]
  * builtin [-sd] [-f file] [name...]
  * set [options] [name...]
  * unset [-fnv] [name...]
@@ -132,6 +133,49 @@ int    b_readonly(int argc,char *argv[],Shbltin_t *context)
 	return(setall(argv,flag,tdata.sh->var_tree, &tdata));
 }
 
+int    b_hash(int argc,char *argv[],Shbltin_t *context)
+{
+	register unsigned flag = NV_NOARRAY | NV_NOSCOPE | NV_ASSIGN | NV_TAGGED;
+	register Dt_t *troot;
+	register int rflag=0, n;
+	struct tdata tdata;
+	NOT_USED(argc);
+	memset((void *)&tdata, 0, sizeof(tdata));
+	tdata.sh = context->shp;
+	troot = tdata.sh->alias_tree;
+
+	/* The hash utility should not be recognized as the alias builtin */
+	tdata.aflag = '-';
+
+	while((n = optget(argv,sh_opthash))) switch(n)
+	{
+		case 'r':
+			rflag = 1;
+			break;
+		case ':':
+			error(2,"%s",opt_info.arg);
+			break;
+		case '?':
+			error(ERROR_usage(0),"%s",opt_info.arg);
+			return 2;
+	}
+	if (error_info.errors)
+		errormsg(SH_DICT,ERROR_usage(2),"%s",optusage(NULL));
+	argv += (opt_info.index-1);
+
+	/* Handle -r */
+	if (rflag)
+	{
+		Namval_t *np = nv_search((char *)PATHNOD, tdata.sh->var_tree, HASH_BUCKET);
+		nv_putval(np, nv_getval(np), NV_RDONLY);
+	}
+
+	troot = tdata.sh->track_tree;
+	/* We must fork in subshells to avoid polluting the parent shell's hash table. */
+	if(tdata.sh->subshell && !tdata.sh->subshare)
+		sh_subfork();
+	return setall(argv, flag, troot, &tdata);
+}
 
 int    b_alias(int argc,register char *argv[],Shbltin_t *context)
 {
@@ -174,29 +218,8 @@ int    b_alias(int argc,register char *argv[],Shbltin_t *context)
 			errormsg(SH_DICT,ERROR_usage(2),"%s",optusage(NIL(char*)));
 		argv += (opt_info.index-1);
 		if(flag&NV_TAGGED)
-		{
-			/* hacks to handle hash -r | -- */
-			if(argv[1] && argv[1][0]=='-')
-			{
-				if(argv[1][1]=='r' && argv[1][2]==0)
-				{
-					Namval_t *np = nv_search((char*)PATHNOD,tdata.sh->var_tree,HASH_BUCKET);
-					nv_putval(np,nv_getval(np),NV_RDONLY);
-					argv++;
-					if(!argv[1])
-						return(0);
-				}
-				if(argv[1][0]=='-')
-				{
-					if(argv[1][1]=='-' && argv[1][2]==0)
-						argv++;
-					else
-						errormsg(SH_DICT, ERROR_exit(1), e_option, argv[1]);
-		}
-			}
 			troot = tdata.sh->track_tree;
 	}
-	}
 	if(context->shp->subshell && !context->shp->subshare)
 		sh_subfork();
 	return(setall(argv,flag,troot,&tdata));

src/cmd/ksh93/data/aliases.c

@@ -33,7 +33,6 @@ const struct shtable2 shtab_aliases[] =
 	"compound",	NV_NOFREE|BLT_DCL,	"typeset -C",
 	"float",	NV_NOFREE|BLT_DCL,	"typeset -lE",
 	"functions",	NV_NOFREE,		"typeset -f",
-	"hash",		NV_NOFREE,		"alias -t --",
 	"history",	NV_NOFREE,		"hist -l",
 	"integer",	NV_NOFREE|BLT_DCL,	"typeset -li",
 	"nameref",	NV_NOFREE|BLT_DCL,	"typeset -n",

src/cmd/ksh93/data/builtins.c

@@ -79,7 +79,7 @@ const struct shtable3 shtab_builtins[] =
 	"newgrp",	NV_BLTIN|BLT_ENV|BLT_SPC,	Bltin(login),
 #endif	/* _bin_newgrp || _usr_bin_newgrp */
 	"alias",	NV_BLTIN|BLT_SPC,		bltin(alias),
-	"hash",		NV_BLTIN|BLT_SPC,		bltin(alias),
+	"hash",		NV_BLTIN,			bltin(hash),
 	"enum",		NV_BLTIN|BLT_ENV|BLT_SPC|BLT_DCL,bltin(enum),
 	"eval",		NV_BLTIN|BLT_ENV|BLT_SPC|BLT_EXIT,bltin(eval),
 	"exit",		NV_BLTIN|BLT_ENV|BLT_SPC,	bltin(return),
@@ -377,7 +377,7 @@ USAGE_LICENSE
 		"definition, or an error occurred.]"
 "}"
 
-"[+SEE ALSO?\bsh\b(1), \bunalias\b(1)]"
+"[+SEE ALSO?\bsh\b(1), \bhash(1), \bunalias\b(1)]"
 ;
 
 const char sh_optbuiltin[] =
@@ -929,6 +929,27 @@ _JOB_
 "[+SEE ALSO?\bwait\b(1), \bps\b(1), \bfg\b(1), \bbg\b(1)]"
 ;
 
+const char sh_opthash[] =
+"[-1c?\n@(#)$Id: hash (ksh community) 2020-06-10 $\n]"
+"[+NAME?hash - display the locations of recently used programs]"
+"[+DESCRIPTION?The \bhash\b utility is used to display or modify "
+	"the hash table, which contains the locations of "
+	"recently used programs.  When \bhash\b is given no arguments, "
+	"it will list all commands in the hash table. If \autility\a is "
+     "supplied, \bhash\b will add that utility to the hash table.]"
+"[r?This option will empty the hash table.  The effect of "
+	"this flag can also be achieved by resetting the value of \bPATH\b.]"
+"\n"
+"\n[utility...]\n"
+"\n"
+"[+EXIT STATUS?]{"
+	"[+0?Successful completion.]"
+	"[+>0?An error occured.]"
+"}"
+
+"[+SEE ALSO?\bsh\b(1), \balias\b(1)]"
+;
+
 const char sh_opthist[]	= 
 "[-1cn?@(#)$Id: hist (AT&T Research) 2000-04-02 $\n]"
 USAGE_LICENSE

src/cmd/ksh93/include/builtins.h

@@ -85,6 +85,7 @@ extern int b_builtin(int, char*[],Shbltin_t*);
 extern int b_cd(int, char*[],Shbltin_t*);
 extern int b_command(int, char*[],Shbltin_t*);
 extern int b_getopts(int, char*[],Shbltin_t*);
+extern int b_hash(int, char*[],Shbltin_t*);
 extern int b_hist(int, char*[],Shbltin_t*);
 extern int b_let(int, char*[],Shbltin_t*);
 extern int b_read(int, char*[],Shbltin_t*);
@@ -159,6 +160,7 @@ extern const char sh_optgetopts[];
 extern const char sh_optbg[];
 extern const char sh_optdisown[];
 extern const char sh_optfg[];
+extern const char sh_opthash[];
 extern const char sh_opthist[];
 extern const char sh_optjobs[];
 extern const char sh_optkill[];

src/cmd/ksh93/sh.1

@@ -796,8 +796,6 @@ but can be unset or redefined:
 .TP
 .B "functions=\(fmtypeset \-f\(fm"
 .TP
-.B "hash=\(fmalias \-t \-\^\-\(fm"
-.TP
 .B "history=\(fmhist \-l\(fm"
 .TP
 .B "integer=\(fmtypeset \-li\(fm"
@@ -6126,6 +6124,21 @@ The option
 .B #
 can only be specified as the first option.
 .TP
+\f3hash\fP \*(OK \f3\-r\fP \f2\^\fP\*(CK \*(OK \f3utility\^\fP \*(CK
+The
+.BR hash
+command can be used to display or modify the hash table.
+The hash table is a list of the most recently used programs
+and their locations.
+If
+.I utility\^
+is supplied,
+.BR hash
+will add that utility to the hash table.
+If
+.B \-r
+is selected, the hash table is reset.
+.TP
 .PD 0
 \f3hist\fP \*(OK \f3\-e\fP \f2ename\^\fP \ \*(CK \*(OK \f3\-nlr\^\fP \*(CK \*(OK \f2first\^\fP \*(OK \f2last\^\fP \*(CK \*(CK
 .TP

src/cmd/ksh93/sh/name.c

@@ -1605,12 +1605,22 @@ void nv_putval(register Namval_t *np, const char *string, int flags)
 #endif /* SHOPT_FIXEDARRAY */
 	if(!(flags&NV_RDONLY) && nv_isattr (np, NV_RDONLY))
 		errormsg(SH_DICT,ERROR_exit(1),e_readonly, nv_name(np));
-	/* The following could cause the shell to fork if assignment
+
+	/*
+	 * Resetting the PATH in a non-forking subshell will reset the parent shell's
+	 * hash table, so work around the problem by forking before sh_assignok
+	 */
+	if(shp->subshell && !shp->subshare && np == PATHNOD)
+		sh_subfork();
+
+	/*
+	 * The following could cause the shell to fork if assignment
 	 * would cause a side effect
 	 */
 	shp->argaddr = 0;
 	if(shp->subshell && !nv_local && !(flags&NV_RDONLY))
 		np = sh_assignok(np,1);
+
 	if(np->nvfun && np->nvfun->disc && !(flags&NV_NODISC) && !nv_isref(np))
 	{
 		/* This function contains disc */

src/cmd/ksh93/tests/alias.sh

@@ -105,6 +105,11 @@ fi
 
 unalias no_such_alias && err_exit 'unalias should return non-zero for unknown alias'
 
+# ======
+# Adding a utility after resetting the hash table should work
+hash -r chmod
+[[ $(hash) == "chmod=$(whence -p chmod)" ]] || err_exit $'resetting the hash table with `hash -r \'utility\'` doesn\'t work correctly'
+
 # ======
 # Attempting to unalias a previously set alias twice should be an error
 alias foo=bar

src/cmd/ksh93/tests/subshell.sh

@@ -709,5 +709,16 @@ v=$(alias al='echo sub'; eval 'al') && [[ $v == sub ]] || err_exit 'alias fails
 v=${ eval 'al'; alias al='echo subshare'; } && [[ $v == 'mainalias' && $(eval 'al') == subshare ]] \
 || err_exit 'alias redefinition fails to survive ${ ...; }'
 
+# Resetting a subshell's hash table should not affect the parent shell
+(hash -r)
+[[ $(hash) ]] || err_exit $'resetting the hash table in a subshell affects the parent shell\'s hash table'
+(PATH="$PATH")
+[[ $(hash) ]] || err_exit $'resetting the PATH in a subshell affects the parent shell\'s hash table'
+
+# Adding a utility to a subshell's hash table should not affect the parent shell
+hash -r chmod
+(hash cat)
+[[ $(hash) == "chmod=$(whence -p chmod)" ]] || err_exit $'changes to a subshell\'s hash table affect the parent shell'
+
 # ======
 exit $((Errors<125?Errors:125))