external/cde
1
0
Fork 0
mirror of git://git.code.sf.net/p/cdesktopenv/code synced 2025-02-13 03:32:24 +00:00
Code Issues Releases Wiki Activity

Fix ${.sh.fun} leaking out of DEBUG trap

Browse source 
The value of the ${.sh.fun} variable, which is supposed to contain
the name of the function currently being executed, leaks out of the
DEBUG trap if it executes a function. Reproducer:

$ fn() { echo "executing the function"; }
$ trap fn DEBUG
$ trap - DEBUG
executing the function
$ echo ${.sh.fun}
fn

${.sh.fun} should be empty outside the function.

Annalysis:

The sh_debug() function in xec.c, which executes the DEBUG trap
action, contains these lines, which are part of restoring the state
after running the trap action with sh_trap():

	nv_putval(SH_PATHNAMENOD,shp->st.filename,NV_NOFREE);
	nv_putval(SH_FUNNAMENOD,shp->st.funname,NV_NOFREE);
 	shp->st = savst;

First the SH_PATHNAMENOD (${.sh.file}) and SH_FUNNAMENOD
(${.sh.fun}) variables get restored from the values in the shell's
scoped information struct (shp->st), but that is done *before*
restoring the parent scope with 'shp->st = savst;'. It should be
done after. Fixing the order is sufficient to fix the bug.

However, I am not convinced that these nv_putval() calls are good
for anything at all. Setting, unsetting, restoring, etc. the
${.sh.fun} and ${.sh.file} variables is already being handled
perfectly well elsewhere in the code for executing functions and
sourcing dot scripts. The DEBUG trap is neither here nor there.
There's no reason for it to get involved with these variables.

I was unable to break anything after simply removing those two
lines. So I strongly suspect this is another case, out of many now,
where a bug in ksh93 is properly fixed by removing some code.

I couldn't get ${.sh.file} to leak similarly -- I think this is
because SH_PATHNAMENOD (and not SH_FUNNOD) is set explicitly in
exfile() in main.c, masking this incorrect restore. It is the only
place where SH_PATHNAMENOD and SH_FUNNOD are not both set.

src/cmd/ksh93/sh/xec.c:
- Remove these two spurious nv_putval() calls.

src/cmd/ksh93/tests/variables.sh:
- Add regression test for leaking ${.sh.fun}.
This commit is contained in:
Martijn Dekker 2021-02-27 01:01:30 +00:00
parent ef8b80cfd7
commit c928046aa9
3 changed files with 10 additions and 2 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

4
NEWS
View file

@ -23,6 +23,10 @@ Any uppercase BUG_* names are modernish shell bug IDs.
part uniquely identified it so the menu only showed one item. Details:
https://www.mail-archive.com/ast-users@lists.research.att.com/msg00436.html
- A bug with ${.sh.fun} in combination with the DEBUG trap has been fixed.
The ${.sh.fun} variable wrongly continued to contain the name of the last
function executed by the DEBUG trap after the trap action completed.
2021-02-21:
- Fixed: The way that SIGWINCH was handled (i.e. the signal emitted when the

2
src/cmd/ksh93/sh/xec.c
View file

@ -697,8 +697,6 @@ int sh_debug(Shell_t *shp, const char *trap, const char *name, const char *subsc
shp->indebug = 0;
if(shp->st.cmdname)
error_info.id = shp->st.cmdname;
nv_putval(SH_PATHNAMENOD,shp->st.filename,NV_NOFREE);
nv_putval(SH_FUNNAMENOD,shp->st.funname,NV_NOFREE);
shp->st = savst;
if(sav != stkptr(stkp,0))
stkset(stkp,sav,offset);

6
src/cmd/ksh93/tests/variables.sh
View file

@ -1192,5 +1192,11 @@ got=$(FPATH=$tmp "$SHELL" ./lineno_autoload 2>&1)
[[ $got == "$exp" ]] || err_exit 'Regression in $LINENO and/or error messages.' \
$'Diff follows:\n'"$(diff -u <(print -r -- "$exp") <(print -r -- "$got") | sed $'s/^/\t| /')"
# ======
# Before 2021-02-26, the DEBUG trap corrupted ${.sh.fun}
unset .sh.fun
got=$(some_func() { :; }; trap some_func DEBUG; trap - DEBUG; print -r "${.sh.fun}")
[[ -z $got ]] || err_exit "\${.sh.fun} leaks out of DEBUG trap (got $(printf %q "$got"))"
# ======
exit $((Errors<125?Errors:125))