From e3aa32a129a6aa8b0b2633436398f6b02b0d1aa6 Mon Sep 17 00:00:00 2001 From: Martijn Dekker Date: Sat, 4 Jun 2022 17:02:05 +0100 Subject: [PATCH] Add --functrace shell option (re: 2a835a2d) A side effect of the bug fixed in 2a835a2d caused the DEBUG trap action to appear to be inherited by subshells, but in a buggy way that could crash the shell. After the fix, the trap is reset in subshells along with all the others, as it should be. Nonetheless, as that bug was present for years, some have come to rely on it. This commit implements that functionality properly. When the new --functrace option is turned on, DEBUG trap actions are now inherited by subshells as well as ksh function scopes. In addition, since it makes logical sense, the new option also causes the -x/--xtrace option's state to be inherited by ksh function scopes. Note that changes made within the scope do not propagate upwards; this is different from bash. (I've opted against adding a -T short-form equivalent as on bash, because -T was formerly a different option on 93u+ (see 63c55ad7) and on mksh it has yet anohter a different meaning. To minimise confusion, I think it's best to have the long-form name only.) src/cmd/ksh93/include/shell.h, src/cmd/ksh93/data/options.c: - Add new "functrace" (SH_FUNCTRACE) long-form shell option. src/cmd/ksh93/sh/subshell.c: sh_subshell(): - When functrace is on, copy the parent's DEBUG trap action into the virtual subshell scope after resetting the trap actions. src/cmd/ksh93/sh/xec.c: sh_funscope(): - When functrace is on and xtrace is on in the parent scope, turn it on in the child scope. - Same DEBUG trap action handling as in sh_subshell(). Resolves: https://github.com/ksh93/ksh/issues/162 --- ANNOUNCE | 6 +++ NEWS | 8 ++++ src/cmd/ksh93/data/builtins.c | 5 ++- src/cmd/ksh93/data/options.c | 1 + src/cmd/ksh93/include/shell.h | 1 + src/cmd/ksh93/include/version.h | 2 +- src/cmd/ksh93/sh.1 | 16 ++++++++ src/cmd/ksh93/sh/subshell.c | 5 +++ src/cmd/ksh93/sh/xec.c | 8 +++- src/cmd/ksh93/tests/basic.sh | 66 ++++++++++++++++++++++++++++++++- 10 files changed, 112 insertions(+), 6 deletions(-) diff --git a/ANNOUNCE b/ANNOUNCE index 4b0f190b3..7520963d0 100644 --- a/ANNOUNCE +++ b/ANNOUNCE @@ -70,6 +70,12 @@ New command line editor features: New features in shell options: +- A new --functrace long-form shell option causes the -x/--xtrace option's + state and the DEBUG trap action to be inherited by function scopes instead + of being reset to default. Changes made to them within a function scope + still do not propagate back to the parent scope. Similarly, this option + also causes the DEBUG trap action to be inherited by subshells. + - The new --histreedit and --histverify options modify history expansion (--histexpand). If --histreedit is on and a history expansion fails, the command line is reloaded into the next prompt's edit buffer, allowing diff --git a/NEWS b/NEWS index 598b10de6..123e7a3fe 100644 --- a/NEWS +++ b/NEWS @@ -3,6 +3,14 @@ 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-06-04: + +- Added a new --functrace long-form shell option which causes the -x/--xtrace + option's state and the DEBUG trap action to be inherited by function scopes + instead of being reset to default. Changes made to them within a function + scope still do not propagate back to the parent scope. Similarly, this + option also causes the DEBUG trap action to be inherited by subshells. + 2022-06-03: - Fixed a couple of bugs that caused floating point variables imported from diff --git a/src/cmd/ksh93/data/builtins.c b/src/cmd/ksh93/data/builtins.c index 183a1b263..5db5a2ca4 100644 --- a/src/cmd/ksh93/data/builtins.c +++ b/src/cmd/ksh93/data/builtins.c @@ -210,6 +210,9 @@ const char sh_set[] = "[+emacs?Enables/disables \bemacs\b editing mode.]" #endif "[+errexit?Equivalent to \b-e\b.]" + "[+functrace?Function scopes and subshells inherit the parent " + "environment's \bDEBUG\b trap action. Function scopes inherit " + "the \b-x\b option's state.]" #if SHOPT_GLOBCASEDET "[+globcasedetect?Pathname expansion and file name completion " "automatically become case-insensitive on file systems where " @@ -1625,7 +1628,7 @@ const char sh_optksh[] = "[+SEE ALSO?\bset\b(1), \bbuiltin\b(1)]" ; const char sh_optset[] = -"+[-1c?\n@(#)$Id: set (AT&T Research) 1999-09-28 $\n]" +"+[-1c?\n@(#)$Id: set (ksh 93u+m) 2022-06-04 $\n]" "[--catalog?" SH_DICT "]" "[+NAME?set - set/unset options and positional parameters]" "[+DESCRIPTION?\bset\b sets or unsets options and positional parameters. " diff --git a/src/cmd/ksh93/data/options.c b/src/cmd/ksh93/data/options.c index 07499ff6b..7f430c647 100644 --- a/src/cmd/ksh93/data/options.c +++ b/src/cmd/ksh93/data/options.c @@ -42,6 +42,7 @@ const Shtable_t shtab_options[] = #endif "errexit", SH_ERREXIT, "noexec", SH_NOEXEC, + "functrace", SH_FUNCTRACE, "noglob", SH_NOGLOB, #if SHOPT_GLOBCASEDET "globcasedetect", SH_GLOBCASEDET, diff --git a/src/cmd/ksh93/include/shell.h b/src/cmd/ksh93/include/shell.h index 9b3fc21ae..020152c5f 100644 --- a/src/cmd/ksh93/include/shell.h +++ b/src/cmd/ksh93/include/shell.h @@ -99,6 +99,7 @@ typedef union Shnode_u Shnode_t; #define SH_NOUNSET 9 #define SH_NOGLOB 10 #define SH_ALLEXPORT 11 +#define SH_FUNCTRACE 12 #define SH_IGNOREEOF 13 #define SH_NOCLOBBER 14 #define SH_MARKDIRS 15 diff --git a/src/cmd/ksh93/include/version.h b/src/cmd/ksh93/include/version.h index f03ddaca5..9c0daad67 100644 --- a/src/cmd/ksh93/include/version.h +++ b/src/cmd/ksh93/include/version.h @@ -21,7 +21,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-06-03" /* must be in this format for $((.sh.version)) */ +#define SH_RELEASE_DATE "2022-06-04" /* 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.1 b/src/cmd/ksh93/sh.1 index e7fa955a7..290c3c34e 100644 --- a/src/cmd/ksh93/sh.1 +++ b/src/cmd/ksh93/sh.1 @@ -7566,6 +7566,22 @@ style in-line editor for command entry. Same as .BR \-e . .TP 8 +.B functrace +Causes the +.B \-x +option's state and the +.B DEBUG +trap action to be inherited by functions defined using the +.B function +keyword (see +.I "Functions\^" +above) instead of being reset to default. +Changes made to them within the function +do not propagate back to the parent scope. +Similarly, this option also causes the +.B DEBUG +trap action to be inherited by subshells. +.TP 8 .B globcasedetect When this option is turned on, globbing (see .I "Pathname Expansion\^" diff --git a/src/cmd/ksh93/sh/subshell.c b/src/cmd/ksh93/sh/subshell.c index e0bab0842..1f9a4e865 100644 --- a/src/cmd/ksh93/sh/subshell.c +++ b/src/cmd/ksh93/sh/subshell.c @@ -532,6 +532,7 @@ Sfio_t *sh_subshell(Shnode_t *t, volatile int flags, int comsub) if(!sh.subshare) { struct subshell *xp; + char *save_debugtrap = 0; #if _lib_fchdir sp->pwdfd = -1; for(xp=sp->prev; xp; xp=xp->prev) @@ -591,7 +592,11 @@ Sfio_t *sh_subshell(Shnode_t *t, volatile int flags, int comsub) sp->coutpipe = sh.coutpipe; sp->cpipe = sh.cpipe[1]; sh.cpid = 0; + if(sh_isoption(SH_FUNCTRACE) && sh.st.trap[SH_DEBUGTRAP] && *sh.st.trap[SH_DEBUGTRAP]) + save_debugtrap = sh_strdup(sh.st.trap[SH_DEBUGTRAP]); sh_sigreset(0); + if(save_debugtrap) + sh.st.trap[SH_DEBUGTRAP] = save_debugtrap; } jmpval = sigsetjmp(checkpoint.buff,0); if(jmpval==0) diff --git a/src/cmd/ksh93/sh/xec.c b/src/cmd/ksh93/sh/xec.c index cf9ca676d..3c21e62f6 100644 --- a/src/cmd/ksh93/sh/xec.c +++ b/src/cmd/ksh93/sh/xec.c @@ -3109,7 +3109,7 @@ int sh_funscope(int argn, char *argv[],int(*fun)(void*),void *arg,int execflg) int isig,jmpval; volatile int r = 0; int n; - char **savsig; + char **savsig, *save_debugtrap = 0; struct funenv *fp = 0; struct checkpt *buffp = (struct checkpt*)stkalloc(sh.stk,sizeof(struct checkpt)); Namval_t *nspace = sh.namespace; @@ -3146,7 +3146,7 @@ int sh_funscope(int argn, char *argv[],int(*fun)(void*),void *arg,int execflg) sh.st.save_tree = sh.var_tree; if(!fun) { - if(nv_isattr(fp->node,NV_TAGGED)) + if(sh_isoption(SH_FUNCTRACE) && is_option(&options,SH_XTRACE) || nv_isattr(fp->node,NV_TAGGED)) sh_onoption(SH_XTRACE); else sh_offoption(SH_XTRACE); @@ -3171,7 +3171,11 @@ int sh_funscope(int argn, char *argv[],int(*fun)(void*),void *arg,int execflg) savsig[isig] = NULL; } } + if(!fun && sh_isoption(SH_FUNCTRACE) && sh.st.trap[SH_DEBUGTRAP] && *sh.st.trap[SH_DEBUGTRAP]) + save_debugtrap = sh_strdup(sh.st.trap[SH_DEBUGTRAP]); sh_sigreset(0); + if(save_debugtrap) + sh.st.trap[SH_DEBUGTRAP] = save_debugtrap; argsav = sh_argnew(argv,&saveargfor); sh_pushcontext(buffp,SH_JMPFUN); errorpush(&buffp->err,0); diff --git a/src/cmd/ksh93/tests/basic.sh b/src/cmd/ksh93/tests/basic.sh index e6102506a..4a52c6bc4 100755 --- a/src/cmd/ksh93/tests/basic.sh +++ b/src/cmd/ksh93/tests/basic.sh @@ -797,8 +797,7 @@ trap - DEBUG # bug compat "(expected $(printf %q "$exp"), got $(printf %q "$got"))" # Make sure the DEBUG trap still exits a POSIX function on exit status 255 -# TODO: same test for ksh function with -o functrace, once we add that option -exp=$'one\ntwo' +exp=$'one\ntwo\nEND' got=$( myfn() { @@ -813,11 +812,74 @@ got=$( } trap '[[ ${.sh.command} == *three ]] && set255' DEBUG myfn + echo END ) trap - DEBUG # bug compat [[ $got == "$exp" ]] || err_exit "DEBUG trap did not trigger return from POSIX function on status 255" \ "(expected $(printf %q "$exp"), got $(printf %q "$got"))" +# Test the new --functrace option +if ! [[ -o ?functrace ]] +then + warning 'shell does not have --functrace; skipping those tests' +else + # Make sure the DEBUG trap is inherited by ksh functions if --functrace is on + exp=$'Debug 0\nDebug 1\nDebugLocal 1\nDebugLocal 2\nFunction\nDebugLocal 1\nDebug 0\nNofunction' + got=$( + function entryfn + { + trap 'echo DebugLocal ${.sh.level}' DEBUG + myfn + : + } + function myfn + { + echo Function + } + set --functrace + trap 'echo Debug ${.sh.level}' DEBUG + entryfn + echo Nofunction + ) + [[ $got == "$exp" ]] || err_exit "DEBUG trap not inherited by ksh function with --functrace on" \ + "(expected $(printf %q "$exp"), got $(printf %q "$got"))" + + # Make sure the DEBUG trap exits a ksh function with --functrace on exit status 255 + exp=$'one\ntwo\nEND' + got=$( + function myfn + { + echo one + echo two + echo three + echo four + } + function set255 + { + return 255 + } + set --functrace + trap '[[ ${.sh.command} == *three ]] && set255' DEBUG + myfn + echo END + ) + [[ $got == "$exp" ]] || err_exit "DEBUG trap did not trigger return from POSIX function on status 255" \ + "(expected $(printf %q "$exp"), got $(printf %q "$got"))" + + # Make sure --functrace causes subshells to inherit the DEBUG trap + exp=$'[DEBUG] 1\nfoo:5\n[DEBUG] 2\nbar:6\n[DEBUG] 3\nbar:6a\n[DEBUG] 2\nbaz:7' + got=$( + typeset -i dbg=0 + set --functrace + trap 'echo "[DEBUG] $((++dbg))"' DEBUG + echo foo:5 + ( echo bar:6; (echo bar:6a) ) + echo baz:7 + ) + [[ $got == "$exp" ]] || err_exit "DEBUG trap not inherited by subshell" \ + "(expected $(printf %q "$exp"), got $(printf %q "$got"))" +fi + # ====== # In ksh93v- and ksh2020 EXIT traps don't work in forked subshells # https://github.com/att/ast/issues/1452