ksh: ksh u+m does not add syntax errors to history, corrupts history

Re the thread at https://groups.google.com/g/korn-shell/c/IGAHm-5UF4o/m/PZk3mqxMAQAJ …

Something is definitely going on with history in u+m. Example:

Version AJMv 93u+m/1.0.0-alpha+c1986c4e 2021-03-06
[11:57 AM][ttys007 +1][~]
[1] mbp13 $ for i in * do echo $i ; done                                       
-ksh: syntax error: `done' unexpected
[11:58 AM][ttys007 +1][~]
[252639022] mbp13 $

Uh, WHAT?

[11:59 AM][ttys007 +1][~]
[252639022] mbp13 $ history
252639007	?for i in * do echo $i ; done
252639008	?for i in * do echo $i ; done
252639009	?for i in * do echo $i ; done
252639010	?for i in * do echo $i ; done
252639011	?for i in * do echo $i ; done
252639012	?for i in * do echo $i ; done
252639013	?for i in * do echo $i ; done
252639014	?for i in * do echo $i ; done
252639015	?for i in * do echo $i ; done
252639016	?for i in * do echo $i ; done
252639017	?for i in * do echo $i ; done
252639018	?for i in * do echo $i ; done
252639019	?for i in * do echo $i ; done
252639020	?for i in * do echo $i ; done
252639021	?for i in * do echo $i ; done
252639022	history
[11:59 AM][ttys007 +1][~]
[252639023] mbp13 $

That junk is not actually in the history file.

[12:01 PM][ttys007 +1][~]
[252639024] mbp13 $ od -c /Users/mwilson/.ksh/histfiles/history.ttys007  
0000000  201 001   f   o   r       i       i   n       *       d   o    
0000020    e   c   h   o       $   i       ;       d   o   n   e  \n  \0
0000040  201  \0   h   i   s   t   o   r   y  \n  \0  \0   e   c   h   o
0000060        $   H   I   S   T   F   I   L   E  \n  \0   o   d       -
0000100    c       /   U   s   e   r   s   /   m   w   i   l   s   o   n
0000120    /   .   k   s   h   /   h   i   s   t   f   i   l   e   s   /
0000140    h   i   s   t   o   r   y   .   t   t   y   s   0   0   7  \n
0000160   \0  \0                                                        
0000162

If you kill that copy of ksh and reopen one, the junk is gone, so its an in-memory corruption.

Version AJMv 93u+m/1.0.0-alpha+c1986c4e 2021-03-06
[12:09 PM][ttys007 +1][~]
[4] mbp13 $ history
1	history
2	echo $HISTFILE
3	od -c /Users/mwilson/.ksh/histfiles/history.ttys007
4	history
[12:09 PM][ttys007 +1][~]
[5] mbp13 $

Now, that was with a new history file (had to go up to tty007 to get a blank one, I wanted the shell to create a new one). With an existing populated history file, u+m silently fails to add the line and there appears to be no corruption:

[12:07 PM][ttys000 +1][~]
[719] mbp13 $ for i in * do echo $i ; done
-ksh: syntax error: `done' unexpected
[12:07 PM][ttys000 +1][~]
[719] mbp13 $ history -1
718	ls
719	history -1
[12:08 PM][ttys000 +1][~]
[720] mbp13 $ ver
Version AJMv 93u+m/1.0.0-alpha+c1986c4e 2021-03-06

ksh 93u+ does not do this, it simply adds the erroneous command line to the history file:

Version AJM 93u+ 2012-08-01
[12:12 PM][ttys007 +2][~]
[2] mbp13 $ for i in * do echo $i ; done
/bin/ksh: syntax error: `done' unexpected
[12:12 PM][ttys007 +2][~]
[3] mbp13 $ history
1	/bin/ksh
2	for i in * do echo $i ; done
3	history
[12:12 PM][ttys007 +2][~]
[4] mbp13 $

About this issue

Original URL
State: closed
Created 3 years ago
Comments: 18

Most upvoted comments

Patch. No regression tests fail, including those added in e999f6b. And the bug seems to be fixed.

This also changes the style of the call to match historic practice. NULL was/is rarely or never used, 0 or NIL(sometype) is generally used instead. As far as I know, it makes no real difference, but I like to make new code look like it belongs there.

diff --git a/src/cmd/ksh93/sh/io.c b/src/cmd/ksh93/sh/io.c
index 0f4be76b..4b83a7e3 100644
--- a/src/cmd/ksh93/sh/io.c
+++ b/src/cmd/ksh93/sh/io.c
@@ -1712,11 +1712,6 @@ void	sh_iorestore(Shell_t *shp, int last, int jmpval)
 	register int 	origfd, savefd, fd;
 	int flag = (last&IOSUBSHELL);
 	last &= ~IOSUBSHELL;
-	/*
-	 * There was an issue with truncating files (see 'ftruncate' below) that was caused by
-	 * out-of-sync streams. So, to be safe, sync all streams before restoring file descriptors.
-	 */
-	sfsync(NULL);
 	for (fd = shp->topfd - 1; fd >= last; fd--)
 	{
 		if(!flag && filemap[fd].subshell)
@@ -1740,7 +1735,10 @@ void	sh_iorestore(Shell_t *shp, int last, int jmpval)
 			return;
 		}
 		if(filemap[fd].tname == Empty && shp->exitval==0)
+		{
+			sfsync(NIL(Sfio_t*));
 			ftruncate(origfd,lseek(origfd,0,SEEK_CUR));
+		}
 		else if(filemap[fd].tname)
 			io_usename(filemap[fd].tname,(int*)0,origfd,shp->exitval?2:1);
 		sh_close(origfd);

McDutchie on Mar 6, 2021

The patch fixes it for me on macOS Catalina 10.15.7 and Linux Mint.

I don’t suppose it matters, but why is the behavior of the bug different depending on whether or not it’s the first write to the history file? If the history file already exists, it just silently fails adding the line to the file. But if the file doesn’t yet exist, and ksh is flushing it out to storage, then you get the corruption.

posguy99 on Mar 7, 2021

I test each commit from the month

git bisect could be a very good friend on such case.

avih on Mar 6, 2021

I test the last commit of each month (e.g., if a bug occurs in a July 31 commit but not in the last commit from June 30, the bug was introduced in July). After that I test each commit from the month the bug first appeared until I find the broken one.

JohnoKing on Mar 6, 2021