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

Fix the exec optimisation mess (re: 17ebfbf6, 6701bb30, d6c9821c)

This commit supersedes @lijog's Solaris patch 280-23332860 (see
17ebfbf6) as this is a more general fix that makes the patch
redundant. Of course its associated regression tests stay.

Reproducer script:

	trap 'echo SIGUSR1 received' USR1
	sh -c 'kill -s USR1 $PPID'

Run as a normal script.
Expected behaviour: prints "SIGUSR1 received"
Actual behaviour: the shell invoking the script terminates. Oops.

As of 6701bb30, ksh again allows an exec-without-fork optimisation
for the last command in a script. So the 'sh' command gets the same
PID as the script, therefore its parent PID ($PPID) is the invoking
script and not the script itself, which has been overwritten in
working memory. This shows that, if there are traps set, the exec
optimisation is incorrect as the expected process is not signalled.

While 6701bb30 reintroduced this problem for scripts, this has
always been an issue for certain other situations: forked command
substitutions, background subshells, and -c option argument
scripts. This commit fixes it in all those cases.

In sh_exec(), case TFORK, the optimisation (flagged in no_fork) was
only blocked for SIGINT and for the EXIT and ERR pseudosignals.
That is wrong. It should be blocked for all signal and pseudosignal
traps, except DEBUG (which is run before the command) and SIGKILL
and SIGSTOP (which cannot be trapped).

(I've also tested the behaviour of other shells. One shell, mksh,
never does an exec optimisation, even if no traps are set. I don't
know if that is intentional or not. I suppose it is possible that a
script might expect to receive a signal without trapping it first,
and they could conceivably be affected the same way by this exec
optimisation. But the ash variants (e.g. Busybox ash, dash, FreeBSD
sh), as well as bash, yash and zsh, all do act like this, so the
behaviour is very widespread. This commit makes ksh act like them.)

Multiple files:
- Remove the sh.errtrap, sh.exittrap and sh.end_fn flags and their
  associated code from the superseded Solaris patch.

src/cmd/ksh93/include/shell.h:
- Add a scoped sh.st.trapdontexec flag for sh_exec() to disable
  exec-without-fork optimisations. It should be in the sh.st scope
  struct because it needs to be reset in subshell scopes.

src/cmd/ksh93/bltins/trap.c: b_trap():
- Set sh.st.trapdontexec if any trap is set and non-empty (an empty
  trap means ignore the signal, which is inherited by an exec'd
  process, so the optimisation is fine in that case).
- Only clear sh.st.trapdontexec if we're not in a ksh function
  scope; unlike subshells, ksh functions fall back to parent traps
  if they don't trap a signal themselves, so a ksh function's
  parent traps also need to disable the exec optimisation.

src/cmd/ksh93/sh/fault.c: sh_sigreset():
- Introduce a new -1 mode for sh_funscope() to use, which acts like
  mode 0 except it does not clear sh.st.trapdontexec. This avoids
  clearing sh.st.trapdontexec for ksh functions scopes (see above).
- Otherwise, clear sh.st.trapdontexec whenever traps are reset.

src/cmd/ksh93/sh/xec.c: check_exec_optimization():
- Consolidate all the exec optimisation logic into this function,
  including the logic from the no_fork flag in sh_exec()/TFORK.
- In the former no_fork flag logic, replace the three checks for
  SIGINT, ERR and EXIT traps with a single check for the
  sh.st.trapdontexec flag.
This commit is contained in:
Martijn Dekker 2022-06-18 16:44:30 +01:00
parent d6c9821c5b
commit 40245e088d
8 changed files with 123 additions and 43 deletions

4
NEWS
View file

@ -10,6 +10,10 @@ Any uppercase BUG_* names are modernish shell bug IDs.
the last component command was an external command and the pipeline was
the last command in a background subshell.
- When any trap except DEBUG, KILL or STOP is set to a non-empty command,
the last command in a script or forked subshell will no longer avoid forking
before executing; this optimization incorrectly bypassed the traps.
2022-06-15:
- Fixed a bug where converting an indexed array into an associative array in