ksh: Process substitution as file name to redirection is not compiled by shcomp

The commands within a process substitution (<(…) an >(…)) are simply not included in parse trees dumped by shcomp. This can be verified with a command like hexdump -C. As a result, process substitutions don’t work when running a bytecode-compiled shell script.

The bug/omission is likely to be somewhere in src/cmd/ksh93/sh/tdump.c which is the code for dumping a parse tree into a file. It may be that src/cmd/ksh93/sh/trestore.c also needs changing.

This problem is also in 93u+, 93v- and ksh2020. Another one of those amazing bugs that have been there for decades. shcomp is basically not usable before this is fixed, unless you know the script does not contain any process substitutions.

About this issue

Original URL
State: closed
Created 3 years ago
Comments: 17

Commits related to this issue

Fix the <>; redirection operator in -c scripts This bug was first reported at https://github.com/att/ast/issues/9. The <>; operator doesn't work correctly if it's used as the last command of a -c sc... — committed to JohnoKing/ksh by JohnoKing 3 years ago
To workaround another segfault caused by #165, avoid procsubs here also — committed to JohnoKing/ksh by JohnoKing 3 years ago
Fix <>; redirection for final command exec optimization (#277) The <>; operator doesn't work correctly if it's used as the last command of a -c script. Reproducer: $ echo test > a; ksh -c 'echo x... — committed to ksh93/ksh by JohnoKing 3 years ago
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 du... — committed to ksh93/ksh by McDutchie 3 years ago

Most upvoted comments

New patch:

--- a/src/cmd/ksh93/sh/tdump.c
+++ b/src/cmd/ksh93/sh/tdump.c
@@ -194,6 +194,11 @@ static int p_redirect(register const struct ionod *iop)
 	{
 		if(iop->iovname)
 			sfputl(outfile,iop->iofile|IOVNM);
+		else if((iop->iofile & IOPROCSUB) && !(iop->iofile & IOLSEEK))
+		{
+			errormsg(SH_DICT,ERROR_exit(1),"process substitution as file name to redirection is not yet implemented for shcomp");
+			UNREACHABLE();
+		}
 		else
 			sfputl(outfile,iop->iofile);
 		p_string(iop->ioname);
@@ -215,6 +220,7 @@ static int p_redirect(register const struct ionod *iop)
 
 static int p_comarg(register const struct comnod *com)
 {
+	error_info.line = com->comline;
 	p_redirect(com->comio);
 	p_arg(com->comset);
 	if(!com->comarg)

Now it does seem to be detected correctly:

$ cat test.ksh
redirect 3<# ((40*8))
cat >&2 < <(echo procsub)
$ arch/*/bin/shcomp test.ksh >/dev/null
test.ksh: line 2: process substitution as file name to redirection is not yet implemented for shcomp

McDutchie on Apr 21, 2021

This is minor, but error messages should be followed by UNREACHABLE() when they don’t return:

--- a/src/cmd/ksh93/sh/tdump.c
+++ b/src/cmd/ksh93/sh/tdump.c
@@ -219,6 +219,11 @@ static int p_comarg(register const struct comnod *com)
 	p_arg(com->comset);
 	if(!com->comarg)
 		sfputl(outfile,-1);
+	else if(com->comio && (com->comio->iofile & IOPROCSUB))
+	{
+		errormsg(SH_DICT,ERROR_exit(1),"process substitution as file name to redirection is not yet implemented for shcomp");
+		UNREACHABLE();
+	}
 	else if(com->comtyp&COMSCAN)
 		p_arg(com->comarg);
 	else

On another note, I tested the patch and it causes the io.sh regression tests to fail under shcomp:

test io(shcomp) begins at 2021-04-21+13:16:12
/tmp/ksh93.shtests.3926.633/shcomp-io.ksh.orig: line 48: process substitution as file name to redirection is not yet implemented for shcomp
test io(shcomp) failed to compile at 2021-04-21+13:16:12 with exit code 1 [ 1 test 1 error ]

JohnoKing on Apr 21, 2021

I’ve studied the code in io.c for executing process substitutions with redirections: https://github.com/ksh93/ksh/blob/6b9703ffdd9a71c3fb7418ba7744fd734e39f454/src/cmd/ksh93/sh/io.c#L1176-L1187

So, iop->ioname may start with ‘\n\0’ and appear to be a string containing a newline, but it’s actually a pointer to the parse tree for the process substitution and should be handled with a typecast to struct argnod*.

Based on that code, I’ve come up with the following provisional patch. It needs an extra hack to avoid the segfault in p_arg(). It does cause the command to be included in the shcomp output. Unfortunately, when trying to run it, ksh segfaults.

Note that the sh_argprocsub() call is not included because that causes a file name like /dev/fd/4 to be included in the shcomp output instead of the process substitution’s parse tree.

--- a/src/cmd/ksh93/sh/tdump.c
+++ b/src/cmd/ksh93/sh/tdump.c
@@ -219,6 +219,16 @@ static int p_comarg(register const struct comnod *com)
 	p_arg(com->comset);
 	if(!com->comarg)
 		sfputl(outfile,-1);
+	else if(com->comio && (com->comio->iofile & IOPROCSUB))
+	{
+		struct argnod *ap = (struct argnod*)stakalloc(ARGVAL+strlen(com->comio->ioname));
+		memset(ap, 0, ARGVAL);
+		if(com->comio->iofile & IOPUT)
+			ap->argflag = ARG_RAW;
+		ap->argchn.ap = (struct argnod*)com->comio->ioname;
+		((struct fornod*)ap->argchn.ap)->fornam = Empty; /* avoid segfault when calling strlen() in p_arg */
+		p_arg(ap);
+	}
 	else if(com->comtyp&COMSCAN)
 		p_arg(com->comarg);
 	else

This test script now compiles like this:

$ cat test.ksh
echo line 1
cat < <(echo line 2)
$ arch/*/bin/shcomp test.ksh >test.kshc
$ hexdump -C test.kshc
00000000  0b 13 08 00 04 00 00 40  00 03 05 65 63 68 6f 05  |.......@...echo.|
00000010  6c 69 6e 65 02 31 00 01  00 a0 80 00 03 0a 02 00  |line.1..........|
00000020  40 00 02 00 00 84 0a 00  40 00 03 05 65 63 68 6f  |@.......@...echo|
00000030  05 6c 69 6e 65 02 32 00  02 00 02                 |.line.2....|
0000003b
$ arch/*/bin/ksh test.kshc
line 1
Memory fault

Backtrace of the crash:

Thread 0 Crashed:: Dispatch queue: com.apple.main-thread
0   libsystem_platform.dylib            0x00007fff5fa1113c _platform_strchr$VARIANT$Base + 28
1   ksh                                 0x0000000100a34486 sh_exec + 4918 (xec.c:1224)
2   ksh                                 0x00000001009bbde6 exfile + 3254 (main.c:586)
3   ksh                                 0x00000001009bcd30 sh_main + 3392 (main.c:358)
4   ksh                                 0x00000001009a1b36 main + 38 (pmain.c:45)
5   libdyld.dylib                       0x00007fff5f8283d5 start + 1

So it crashes here, in strchr(): https://github.com/ksh93/ksh/blob/6b9703ffdd9a71c3fb7418ba7744fd734e39f454/src/cmd/ksh93/sh/xec.c#L1224

That’s as far as I got for now, I hope this is some progress.

McDutchie on Apr 6, 2021