Re: Help me track down a tough bug? (probably funcfiletrace, subshells and possibly I/O redirection)

[Peter Stephenson <p.w.stephenson@xxxxxxxxxxxx>]

On Mon, 29 Sep 2008 15:25:28 +0100
Peter Stephenson <pws@xxxxxxx> wrote:
> "Rocky Bernstein" wrote:
> > ./zshdb2.sh
> > ./zshdb2.sh:39 ./zshdb2.sh:34   # Debug output in lib/frame.sh
> > 
> > # Above should be: ./lib/hook.sh:5 ./zshdb2.sh:34
> > # note: 34+5=39
> 
> Ah, I see, so this should be fairly obvious once I track back and see
> what the code referred to is... I'll try and get round to this this
> evening.

Wasn't so easy, since there was quite a lot of irrelevant stuff to
strip.  It turns out the problem is handling of functions inside traps.
These inherit the behaviour of the parent (eval-style, i.e. not
TRAP...() { ... } ) trap that the line number isn't updated, it's kept
as that in the line that caused the trap.  This causes oddities like
this, so I think it wants turning off so that the functions called from
the trap act normally.  This seems to accord with the principle of least
surprise, too.  That seems to fix the problem, but I'm sure I'll be
hearing if it doesn't.  I suspect sources inside eval-style traps behave
similarly and ought to be fixed, which could be done with sourcelevel,
but sufficient unto the day etc.  (The tests could also be done by
pushing a special FS_TRAP type onto funcstack, but that has obvious
visible consequences.  funcstack is a relative latecomer and could have
been used for also sorts of things we now do other ways.)

Responses avoiding "it should have been done completely differently from
the start", "it would be nice to have an option to do it completely
differently" and "other languages do this much better" preferred.

Index: Src/exec.c
===================================================================
RCS file: /cvsroot/zsh/zsh/Src/exec.c,v
retrieving revision 1.157
diff -u -r1.157 exec.c
--- Src/exec.c	29 Sep 2008 08:46:32 -0000	1.157
+++ Src/exec.c	29 Sep 2008 21:33:16 -0000
@@ -992,7 +992,7 @@
 	return (lastval = 1);
 
     /* In evaluated traps, don't modify the line number. */
-    if ((!intrap || trapisfunc) && !ineval && code)
+    if (!IN_EVAL_TRAP() && !ineval && code)
 	lineno = code - 1;
 
     code = wc_code(*state->pc++);
@@ -1053,7 +1053,7 @@
 	ltype = WC_LIST_TYPE(code);
 	csp = cmdsp;
 
-	if ((!intrap || trapisfunc) && !ineval) {
+	if (!IN_EVAL_TRAP() && !ineval) {
 	    /*
 	     * Ensure we have a valid line number for debugging,
 	     * unless we are in an evaluated trap in which case
@@ -1543,7 +1543,7 @@
 	return;
 
     /* In evaluated traps, don't modify the line number. */
-    if ((!intrap || trapisfunc) && !ineval && WC_PIPE_LINENO(pcode))
+    if (!IN_EVAL_TRAP() && !ineval && WC_PIPE_LINENO(pcode))
 	lineno = WC_PIPE_LINENO(pcode) - 1;
 
     if (pline_level == 1) {
@@ -4628,6 +4628,7 @@
     es->trap_return = trap_return;
     es->trap_state = trap_state;
     es->trapisfunc = trapisfunc;
+    es->traplocallevel = traplocallevel;
     es->noerrs = noerrs;
     es->subsh_close = subsh_close;
     es->underscore = ztrdup(underscore);
@@ -4657,6 +4658,7 @@
     trap_return = exstack->trap_return;
     trap_state = exstack->trap_state;
     trapisfunc = exstack->trapisfunc;
+    traplocallevel = exstack->traplocallevel;
     noerrs = exstack->noerrs;
     subsh_close = exstack->subsh_close;
     setunderscore(exstack->underscore);
Index: Src/prompt.c
===================================================================
RCS file: /cvsroot/zsh/zsh/Src/prompt.c,v
retrieving revision 1.55
diff -u -r1.55 prompt.c
--- Src/prompt.c	25 Sep 2008 11:26:03 -0000	1.55
+++ Src/prompt.c	29 Sep 2008 21:33:18 -0000
@@ -727,7 +727,7 @@
 		break;
 	    case 'I':
 		if (funcstack && funcstack->tp != FS_SOURCE &&
-		    (!intrap || trapisfunc)) {
+		    !IN_EVAL_TRAP()) {
 		    /*
 		     * We're in a function or an eval with
 		     * EVALLINENO.  Calculate the line number in
@@ -751,7 +751,7 @@
 		break;
 	    case 'x':
 		if (funcstack && funcstack->tp != FS_SOURCE &&
-		    (!intrap || trapisfunc))
+		    !IN_EVAL_TRAP())
 		    promptpath(funcstack->filename ? funcstack->filename : "",
 			       arg, 0);
 		else
Index: Src/signals.c
===================================================================
RCS file: /cvsroot/zsh/zsh/Src/signals.c,v
retrieving revision 1.52
diff -u -r1.52 signals.c
--- Src/signals.c	26 Sep 2008 09:11:30 -0000	1.52
+++ Src/signals.c	29 Sep 2008 21:33:19 -0000
@@ -1077,6 +1077,13 @@
 int trapisfunc;
 
 /*
+ * If the current trap is not a function, at what function depth
+ * did the trap get called?
+ */
+/**/
+int traplocallevel;
+
+/*
  * sig is the signal number.
  * *sigtr is the value to be taken as the field in sigtrapped (since
  *   that may have changed by this point if we are exiting).
@@ -1132,6 +1139,7 @@
     /* execsave will save the old trap_return and trap_state */
     execsave();
     breaks = retflag = 0;
+    traplocallevel = locallevel;
     runhookdef(BEFORETRAPHOOK, NULL);
     if (*sigtr & ZSIG_FUNC) {
 	int osc = sfcontext;
Index: Src/zsh.h
===================================================================
RCS file: /cvsroot/zsh/zsh/Src/zsh.h,v
retrieving revision 1.147
diff -u -r1.147 zsh.h
--- Src/zsh.h	29 Sep 2008 15:18:55 -0000	1.147
+++ Src/zsh.h	29 Sep 2008 21:33:20 -0000
@@ -924,6 +924,7 @@
     int trap_return;
     int trap_state;
     int trapisfunc;
+    int traplocallevel;
     int noerrs;
     int subsh_close;
     char *underscore;
@@ -2261,6 +2262,9 @@
     TRAP_STATE_FORCE_RETURN
 };
 
+#define IN_EVAL_TRAP() \
+    (intrap && !trapisfunc && traplocallevel == locallevel)
+
 /***********/
 /* Sorting */
 /***********/


-- 
Peter Stephenson <p.w.stephenson@xxxxxxxxxxxx>
Web page now at http://homepage.ntlworld.com/p.w.stephenson/