Further fix '<>;' and fix crash on 32-bit systems (re: 6701bb30)

Accessing t->tre.treio for every sh_exec() run is invalid because
't' is of type Shnode_t, which is a union that can contain many
different kinds of structs. As all members of a union occupy the
same address space, only one can be used at a time. Which member is
valid to access depends on the node type sh_exec() was called with.
The invalid access triggered a crash on 32-bit systems when
executing an arithmetic command like ((x=1)).

The t->tre.treio union member should be accessed for a simple
command (case TCOM in sh_exec()). The fix is also needed for
redirections attached to blocks (case TSETIO) in which case the
union member to use is t->fork.forkio.

src/cmd/ksh93/sh/xec.c:
- Add check_exec_optimization() function that checks for all the
  conditions where the exec optimisation should not be done. For
  redirections we need to loop through the whole list to check for
  an IOREWRITE (<>;) one.
- sh_exec(): case TCOM (simple command): Only bother to call
  check_exec_optimization() if there are either command arguments
  or redirections (IOW: don't bother for bare variable
  assignments), so move it to within the if(io||argn) block.
- sh_exec(): case TSETIO: This needs a similar fix. To avoid the
  optimization breaking again if the last command is a subshell
  with a <>; redirection attached, we need to not only set execflg
  to 0 but also clear the SH_NOFORK state bit from the 'flags'
  variable which is passed on to the recursive sh_exec() call.

src/cmd/ksh93/tests/io.sh:
- Update and expand tests. Add tests for redirections attached to
  simple commands (TCOM) and various kinds of code block (TSETIO).

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

NEWS

@@ -8,10 +8,8 @@ Any uppercase BUG_* names are modernish shell bug IDs.
 - Fixed a bug in emacs mode: after using tab completion to complete the name
   of a directory, it was not possible to type numbers after the slash.
 
-2021-04-15:
-
 - Fixed an optimization bug that caused the <>; redirection operator to fail
-  when used in -c scripts.
+  when used with the last command in a -c script.
 
 2021-04-14:
 

src/cmd/ksh93/sh/xec.c

@@ -958,6 +958,21 @@ static Namval_t *enter_namespace(Shell_t *shp, Namval_t *nsp)
 }
 #endif /* SHOPT_NAMESPACE */
 
+/*
+ * Check whether to execve(2) the final command or make its redirections permanent.
+ */
+static int check_exec_optimization(struct ionod *iop)
+{
+	if(sh.subshell || sh.exittrap || sh.errtrap)
+		return(0);
+	/* '<>;' (IOREWRITE) redirections are incompatible with exec */
+	while(iop && !(iop->iofile & IOREWRITE))
+		iop = iop->ionxt;
+	if(iop)
+		return(0);
+	return(1);
+}
+
 int sh_exec(register const Shnode_t *t, int flags)
 {
 	register Shell_t	*shp = sh_getinterp();
@@ -1003,8 +1018,6 @@ int sh_exec(register const Shnode_t *t, int flags)
 		shp->exitval=0;
 		shp->lastsig = 0;
 		shp->lastpath = 0;
-		if(shp->exittrap || shp->errtrap || (t->tre.treio && (t->tre.treio->iofile&IOREWRITE)))
-			execflg = 0; /* don't run the command with execve(2) */
 		switch(type&COMMSK)
 		{
 		    case TCOM:
@@ -1174,6 +1187,8 @@ int sh_exec(register const Shnode_t *t, int flags)
 				int tflags = 1;
 				if(np &&  nv_isattr(np,BLT_DCL))
 					tflags |= 2;
+				if(execflg && !check_exec_optimization(io))
+					execflg = 0;
 				if(argn==0)
 				{
 					/* fake 'true' built-in */
@@ -1848,8 +1863,6 @@ int sh_exec(register const Shnode_t *t, int flags)
 			int 	jmpval, waitall = 0;
 			int 	simple = (t->fork.forktre->tre.tretyp&COMMSK)==TCOM;
 			struct checkpt *buffp = (struct checkpt*)stkalloc(shp->stk,sizeof(struct checkpt));
-			if(shp->subshell)
-				execflg = 0;
 			sh_pushcontext(shp,buffp,SH_JMPIO);
 			if(type&FPIN)
 			{
@@ -1875,6 +1888,11 @@ int sh_exec(register const Shnode_t *t, int flags)
 			{
 				if(shp->comsub==1)
 					tsetio = 1;
+				if(execflg && !check_exec_optimization(t->fork.forkio))
+				{
+					execflg = 0;
+					flags &= ~sh_state(SH_NOFORK);
+				}
 				sh_redirect(shp,t->fork.forkio,execflg);
 				(t->fork.forktre)->tre.tretyp |= t->tre.tretyp&FSHOWME;
 				sh_exec(t->fork.forktre,flags&~simple);

src/cmd/ksh93/tests/io.sh

@@ -491,28 +491,67 @@ n=$( exec {n}< /dev/null; print -r -- $n)
 # ======
 # Truncating a file using <> and >#num
 # https://github.com/att/ast/issues/61
-
-for ((i=1; i<=10; i++)) do print "$i"; done | tee "$tmp/nums2" > "$tmp/nums1"
-expect=$'1\n2\n3\n4'
-
-(1<>;"$tmp/nums1" >#5)
-actual=$(cat "$tmp/nums1")
-[[ "$actual" = "$expect" ]] || err_exit "Failed to truncate file in subshell \
-(expected $(printf %q "$expect"), got $(printf %q "$actual"))"
-
 # The <>; redirection operator didn't work correctly in -c scripts
 # https://github.com/att/ast/issues/9
-"$SHELL" -c '1<>;"$1/nums2" >#5' x "$tmp"
+# https://github.com/ksh93/ksh/issues/278
-actual=$(cat "$tmp/nums2")
-[[ "$actual" = "$expect" ]] || err_exit "Failed to truncate file in -c script \
-(expected $(printf %q "$expect"), got $(printf %q "$actual"))"
 
-echo test > "$tmp/ast-9-reproducer"
+# the extra '3>&2 3>&-' is to verify it all keeps working with multiple redirections
-"$SHELL" -c "echo x 1<>; \"$tmp/ast-9-reproducer\""
+typeset -A exp=(
-exp=x
+	['3>&2 3>&- 1<>;nums >#5']=$'1\n2\n3\n4'
-got=$(cat "$tmp/ast-9-reproducer")
+	[': 3>&2 3>&- 1<>;nums >#5']=$'1\n2\n3\n4'
-[[ $exp == "$got" ]] || err_exit "<>; redirection operator fails in -c scripts \
+	['{ :; } 3>&2 3>&- 1<>;nums >#5']=$'1\n2\n3\n4'
-(expected $(printf %q "$exp"), got $(printf %q "$got"))"
+	['(:) 3>&2 3>&- 1<>;nums >#5']=$'1\n2\n3\n4'
+	['while false; do :; done 3>&2 3>&- 1<>;nums >#5']=$'1\n2\n3\n4'
+	['until true; do :; done 3>&2 3>&- 1<>;nums >#5']=$'1\n2\n3\n4'
+	['if false; then :; fi 3>&2 3>&- 1<>;nums >#5']=$'1\n2\n3\n4'
+	['case x in y) :;; esac 3>&2 3>&- 1<>;nums >#5']=$'1\n2\n3\n4'
+	['fn() { :; } 3>&2 3>&- 1<>;nums >#5; fn']=$'1\n2\n3\n4'
+	['echo x 3>&2 3>&- 1<>;nums']=x
+	['{ echo x; } 3>&2 3>&- 1<>;nums']=x
+	['(echo x) 3>&2 3>&- 1<>;nums']=x
+	['while :; do echo x; break; done 3>&2 3>&- 1<>;nums']=x
+	['until ! :; do echo x; break; done 3>&2 3>&- 1<>;nums']=x
+	['if :; then echo x; fi 3>&2 3>&- 1<>;nums']=x
+	['case x in x) echo x;; esac 3>&2 3>&- 1<>;nums']=x
+	['fn() { echo x; } 3>&2 3>&- 1<>;nums; fn']=x
+)
+nums=$'1\n2\n3\n4\n5\n6\n7\n8\n9\n10'
+for script in "${!exp[@]}"
+do
+	echo "$nums" >nums
+	eval "$script"
+	got=$(<nums)
+	[[ $got == "${exp[$script]}" ]] || err_exit "IOREWRITE: '$script' failed in main shell" \
+		"(expected $(printf %q "${exp[$script]}"), got $(printf %q "$got"))"
+	echo "$nums" >nums
+	eval "( $script )"
+	got=$(<nums)
+	[[ $got == "${exp[$script]}" ]] || err_exit "IOREWRITE: '$script' failed in subshell" \
+		"(expected $(printf %q "${exp[$script]}"), got $(printf %q "$got"))"
+	echo "$nums" >nums
+	"$SHELL" -c "$script"
+	got=$(<nums)
+	[[ $got == "${exp[$script]}" ]] || err_exit "IOREWRITE: '$script' failed as -c script" \
+		"(expected $(printf %q "${exp[$script]}"), got $(printf %q "$got"))"
+	echo "$nums" >nums
+	echo "$script" >test.sh
+	"$SHELL" test.sh
+	got=$(<nums)
+	[[ $got == "${exp[$script]}" ]] || err_exit "IOREWRITE: '$script' failed as regular script" \
+		"(expected $(printf %q "${exp[$script]}"), got $(printf %q "$got"))"
+done
+unset exp script
+
+got=$("$SHELL" -c $'trap "echo bye" 0\n{ echo hi; } >/dev/null' 2>&1)
+exp=bye
+[[ $got == "$exp" ]] || err_exit "redirection in last -c script command persists for trap" \
+	"(expected $(printf %q "$exp"), got $(printf %q "$got"))"
+
+echo $'trap "echo bye" 0\n{ echo hi; } >/dev/null' >test.sh
+got=$("$SHELL" test.sh 2>&1)
+exp=bye
+[[ $got == "$exp" ]] || err_exit "redirection in last regular script command persists for trap" \
+	"(expected $(printf %q "$exp"), got $(printf %q "$got"))"
 
 # ======
 # Exit behaviour of 'exec', 'command exec', 'redirect' on redirections
@@ -538,7 +577,7 @@ actual=$( (redirect /dev/null/foo >$tmp/wrong_redirect) 2>&1; echo " status = $?
 # Process substitution
 
 # An output process substitution should work when combined with a redirection.
-result=$("$SHELL" -c 'echo ok > >(sed s/ok/good/); wait')
+result=$("$SHELL" -c 'echo ok > >(sed s/ok/good/); wait' 2>&1)
 [[ $result == good ]] || err_exit 'process substitution does not work with redirections' \
 				"(expected 'good', got $(printf %q "$result"))"