3.1.6-bart-8: BUG: bad node type in freenode() [and others]

3.1.6-bart-8: BUG: bad node type in freenode() [and others]

[Bart Schaefer]

[-- Attachment #1: Type: text/plain, Size: 1850 bytes --]

I've just spent the last hour trying to narrow this down, without much
success.  It appears to have something to do with executing the trap at
function exit that is defined by

	trap "$r ; trap - 0 2 3 15 ZERR ; return 1" 0 2 3 15 ZERR

but there must be something else involved because I can't reproduce it
with a function that has only such a trap.

This is confounded by the fact that using "shift" during a "while getopts"
loop appears to cause some sort of confusion; maybe the two together are
what triggers the bug, or maybe it has something to do with "compctl -T",
which is also used in the attached sample function.

Anyway, to reproduce you must autoload the two attached functions, then:

zagzig<1> ftpupdate-pack
1) create    3) symlink   5) chmod     7) rmdir     9) md5sum    11) send     
2) link      4) rename    6) mkdir     8) unlink    10) edit     12) quit     
ftpupdate command: create zsh-3.0.7.tar.gz
ftpupdate-pack: bad option: -3				<-- GETOPTS ODDITY
ftpupdate command: quit

At this point I've gotten both

BUG: bad node type in freenode()
BUG: attempt to free more than allocated.

and a simple crash with no BUG message at all.  The stack trace below is
from the "bad node type" variant.

(gdb) where
#0  0x4004e811 in __kill ()
#1  0x4004e63f in raise (sig=6) at ../sysdeps/posix/raise.c:27
#2  0x4004f84f in abort () at ../sysdeps/generic/abort.c:83
#3  0x8095fda in ifreestruct (a=0x812e138) at ../../zsh-3.1.6/Src/utils.c:2106
#4  0x8095dd1 in freestructs () at ../../zsh-3.1.6/Src/utils.c:2049
#5  0x8068a99 in loop (toplevel=1, justonce=0)
    at ../../zsh-3.1.6/Src/init.c:145
#6  0x804a4f9 in main (argc=1, argv=0xbffff6e4)
    at ../../zsh-3.1.6/Src/main.c:89

-- 
Bart Schaefer                                 Brass Lantern Enterprises
http://www.well.com/user/barts              http://www.brasslantern.com

[-- Attachment #2: zleread --]
[-- Type: text/plain , Size: 379 bytes --]

#! /usr/local/bin/zsh -f
emulate zsh
setopt localoptions
local input opt ropt n=0
ropt=()
while getopts :AEe opt
do
    case $opt in
    [?]) read $*; return $?;;
    +*) break;;
    *) ropt=($ropt -$opt); ((++n));
    esac
done
if [[ -t 0 ]]
then
    shift $n
    vared -hp "${${(M)1%%\?*}#\?}" input
    print -r $input | read $ropt ${1%%\?*} $*[2,-1]
    return $?
fi
read $*

[-- Attachment #3: Text --]
[-- Type: text/plain , Size: 3942 bytes --]

#!/bin/zsh -f
emulate zsh
setopt localoptions nomultios
local f m s

if (($#)) then
    while getopts :m: f
    do
	case $f in
	m) m=$OPTARG;;
	[?]) echo 1>&2 ${0}: bad option: -$OPTARG; return 1;;
	esac
	shift $((OPTIND-1))
    done
    for f
    do
	case $f:e in
	--) continue;;
	[zZ]|gz|bzip2) ;;
	*) echo 1>&2 gzip $f; gzip $f; f=$f.gz;;
	esac
	s=${${=$(md5sum $f)}[1]}
	uuencode $f /dev/stdout |
	sed "s,^begin [0-9]*... /dev/stdout,create $f ${m:-0644} $s,"
    done
    return 0
fi

f=${TMPPREFIX}-ftpupdate.$$
: >| $f
local c p r l REPLY
l=(create link symlink rename chmod mkdir rmdir unlink md5sum edit send quit)
r="$(compctl -LT) ; /bin/rm -f $f"
trap "$r ; trap - 0 2 3 15 ZERR ; return 1" 0 2 3 15 ZERR
compctl -Tx 'p[1,-1]' -f - 'W[0,*]' -k l ${${ZSH_VERSION##3.0*}:+-tn}
local PS3="ftpupdate command: "
select c in $l
do
    p=
    m=
    REPLY=($=REPLY)
    compctl -Tx 'W[0,*]' -f ${${ZSH_VERSION##3.0*}:+-tn}
    case ${c:=$REPLY[1]} in
      mkdir)
	[[ -n "${p:=$REPLY[2]}" ]] || zleread p"?Enter path: " m
	print -R $c ${p:?'Missing path!'} ${${m:=$REPLY[3]}:-0755} >>$f
	;;
      rmdir|unlink|md5sum)
	[[ -n "${p:=$REPLY[2]}" ]] || zleread p"?Enter path: "
	print -R $c ${p:?'Missing path!'} >>$f
	;;
      create)
	[[ -n "${p:=$REPLY[2]}" ]] || zleread p"?Enter path: " m
	ftpupdate-pack -m ${${m:=$REPLY[3]}:-0644} ${p:?'Missing path!'} >>$f
	;;
      chmod)
	[[ -n "${m:=$REPLY[2]}" ]] || zleread m"?Enter mode: " p
	[[ -n "${p:=$REPLY[3]}" ]] || zleread p"?Enter path: "
	print -R $c ${m:?'Missing mode!'} ${p:?'Missing path!'} >>$f
	;;
      rename|link|symlink)
	[[ -n "${p:=$REPLY[2]}" ]] || zleread p"?Enter source: " m
	[[ -n "${m:=$REPLY[3]}" ]] || zleread m"?Enter destination: "
	print -R $c ${p:?'Missing source!'} ${m:?'Missing destination!'} >>$f
	;;
      edit)
	echo -n "Waiting for editor ..."; emacs $f; echo;;
      send)
	less < $f
	read -q c"?Send to ftpupdate@zsh.org now? [yn] " &&
	    pgp -fs < $f | mail -n ftpupdate@zsh.org && break;;
      quit) break;;
  *) less <<'EOF'
  mkdir <pathname> <mode>
      Create a directory.  Existing files will not be overwritten.
      The mode *must* include the bits 0700.

  rmdir <pathname>
      Remove a directory.

  create <pathname> <mode> <md5sum>
      Create a regular file; any existing non-directory of that name
      is deleted.  The MD5 checksum is checked, and stored in the MD5SUM
      file.  The command must be followed by uuencoded data, sans "begin"
      line, terminated by "end".

  md5sum <pathname>
      Regenerate the MD5SUM entry for a regular file.  This should only
      be used if the MD5SUM entry is missing or corrupted.

  chmod <mode> <pathname>
      Change mode of the specified file to the specified octal value.
      If modifying a directory, the mode *must* include the bits 0700.
      Symbolic links cannot be modified in this manner.

  unlink <pathname>
      Remove a non-directory.

  rename <source-pathname> <destination-pathname>
      Rename/move an existing file.  A non-directory, when renamed,
      can replace an existing non-directory.

  link <source-pathname> <destination-pathname>
      Create a hard link to a non-directory.  Existing files will not
      be overwritten.

  symlink <source-pathname> <destination-pathname> [<md5sum>]
      Create a symbolic link; any existing non-directory of that name is
      deleted.  The MD5 checksum, if given, is stored in the appropriate
      MD5SUM file; this should be the MD5 checksum of the file the link
      points to.
      The <source-pathname> can be any sequence of permitted characters,
      even if it refers to a reserved filename or a filename that the
      user does not have permission to examine or modify.  Note that
      this pathname is interpreted relative to the directory containing
      the symbolic link.

EOF
    ;;
    esac
    compctl -Tx 'p[1,-1]' -f - 'W[0,*]' -k l ${${ZSH_VERSION##3.0*}:+-tn}
done
eval "$r"

Re: 3.1.6-bart-8: BUG: bad node type in freenode() [and others]

[Sven Wischnowsky]

Bart Schaefer wrote:

> I've just spent the last hour trying to narrow this down, without much
> success.  It appears to have something to do with executing the trap at
> function exit that is defined by
> 
> 	trap "$r ; trap - 0 2 3 15 ZERR ; return 1" 0 2 3 15 ZERR
> 
> but there must be something else involved because I can't reproduce it
> with a function that has only such a trap.
> 
> This is confounded by the fact that using "shift" during a "while getopts"
> loop appears to cause some sort of confusion; maybe the two together are
> what triggers the bug, or maybe it has something to do with "compctl -T",
> which is also used in the attached sample function.

As far as I can see this is caused by dosavetrap() storing the pointer 
to the trap code in its savetrap struct and then calling unsettrap()
(which calls freestruct()). Later the saved pointer is re-installed
and even later it is freed again. Boom.

I'm not entirely sure that the patch below is the right thing to do
(I'm never really sure about that part of the code). But this gives me 
the opportunity to mention something else from my list which I wanted
to ask Peter about:

Some time ago Clint Adams wrote (in 8457):

> % TRAPEXIT() { print boom }
> % exit
> boom
> zsh: 8101 segmentation fault (core dumped)  zsh -f
> 
> 
> Reversing the removehashnode hunk in 7349 fixes this,
> but of course the TRAPQUIT example now segfaults.
> 
> What to do?

I answered this with (in 8473):

> The problem occurred as soon as a functions was called...
> 
> The problem was that `dosavetrap()' when called from `starttrapscope()' 
> removed the definition for `TRAPEXIT()' but didn't reset the entry for 
> it in `sigtrapped[]'. In `endtrapscope()' we tested `sigtrapped[SIGEXIT]' 
> which was still `!= 0' even though we had removed the handler function. 
> Then we tried to get the function definition, got a NULL pointer and
> later tried to execute that. Bang.
> 
> 
> This patch resets the `sigtrapped[]' entry in `dosavetrap()'. I'm not
> sure if this is enough, maybe we have to execute some code from
> `unsettrap()' there, too. So I think the final solution has to wait
> until Peter reappears.

So... Peter?

Bye
 Sven

P.S.: Hello Peter, welcome back.

diff -u oldsrc/signals.c Src/signals.c
--- oldsrc/signals.c	Mon Nov 22 10:49:26 1999
+++ Src/signals.c	Mon Nov 22 12:44:36 1999
@@ -666,6 +666,7 @@
 	st->list = removehashnode(shfunctab, func);
     } else {
 	st->list = sigfuncs[sig];
+	sigfuncs[sig] = NULL;
 	unsettrap(sig);
     }
     sigtrapped[sig] = 0;

--
Sven Wischnowsky                         wischnow@informatik.hu-berlin.de