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

Regression: Double redirection in command substitution causes shell hang #784

Closed vmihalko closed 3 weeks ago

vmihalko commented 2 months ago

Reproducer

#!/usr/bin/env ksh

BugFunction() {
  { echo "funA" >&2; } >&2
  echo A
}

NoBugFunction() {
  echo "funB" >&2; >&2
  echo B
}

Result=$(BugFunction)
echo $Result
Result2=$(NoBugFunction)
echo $Result2

Steps to reproduce

$ ./arch/linux.i386-64/bin/ksh test.ksh 
funA

funB
B

KSH version

sh (AT&T Research) 93u+m/1.1.0-alpha+7170ac01 2024-08-28

What happens

Double redirection seems to cause a hang or unexpected behavior.

Git Bisect

I've tracked the bug to a specific commit: https://github.com/ksh93/ksh/commit/2d4a78756411cd983dfa09e7c4b0370797e532c8

McDutchie commented 3 weeks ago

Thanks for the report and for doing the bisect. Looks like the forking workaround must also be implemented for the TSETIO case in sh_exec(), as that is where redirections for compound commands are handled.

diff --git a/src/cmd/ksh93/sh/xec.c b/src/cmd/ksh93/sh/xec.c
index d48b72dc3..c90b13b1f 100644
--- a/src/cmd/ksh93/sh/xec.c
+++ b/src/cmd/ksh93/sh/xec.c
@@ -1712,21 +1712,22 @@ int sh_exec(const Shnode_t *t, int flags)
            int     jmpval, waitall = 0;
            int     simple = (t->fork.forktre->tre.tretyp&COMMSK)==TCOM;
            struct checkpt *buffp = stkalloc(sh.stk,sizeof(struct checkpt));
-           if(sh.subshell && !sh.subshare && t->fork.forkio)
+           if(sh.subshell && !sh.subshare)
            {
-               /* Subshell forking workaround for https://github.com/ksh93/ksh/issues/161
-                * Check each redirection for >&- or <&-
+               /* Subshell forking workaround for:
+                * https://github.com/ksh93/ksh/issues/161 (check each redirection for >&- or <&-)
+                * https://github.com/ksh93/ksh/issues/784 (check for stdout in a command substitution)
                 * TODO: find the elusive real fix */
-               struct ionod *i = t->fork.forkio;
-               do
+               struct ionod *i;
+               for (i = t->fork.forkio; i; i = i->ionxt)
                {
-                   if((i->iofile & ~(IOUFD|IOPUT)) == (IOMOV|IORAW) && !strcmp(i->ioname,"-"))
+                   unsigned f = i->iofile;
+                   if ((f & ~(IOUFD|IOPUT))==(IOMOV|IORAW) && !strcmp(i->ioname,"-") || (f & IOUFD)==1 && sh.comsub)
                    {
                        sh_subfork();
                        break;
                    }
                }
-               while(i = i->ionxt);
            }
            sh_pushcontext(buffp,SH_JMPIO);
            if(type&FPIN)