Skip to content

Commit

Permalink
Revert buggy exec optimisation (re: d6c9821)
Browse files Browse the repository at this point in the history
As of 2022-06-18, ksh 93u+m is not capable of being used as /bin/sh
while building GNU binutils. The execution of some of its build
system's dot scripts is incorrectly aborted as an external 'sed'
command is execve(2)'d without forking. This means that incorrect
exec optimization was happening.

Unfortunately I have not been able to derive a minimal reproducer
of the problem yet because the GNU binutils build scripts are very
complex. Pending further research, the optimisation is reverted.
Even if a way to make it work is found, it will not be reintroduced
to the 1.0 branch.

Thanks to @atheik for finding the problem and identifying the
commit that introduced it.

Resolves: #507
  • Loading branch information
McDutchie committed Aug 5, 2022
1 parent c848433 commit 9c97439
Show file tree
Hide file tree
Showing 5 changed files with 24 additions and 17 deletions.
7 changes: 7 additions & 0 deletions NEWS
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,13 @@ This documents significant changes in the 1.0 branch of ksh 93u+m.
For full details, see the git log at: https://github.com/ksh93/ksh/tree/1.0
Uppercase BUG_* IDs are shell bug IDs as used by the Modernish shell library.

2022-08-05:

- Reverted a buggy exec optimisation introduced on 2022-06-18. It is known
to make the build scripts of GNU binutils produce corrupted results.

- Release 1.0.1.

2022-08-01:
_ _ ___ _____ ___ ___ ___
| | _____| |__ / _ \___ / _ _ _ _ __ ___ / / | / _ \ / _ \
Expand Down
4 changes: 2 additions & 2 deletions src/cmd/ksh93/include/version.h
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,8 @@
#include <releaseflags.h>

#define SH_RELEASE_FORK "93u+m" /* only change if you develop a new ksh93 fork */
#define SH_RELEASE_SVER "1.0.0" /* semantic version number: https://semver.org */
#define SH_RELEASE_DATE "2022-08-01" /* must be in this format for $((.sh.version)) */
#define SH_RELEASE_SVER "1.0.1" /* semantic version number: https://semver.org */
#define SH_RELEASE_DATE "2022-08-05" /* must be in this format for $((.sh.version)) */
#define SH_RELEASE_CPYR "(c) 2020-2022 Contributors to ksh " SH_RELEASE_FORK

/* Scripts sometimes field-split ${.sh.version}, so don't change amount of whitespace. */
Expand Down
6 changes: 1 addition & 5 deletions src/cmd/ksh93/sh/xec.c
Original file line number Diff line number Diff line change
Expand Up @@ -1927,11 +1927,7 @@ int sh_exec(register const Shnode_t *t, int flags)
t = t->lst.lstrit;
}
while(t->tre.tretyp == TLST);
/*
* if sh_state(SH_FORKED) was turned on in the meantime, mix it in to the flags to allow a last-command
* no_fork optimization via execflg2 -- this happens when a subshell forks mid-execution (sh_subfork())
*/
sh_exec(t,flags|sh_isstate(SH_FORKED));
sh_exec(t,flags);
break;
}

Expand Down
9 changes: 0 additions & 9 deletions src/cmd/ksh93/tests/basic.sh
Original file line number Diff line number Diff line change
Expand Up @@ -927,15 +927,6 @@ esac
# ======
# Test exec optimization of last command in script or subshell
(
ulimit -t unlimited 2>/dev/null # fork subshell
print "${.sh.pid:-$("$SHELL" -c 'echo "$PPID"')}" # fallback for pre-93u+m ksh without ${.sh.pid}
"$SHELL" -c 'print "$$"'
) >out
pid1= pid2=
{ read pid1 && read pid2; } <out && let "pid1 == pid2" \
|| err_exit "last command in forked subshell not exec-optimized ($pid1 != $pid2)"
got=$(
ulimit -t unlimited 2>/dev/null # fork subshell
print "${.sh.pid:-$("$SHELL" -c 'echo "$PPID"')}" # fallback for pre-93u+m ksh without ${.sh.pid}
Expand Down
15 changes: 14 additions & 1 deletion src/cmd/ksh93/tests/functions.sh
Original file line number Diff line number Diff line change
Expand Up @@ -1034,9 +1034,22 @@ function foo
typeset pid
$tmp1 > $tmp2 & pid=$!
wait $!
[[ $(< $tmp2) == $pid ]] || err_exit 'wrong PID for & job in function'
got=$(< $tmp2)
[[ $got == "$pid" ]] || err_exit 'wrong PID for & job in function' \
"(expected $(printf %q "$pid"), got $(printf %q "$got"))"
}
foo
foo()
{
$tmp1 > $tmp2 & pid=$!
wait $!
got=$(< $tmp2)
[[ $got == "$pid" ]] || err_exit 'wrong PID for & job in POSIX function' \
"(expected $(printf %q "$pid"), got $(printf %q "$got"))"
}
foo
unset pid
unset -f foo
# make sure compiled functions work
[[ $(tmp=$tmp $SHELL <<- \++++
cat > $tmp/functions <<- \EOF
Expand Down

0 comments on commit 9c97439

Please sign in to comment.