Fix signal/trap behaviour in ksh functions (rhbz#1454804)

Prior discussion:
https://bugzilla.redhat.com/1454804

On 2017-05-23 13:33:25 UTC, Paulo Andrade wrote:
> In previous ksh versions, when exiting the scope of a ksh
> (not posix) function, it would restore the trap table of
> the "calling context" and if the reason the function exited
> was a signal, it would call sh_fault() passing as argument
> the signal value.
>   Newer ksh checks it, but calls kill(getpid(), signal_number)
> after restoring the trap table, but only calls for SIGINT and
> SIGQUIT.
[...]
>   The old way appears to have been more appropriate, but there
> must be a reason to only pass SIGINT and SIGQUIT as it is an
> explicit patch.

The last paragraph is where I differ. This would not be the first
example of outright breakage that appeared to be added deliberately
and that 93u+m has fixed or removed, see e.g. 8477d2c ('printf %H'
had code that deleted all multibyte characters), cefe087, or
781f0a3. Sometimes it seems the developers added a little
experiment and then forgot all about it, so it became a misfeature.

In this instance, the correct pre-2012 ksh behaviour is still
explicitly documented in (k)sh.1: "A trap condition that is not
caught or ignored by the function causes the function to terminate
and the condition to be passed on to the caller". Meaning, if there
is no function-local trap, the signal defaults to the parent scope.
There is no language that limits this to SIGINT and SIGQUIT only.
It also makes no sense at all to do so -- signals such as SIGPIPE,
SIGTERM, or SIGSEGV need to be caught by default and to do
otherwise results in misbehaviour by default.

src/cmd/ksh93/sh/xec.c: sh_funscope():
- When resending a signal after restoring the global traps state,
  remove the spurious check that limits this to SIGINT and SIGQUIT.
- Replace it with a check for nsig!=0, as that means there were
  parent trap states to restore. Otherwise 'kill' may be called
  with an invalid signal argument, causing a crash on macOS.

src/cmd/ksh93/tests/signal.sh:
- Update a test to check that a function-local SIGTERM trap is
  triggered correctly when signalled from another process.
- Complete the tests for 3aee10d; this bug needed fixing before
  we could test that previous fix in a ksh function scope.
- Add a test for triggering global traps from ksh functions,
  testing multiple POSIX-standard signals.

NEWS

@@ -3,6 +3,15 @@ For full details, see the git log at: https://github.com/ksh93/ksh
 
 Any uppercase BUG_* names are modernish shell bug IDs.
 
+2020-09-28:
+
+- While executing a ksh-style function, ksh 93u+ ignored all signals for which
+  the function had not set a local trap, except for SIGINT and SIGQUIT. This
+  was contrary to the manual, which states that a "trap condition that is not
+  caught or ignored by the function causes the function to terminate and the
+  condition to be passed on to the caller". This has now been fixed in 93u+m to
+  match the documentation, so that e.g. global traps work as expected again.
+
 2020-09-27:
 
 - The shell's lexical analysis of a 'case' statement within a do...done block

src/cmd/ksh93/include/version.h

@@ -17,4 +17,4 @@
 *                  David Korn <dgk@research.att.com>                   *
 *                                                                      *
 ***********************************************************************/
-#define SH_RELEASE	"93u+m 2020-09-27"
+#define SH_RELEASE	"93u+m 2020-09-28"

src/cmd/ksh93/sh/xec.c

@@ -3193,7 +3193,7 @@ int sh_funscope(int argn, char *argv[],int(*fun)(void*),void *arg,int execflg)
 	}
 	if(jmpval)
 		r=shp->exitval;
-	if(r>SH_EXITSIG && ((r&SH_EXITMASK)==SIGINT || ((r&SH_EXITMASK)==SIGQUIT)))
+	if(nsig && r>SH_EXITSIG)
 		kill(shgd->current_pid,r&SH_EXITMASK);
 	if(jmpval > SH_JMPFUN)
 	{

src/cmd/ksh93/tests/signal.sh

@@ -426,6 +426,8 @@ then	{
 	[[ $(kill -l $?) == RTMIN ]] || err_exit 'wait interrupted by signal not caught should exit with the value of that signal+256'
 fi
 
+# sh.1: "A trap condition that is not caught or ignored by the function causes
+# the function to terminate and the condition to be passed on to the caller."
 function b
 {
 	sleep .3
@@ -434,16 +436,17 @@ function b
 
 function a
 {
-	trap 'print int'  TERM
+	trap 'endc=1' TERM
 	b
 	enda=1
 }
 
 { sleep .1;kill -s TERM $$;}&
-unset enda endb
+unset enda endb endc
 a
 [[ $endb ]] &&  err_exit 'TERM signal did not kill function b'
 [[ $enda == 1 ]] || err_exit 'TERM signal killed function a'
+[[ $endc == 1 ]] || err_exit 'TERM trap not triggered in function a'
 
 # ======
 # Exit status checks
@@ -494,5 +497,35 @@ got=$(export sig; "$SHELL" -c '
 ((!(e = $?))) && [[ $got == "$exp" ]] || err_exit "failed to handle SIG$sig from subshell" \
 	"(got status $e$( ((e>128)) && print -n / && kill -l "$e"), $(printf %q "$got"))"
 
+got=$(export sig; "$SHELL" -c '
+	function tryTrap
+	{
+		kill -s "$1" "$$"
+	}
+	trap "print '\''OK: $sig'\''" "$sig"
+	tryTrap "$sig"
+	trap - "$sig"
+' 2>&1)
+((!(e = $?))) && [[ $got == "$exp" ]] || err_exit "failed to handle SIG$sig from ksh function" \
+	"(got status $e$( ((e>128)) && print -n / && kill -l "$e"), $(printf %q "$got"))"
+
+# ======
+# ksh-style functions didn't handle signals other than SIGINT and SIGQUIT (rhbz#1454804)
+exp="HUP INT PIPE QUIT TERM USR1 USR2"
+got=$(export exp; "$SHELL" -c '
+	function tryTrap
+	{
+		kill -s "$1" "$$"
+	}
+	for sig in $exp			# split
+	do	trap "print -n '\''$sig '\''" "$sig"
+		tryTrap "$sig"
+		trap - "$sig"
+	done
+' 2>&1)
+got=${got% }	# rm final space
+((!(e = $?))) && [[ $got == "$exp" ]] || err_exit "ksh function ignores global signal traps" \
+	"(got status $e$( ((e>128)) && print -n / && kill -l "$e"), $(printf %q "$got"))"
+
 # ======
 exit $((Errors<125?Errors:125))