From 4df6d674a08c7ca92e70e2c44b9ebd3756071719 Mon Sep 17 00:00:00 2001 From: Martijn Dekker Date: Sat, 2 Jul 2022 17:50:52 +0200 Subject: [PATCH] Fix signal exit status of last command in subshell (re: b3050769) Reproducer (on macOS/*BSD where SIGUSR1 has signal number 30): $ ksh -c '(sh -c '\''kill -s USR1 $$'\''); echo $?' ksh: 54220: User signal 1 30 Expected output for $?: 286, not 30. The signal is not reflected in the 9th bit of the exit status. This bug was introduced for virtual subshells in b3050769 but exists in every ksh93 version for real (forked) subshells: $ ksh -c '(ulimit -t unlimited; trap : EXIT; \ sh -c '\''kill -s USR1 $$'\''); echo $?' ksh: 54267: User signal 1 30 (As of d6c9821c, a dummy trap is needed to trigger the bug, or it will be masked by the exec optimization for the sh invocation.) This is caused by the exit status being masked to 8 bits when a subshell terminates. For a real subshell, this is inevitable as the kernel does this. As of b3050769, virtual subshells behave in a manner consistent with real subshells in this regard. However, for both virtual and real subshells, if its last command was terminated by a signal, then that should still be reflected in the 9th bit of ksh's exit stauts. The root of the problem is that ksh simply cannot rely internally on the 9th bit of the exit status to determine if a command exited due to a signal. The 9th bit may be trimmed by a subshell or may be set by 'return' without a signal being involved. This commit fixes it by introducing a separate flag which will be a reliable indicator of this. src/cmd/ksh93/include/shell.h: - Add sh.chldexitsig flag (set if the last command was a child process that exited due to a signal). src/cmd/ksh93/sh/jobs.c: job_wait(): - When the last child process exited due to a signal, not only set the 9th (SH_EXITSIG) bit of sh.exitval but also sh.chldexitsig. src/cmd/ksh93/sh/subshell.c: sh_subshell(): - Fix the virtual subshell reproducer above. After trimming the exit status to 8 bit, set the 9th bit if sh.chldexitsig is set. This needs to be done in two places: one that runs in the parent process after sh_subfork() and one for the regular virtual subshell exit. src/cmd/ksh93/sh/fault.c: - sh_trap(): Save and restore sh.chldexitsig so that this fix does not get deactivated if a trap is set. - sh_done(): - Fix the real subshell reproducer above. When the last command of a real subshell is a child process that exited due to a signal (i.e., if (sh.chldexitsig && sh.realsubshell)), then activate the code to pass down the signal to the parent process. Since there is no way to pass a 9-bit exit status to a parent process, this is the only way to ensure a correct exit status in the parent shell environment. - When exiting the main shell, use sh.chldexitsig and not the unreliable SH_EXITSIG bit to determine if the 8th bit needs to be set for a portable exit status indicating its last command exited due to a signal. --- NEWS | 6 ++++++ src/cmd/ksh93/include/shell.h | 1 + src/cmd/ksh93/include/version.h | 2 +- src/cmd/ksh93/sh/fault.c | 8 ++++---- src/cmd/ksh93/sh/jobs.c | 3 +++ src/cmd/ksh93/sh/subshell.c | 4 ++++ src/cmd/ksh93/sh/xec.c | 3 +-- src/cmd/ksh93/tests/subshell.sh | 18 ++++++++++++++++++ 8 files changed, 38 insertions(+), 7 deletions(-) diff --git a/NEWS b/NEWS index a1324acd8..82f453eb4 100644 --- a/NEWS +++ b/NEWS @@ -3,6 +3,12 @@ For full details, see the git log at: https://github.com/ksh93/ksh/tree/1.0 Any uppercase BUG_* names are modernish shell bug IDs. +2022-07-02: + +- Fixed a bug where, if the last command in a subshell was an external + command that was terminated by a signal, the exit status ($?) of the + subshell did not reflect this by adding 256 to the signal number. + 2022-07-01: - In scripts, $COLUMNS and $LINES are now kept up to date in scripts at diff --git a/src/cmd/ksh93/include/shell.h b/src/cmd/ksh93/include/shell.h index 94379ec72..406b9b424 100644 --- a/src/cmd/ksh93/include/shell.h +++ b/src/cmd/ksh93/include/shell.h @@ -319,6 +319,7 @@ struct Shell_s char indebug; /* set when in debug trap */ unsigned char ignsig; /* ignored signal in subshell */ unsigned char lastsig; /* last signal received */ + char chldexitsig; /* set if the last command was a child process that exited due to a signal */ char pathinit; /* pathinit called from subshell */ char comsub; /* set to 1 when in `...`, 2 when in ${ ...; }, 3 when in $(...) */ char subshare; /* set when comsub==2 (shared-state ${ ...; } command substitution) */ diff --git a/src/cmd/ksh93/include/version.h b/src/cmd/ksh93/include/version.h index 7c6f09017..06a605bc3 100644 --- a/src/cmd/ksh93/include/version.h +++ b/src/cmd/ksh93/include/version.h @@ -23,7 +23,7 @@ #define SH_RELEASE_FORK "93u+m" /* only change if you develop a new ksh93 fork */ #define SH_RELEASE_SVER "1.0.0-beta.2" /* semantic version number: https://semver.org */ -#define SH_RELEASE_DATE "2022-07-01" /* must be in this format for $((.sh.version)) */ +#define SH_RELEASE_DATE "2022-07-02" /* 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. */ diff --git a/src/cmd/ksh93/sh/fault.c b/src/cmd/ksh93/sh/fault.c index a4976747b..5c46187fe 100644 --- a/src/cmd/ksh93/sh/fault.c +++ b/src/cmd/ksh93/sh/fault.c @@ -482,6 +482,7 @@ int sh_trap(const char *trap, int mode) int jmpval, savxit = sh.exitval, savxit_return; int was_history = sh_isstate(SH_HISTORY); int was_verbose = sh_isstate(SH_VERBOSE); + char save_chldexitsig = sh.chldexitsig; int staktop = staktell(); char *savptr = stakfreeze(0); struct checkpt buff; @@ -529,6 +530,7 @@ int sh_trap(const char *trap, int mode) sh_onstate(SH_HISTORY); if(was_verbose) sh_onstate(SH_VERBOSE); + sh.chldexitsig = save_chldexitsig; exitset(); if(jmpval>SH_JMPTRAP && (((struct checkpt*)sh.jmpbuffer)->prev || ((struct checkpt*)sh.jmpbuffer)->mode==SH_JMPSCRIPT)) siglongjmp(*sh.jmplist,jmpval); @@ -682,7 +684,7 @@ noreturn void sh_done(register int sig) sfsync((Sfio_t*)sfstdin); sfsync((Sfio_t*)sh.outpool); sfsync((Sfio_t*)sfstdout); - if(savxit&SH_EXITSIG && (savxit&SH_EXITMASK) == sh.lastsig) + if((sh.chldexitsig && sh.realsubshell) || (savxit&SH_EXITSIG && (savxit&SH_EXITMASK) == sh.lastsig)) sig = savxit&SH_EXITMASK; if(sig) { @@ -707,10 +709,8 @@ noreturn void sh_done(register int sig) if(sh_isoption(SH_NOEXEC)) kiaclose((Lex_t*)sh.lex_context); #endif /* SHOPT_KIA */ - /* Exit with portable 8-bit status (128 + signum) if last child process exits due to signal */ - if (savxit & SH_EXITSIG) + if(sh.chldexitsig) savxit = savxit & ~SH_EXITSIG | 0200; - exit(savxit&SH_EXITMASK); } diff --git a/src/cmd/ksh93/sh/jobs.c b/src/cmd/ksh93/sh/jobs.c index 4938f26cc..974330b2e 100644 --- a/src/cmd/ksh93/sh/jobs.c +++ b/src/cmd/ksh93/sh/jobs.c @@ -1508,7 +1508,10 @@ int job_wait(register pid_t pid) { sh.exitval=px->p_exit; if(px->p_flag&P_SIGNALLED) + { sh.exitval |= SH_EXITSIG; + sh.chldexitsig = 1; + } if(intr) px->p_flag &= ~P_EXITSAVE; } diff --git a/src/cmd/ksh93/sh/subshell.c b/src/cmd/ksh93/sh/subshell.c index 92b2a78a7..ab91f76dc 100644 --- a/src/cmd/ksh93/sh/subshell.c +++ b/src/cmd/ksh93/sh/subshell.c @@ -680,6 +680,8 @@ Sfio_t *sh_subshell(Shnode_t *t, volatile int flags, int comsub) if(jmpval==SH_JMPSCRIPT) siglongjmp(*sh.jmplist,jmpval); sh.exitval &= SH_EXITMASK; + if(sh.chldexitsig) + sh.exitval |= SH_EXITSIG; sh_done(0); } if(!sh.savesig) @@ -873,6 +875,8 @@ Sfio_t *sh_subshell(Shnode_t *t, volatile int flags, int comsub) /* Real subshells have their exit status truncated to 8 bits by the kernel. * Since virtual subshells should be indistinguishable, do the same here. */ sh.exitval &= SH_EXITMASK; + if(sh.chldexitsig) + sh.exitval |= SH_EXITSIG; } sh.subshare = sp->subshare; sh.subdup = sp->subdup; diff --git a/src/cmd/ksh93/sh/xec.c b/src/cmd/ksh93/sh/xec.c index 68bb5a9b6..910fcdcc5 100644 --- a/src/cmd/ksh93/sh/xec.c +++ b/src/cmd/ksh93/sh/xec.c @@ -952,6 +952,7 @@ int sh_exec(register const Shnode_t *t, int flags) sh_offstate(SH_ERREXIT); sh.exitval=0; sh.lastsig = 0; + sh.chldexitsig = 0; sh.lastpath = 0; switch(type&COMMSK) { @@ -1854,8 +1855,6 @@ int sh_exec(register const Shnode_t *t, int flags) sh_popcontext(buffp); if(jmpval > SH_JMPEXIT) siglongjmp(*sh.jmplist,jmpval); - if(sh.exitval > 256) - sh.exitval -= 128; sh_done(0); } else diff --git a/src/cmd/ksh93/tests/subshell.sh b/src/cmd/ksh93/tests/subshell.sh index 4bcf5b5f4..43f461aff 100755 --- a/src/cmd/ksh93/tests/subshell.sh +++ b/src/cmd/ksh93/tests/subshell.sh @@ -1153,5 +1153,23 @@ exp=OK0 [[ $got == $exp ]] || err_exit "capturing output from ksh when piped doesn't work correctly" \ "(expected $(printf %q "$exp"), got $(printf %q "$got"))" +# ====== +# A virtual subshell should trim its exit status to 8 bits just like a real subshell, but if +# its last command was killed by a signal then that should still be reflected in the 9th bit. +sig=USR1 +exp=$((${ kill -l "$sig"; }|0x100)) +{ (:; "$SHELL" -c "kill -s $sig \$\$"); } 2>/dev/null +let "(got=$?)==exp" || err_exit "command killed by signal in virtual subshell: expected status $exp, got status $got" +{ (ulimit -t unlimited; "$SHELL" -c "kill -s $sig \$\$"); } 2>/dev/null +let "(got=$?)==exp" || err_exit "command killed by signal in real subshell: expected status $exp, got status $got" +{ (trap : EXIT; "$SHELL" -c "kill -s $sig \$\$"); } 2>/dev/null +let "(got=$?)==exp" || err_exit "command killed by signal in virtual subshell with trap: expected status $exp, got status $got" +{ (ulimit -t unlimited; trap : EXIT; "$SHELL" -c "kill -s $sig \$\$"); } 2>/dev/null +let "(got=$?)==exp" || err_exit "command killed by signal in real subshell with trap: expected status $exp, got status $got" +(:; exit "$exp") +let "(got=$?)==(exp&0xFF)" || err_exit "fake signal exit from virtual subshell: expected status $((exp&0xFF)), got status $got" +(ulimit -t unlimited 2>/dev/null; exit "$exp") +let "(got=$?)==(exp&0xFF)" || err_exit "fake signal exit from real subshell: expected status $((exp&0xFF)), got status $got" + # ====== exit $((Errors<125?Errors:125))