Fix vi mode crashes when going back one word (#246)

This bug was originally reported at <att#1467>.
A crash can occur when using the 'b' or 'B' vi mode commands to go back
one word. I was able to reproduce these crashes with 100% consistency on
an OpenBSD virtual machine when ksh is compiled with -D_std_malloc.
Reproducer:
    $ set -o vi
    $ asdf <ESC> <b or B>

The fix is based on Matthew DeVore's analysis:
> I suspect this is caused by this line:
>> while (vi_isalph(tcur_virt) && tcur_virt >= first_virt) --tcur_virt;
> which is in the b codepath. It checks vi_isalph(tcur_virt) before checking
> if tcur_virt is in range. These two clauses should be reversed. Note that
> line 316 is a similar check for pressing B, and there the tcur_virt value
> is checked first.

src/cmd/ksh93/edit/vi.c:
- Check tcur_virt before using isalph() or isblank() to fix both crashes.
  At the start of the backword() while loop this check was performed
  twice, so the redundant check has been removed.

src/cmd/ksh93/tests/pty.sh:
- Add a regression test for the b, B, w and W editor commands.

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.
 
+2021-03-29:
+
+- Fixed an intermittent crash that could occur in vi mode when using the 'b'
+  or 'B' commands to go back one word.
+
 2021-03-27:
 
 - The 'test' builtin will now show an error message when given the invalid ']]'

src/cmd/ksh93/edit/vi.c

@@ -664,8 +664,7 @@ static void backword(Vi_t *vp,int nwords, register int cmd)
 	register int tcur_virt = cur_virt;
 	while( nwords-- && tcur_virt > first_virt )
 	{
-		if( !isblank(tcur_virt) && isblank(tcur_virt-1)
-			&& tcur_virt>first_virt )
+		if( !isblank(tcur_virt) && isblank(tcur_virt-1) )
 			--tcur_virt;
 		else if(cmd != 'B')
 		{
@@ -674,21 +673,20 @@ static void backword(Vi_t *vp,int nwords, register int cmd)
 			if((!cur && last) || (cur && !last))
 				--tcur_virt;
 		}
-		while( isblank(tcur_virt) && tcur_virt>=first_virt )
+		while( tcur_virt >= first_virt && isblank(tcur_virt) )
 			--tcur_virt;
 		if( cmd == 'B' )
 		{
-			while( !isblank(tcur_virt) && tcur_virt>=first_virt )
+			while( tcur_virt >= first_virt && !isblank(tcur_virt) )
 				--tcur_virt;
 		}
 		else
 		{
 			if(isalph(tcur_virt))
-				while( isalph(tcur_virt) && tcur_virt>=first_virt )
+				while( tcur_virt >= first_virt && isalph(tcur_virt) )
 					--tcur_virt;
 			else
-				while( !isalph(tcur_virt) && !isblank(tcur_virt)
-					&& tcur_virt>=first_virt )
+				while( tcur_virt >= first_virt && !isalph(tcur_virt) && !isblank(tcur_virt) )
 					--tcur_virt;
 		}
 		cur_virt = ++tcur_virt;
@@ -1235,23 +1233,22 @@ static void endword(Vi_t *vp, int nwords, register int cmd)
 	register int tcur_virt = cur_virt;
 	while( nwords-- )
 	{
-		if( !isblank(tcur_virt) && tcur_virt<=last_virt )
+		if( tcur_virt <= last_virt && !isblank(tcur_virt) )
 			++tcur_virt;
-		while( isblank(tcur_virt) && tcur_virt<=last_virt )
+		while( tcur_virt <= last_virt && isblank(tcur_virt) )
 			++tcur_virt;	
 		if( cmd == 'E' )
 		{
-			while( !isblank(tcur_virt) && tcur_virt<=last_virt )
+			while( tcur_virt <= last_virt && !isblank(tcur_virt) )
 				++tcur_virt;
 		}
 		else
 		{
 			if( isalph(tcur_virt) )
-				while( isalph(tcur_virt) && tcur_virt<=last_virt )
+				while( tcur_virt <= last_virt && isalph(tcur_virt) )
 					++tcur_virt;
 			else
-				while( !isalph(tcur_virt) && !isblank(tcur_virt)
-					&& tcur_virt<=last_virt )
+				while( tcur_virt <= last_virt && !isalph(tcur_virt) && !isblank(tcur_virt) )
 					++tcur_virt;
 		}
 		if( tcur_virt > first_virt )
@@ -1274,24 +1271,23 @@ static void forward(Vi_t *vp,register int nwords, int cmd)
 	{
 		if( cmd == 'W' )
 		{
-			while( !isblank(tcur_virt) && tcur_virt < last_virt )
+			while( tcur_virt < last_virt && !isblank(tcur_virt) )
 				++tcur_virt;
 		}
 		else
 		{
 			if( isalph(tcur_virt) )
 			{
-				while( isalph(tcur_virt) && tcur_virt<last_virt )
+				while( tcur_virt < last_virt && isalph(tcur_virt) )
 					++tcur_virt;
 			}
 			else
 			{
-				while( !isalph(tcur_virt) && !isblank(tcur_virt)
-					&& tcur_virt < last_virt )
+				while( tcur_virt < last_virt && !isalph(tcur_virt) && !isblank(tcur_virt) )
 					++tcur_virt;
 			}
 		}
-		while( isblank(tcur_virt) && tcur_virt < last_virt )
+		while( tcur_virt < last_virt && isblank(tcur_virt) )
 			++tcur_virt;
 	}
 	cur_virt = tcur_virt;
@@ -1606,7 +1602,7 @@ static int mvcursor(register Vi_t* vp,register int motion)
 
 	case '^':		/** First nonblank character **/
 		tcur_virt = first_virt;
-		while( isblank(tcur_virt) && tcur_virt < last_virt )
+		while( tcur_virt < last_virt && isblank(tcur_virt) )
 			++tcur_virt;
 		break;
 

src/cmd/ksh93/include/version.h

@@ -20,7 +20,7 @@
 
 #define SH_RELEASE_FORK	"93u+m"		/* only change if you develop a new ksh93 fork */
 #define SH_RELEASE_SVER	"1.0.0-alpha"	/* semantic version number: https://semver.org */
-#define SH_RELEASE_DATE	"2021-03-27"	/* must be in this format for $((.sh.version)) */
+#define SH_RELEASE_DATE	"2021-03-29"	/* must be in this format for $((.sh.version)) */
 #define SH_RELEASE_CPYR	"(c) 2020-2021 Contributors to ksh " SH_RELEASE_FORK
 
 /* Scripts sometimes field-split ${.sh.version}, so don't change amount of whitespace. */

src/cmd/ksh93/tests/pty.sh

@@ -770,5 +770,18 @@ w : XX\t
 r ^:test-1: : XXX\r\n$
 !
 
+((SHOPT_VSH)) && tst $LINENO <<"!"
+L Using b, B, w and W commands in vi mode
+# https://github.com/att/ast/issues/1467
+
+d 15
+p :test-1:
+w set -o vi
+r ^:test-1: set -o vi\r\n$
+w echo asdf\EbwBWa
+r ^:test-2: echo asdf\r\n$
+r ^asdf\r\n$
+!
+
 # ======
 exit $((Errors<125?Errors:125))