ksh93 / ksh

ksh 93u+m: KornShell lives! | Latest release: https://github.com/ksh93/ksh/releases
Eclipse Public License 2.0
192 stars 32 forks source link

Setting TMOUT in a virtual subshell removes the variable's special meaning #782

Closed sckendall2 closed 3 weeks ago

sckendall2 commented 2 months ago

In a terminal, assuming ksh is the shell under test, run these commands:

ksh -c 'TMOUT=1; read v; print $?'
ksh -c '( TMOUT=0 ); TMOUT=1; read v; print $?'

The first command prints 1 because read times out after a second and yields status 1. So far so good. This is the documented behavior of read.

The second command should do the same. Instead, it hangs. That's the bug.

ksh93 has always had this bug. I've reproduced it in "Version M 93t+ 2009-05-01" in AIX 7.2; and in "Version AJM 93u+m/1.0.6 2023-06-13", RPM package ksh-1.0.6-3.el9.x86_64, on RHEL 9.3.

Apparently, ksh has optimized away the subshell process incorrectly, removing TMOUT's special meaning. You can see this by adding a command that blocks the optimization. E.g., the following correctly times out and prints 1:

ksh -c '( ulimit $(ulimit); TMOUT=0 ); TMOUT=1; read v; print $?'

FWIW, I suspected that all the variables named in this sentence (from the man page) would behave like TMOUT if set in an optimized-away subshell:

Unsetting LINENO, MAILCHECK, OPTARG, OPTIND, RANDOM, SECONDS, TMOUT, and _ removes their special meaning even if they are subsequently assigned to.

I tried with SECONDS and _. But, FWIW, I had no success. At least unset SECONDS has the documented effect, but even unset _ did not remove that variable's special meaning.

So, we have a bonus super-minor documentation bug: _ should be removed from that sentence in the man page.

McDutchie commented 3 weeks ago

The value of TMOUT is initialised as a pointer into the static scope struct, sh.st, which gets saved and restored for virtual subshells and ksh functions (so the effect of TMOUT is always local to a ksh function). https://github.com/ksh93/ksh/blob/5def43983de3ecfa38c805c02a1f0d6f1581160c/src/cmd/ksh93/sh/init.c#L1918

This means their value pointers should be copied in the sh_assignok function, along with those of a couple of other variables initialised this way.

Please test this patch:

diff --git a/src/cmd/ksh93/sh/subshell.c b/src/cmd/ksh93/sh/subshell.c
index 763724152..1d1b5c832 100644
--- a/src/cmd/ksh93/sh/subshell.c
+++ b/src/cmd/ksh93/sh/subshell.c
@@ -312,6 +312,9 @@ void sh_assignok(Namval_t *np,int add)
    save = sh.subshell;
    sh.subshell = 0;
    mp->nvname = np->nvname;
+   /* Copy value pointers for variables whose values are pointers into the static scope, sh.st */
+   if(!nv_isnonptr(np) && np->nvalue.cp >= (char*)&sh.st && np->nvalue.cp < (char*)&sh.st + sizeof(struct sh_scoped))
+       mp->nvalue = np->nvalue;
    if(nv_isattr(np,NV_NOFREE))
        nv_onattr(mp,NV_IDENT);
    nv_clone(np,mp,(add?(nv_isnull(np)?0:NV_NOFREE)|NV_ARRAY:NV_MOVE));