Skip to content

Commit

Permalink
Fix 'exit' default status in trap action (re: 092b90d)
Browse files Browse the repository at this point in the history
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
092b90d. 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: #667
Resolves: #687
  • Loading branch information
McDutchie committed Dec 25, 2023
1 parent e9182bd commit aea9915
Show file tree
Hide file tree
Showing 8 changed files with 30 additions and 5 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 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
Expand Down
3 changes: 2 additions & 1 deletion src/cmd/ksh93/bltins/cflow.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down
3 changes: 2 additions & 1 deletion src/cmd/ksh93/include/shell.h
Original file line number Diff line number Diff line change
Expand Up @@ -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 */
Expand All @@ -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;
Expand Down
2 changes: 1 addition & 1 deletion src/cmd/ksh93/include/version.h
Original file line number Diff line number Diff line change
Expand Up @@ -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. */
Expand Down
1 change: 1 addition & 0 deletions src/cmd/ksh93/sh/fault.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down
1 change: 1 addition & 0 deletions src/cmd/ksh93/sh/subshell.c
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand Down
2 changes: 2 additions & 0 deletions src/cmd/ksh93/sh/xec.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
16 changes: 14 additions & 2 deletions src/cmd/ksh93/tests/exit.sh
Original file line number Diff line number Diff line change
Expand Up @@ -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))

0 comments on commit aea9915

Please sign in to comment.