[arnova@xs4all.nl: Bug#293364: Bug in ZSH (zshell) using Debian-testing (Sarge)]

[arnova@xs4all.nl: Bug#293364: Bug in ZSH (zshell) using Debian-testing (Sarge)]

[Clint Adams]

#0  0x00030bdc in loadautofn ()
#1  0x00030b80 in getproc ()
#2  0x0002d5d8 in execsubst ()
#3  0x0002bca0 in execlist ()
#4  0x0002b1d8 in execlist ()
#5  0x0002af0c in execlist ()
#6  0x0002a894 in execode ()
#7  0x00031348 in runshfunc ()
#8  0x00030fe0 in doshfunc ()
#9  0x0006d0f0 in dotrapargs ()
#10 0x00025288 in zexit ()
#11 0x00024f54 in bin_break ()
#12 0x0001a7c0 in execbuiltin ()
#13 0x0002d8c4 in execsubst ()
#14 0x0002bca0 in execlist ()
#15 0x0002b1d8 in execlist ()
#16 0x0002af0c in execlist ()
#17 0x0002a894 in execode ()
#18 0x0003f0c0 in loop ()
#19 0x00041a64 in zsh_main ()
#20 0x70171034 in __libc_start_main () from /lib/v9/libc.so.6
#21 0x00019f74 in _start ()
#22 0x00019f74 in _start ()
Previous frame identical to this frame (corrupt stack?)

----- Forwarded message from Arno van Amersfoort <arnova@xs4all.nl> -----

Package: zsh
Version: 4.2.3-1

 When I exit a zshell (for example when exiting from su) zshell segfaults 
 when:
 - /usr/share/zsh/functions/TRAPEXIT exists and is executable
 - And the function is loaded (for example from /etc/zsh/zshrc with 
 "autoload ${^/usr/share/zsh/functions}/*(.xN:t)" )

 I am using Debian GNU/Linux 3.2 (testing/sarge)



----- End forwarded message -----

Re: [arnova@xs4all.nl: Bug#293364: Bug in ZSH (zshell) using Debian-testing (Sarge)]

[Peter Stephenson]

Clint Adams wrote:
>  When I exit a zshell (for example when exiting from su) zshell segfaults 
>  when:
>  - /usr/share/zsh/functions/TRAPEXIT exists and is executable
>  - And the function is loaded (for example from /etc/zsh/zshrc with 
>  "autoload ${^/usr/share/zsh/functions}/*(.xN:t)" )
> 
>  I am using Debian GNU/Linux 3.2 (testing/sarge)

I remember not being satisfied with the results of the first round of
fixing autoloaded traps, so I'm not surprises something still needs
fixing.  It was quite low priority, since it's not the natural way to
use a trap, anything to do with traps is complicated, and anything to do
with autoloaded traps doubly so, so I left it.  I'll try and look it
over the next few days.

If we'd never had function-style traps in the first place this wouldn't
have happened...

-- 
Peter Stephenson <pws@csr.com>                  Software Engineer
CSR PLC, Churchill House, Cambridge Business Park, Cowley Road
Cambridge, CB4 0WZ, UK                          Tel: +44 (0)1223 692070


**********************************************************************
This email and any files transmitted with it are confidential and
intended solely for the use of the individual or entity to whom they
are addressed. If you have received this email in error please notify
the system manager.

**********************************************************************

Re: [arnova@xs4all.nl: Bug#293364: Bug in ZSH (zshell) using Debian-testing (Sarge)]

[Peter Stephenson]

Clint Adams wrote:
> ----- Forwarded message from Arno van Amersfoort <arnova@xs4all.nl> -----
> 
> Package: zsh
> Version: 4.2.3-1
> 
>  When I exit a zshell (for example when exiting from su) zshell segfaults 
>  when:
>  - /usr/share/zsh/functions/TRAPEXIT exists and is executable
>  - And the function is loaded (for example from /etc/zsh/zshrc with 
>  "autoload ${^/usr/share/zsh/functions}/*(.xN:t)" )
> 
>  I am using Debian GNU/Linux 3.2 (testing/sarge)
> 
> 
> 
> ----- End forwarded message -----

3 1/2 hours of my life later...

Eprogs need to be set specially for not-yet-autoloaded functions,
apparently.

I have also rewritten function-style traps so that the code to execute
always comes from the function table.  This wasn't the problem here, but
it makes me very much happier.  The duplication in trapfuncs was always
rather hairy in cases like this where the function entry could be
manipulated behind your back.  trapfuncs has been renamed traplists to
reflect the new use.

I discoverd that C03traps exited early, so the last tests weren't called
properly (and two were wrong, including the one that caused the shell to
exit).  Now they are all executed, including the one I've added.

I'm reasonably happy this ought to be safe enough to go onto
zsh-4_2-patches.  (Note weasel words.)

Note also this needs patch -p1 since I made it offline.  However, I'll
commit it as soon as the message appears.

diff -ru zsh.old/Src/builtin.c zsh/Src/builtin.c
--- zsh.old/Src/builtin.c	2005-01-15 19:00:20.000000000 +0000
+++ zsh/Src/builtin.c	2005-02-06 18:49:07.352658776 +0000
@@ -2617,14 +2617,12 @@
 	    shfunctab->addnode(shfunctab, ztrdup(*argv), shf);
 
 	    if (signum != -1) {
-		if (settrap(signum, shf->funcdef)) {
+		if (settrap(signum, NULL, ZSIG_FUNC)) {
 		    shfunctab->removenode(shfunctab, *argv);
 		    shfunctab->freenode((HashNode)shf);
 		    returnval = 1;
 		    ok = 0;
 		}
-		else
-		    sigtrapped[signum] |= ZSIG_FUNC;
 	    }
 
 	    if (ok && OPT_ISSET(ops,'X') &&
@@ -4967,10 +4965,10 @@
 		    shfunctab->printnode(hn, 0);
 		DPUTS(!hn, "BUG: I did not find any trap functions!");
 	    } else if (sigtrapped[sig]) {
-		if (!sigfuncs[sig])
+		if (!siglists[sig])
 		    printf("trap -- '' %s\n", sigs[sig]);
 		else {
-		    s = getpermtext(sigfuncs[sig], NULL);
+		    s = getpermtext(siglists[sig], NULL);
 		    printf("trap -- ");
 		    quotedzputs(s, stdout);
 		    printf(" %s\n", sigs[sig]);
@@ -5013,7 +5011,7 @@
 	    break;
 	}
 	t = dupeprog(prog, 0);
-	if (settrap(sig, t))
+	if (settrap(sig, t, 0))
 	    freeeprog(t);
     }
     return *argv != NULL;
diff -ru zsh.old/Src/exec.c zsh/Src/exec.c
--- zsh.old/Src/exec.c	2005-02-05 17:25:22.000000000 +0000
+++ zsh/Src/exec.c	2005-02-06 18:49:31.972915928 +0000
@@ -2650,8 +2650,8 @@
 		unsettrap(sig);
     if (!(monitor = isset(MONITOR))) {
 	if (how & Z_ASYNC) {
-	    settrap(SIGINT, NULL);
-	    settrap(SIGQUIT, NULL);
+	    settrap(SIGINT, NULL, 0);
+	    settrap(SIGQUIT, NULL, 0);
 	    if (isatty(0)) {
 		close(0);
 		if (open("/dev/null", O_RDWR | O_NOCTTY)) {
@@ -3340,13 +3340,12 @@
 	/* is this shell function a signal trap? */
 	if (!strncmp(s, "TRAP", 4) &&
 	    (signum = getsignum(s + 4)) != -1) {
-	    if (settrap(signum, shf->funcdef)) {
+	    if (settrap(signum, NULL, ZSIG_FUNC)) {
 		freeeprog(shf->funcdef);
 		zfree(shf, sizeof(*shf));
 		state->pc = end;
 		return 1;
 	    }
-	    sigtrapped[signum] |= ZSIG_FUNC;
 
 	    /*
 	     * Remove the old node explicitly in case it has
diff -ru zsh.old/Src/hashtable.c zsh/Src/hashtable.c
--- zsh.old/Src/hashtable.c	2004-06-08 22:24:19.000000000 +0100
+++ zsh/Src/hashtable.c	2005-02-06 18:49:53.961573144 +0000
@@ -819,7 +819,6 @@
     if (!strncmp(hn->nam, "TRAP", 4)) {
 	int signum = getsignum(hn->nam + 4);
 	sigtrapped[signum] &= ~ZSIG_FUNC;
-	sigfuncs[signum] = NULL;
 	unsettrap(signum);
     }
 }
@@ -838,8 +837,7 @@
     if (!strncmp(shf->nam, "TRAP", 4)) {
 	int signum = getsignum(shf->nam + 4);
 	if (signum != -1) {
-	    settrap(signum, shf->funcdef);
-	    sigtrapped[signum] |= ZSIG_FUNC;
+	    settrap(signum, NULL, ZSIG_FUNC);
 	}
     }
 }
diff -ru zsh.old/Src/Modules/parameter.c zsh/Src/Modules/parameter.c
--- zsh.old/Src/Modules/parameter.c	2005-01-22 00:38:28.000000000 +0000
+++ zsh/Src/Modules/parameter.c	2005-02-06 18:54:07.519026552 +0000
@@ -332,13 +332,12 @@
 
     if (!strncmp(name, "TRAP", 4) &&
 	(sn = getsignum(name + 4)) != -1) {
-	if (settrap(sn, shf->funcdef)) {
+	if (settrap(sn, NULL, ZSIG_FUNC)) {
 	    freeeprog(shf->funcdef);
 	    zfree(shf, sizeof(*shf));
 	    zsfree(val);
 	    return;
 	}
-	sigtrapped[sn] |= ZSIG_FUNC;
     }
     shfunctab->addnode(shfunctab, ztrdup(name), shf);
     zsfree(val);
diff -ru zsh.old/Src/Modules/zftp.c zsh/Src/Modules/zftp.c
--- zsh.old/Src/Modules/zftp.c	2004-12-12 20:19:48.000000000 +0000
+++ zsh/Src/Modules/zftp.c	2005-02-06 18:39:58.503096592 +0000
@@ -436,7 +436,8 @@
     } else
 	alarm(0);
     if (sigtrapped[SIGALRM] || interact) {
-	if (sigfuncs[SIGALRM] || !sigtrapped[SIGALRM])
+	if (siglists[SIGALRM] || !sigtrapped[SIGALRM] ||
+	    (sigtrapped[SIGALRM] & ZSIG_FUNC))
 	    install_handler(SIGALRM);
 	else
 	    signal_ignore(SIGALRM);
@@ -452,7 +453,7 @@
 zfunpipe()
 {
     if (sigtrapped[SIGPIPE]) {
-	if (sigfuncs[SIGPIPE])
+	if (siglists[SIGPIPE] || (sigtrapped[SIGPIPE] & ZSIG_FUNC))
 	    install_handler(SIGPIPE);
 	else
 	    signal_ignore(SIGPIPE);
diff -ru zsh.old/Src/parse.c zsh/Src/parse.c
--- zsh.old/Src/parse.c	2004-10-06 20:22:52.000000000 +0100
+++ zsh/Src/parse.c	2005-02-06 20:03:30.949088976 +0000
@@ -2105,6 +2105,16 @@
 	errflag = 1;
 }
 
+/*
+ * Duplicate a programme list, on the heap if heap is 1, else
+ * in permanent storage.
+ *
+ * Be careful in case p is the Eprog for a function which will
+ * later be autoloaded.  The shf element of the returned Eprog
+ * must be set appropriately by the caller.  (Normally we create
+ * the Eprog in this case by using mkautofn.)
+ */
+
 /**/
 mod_export Eprog
 dupeprog(Eprog p, int heap)
diff -ru zsh.old/Src/signals.c zsh/Src/signals.c
--- zsh.old/Src/signals.c	2005-01-15 19:00:22.000000000 +0000
+++ zsh/Src/signals.c	2005-02-06 19:57:31.150786632 +0000
@@ -36,10 +36,19 @@
 /**/
 mod_export int sigtrapped[VSIGCOUNT];
 
-/* trap functions for each signal */
+/*
+ * Trap programme lists for each signal.
+ *
+ * If (sigtrapped[sig] & ZSIG_FUNC) is set, this isn't used.
+ * The corresponding shell function is used instead.
+ *
+ * Otherwise, if sigtrapped[sig] is not zero, this is NULL when a signal
+ * is to be ignored, and if not NULL contains the programme list to be
+ * eval'd.
+ */
 
 /**/
-mod_export Eprog sigfuncs[VSIGCOUNT];
+mod_export Eprog siglists[VSIGCOUNT];
 
 /* Total count of trapped signals */
 
@@ -682,7 +691,7 @@
 
 /*
  * Save the current trap by copying it.  This does nothing to
- * the existing value of sigtrapped or sigfuncs.
+ * the existing value of sigtrapped or siglists.
  */
 
 static void
@@ -704,15 +713,18 @@
 	    newshf->nam = ztrdup(shf->nam);
 	    newshf->flags = shf->flags;
 	    newshf->funcdef = dupeprog(shf->funcdef, 0);
+	    if (shf->flags & PM_UNDEFINED)
+		newshf->funcdef->shf = newshf;
 	}
 #ifdef DEBUG
 	else dputs("BUG: no function present with function trap flag set.");
 #endif
+	DPUTS(siglists[sig], "BUG: function signal has eval list, too.");
 	st->list = newshf;
     } else if (sigtrapped[sig]) {
-	st->list = sigfuncs[sig] ? dupeprog(sigfuncs[sig], 0) : NULL;
+	st->list = siglists[sig] ? dupeprog(siglists[sig], 0) : NULL;
     } else {
-	DPUTS(sigfuncs[sig], "BUG: sigfuncs not null for untrapped signal");
+	DPUTS(siglists[sig], "BUG: siglists not null for untrapped signal");
 	st->list = NULL;
     }
     if (!savetraps)
@@ -723,9 +735,24 @@
     zinsertlinknode(savetraps, (LinkNode)savetraps, st);
 }
 
+
+/*
+ * Set a trap:  note this does not handle manipulation of
+ * the function table for TRAPNAL functions.
+ *
+ * sig is the signal number.
+ *
+ * l is the list to be eval'd for a trap defined with the "trap"
+ * builtin and should be NULL for a function trap.
+ *
+ * flags includes any additional flags to be or'd into sigtrapped[sig],
+ * in particular ZSIG_FUNC; the basic flags will be assigned within
+ * settrap.
+ */
+
 /**/
 mod_export int
-settrap(int sig, Eprog l)
+settrap(int sig, Eprog l, int flags)
 {
     if (sig == -1)
         return 1;
@@ -741,8 +768,10 @@
     queue_signals();
     unsettrap(sig);
 
-    sigfuncs[sig] = l;
-    if (empty_eprog(l)) {
+    DPUTS((flags & ZSIG_FUNC) && l,
+	  "BUG: trap function has passed eval list, too");
+    siglists[sig] = l;
+    if (!(flags & ZSIG_FUNC) && empty_eprog(l)) {
 	sigtrapped[sig] = ZSIG_IGNORED;
         if (sig && sig <= SIGCOUNT &&
 #ifdef SIGWINCH
@@ -765,7 +794,7 @@
      * sigtrapped[sig] is zero or not, i.e. a test without a mask
      * works just the same.
      */
-    sigtrapped[sig] |= (locallevel << ZSIG_SHIFT);
+    sigtrapped[sig] |= (locallevel << ZSIG_SHIFT) | flags;
     unqueue_signals();
     return 0;
 }
@@ -829,7 +858,7 @@
     /*
      * At this point we free the appropriate structs.  If we don't
      * want that to happen then either the function should already have been
-     * removed from shfunctab, or the entry in sigfuncs should have been set
+     * removed from shfunctab, or the entry in siglists should have been set
      * to NULL.  This is no longer necessary for saving traps as that
      * copies the structures, so here we are remove the originals.
      * That causes a little inefficiency, but a good deal more reliability.
@@ -841,15 +870,14 @@
 	 * As in dosavetrap(), don't call removeshfuncnode() because
 	 * that calls back into unsettrap();
 	 */
-	sigfuncs[sig] = NULL;
 	if (node)
 	    removehashnode(shfunctab, node->nam);
 	unqueue_signals();
 
 	return node;
-    } else if (sigfuncs[sig]) {
-	freeeprog(sigfuncs[sig]);
-	sigfuncs[sig] = NULL;
+    } else if (siglists[sig]) {
+	freeeprog(siglists[sig]);
+	siglists[sig] = NULL;
     }
     unqueue_signals();
 
@@ -894,9 +922,9 @@
 	if (exittr & ZSIG_FUNC) {
 	    exitfn = removehashnode(shfunctab, "TRAPEXIT");
 	} else {
-	    exitfn = sigfuncs[SIGEXIT];
+	    exitfn = siglists[SIGEXIT];
+	    siglists[SIGEXIT] = NULL;
 	}
-	sigfuncs[SIGEXIT] = NULL;
 	if (sigtrapped[SIGEXIT] & ZSIG_TRAPPED)
 	    nsigtrapped--;
 	sigtrapped[SIGEXIT] = 0;
@@ -911,11 +939,12 @@
 	    remnode(savetraps, ln);
 
 	    if (st->flags && (st->list != NULL)) {
-		Eprog prog = (st->flags & ZSIG_FUNC) ?
-		    ((Shfunc) st->list)->funcdef : (Eprog) st->list;
 		/* prevent settrap from saving this */
 		dontsavetrap++;
-		settrap(sig, prog);
+		if (st->flags & ZSIG_FUNC)
+		    settrap(sig, NULL, ZSIG_FUNC);
+		else
+		    settrap(sig, (Eprog) st->list, 0);
 		dontsavetrap--;
 		/*
 		 * counting of nsigtrapped should presumably be handled
@@ -946,7 +975,7 @@
 }
 
 /* Execute a trap function for a given signal, possibly
- * with non-standard sigtrapped & sigfuncs values
+ * with non-standard sigtrapped & siglists values
  */
 
 /* Are we already executing a trap? */
@@ -1097,9 +1126,24 @@
 void
 dotrap(int sig)
 {
+    Eprog funcprog;
+
+    if (sigtrapped[sig] & ZSIG_FUNC) {
+	HashNode hn = gettrapnode(sig, 0);
+	if (hn)
+	    funcprog = ((Shfunc)hn)->funcdef;
+	else {
+#ifdef DEBUG
+	    dputs("BUG: running function trap which has escaped.");
+#endif
+	    funcprog = NULL;
+	}
+    } else
+	funcprog = siglists[sig];
+
     /* Copied from dotrapargs(). */
-    if ((sigtrapped[sig] & ZSIG_IGNORED) || !sigfuncs[sig] || errflag)
+    if ((sigtrapped[sig] & ZSIG_IGNORED) || !funcprog || errflag)
 	return;
 
-    dotrapargs(sig, sigtrapped+sig, sigfuncs[sig]);
+    dotrapargs(sig, sigtrapped+sig, funcprog);
 }
diff -ru zsh.old/Test/C03traps.ztst zsh/Test/C03traps.ztst
--- zsh.old/Test/C03traps.ztst	2004-07-27 21:59:33.000000000 +0100
+++ zsh/Test/C03traps.ztst	2005-02-06 20:00:45.012315200 +0000
@@ -183,17 +183,21 @@
   }
   testunset
   f
-1: more sophisticated error trapping
+  print status $?
+  unfunction TRAPZERR
+0: more sophisticated error trapping
 >f
 >ERR-or!
 >f
 >t
+>f
 >t
 >f
 >ERR-or!
 >testunset
 >f
 >ERR-or!
+>status 1
 
   f() {
     setopt localtraps
@@ -216,6 +220,32 @@
   fn
 1: ksh-style EXIT traps preserve return value
 
-  inner() { trap 'return 3' EXIT; return 2: }
+  inner() { trap 'return 3' EXIT; return 2; }
   outer() { inner; return 1; }
+  outer
 3: ksh-style EXIT traps can force return status of enclosing function
+
+# Autoloaded traps are horrid, but unfortunately people expect
+# them to work if we support them.
+  echo "print Running exit trap" >TRAPEXIT
+  $ZTST_testdir/../Src/zsh -fc '
+    fpath=(. $fpath)
+    autoload TRAPEXIT
+    print "Exiting, attempt 1"
+    exit
+    print "What?"
+  '
+  $ZTST_testdir/../Src/zsh -fc '
+    fpath=(. $fpath)
+    autoload TRAPEXIT;
+    fn() { print Some function }
+    fn
+    print "Exiting, attempt 2"
+    exit
+  '
+0: autoloaded TRAPEXIT (exit status > 128 indicates an old bug is back)
+>Exiting, attempt 1
+>Running exit trap
+>Some function
+>Exiting, attempt 2
+>Running exit trap

-- 
Peter Stephenson <pws@pwstephenson.fsnet.co.uk>
Work: pws@csr.com
Web: http://www.pwstephenson.fsnet.co.uk

Re: [arnova@xs4all.nl: Bug#293364: Bug in ZSH (zshell) using Debian-testing (Sarge)]

[Peter Stephenson]

Peter Stephenson wrote:
> Clint Adams wrote:
> > ----- Forwarded message from Arno van Amersfoort <arnova@xs4all.nl> -----
> >  When I exit a zshell (for example when exiting from su) zshell segfaults 
> >  when:
> >  - /usr/share/zsh/functions/TRAPEXIT exists and is executable
> >  - And the function is loaded (for example from /etc/zsh/zshrc with 
> >  "autoload ${^/usr/share/zsh/functions}/*(.xN:t)" )
> 
> I'm reasonably happy this ought to be safe enough to go onto
> zsh-4_2-patches.  (Note weasel words.)

I've copied across the key fix and the tests, which now pass.  This
certainly ought to be an improvement.

-- 
Peter Stephenson <pws@csr.com>                  Software Engineer
CSR PLC, Churchill House, Cambridge Business Park, Cowley Road
Cambridge, CB4 0WZ, UK                          Tel: +44 (0)1223 692070


**********************************************************************
This email and any files transmitted with it are confidential and
intended solely for the use of the individual or entity to whom they
are addressed. If you have received this email in error please notify
the system manager.

**********************************************************************