Syntax error leaves ksh stuck

[posguy99]

A completely errant paste that wasn't meant for the terminal window exposed this...

Reproducer:

$ echo $KSH_VERSION
Version AJM 93u+m/1.1.0-alpha+b5d1f098 2024-08-23
$ for ((i = 5 , i = 0, --i)) 
-ksh: syntax error: `{' unmatched
-ksh: syntax error: `{' unmatched
-ksh: syntax error: `{' unmatched
-ksh: syntax error: `{' unmatched
-ksh: syntax error: `{' unmatched
-ksh: syntax error: `{' unmatched
-ksh: syntax error: `{' unmatched
-ksh: syntax error: `{' unmatched
-ksh: syntax error: `{' unmatched

Continues forever until you kill the shell.

ksh93u+ does not have the problem:

[82] mbp-m2 $ echo $KSH_VERSION         
Version AJM 93u+ 2012-08-01
$ for ((i = 5 , i = 0, --i))
/bin/ksh: syntax error: `))' unexpected
$

Obviously bad code shouldn't cause the shell to get stuck.

[posguy99]

Even simpler reproducer:

$ for(())
-ksh: syntax error: `{' unmatched
-ksh: syntax error: `{' unmatched
-ksh: syntax error: `{' unmatched
-ksh: syntax error: `{' unmatched

Continues forever.

[McDutchie]

Thanks for the report. On my end, ksh93u+ and ksh2020 are also broken, just not in the same way.

[McDutchie]

The current infinite-loop manifestation of the bug was introduced in: f8f2c4b (found by trying old commits)

[McDutchie]

So, after some debugging and backtracing, I've found that what happens is this:

1. arithfor() in parse.c is called to parse the arithmetic for loop
2. arithfor() saves current input stream and opens a new input stream to separate each of the three expressions in (( ... ))
3. on line 758, arithfor() calls sh_lexskip() to find the next ;
4. in lex.c line 1749, sh_lexskip() calls the main lexer function, sh_lex()
5. in lex.c line 375, sh_lex() calls sh_syntax() to throw the syntax error in the reproducer
6. sh_syntax() calls errormsg() from libast, which causes a longjmp straight back to the prompt, without giving arithfor() a chance to restore the input stream that it saved in step 2. This is the bug. We can get all sorts of unpredictable/undefined behaviour that way.

I can think of two strategies to fix this:

1. Push context and sigsetjmp in arithfor() before calling sh_lexskip(), so that it longjmps back to arithfor() and it can restore the input stream before doing its own longjmp. This may carry a minor performance hit for nested arithmetic for loops.
2. Since arithfor() throws a syntax error itself when needed anyway (and does so after restoring the input stream), we could also find a way for sh_lex() not to throw that syntax error when called from sh_lexskip.

Strategy 2 might be the best way. It just so happens that sh_lexskip sets its own flag, lp->lexd.noarg, that we can use in sh_lex() to tell if it was called from sh_lexskip(). So we could use that to prevent it from throwing the syntax error and just return instead, handing control back to arithfor(), so that it can throw the syntax error after restoring state.

Here is a patch that does that. Please test.

diff --git a/src/cmd/ksh93/sh/lex.c b/src/cmd/ksh93/sh/lex.c
index ed8ed2db6..d60cb1cb9 100644
--- a/src/cmd/ksh93/sh/lex.c
+++ b/src/cmd/ksh93/sh/lex.c
@@ -344,7 +344,7 @@ int sh_lex(Lex_t* lp)
 				/* end-of-file */
 				if(mode==ST_BEGIN)
 					return lp->token=EOFSYM;
-				if(mode >ST_NORM && lp->lexd.level>0)
+				if(mode >ST_NORM && lp->lexd.level>0 && !lp->lexd.noarg)
 				{
 					switch(c=endchar(lp))
 					{

[posguy99]

That stopped the infinite loop.

$ for ((i = 5 , i == 0, --i))
arch/darwin.arm64-64/bin/ksh: syntax error: `))' unexpected
$ for (())
arch/darwin.arm64-64/bin/ksh: syntax error: `))' unexpected
$ echo $KSH_VERSION
Version AJM 93u+m/1.1.0-alpha+b75df92a/MOD 2024-08-26

Works for me.