Syntax errors in `~/.kshrc` cause ksh to exit on startup #281

JohnoKing · 2021-04-21T10:11:27Z

There are two regressions related to how ksh handles syntax errors in the .kshrc file. If ~/.kshrc or the file pointed to by $ENV have a syntax error, ksh exits during startup. Additionally, the error message printed is incorrect:

$ cat /tmp/synerror
((
echo foo

# ksh93u+m
$ ENV=/tmp/synerror arch/*/bin/ksh -ic 'echo ${.sh.version}'
/tmp/synerror: syntax error: `/t/tmp/synerror' unmatched

# ksh93u+
$ ENV=/tmp/synerror ksh93u -ic 'echo ${.sh.version}'                    
/tmp/synerror: syntax error: `(' unmatched
Version AJM 93u+ 2012-08-01

The regression that causes the incorrect error message was introduced by commit cb67a01. The other bug that causes ksh to exit on startup was introduced by commit ceb77b1.

The text was updated successfully, but these errors were encountered:

McDutchie · 2021-04-21T11:19:37Z

Thanks for finding the commits that introduced these, that made it much easier. Please try this:

--- a/src/cmd/ksh93/sh/lex.c
+++ b/src/cmd/ksh93/sh/lex.c
@@ -2060,6 +2060,7 @@ static char	*fmttoken(Lex_t *lp, register int sym)
 		return((char*)sh_translate(e_endoffile));
 	if(sym==NL)
 		return((char*)sh_translate(e_newline));
+	stakfreeze(0);
 	stakputc(sym);
 	if(sym&SYMREP)
 		stakputc(sym);
--- a/src/cmd/ksh93/sh/main.c
+++ b/src/cmd/ksh93/sh/main.c
@@ -424,7 +424,7 @@ static void	exfile(register Shell_t *shp, register Sfio_t *iop,register int fno)
 		sfsync(shp->outpool);
 		shp->st.execbrk = shp->st.breakcnt = 0;
 		/* check for return from profile or env file */
-		if(sh_isstate(SH_PROFILE) && (jmpval==SH_JMPFUN || jmpval==SH_JMPEXIT || jmpval==SH_JMPERREXIT))
+		if(sh_isstate(SH_PROFILE) && (jmpval==SH_JMPFUN || jmpval==SH_JMPEXIT) || !sh_isstate(SH_PROFILE) && jmpval==SH_JMPERREXIT)
 		{
 			sh_setstate(states);
 			goto done;
@@ -603,7 +603,7 @@ done:
 	}
 	if(jmpval == SH_JMPSCRIPT)
 		siglongjmp(*shp->jmplist,jmpval);
-	else if(jmpval == SH_JMPEXIT || jmpval == SH_JMPERREXIT)
+	else if(jmpval == SH_JMPEXIT || !sh_isstate(SH_PROFILE) && jmpval == SH_JMPERREXIT)
 		sh_done(shp,0);
 	if(fno>0)
 		sh_close(fno);

JohnoKing · 2021-04-21T11:28:47Z

I can confirm the new patch fixes both of the regressions.

JohnoKing · 2021-04-21T11:57:33Z

It looks like the patch will need some more work. I've ran it against the regression tests and got these two failures:

$ bin/shtests -p pty
#### Regression-testing /home/johno/GitRepos/KornShell/ksh/arch/linux.i386-64/bin/ksh ####
test pty begins at 2021-04-21+04:55:58
	pty.sh[683]: syntax error added to history file: line 694: expected "^:test-2: fc -lN1\r\n$", got "\r\n"
	pty.sh[683]: syntax error added to history file: line 695: expected "\tdo something\r\n$", got "fc -lN1\r\n"
test pty failed at 2021-04-21+04:56:12 with exit code 2 [ 34 tests 2 errors ]
Total errors: 2

JohnoKing · 2021-04-21T15:38:29Z

Commit ceb77b1 is meant to fix rhbz#1212992. The reproducer for the bug is in comment 3. I've tried it against ksh93u+ and it crashes (as expected), but even when I revert Red Hat's patch it seems to work just fine in ksh93u+m.

# Note: Both tests below are run with vmalloc

# ksh93u+
$ LD_PRELOAD=/home/johno/Downloads/write.so valgrind ksh93u  -l
Memory fault  # After loops invoking sh_assignok()


# ksh93u+m, with the diskfull patch reverted
$ LD_PRELOAD=/home/johno/Downloads/write.so valgrind arch/*/bin/ksh  -l
==8324== Memcheck, a memory error detector
==8324== Copyright (C) 2002-2017, and GNU GPL'd, by Julian Seward et al.
==8324== Using Valgrind-3.16.1 and LibVEX; rerun with -h for copyright info
==8324== Command: arch/linux.i386-64/bin/ksh -l
==8324== 
$

Possibly the diskfull patch is no longer necessary? I'll trace which commit fixed the memory fault.

JohnoKing · 2021-04-21T16:14:44Z

I've found that commit 970069a is what fixed the memory fault. Additionally, I've tested Red Hat's patch against ksh93u+ (and ksh93u+m with the aforementioned commit reverted) and on my system the diskfull patch doesn't fix the crash at all. So, unless I'm missing something I believe the best way to fix this bug is to revert the broken diskfull patch. New diff below (all the regression tests pass, including Red Hat's reproducer):

--- a/src/cmd/ksh93/sh/lex.c
+++ b/src/cmd/ksh93/sh/lex.c
@@ -2060,6 +2060,7 @@ static char	*fmttoken(Lex_t *lp, register int sym)
 		return((char*)sh_translate(e_endoffile));
 	if(sym==NL)
 		return((char*)sh_translate(e_newline));
+	stakfreeze(0);
 	stakputc(sym);
 	if(sym&SYMREP)
 		stakputc(sym);
--- a/src/cmd/ksh93/sh/main.c
+++ b/src/cmd/ksh93/sh/main.c
@@ -424,7 +424,7 @@ static void	exfile(register Shell_t *shp, register Sfio_t *iop,register int fno)
 		sfsync(shp->outpool);
 		shp->st.execbrk = shp->st.breakcnt = 0;
 		/* check for return from profile or env file */
-		if(sh_isstate(SH_PROFILE) && (jmpval==SH_JMPFUN || jmpval==SH_JMPEXIT || jmpval==SH_JMPERREXIT))
+		if(sh_isstate(SH_PROFILE) && (jmpval==SH_JMPFUN || jmpval==SH_JMPEXIT))
 		{
 			sh_setstate(states);
 			goto done;
@@ -603,7 +603,7 @@ done:
 	}
 	if(jmpval == SH_JMPSCRIPT)
 		siglongjmp(*shp->jmplist,jmpval);
-	else if(jmpval == SH_JMPEXIT || jmpval == SH_JMPERREXIT)
+	else if(jmpval == SH_JMPEXIT)
 		sh_done(shp,0);
 	if(fno>0)
 		sh_close(fno);

McDutchie · 2021-04-21T17:52:58Z

It looks like the patch will need some more work. I've ran it against the regression tests and got these two failures:

$ bin/shtests -p pty
#### Regression-testing /home/johno/GitRepos/KornShell/ksh/arch/linux.i386-64/bin/ksh ####
test pty begins at 2021-04-21+04:55:58
	pty.sh[683]: syntax error added to history file: line 694: expected "^:test-2: fc -lN1\r\n$", got "\r\n"
	pty.sh[683]: syntax error added to history file: line 695: expected "\tdo something\r\n$", got "fc -lN1\r\n"
test pty failed at 2021-04-21+04:56:12 with exit code 2 [ 34 tests 2 errors ]
Total errors: 2

FWIW, on the Mac, the test fails like this:

 bin/shtests -p pty
#### Regression-testing /usr/local/src/ksh93/ksh/arch/darwin.i386-64/bin/ksh ####
test pty begins at 2021-04-21+18:50:13
	pty.sh[683]: syntax error added to history file: line 693: write failed [Input/output error]
test pty failed at 2021-04-21+18:50:28 with exit code 1 [ 34 tests 1 error ]
Total errors: 1
CPU time       user:      system:
main:      0m00.008s    0m00.020s
tests:     0m00.333s    0m01.013s

Thanks for doing the testing and figuring out that the diskfull patch should be reverted. Your results make sense to me and I'm happy to take your word for it.

JohnoKing changed the title ~~Syntax errors in kshrc cause ksh to exit on startup~~ Syntax errors in ~/.kshrc cause ksh to exit on startup Apr 21, 2021

McDutchie added the bug Something is not working label Apr 21, 2021

McDutchie closed this as completed in b7dde4e Apr 21, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Syntax errors in `~/.kshrc` cause ksh to exit on startup #281

Syntax errors in `~/.kshrc` cause ksh to exit on startup #281

JohnoKing commented Apr 21, 2021 •

edited

Loading

McDutchie commented Apr 21, 2021

JohnoKing commented Apr 21, 2021

JohnoKing commented Apr 21, 2021

JohnoKing commented Apr 21, 2021

JohnoKing commented Apr 21, 2021 •

edited

Loading

McDutchie commented Apr 21, 2021 •

edited

Loading

Syntax errors in ~/.kshrc cause ksh to exit on startup #281

Syntax errors in ~/.kshrc cause ksh to exit on startup #281

Comments

JohnoKing commented Apr 21, 2021 • edited Loading

McDutchie commented Apr 21, 2021

JohnoKing commented Apr 21, 2021

JohnoKing commented Apr 21, 2021

JohnoKing commented Apr 21, 2021

JohnoKing commented Apr 21, 2021 • edited Loading

McDutchie commented Apr 21, 2021 • edited Loading

Syntax errors in `~/.kshrc` cause ksh to exit on startup #281

Syntax errors in `~/.kshrc` cause ksh to exit on startup #281

JohnoKing commented Apr 21, 2021 •

edited

Loading

JohnoKing commented Apr 21, 2021 •

edited

Loading

McDutchie commented Apr 21, 2021 •

edited

Loading