Skip to content

Commit

Permalink
Fix pipefail exit status of signalled process to be > 256
Browse files Browse the repository at this point in the history
Koichi Nakashima (@ko1nksm) reports:
>     $ set -o pipefail
>     $ seq 1000000 | head > /dev/null
>     $ echo $?
>     13
>
> The seq command terminated by SIGPIPE, but the exit status is 13.
> The reason I think this is problematic is that we don't know that
> the seq command terminated by a signal (and I wish ksh had
> PIPESTATUS). And POSIX specifies an exit status of 128 or higher
> when exiting on a signal.
>
> From my investigation, it appears that this behavior was changed
> in ksh93u+, although I am not sure if this was intentional or
> not. On ksh93s+ (debian 6.0) and ksh93t+ (OpenIndiana 2013.08) it
> works as follows:
>
>     $ set -o pipefail
>     $ seq 1000000 | head > /dev/null
>     $ echo $?
>     269

I was testing my stash of historic ksh versions compiled on Debian
arm64 from the ksh93-history repo. Version 93u 2011-03-10 and
before do not have the bug; the bug is present as of 93u+
2011-04-15, which rewrote the code for the pipefail option.

src/cmd/ksh93/sh/jobs.c: job_unpost():
- When saving a process' exit status for the pipefail option, also
  set the 9th bit (a.k.a. SH_EXITSIG) if the process was signalled.

Discussion: #755 (comment)
  • Loading branch information
McDutchie committed Jul 26, 2024
1 parent f7785d8 commit f1ffd60
Show file tree
Hide file tree
Showing 3 changed files with 16 additions and 0 deletions.
4 changes: 4 additions & 0 deletions NEWS
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,10 @@ Uppercase BUG_* IDs are shell bug IDs as used by the Modernish shell library.
parent/main shell, in some cases causing a script to abort. Bug introduced
on 2022-07-02.

- Fixed a bug in the pipefail shell option where the status of a signalled
non-rightmost process in a pipeline was just the signal number, instead of
that number + 256. Bug introduced in ksh 93u+ 2011-04-15.

2024-07-24:

- Fixed a serious bug in the arithmetic subsystem that was triggered on
Expand Down
5 changes: 5 additions & 0 deletions src/cmd/ksh93/sh/jobs.c
Original file line number Diff line number Diff line change
Expand Up @@ -1639,8 +1639,13 @@ static struct process *job_unpost(struct process *pwtop,int notify)
job_unlink(pwtop);
for(pw=pwtop; pw; pw=pw->p_nxtproc)
{
/* save the exit status for the pipefail option */
if(pw && pw->p_exitval)
{
*pw->p_exitval = pw->p_exit;
if(pw->p_flag&P_SIGNALLED)
*pw->p_exitval |= SH_EXITSIG;
}
/* save the exit status for background jobs */
if((pw->p_flag&P_EXITSAVE) || pw->p_pid==sh.spid)
{
Expand Down
7 changes: 7 additions & 0 deletions src/cmd/ksh93/tests/options.sh
Original file line number Diff line number Diff line change
Expand Up @@ -645,5 +645,12 @@ exp=$'+ true\n+ 1> /dev/null 2>& 1 3>& 1 4>& 3'
[[ $got == "$exp" ]] || err_exit "showme doesn't print redirects properly" \
"(expected $(printf %q "$exp"), got $(printf %q "$got"))"

# ======
# pipefail did not set 9th bit in exit status if a process got signalled
# https://github.com/ksh93/ksh/discussions/755#discussioncomment-9925394
got=$(set --pipefail; "$SHELL" -c 'kill -s PIPE $$' | true; echo $?)
exp=$(( ${ kill -l PIPE; } + 256 ))
[[ $got == "$exp" ]] || err_exit "status of signalled process in pipe with pipefail (expected $exp, got $got)"

# ======
exit $((Errors<125?Errors:125))

0 comments on commit f1ffd60

Please sign in to comment.