Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Browse files
Browse the repository at this point in the history
The _ variable is used for various context-dependent things and is
sometimes a name reference (see man page under Variables). The $_
state changes are handled by set_instance() and unset_instance() in
xec.c. In the following reproducer, $_ became corrupted, crashing
the shell:
foo.get() { .sh.value=foo; }
bar.get() { .sh.value=${ echo foo; }; }
(: $bar; : $foo)
echo "$_"
So this involves two .get discipline functions, a virtual subshell,
and a shared-state command substitution in the second function.
The $_ corruption in the reproducer was happening as of 88a1f3d,
which began to robustify discipline functions by introducing a
context push and sigsetjmp/siglongjmp to ensure they can restore
state if an error occurs. As of that commit, the reproducer prints
the empty string, and no value can be assigned to _ afterwards.
While the state is restored correctly, it fails to actually execute
a siglongjmp when necessary. For example, if sh_subfork() is called
to fork a virtual subshell (causing jmpval == SH_JMPSUB), we need
to siglongjmp in the parent process, or entirely incorrect things
are going to happen, such as unset_instance() not being called when
it should be. Commit 430e478 introduced just such a sh_subfork()
call, exposing this problem.
The bug was further exacerbated by 430e478. As of that commit, the
reproducer crashes the shell. The real problem here is not in that
commit, though, but in faulty sigsetjmp/siglongjmp logic in
sh_fun(), the main interface to call a function, which is used by
the discipline function routines. If a shell command run via the
sh_funct() call executed a siglongjmp, it failed (among a few other
things) to call unset_instance() to restore the state of $_.
Moral of the story:
1. After sigsetjmp and sh_pushcontext, never fail to siglongjmp for
a sufficiently high jmpval (see SH_JMP* in fault.h, and all the
uses of sh_pushcontext()).
2. Always make sure the state is fully restored before siglongjmp.
Thanks to @phidebian for all the help analysing the root cause of
this problem.
src/cmd/ksh93/sh/xec.c: sh_fun():
- Fix sigsetjmp logic by (a) making it cover the sh_funct() call,
so that a siglongjmp from elsewhere will return here, and (b)
delaying our own siglongjmp call until state is fully restored.
- Push context with the jmpval SH_JMPCMD for a built-in command
(which can also be run via this function) and SH_JMPFUN for a
function; this could be needed for correct siglongjmp behaviour
if an error occurs elsewhere, as various code paths will be able
to detect whether it was a command or a function that failed.
- While we're here, switch AST stack use from old stak(3) to new
stk(3) API. This was already done in many places and will
eventually be done in the entire codebase for consistency.
src/cmd/ksh93/sh/nvdisc.c: assign(), lookup():
- Push context with SH_JMPFUN as we're going to run a function.
- Add the missing siglongjmp if jmpval >= SH_JMPFUN. Do not
siglongjmp until the state is fully restored.
Resolves: #616- Loading branch information
