From 32d1abb1baecb88e9cbb61a95220ff3c866583a5 Mon Sep 17 00:00:00 2001 From: Martijn Dekker Date: Thu, 22 Apr 2021 03:25:24 +0100 Subject: [PATCH] shcomp: fix redirection with process substitution The commands within a process substitution used as an argument to a redirection (e.g. < <(...) or > >(...)) are simply not included in parse trees dumped by shcomp. This can be verified with a command like hexdump -C. As a result, these process substitutions do not work when running a bytecode-compiled shell script. The fix is surprisingly simple. A process substitution is encoded as a complete parse tree. When used with a redirection, that parse tree is used as the file name for the redirection. All we need to do is treat the "file name" as a parse tree instead of a string if flags indicate a process substitution. A process substitution is detected by the struct ionod field 'iofile'. Checking the IOPROCSUB bit flag is not enough. We also need to exclude the IOLSEEK flag as that form of redirection may use the IOARITH flag which has the same bit value as IOPROCSUB (see include/shnodes.h). src/cmd/ksh93/sh/tdump.c: p_redirect(): - Call p_tree() instead of p_string() for a process substitution. src/cmd/ksh93/sh/trestore.c: r_redirect(): - Call r_tree() instead of r_string() for a process substitution. src/cmd/ksh93/include/version.h: - Bump the shcomp binary header version as this change is not backwards compatible; previous trestore.c versions don't know how to read the newly compiled process substitutions and would crash. src/cmd/ksh93/tests/io.sh: - Add test. src/cmd/ksh93/tests/builtins.sh, src/cmd/ksh93/tests/options.sh: - Revert shcomp workarounds. (re: 6701bb30) Resolves: https://github.com/ksh93/ksh/issues/165 --- NEWS | 5 +++++ src/cmd/ksh93/include/version.h | 7 ++----- src/cmd/ksh93/sh/tdump.c | 5 ++++- src/cmd/ksh93/sh/trestore.c | 5 ++++- src/cmd/ksh93/tests/builtins.sh | 5 +---- src/cmd/ksh93/tests/io.sh | 11 +++++++++++ src/cmd/ksh93/tests/options.sh | 8 ++------ 7 files changed, 29 insertions(+), 17 deletions(-) diff --git a/NEWS b/NEWS index 77f69e4ce486..4ba1ecdd258b 100644 --- a/NEWS +++ b/NEWS @@ -3,6 +3,11 @@ For full details, see the git log at: https://github.com/ksh93/ksh Any uppercase BUG_* names are modernish shell bug IDs. +2021-04-22: + +- shcomp (the shell bytecode compiler) was fixed to correctly compile process + substitutions used as the file name to a redirection, as in 'cmd < <(cmd)'. + 2021-04-21: - Fixed a bug introduced on 2020-09-28 that caused an interactive ksh to exit diff --git a/src/cmd/ksh93/include/version.h b/src/cmd/ksh93/include/version.h index b9725ca5cf78..bf4fb6ef222a 100644 --- a/src/cmd/ksh93/include/version.h +++ b/src/cmd/ksh93/include/version.h @@ -20,7 +20,7 @@ #define SH_RELEASE_FORK "93u+m" /* only change if you develop a new ksh93 fork */ #define SH_RELEASE_SVER "1.0.0-alpha" /* semantic version number: https://semver.org */ -#define SH_RELEASE_DATE "2021-04-21" /* must be in this format for $((.sh.version)) */ +#define SH_RELEASE_DATE "2021-04-22" /* must be in this format for $((.sh.version)) */ #define SH_RELEASE_CPYR "(c) 2020-2021 Contributors to ksh " SH_RELEASE_FORK /* Scripts sometimes field-split ${.sh.version}, so don't change amount of whitespace. */ @@ -39,8 +39,5 @@ * For shcomp: the version number (0-255) for the binary bytecode header. * Only increase very rarely, i.e.: if incompatible changes are made that * cause bytecode from newer versions to fail on older versions of ksh. - * - * The version number was last increased in 2021 for ksh 93u+m because - * most of the predefined aliases were converted to builtin commands. */ -#define SHCOMP_HDR_VERSION 4 +#define SHCOMP_HDR_VERSION 5 diff --git a/src/cmd/ksh93/sh/tdump.c b/src/cmd/ksh93/sh/tdump.c index d3d82d87ae84..023688d8c1b8 100644 --- a/src/cmd/ksh93/sh/tdump.c +++ b/src/cmd/ksh93/sh/tdump.c @@ -196,7 +196,10 @@ static int p_redirect(register const struct ionod *iop) sfputl(outfile,iop->iofile|IOVNM); else sfputl(outfile,iop->iofile); - p_string(iop->ioname); + if((iop->iofile & IOPROCSUB) && !(iop->iofile & IOLSEEK)) + p_tree((Shnode_t*)iop->ioname); /* process substitution as file name to redirection */ + else + p_string(iop->ioname); /* file name, descriptor, etc. */ if(iop->iodelim) { p_string(iop->iodelim); diff --git a/src/cmd/ksh93/sh/trestore.c b/src/cmd/ksh93/sh/trestore.c index 6fab1189383b..d9fdc9676efc 100644 --- a/src/cmd/ksh93/sh/trestore.c +++ b/src/cmd/ksh93/sh/trestore.c @@ -224,7 +224,10 @@ static struct ionod *r_redirect(Shell_t* shp) else iopold->ionxt = iop; iop->iofile = l; - iop->ioname = r_string(shp->stk); + if((l & IOPROCSUB) && !(l & IOLSEEK)) + iop->ioname = (char*)r_tree(shp); /* process substitution as file name to redirection */ + else + iop->ioname = r_string(shp->stk); /* file name, descriptor, etc. */ if(iop->iodelim = r_string(shp->stk)) { iop->iosize = sfgetl(infile); diff --git a/src/cmd/ksh93/tests/builtins.sh b/src/cmd/ksh93/tests/builtins.sh index 5abf949d3aa6..e6fe731ca088 100755 --- a/src/cmd/ksh93/tests/builtins.sh +++ b/src/cmd/ksh93/tests/builtins.sh @@ -1012,9 +1012,6 @@ EOF # ====== # Builtins should handle unrecognized options correctly -# Note: To workaround https://github.com/ksh93/ksh/issues/165, put the list -# of builtins in a file, then read from that. -builtin > "$tmp/builtin-list" while IFS= read -r bltin <&3 do case $bltin in echo | test | true | false | \[ | : | getconf | */getconf | uname | */uname | catclose | catgets | catopen | Dt* | _Dt* | X* | login | newgrp ) @@ -1028,7 +1025,7 @@ do case $bltin in esac [[ $actual == *"$expect"* ]] || err_exit "$bltin should show usage info on unrecognized options" \ "(expected string containing $(printf %q "$expect"), got $(printf %q "$actual"))" -done 3< "$tmp/builtin-list" +done 3< <(builtin) # ====== # The 'alarm' builtin could make 'read' crash due to IFS table corruption caused by unsafe asynchronous execution. diff --git a/src/cmd/ksh93/tests/io.sh b/src/cmd/ksh93/tests/io.sh index 2d6976a36713..21a1f23f764b 100755 --- a/src/cmd/ksh93/tests/io.sh +++ b/src/cmd/ksh93/tests/io.sh @@ -841,5 +841,16 @@ exp='Foo bar' [[ $exp == $got ]] || err_exit "BUG_CSUBSTDO: Closing stdout outside of command substitution breaks stdout inside of command substitution" \ "(expected $(printf %q "$exp"), got $(printf %q "$got"))" +# ====== +# In shcomp, process substitution did not work when used as the file name to a redirection. +# https://github.com/ksh93/ksh/issues/165 +# Note: avoid testing this in a command substitution, as those are always parsed at runtime, +# meaning shcomp will also include them as literal source text instead of compiling them. +echo ok > >(cat >out1) +wait "$!" # the procsub is run asynchronously, so wait before reading from the file +cat >out2 < <(case x in x) cat out1;; esac) +[[ $(