From aea991588b93b92234c4c9dd0467614c502f9b91 Mon Sep 17 00:00:00 2001 From: Martijn Dekker Date: Mon, 25 Dec 2023 03:41:18 +0000 Subject: [PATCH] Fix 'exit' default status in trap action (re: 092b90da) Reproducer: (trap 'false; exit' EXIT; exit 9); echo $? Expected output: 9 Actual output: 1 In a trap action, the default exit status passed down by an 'exit' with no arguments is the exit status of the last command *before* the trap action. So, *that* was the point of the sh.oldexit variable removed in 092b90da. BUG_LOOPRET2 was that sh.oldexit was unconditionally used for the default exit status of 'exit' and 'return' in b_return(). The fix is to restore it, while using it correctly this time (i.e., depending on sh.intrap) so that BUG_LOOPRET2 is not introduced. Thanks to @sckendall2 and @jghub for the bug reports. Resolves: https://github.com/ksh93/ksh/issues/667 Resolves: https://github.com/ksh93/ksh/issues/687 --- NEWS | 7 +++++++ src/cmd/ksh93/bltins/cflow.c | 3 ++- src/cmd/ksh93/include/shell.h | 3 ++- src/cmd/ksh93/include/version.h | 2 +- src/cmd/ksh93/sh/fault.c | 1 + src/cmd/ksh93/sh/subshell.c | 1 + src/cmd/ksh93/sh/xec.c | 2 ++ src/cmd/ksh93/tests/exit.sh | 16 ++++++++++++++-- 8 files changed, 30 insertions(+), 5 deletions(-) diff --git a/NEWS b/NEWS index 38b7f361190c..e797fbc0173f 100644 --- a/NEWS +++ b/NEWS @@ -2,6 +2,13 @@ This documents significant changes in the dev branch of ksh 93u+m. For full details, see the git log at: https://github.com/ksh93/ksh Uppercase BUG_* IDs are shell bug IDs as used by the Modernish shell library. +2023-12-25: + +- Fixed a regression in the behavior of 'exit' in a trap action, introduced + on 2020-09-09. The default exit status, used when no argument is given to + 'exit', is now once again the exit status of the last command executed + *before* the trap action, as expected. + 2023-09-16: - The 'kill' built-in command was meant to refuse to issue SIGSTOP to the diff --git a/src/cmd/ksh93/bltins/cflow.c b/src/cmd/ksh93/bltins/cflow.c index b7cfd1991dfb..dfc95bb345fa 100644 --- a/src/cmd/ksh93/bltins/cflow.c +++ b/src/cmd/ksh93/bltins/cflow.c @@ -76,7 +76,8 @@ int b_return(int n, char *argv[],Shbltin_t *context) } else { - n = sh.savexit; /* no argument: pass down $? */ + /* no argument: pass down $? (but in a trap action, pass down the pre-trap $?) */ + n = sh.intrap ? sh.oldexit : sh.savexit; if(do_exit) n &= SH_EXITMASK; } diff --git a/src/cmd/ksh93/include/shell.h b/src/cmd/ksh93/include/shell.h index f91e0e47802c..32b18780f53b 100644 --- a/src/cmd/ksh93/include/shell.h +++ b/src/cmd/ksh93/include/shell.h @@ -302,6 +302,7 @@ struct Shell_s char *comdiv; /* points to sh -c argument */ char *prefix; /* prefix for compound assignment */ sigjmp_buf *jmplist; /* longjmp return stack */ + int oldexit; /* saves pre-trap exit status for 'exit' default in trap actions */ pid_t bckpid; /* background process id */ pid_t cpid; pid_t spid; /* subshell process id */ @@ -310,7 +311,7 @@ struct Shell_s int topfd; int savesig; unsigned char *sigflag; /* pointer to signal states */ - char intrap; + char intrap; /* set while executing a trap action */ char forked; char binscript; char funload; diff --git a/src/cmd/ksh93/include/version.h b/src/cmd/ksh93/include/version.h index 8c39e94892e2..5df715d9f4c1 100644 --- a/src/cmd/ksh93/include/version.h +++ b/src/cmd/ksh93/include/version.h @@ -18,7 +18,7 @@ #define SH_RELEASE_FORK "93u+m" /* only change if you develop a new ksh93 fork */ #define SH_RELEASE_SVER "1.1.0-alpha" /* semantic version number: https://semver.org */ -#define SH_RELEASE_DATE "2023-09-24" /* must be in this format for $((.sh.version)) */ +#define SH_RELEASE_DATE "2023-12-25" /* must be in this format for $((.sh.version)) */ #define SH_RELEASE_CPYR "(c) 2020-2023 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 756d2c57e0d0..3ba09a028e91 100644 --- a/src/cmd/ksh93/sh/fault.c +++ b/src/cmd/ksh93/sh/fault.c @@ -666,6 +666,7 @@ noreturn void sh_done(int sig) if(t=sh.st.trapcom[0]) { sh.st.trapcom[0]=0; /* should free but not long */ + sh.oldexit = savxit; sh_trap(t,0); savxit = sh.exitval; } diff --git a/src/cmd/ksh93/sh/subshell.c b/src/cmd/ksh93/sh/subshell.c index 19eb48c4cd8f..954265da0b57 100644 --- a/src/cmd/ksh93/sh/subshell.c +++ b/src/cmd/ksh93/sh/subshell.c @@ -657,6 +657,7 @@ Sfio_t *sh_subshell(Shnode_t *t, volatile int flags, int comsub) /* trap on EXIT not handled by child */ char *trap=sh.st.trapcom[0]; sh.st.trapcom[0] = 0; /* prevent recursion */ + sh.oldexit = sh.exitval; sh_trap(trap,0); free(trap); } diff --git a/src/cmd/ksh93/sh/xec.c b/src/cmd/ksh93/sh/xec.c index e9b49f47f22a..f3dc73b65b51 100644 --- a/src/cmd/ksh93/sh/xec.c +++ b/src/cmd/ksh93/sh/xec.c @@ -901,6 +901,8 @@ int sh_exec(const Shnode_t *t, int flags) sh_offstate(SH_DEFPATH); if(!(flags & sh_state(SH_ERREXIT))) sh_offstate(SH_ERREXIT); + if(!sh.intrap) + sh.oldexit = sh.exitval; sh.exitval=0; sh.lastsig = 0; sh.chldexitsig = 0; diff --git a/src/cmd/ksh93/tests/exit.sh b/src/cmd/ksh93/tests/exit.sh index 0ff16aa6b226..fdc4529b0d1d 100755 --- a/src/cmd/ksh93/tests/exit.sh +++ b/src/cmd/ksh93/tests/exit.sh @@ -228,9 +228,21 @@ unset exp got sig # ====== # trap status tests +exp=0 +(trap 'false; exit' EXIT; true; exit) +let "(got=$?)==exp" || err_exit "pre-trap exit status not preserved (got $got, expected $exp)" + exp=1 -(trap 'false; exit' EXIT; true) -let "(got=$?)==exp" || err_exit "passing down exit status from EXIT trap failed (got $got, expected $exp)" +(trap 'true; exit' EXIT; false; exit) +let "(got=$?)==exp" || err_exit "pre-trap exit status not preserved (got $got, expected $exp)" + +exp=7 +(trap 'false; exit 7' EXIT; exit 9) +let "(got=$?)==exp" || err_exit "explicit exit status in trap not honoured (got $got, expected $exp)" + +exp=9 +(trap 'false; exit' EXIT; exit 9) +let "(got=$?)==exp" || err_exit "explicit exit status outside trap not honoured (got $got, expected $exp)" # ====== exit $((Errors<125?Errors:125))