1
0
Fork 0
mirror of git://git.code.sf.net/p/cdesktopenv/code synced 2025-03-09 15:50:02 +00:00

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 1<>; a'; cat a
  x
  st
This bug is caused by ksh running the last command of -c scripts
with execve(2) instead of posix_spawn(3) or fork(2). The <>;
operator is noted by the man page as being incompatible with the
exec builtin (see also the ksh93u+ man page), so it's not
surprising this bug occurs when ksh runs a command using execve:

> <>;word cannot be used with the exec and redirect built-ins.

The ksh2020 fix simply removed the code required for ksh to use
this optimization at all. It's not a performance friendly fix and
only papers over the bug, so this commit provides a better fix.

This bug was first reported at:
https://github.com/att/ast/issues/9

In addition, this commit re-enables the execve(2) optimization for
the last command for scripts loaded from a file. It was enabled in
in older ksh versions, and was only disabled in interactive shells:
https://github.com/ksh93/ast-open-history/blob/2011-06-30/src/cmd/ksh93/sh/main.c#L593-L599
It was changed on 2011-12-24 to only be used for -c scripts:
https://github.com/ksh93/ast-open-history/blob/2011-12-24/src/cmd/ksh93/sh/main.c#L593-L599

We think there is no good reason why scripts loaded from a file
should be optimised less than scripts loaded from a -c argument.
They're both scripts; there's no essential difference between them.
So this commit reverts that change. If there is a bug left in the
optimization after this fix, this revert increases the chance of
exposing it so that it can be fixed.

src/cmd/ksh93/sh/xec.c:
- The IOREWRITE flag is set when handling the <>; operator, so to
  fix this bug, avoid exec'ing the last command if it uses <>;. See
  also commit 17ebfbf6, which fixed another issue related to the
  execve optimization.

src/cmd/ksh93/tests/io.sh:
- Enable a regression test that was failing because of this bug.
- Add the reproducer from https://github.com/att/ast/issues/9 as a
  regression test.

src/cmd/ksh93/sh/main.c:
- Only avoid the non-forking optimization in interactive shells.

src/cmd/ksh93/tests/signal.sh:
- Add an extra comment to avoid the non-forking optimization in the
  regression test for rhbz#1469624.
- If the regression test for rhbz#1469624 fails, show the incorrect
  exit status in the error message.

src/cmd/ksh93/tests/builtins.sh,
src/cmd/ksh93/tests/options.sh:
- This bugfix was causing the options regression test to segfault
  when run under shcomp. The cause is the same as
  <https://github.com/ksh93/ksh/issues/165>, so as a workaround,
  avoid parsing process substitutions with shcomp until that is
  fixed. This workaround should also avoid the other problem
  detailed in <https://github.com/ksh93/ksh/issues/274>.

Resolves: https://github.com/ksh93/ksh/issues/274
This commit is contained in:
Johnothan King 2021-04-15 10:29:50 -07:00 committed by GitHub
parent 2fdf394b99
commit 6701bb30de
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
8 changed files with 31 additions and 10 deletions

View file

@ -575,7 +575,7 @@ static void exfile(register Shell_t *shp, register Sfio_t *iop,register int fno)
{
execflags = sh_state(SH_ERREXIT)|sh_state(SH_INTERACTIVE);
/* The last command may not have to fork */
if(!sh_isstate(SH_PROFILE) && sh_isoption(SH_CFLAG) &&
if(!sh_isstate(SH_PROFILE) && !sh_isstate(SH_INTERACTIVE) &&
(fno<0 || !(shp->fdstatus[fno]&(IOTTY|IONOSEEK)))
&& !sfreserve(iop,0,0))
{