Fix certain syntax errors not being recorded in history

For the following types of syntax error, the command line is not recorded in the shell history for retrieval and correction: $ a[]=12 -ksh: syntax error at line 1: `[]' empty subscript $ function foo $bad { :; } -ksh: syntax error at line 1: invalid reference list $ echo $(( 1 << 8 ) ) -ksh: syntax error at line 1: `<<8' here-document not contained within command substitution Up-arrow will not work to get the above command lines back. It's also odd that these report a line number (1) though we're working on the interactive command line and other syntax errors do not report a line number, e.g.: $ ) -ksh: syntax error: `)' unexpected Analysis: Syntax errors are normally thrown via a call to sh_syntax(), which does the I/O stream flushing and closing necessary to ensure that the command line gets added to the history before calling errormsg(). But the above three problematic errors call errormsg() directly. The fix is to refactor sh_syntax() to handle these errors properly and call sh_syntax() when these occur. src/cmd/ksh93/sh/lex.c, src/cmd/ksh93/sh/parse.c: - Add 'special' argument to sh_syntax() to handle the three cases above. A value of zero is a regular syntax error and 1-3 are the special messages above. - Construct the error message in parts using the sh.strbuf Sfio string buffer. This fixes the cosmetic 'at line 1' problem on interactive shells. src/cmd/ksh93/data/{keywords,lexstates}.c, src/cmd/ksh93/include/{shlex,lexstates}.h: - Refactor and rename the syntax error-related messages to make the above changes possible. - Group them all together in lexstates.{c,h} for clarity. Resolves: #775
ksh93 · Aug 23, 2024 · abeb5ea · abeb5ea
1 parent ac598d2
commit abeb5ea
Show file tree

Hide file tree

Showing 8 changed files with 108 additions and 104 deletions.
diff --git a/NEWS b/NEWS
@@ -2,6 +2,11 @@ This documents significant changes in the 1.0 branch of ksh 93u+m.
 For full details, see the git log at: https://github.com/ksh93/ksh/tree/1.0
 Uppercase BUG_* IDs are shell bug IDs as used by the Modernish shell library.
 
+2024-08-22:
+
+- Fixed: command lines containing certain specific kinds of syntax error
+  were not entered into the shell's history for retrieval and correction.
+
 2024-08-21:
 
 - Fixed a corner-case crashing bug in shell discipline functions.

diff --git a/src/cmd/ksh93/data/keywords.c b/src/cmd/ksh93/data/keywords.c
@@ -2,7 +2,7 @@
 *                                                                      *
 *               This software is part of the ast package               *
 *          Copyright (c) 1982-2012 AT&T Intellectual Property          *
-*          Copyright (c) 2020-2023 Contributors to ksh 93u+m           *
+*          Copyright (c) 2020-2024 Contributors to ksh 93u+m           *
 *                      and is licensed under the                       *
 *                 Eclipse Public License, Version 2.0                  *
 *                                                                      *
@@ -51,9 +51,3 @@ const Shtable_t shtab_reserved[] =
 		"}",		RBRACE,
 		"",		0,
 };
-
-const char	e_unexpected[]	= "unexpected";
-const char	e_unmatched[]	= "unmatched";
-const char	e_endoffile[]	= "end of file";
-const char	e_newline[]	= "newline";
-
diff --git a/src/cmd/ksh93/data/lexstates.c b/src/cmd/ksh93/data/lexstates.c
@@ -2,7 +2,7 @@
 *                                                                      *
 *               This software is part of the ast package               *
 *          Copyright (c) 1982-2011 AT&T Intellectual Property          *
-*          Copyright (c) 2020-2023 Contributors to ksh 93u+m           *
+*          Copyright (c) 2020-2024 Contributors to ksh 93u+m           *
 *                      and is licensed under the                       *
 *                 Eclipse Public License, Version 2.0                  *
 *                                                                      *
@@ -729,11 +729,19 @@ const char *sh_lexrstates[ST_NONE] =
 const char e_lexversion[]	= "%d: invalid binary script version";
 const char e_lexspace[]		= "line %d: use space or tab to separate operators %c and %c";
 const char e_lexslash[]		= "line %d: $ not preceded by \\";
-const char e_lexsyntax1[]	= "syntax error at line %d: `%s' %s";
-const char e_lexsyntax2[]	= "syntax error: `%s' %s";
-const char e_lexsyntax3[]	= "syntax error at line %d: duplicate label %s";
-const char e_lexsyntax4[]	= "syntax error at line %d: invalid reference list";
-const char e_lexsyntax5[]	= "syntax error at line %d: `<<%s' here-document not contained within command substitution";
+
+/* syntax error messages */
+const char e_syntaxerror[]	= "syntax error: ";
+const char e_syntaxerror_at[]	= "syntax error at line %d: ";
+const char e_unexpected[]	= "`%s' unexpected";
+const char e_unmatched[]	= "`%s' unmatched";
+const char e_emptysubscr[]	= "[]: empty subscript";
+const char e_badreflist[]	= "invalid reference list";
+const char e_heredoccomsub[]	= "`<<%s' here-document not contained within command substitution";
+const char e_endoffile[]	= "end of file";
+const char e_newline[]		= "newline";
+
+/* noexec linter warning messages */
 const char e_lexwarnvar[]	= "line %d: in '((%s))', using '$' as in '$%.*s' is slower and can introduce rounding errors";
 const char e_lexarithwarn[]	= "line %d: %s is slower than ((%.*s%s";
 const char e_lexobsolete1[]	= "line %d: `...` obsolete, use $(...)";

diff --git a/src/cmd/ksh93/include/lexstates.h b/src/cmd/ksh93/include/lexstates.h
@@ -129,11 +129,17 @@ extern const char *sh_lexrstates[ST_NONE];
 extern const char e_lexversion[];
 extern const char e_lexspace[];
 extern const char e_lexslash[];
-extern const char e_lexsyntax1[];
-extern const char e_lexsyntax2[];
-extern const char e_lexsyntax3[];
-extern const char e_lexsyntax4[];
-extern const char e_lexsyntax5[];
+
+extern const char e_syntaxerror[];
+extern const char e_syntaxerror_at[];
+extern const char e_unexpected[];
+extern const char e_unmatched[];
+extern const char e_emptysubscr[];
+extern const char e_badreflist[];
+extern const char e_heredoccomsub[];
+extern const char e_endoffile[];
+extern const char e_newline[];
+
 extern const char e_lexwarnvar[];
 extern const char e_lexarithwarn[];
 extern const char e_lexobsolete1[];

diff --git a/src/cmd/ksh93/include/shlex.h b/src/cmd/ksh93/include/shlex.h
@@ -168,11 +168,6 @@ typedef struct  _shlex_
 
 #define SH_COMPASSIGN	010	/* allow compound assignments only */
 
-extern const char		e_unexpected[];
-extern const char		e_unmatched[];
-extern const char		e_endoffile[];
-extern const char		e_newline[];
-
 /* odd chars */
 #define LBRACE	'{'
 #define RBRACE	'}'
@@ -185,7 +180,7 @@ extern int		sh_lex(Lex_t*);
 extern Shnode_t		*sh_dolparen(Lex_t*);
 extern Lex_t		*sh_lexopen(Lex_t*, int);
 extern void 		sh_lexskip(Lex_t*,int,int,int);
-extern noreturn void 	sh_syntax(Lex_t*);
+extern noreturn void 	sh_syntax(Lex_t*, int);
 #if SHOPT_KIA
     extern int                  kiaclose(Lex_t *);
     extern unsigned long        kiaentity(Lex_t*, const char*,int,int,int,int,unsigned long,int,int,const char*);

diff --git a/src/cmd/ksh93/include/version.h b/src/cmd/ksh93/include/version.h
@@ -18,7 +18,7 @@
 
 #define SH_RELEASE_FORK	"93u+m"		/* only change if you develop a new ksh93 fork */
 #define SH_RELEASE_SVER	"1.0.11-beta"	/* semantic version number: https://semver.org */
-#define SH_RELEASE_DATE	"2024-08-21"	/* must be in this format for $((.sh.version)) */
+#define SH_RELEASE_DATE	"2024-08-22"	/* must be in this format for $((.sh.version)) */
 #define SH_RELEASE_CPYR	"(c) 2020-2024 Contributors to ksh " SH_RELEASE_FORK
 
 /* Scripts sometimes field-split ${.sh.version}, so don't change amount of whitespace. */

diff --git a/src/cmd/ksh93/sh/lex.c b/src/cmd/ksh93/sh/lex.c
@@ -340,7 +340,7 @@ int sh_lex(Lex_t* lp)
 					else
 					{
 						lp->token = -1;
-						sh_syntax(lp);
+						sh_syntax(lp,0);
 					}
 				}
 				/* end-of-file */
@@ -374,7 +374,7 @@ int sh_lex(Lex_t* lp)
 					}
 					lp->lasttok = c;
 					lp->token = EOFSYM;
-					sh_syntax(lp);
+					sh_syntax(lp,0);
 				}
 				goto breakloop;
 			case S_COM:
@@ -414,7 +414,7 @@ int sh_lex(Lex_t* lp)
 						lp->lasttok = IODOCSYM;
 						lp->token = EOFSYM;
 						lp->lastline = c;
-						sh_syntax(lp);
+						sh_syntax(lp,0);
 					}
 					if(!lp->lexd.dolparen)
 						lp->lexd.nocopy--;
@@ -501,7 +501,7 @@ int sh_lex(Lex_t* lp)
 							{	/* throw "`(' unmatched" error */
 								lp->lasttok = LPAREN;
 								lp->token = EOFSYM;
-								sh_syntax(lp);
+								sh_syntax(lp,0);
 							}
 							return r;
 						}
@@ -547,7 +547,7 @@ int sh_lex(Lex_t* lp)
 						{
 							lp->token = c = IORDWRSYMT;
 							if(lp->inexec)
-								sh_syntax(lp);
+								sh_syntax(lp,0);
 						}
 						else if(n>0)
 							fcseek(-LEN);
@@ -561,7 +561,7 @@ int sh_lex(Lex_t* lp)
 						if(lp->inexec)
 						{
 							lp->token = c;
-							sh_syntax(lp);
+							sh_syntax(lp,0);
 						}
 					}
 					else
@@ -972,7 +972,7 @@ int sh_lex(Lex_t* lp)
 							if(c!='%')
 							{
 								lp->token = n;
-								sh_syntax(lp);
+								sh_syntax(lp,0);
 							}
 							else if(lp->lexd.warn)
 								errormsg(SH_DICT,ERROR_warn(0),e_lexquote,sh.inlineno,'%');
@@ -1005,7 +1005,7 @@ int sh_lex(Lex_t* lp)
 				if(n!='$')
 				{
 					lp->token = c;
-					sh_syntax(lp);
+					sh_syntax(lp,0);
 				}
 				else
 				{
@@ -1110,7 +1110,7 @@ int sh_lex(Lex_t* lp)
 				if(c!=n && lp->lex.incase<TEST_RE)
 				{
 					lp->token = c;
-					sh_syntax(lp);
+					sh_syntax(lp,0);
 				}
 				if(c==RBRACE && (mode==ST_NAME||mode==ST_NORM))
 					goto epat;
@@ -1148,10 +1148,7 @@ int sh_lex(Lex_t* lp)
 					if(n>0 && n==']')
 					{
 						if(mode==ST_NAME)
-						{
-							errormsg(SH_DICT,ERROR_exit(SYNBAD),e_lexsyntax1, sh.inlineno, "[]", "empty subscript");
-							UNREACHABLE();
-						}
+							sh_syntax(lp,1);
 						if(!epatchar || epatchar=='%')
 							continue;
 					}
@@ -1655,7 +1652,7 @@ static int comsub(Lex_t *lp, int endtok)
 			    case EOFSYM:
 				lp->lastline = line;
 				lp->lasttok = endtok;
-				sh_syntax(lp);
+				sh_syntax(lp,0);
 				/* UNREACHABLE */
 			    case IOSEEKSYM:
 				if(fcgetc(c)!='#' && c>0)
@@ -1692,11 +1689,8 @@ static int comsub(Lex_t *lp, int endtok)
 	lp->lex = save;
 	lp->assignok = (endchar(lp)==RBRACT?assignok:0);
 	if(lp->heredoc && !inheredoc)
-	{
 		/* here-document isn't fully contained in command substitution */
-		errormsg(SH_DICT,ERROR_exit(SYNBAD),e_lexsyntax5,sh.inlineno,lp->heredoc->ioname);
-		UNREACHABLE();
-	}
+		sh_syntax(lp,3);
 	return messages;
 }
 
@@ -2110,20 +2104,10 @@ static char	*fmttoken(Lex_t *lp, int sym)
 /*
  * print a bad syntax message
  */
-noreturn void sh_syntax(Lex_t *lp)
+noreturn void sh_syntax(Lex_t *lp, int special)
 {
-	const char *cp = sh_translate(e_unexpected);
-	char *tokstr;
-	int tok = lp->token;
+	const int eof = lp->token==EOFSYM && lp->lasttok;
 	Sfio_t *sp;
-	if((tok==EOFSYM) && lp->lasttok)
-	{
-		tok = lp->lasttok;
-		cp = sh_translate(e_unmatched);
-	}
-	else
-		lp->lastline = sh.inlineno;
-	tokstr = fmttoken(lp,tok);
 	if((sp=fcfile()) || (sh.infd>=0 && (sp=sh.sftable[sh.infd])))
 	{
 		/* clear out any pending input */
@@ -2139,10 +2123,28 @@ noreturn void sh_syntax(Lex_t *lp)
 	sh.st.firstline = lp->firstline;
 	/* reset lexer state */
 	sh_lexopen(lp, 0);
-	if(!sh_isstate(SH_INTERACTIVE) && !sh_isstate(SH_PROFILE))
-		errormsg(SH_DICT,ERROR_exit(SYNBAD),e_lexsyntax1,lp->lastline,tokstr,cp);
+	/* construct error message */
+	if (sh_isstate(SH_INTERACTIVE) || sh_isstate(SH_PROFILE))
+		sfprintf(sh.strbuf, sh_translate(e_syntaxerror));
+	else
+		sfprintf(sh.strbuf, sh_translate(e_syntaxerror_at), eof ? sh.inlineno : lp->lastline);
+	if (special==1)
+		sfprintf(sh.strbuf, sh_translate(e_emptysubscr));
+	else if (special==2)
+		sfprintf(sh.strbuf, sh_translate(e_badreflist));
+	else if (special==3)
+		sfprintf(sh.strbuf, sh_translate(e_heredoccomsub), lp->heredoc->ioname);
 	else
-		errormsg(SH_DICT,ERROR_exit(SYNBAD),e_lexsyntax2,tokstr,cp);
+	{
+		const char *msg;
+		int tok;
+		if (eof)
+			tok = lp->lasttok, msg = sh_translate(e_unmatched);
+		else
+			tok = lp->token, msg = sh_translate(e_unexpected);
+		sfprintf(sh.strbuf, msg, fmttoken(lp, tok));
+	}
+	errormsg(SH_DICT, ERROR_exit(SYNBAD), "%s", sfstruse(sh.strbuf));
 	UNREACHABLE();
 }