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

["Rocky Bernstein" <rocky.bernstein@xxxxxxxxx>]

In general I think this will help. Not just because it helps the
debugger. Later (* * * * below) I'll elaborate on this.

But in short, either this patch doesn't solve this particular problem
or I hand-applied the patch incorrectly.

(A cut and paste to a file caused many rejections by "patch", so I did
the rest by hand. It's possible I missed something. Running the
regression test caused a failures in E01options.ztst with an eval and
$LINENO, and another in C05debug.ztst with debug-trap-bug, but I think
this is to be expected. )

When I run zsh with those patches on this program:
(
    x=$(print $LINENO); print $x
)

I still get 1. Is that what one gets with the patch applied?

Note that there are other problems encountered that not explained by
that patch. In particular, it doesn't explain the position zshdb2.sh:6
in the output that went:

./command/eval.sh:4 ./command/eval.sh:22 ./lib/processor.sh:67
./lib/processor.sh:16 ./zshdb2.sh:6 ./testing.sh:1 ./zshdb2.sh:34

The correct position there is ./zshdb2.sh:9 which does appear earlier.
Also it doesn't explain the missing funcfiletrace output which could
be explained if somehow if zsh didn't think there were any function
calls.

Finally there is a history bug, but for that's something probably
totally different and likely just a bug in the debugger code.

* * * *
Let me see if I understand the situation correctly: there is an
internal routine called parse_string() which can be called though eval
as well as backtick. For eval, one can argue its the right thing, but
for backtick seeing 1 as the value of $LINENO might be a bit odd.

zsh is not the only programming language where this issue comes up.
I've seen it in Python and Ruby. In both, when eval'ing a string, the
line number is reset to 1 and the filename is something bogus. In the
Python debugger, pydb, what I do is search for such eval functions in
the traceback and use the line number of the thing that called that
instead. I can't do that here because "eval" or "backtick" or
"parse_string" doesn't appear as a function call in funcstack or any
of the other arrays, It might be cool to put somewhere in funcstack or
funcfiletrace the ZSH_SUBSHELL value.

Ruby is a little nicer here (and that generally seems to be the case).
By default, the line number is reset, but if you want you can supply a
line number and file name as the optional final parameters.

However Ola Blini has observed that really the surrounding context
should be the default; he calls the workaround an "anti-pattern." See:
http://ola-bini.blogspot.com/search/label/eval

On Sun, Sep 28, 2008 at 5:16 PM, Peter Stephenson
<p.w.stephenson@xxxxxxxxxxxx> wrote:
> On Sun, 28 Sep 2008 15:19:37 -0400
> "Rocky Bernstein" <rocky.bernstein@xxxxxxxxx> wrote:
>> There is what looks to me a bug in the recent funcfiletrace that I've
>> been trying to isolate.
>>
>> #!/usr/local/bin/zsh
>> # Test debugger handling of subshells
>> (
>>     x=$(print 5; print 6)
>> )
>> (/tmp/zshdb/testing.sh:1):
>> print 5                            # In a 2nd subshell, backtick
>> zshdb<((7))> where                 # ((..)) indicates this.
>>
>> What's wrong is that we aren't on line 1 of testing.sh.
>
> exec.c:parse_string() resets the line number for every invocation.
> That's fine for eval, where we've fixed this up explicitly, but doesn't
> work in the call from exec.c:getoutput() which is the case here.
> Obviously that's not consistent with the fact that it doesn't have its
> own file; we don't do the special eval handling of reconstructing the
> file location in this case.
>
> The reason (...) is different is it's parsed straight away when the
> shell reads it in; it's not stored as a string argument to be looked at
> again later.
>
> It's possible we can simply get away with not resetting the line number
> in any case without its own entry on the function stack.  That's about
> the simplest fix, but changes the meaning of line numbers in all string
> evaluations apart from eval, trap and functions, where we have the
> special handling.  I'm coming round to the view that this is nonetheless
> the best way of salvaging consistency of the debugging environment.
> Have a go at this and see if it looks sensible.
>
> Index: Src/builtin.c
> ===================================================================
> RCS file: /cvsroot/zsh/zsh/Src/builtin.c,v
> retrieving revision 1.209
> diff -u -r1.209 builtin.c
> --- Src/builtin.c       27 Sep 2008 19:57:33 -0000      1.209
> +++ Src/builtin.c       28 Sep 2008 21:11:21 -0000
> @@ -4781,7 +4781,7 @@
>     } else
>        fpushed = 0;
>
> -    prog = parse_string(zjoin(argv, ' ', 1));
> +    prog = parse_string(zjoin(argv, ' ', 1), 1);
>     if (prog) {
>        if (wc_code(*prog->prog) != WC_LIST) {
>            /* No code to execute */
> @@ -5781,7 +5781,7 @@
>     arg = *argv++;
>     if (!*arg)
>        prog = &dummy_eprog;
> -    else if (!(prog = parse_string(arg))) {
> +    else if (!(prog = parse_string(arg, 1))) {
>        zwarnnam(name, "couldn't parse trap command");
>        return 1;
>     }
> Index: Src/exec.c
> ===================================================================
> RCS file: /cvsroot/zsh/zsh/Src/exec.c,v
> retrieving revision 1.156
> diff -u -r1.156 exec.c
> --- Src/exec.c  26 Sep 2008 09:11:29 -0000      1.156
> +++ Src/exec.c  28 Sep 2008 21:11:22 -0000
> @@ -188,17 +188,21 @@
>
>  /**/
>  mod_export Eprog
> -parse_string(char *s)
> +parse_string(char *s, int reset_lineno)
>  {
>     Eprog p;
> -    zlong oldlineno = lineno;
> +    zlong oldlineno;
>
>     lexsave();
>     inpush(s, INP_LINENO, NULL);
>     strinbeg(0);
> -    lineno = 1;
> +    if (reset_lineno) {
> +       oldlineno = lineno;
> +       lineno = 1;
> +    }
>     p = parse_list();
> -    lineno = oldlineno;
> +    if (reset_lineno)
> +       lineno = oldlineno;
>     if (tok == LEXERR && !lastval)
>        lastval = 1;
>     strinend();
> @@ -954,7 +958,7 @@
>     Eprog prog;
>
>     pushheap();
> -    if ((prog = parse_string(s)))
> +    if ((prog = parse_string(s, 0)))
>        execode(prog, dont_change_job, exiting);
>     popheap();
>  }
> @@ -3445,7 +3449,7 @@
>     pid_t pid;
>     char *s;
>
> -    if (!(prog = parse_string(cmd)))
> +    if (!(prog = parse_string(cmd, 0)))
>        return NULL;
>
>     if ((s = simple_redir_name(prog, REDIR_READ))) {
> @@ -3566,7 +3570,7 @@
>        return NULL;
>     }
>     *str = '\0';
> -    if (str[1] || !(prog = parse_string(cmd + 2))) {
> +    if (str[1] || !(prog = parse_string(cmd + 2, 0))) {
>        zerr("parse error in process substitution");
>        return NULL;
>     }
> @@ -4453,7 +4457,7 @@
>                    d = metafy(d, rlen, META_REALLOC);
>
>                    scriptname = dupstring(s);
> -                   r = parse_string(d);
> +                   r = parse_string(d, 1);
>                    scriptname = oldscriptname;
>
>                    if (fname)
> Index: Src/glob.c
> ===================================================================
> RCS file: /cvsroot/zsh/zsh/Src/glob.c,v
> retrieving revision 1.65
> diff -u -r1.65 glob.c
> --- Src/glob.c  11 May 2008 19:55:21 -0000      1.65
> +++ Src/glob.c  28 Sep 2008 21:11:23 -0000
> @@ -3329,7 +3329,7 @@
>  {
>     Eprog prog;
>
> -    if ((prog = parse_string(str))) {
> +    if ((prog = parse_string(str, 0))) {
>        int ef = errflag, lv = lastval, ret;
>
>        unsetparam("reply");
> Index: Src/parse.c
> ===================================================================
> RCS file: /cvsroot/zsh/zsh/Src/parse.c,v
> retrieving revision 1.75
> diff -u -r1.75 parse.c
> --- Src/parse.c 24 Sep 2008 19:19:56 -0000      1.75
> +++ Src/parse.c 28 Sep 2008 21:11:24 -0000
> @@ -2831,7 +2831,7 @@
>        close(fd);
>        file = metafy(file, flen, META_REALLOC);
>
> -       if (!(prog = parse_string(file)) || errflag) {
> +       if (!(prog = parse_string(file, 1)) || errflag) {
>            errflag = 0;
>            close(dfd);
>            zfree(file, flen);
> Index: Src/Modules/parameter.c
> ===================================================================
> RCS file: /cvsroot/zsh/zsh/Src/Modules/parameter.c,v
> retrieving revision 1.49
> diff -u -r1.49 parameter.c
> --- Src/Modules/parameter.c     11 Sep 2008 17:14:39 -0000      1.49
> +++ Src/Modules/parameter.c     28 Sep 2008 21:11:24 -0000
> @@ -279,7 +279,7 @@
>
>     val = metafy(val, strlen(val), META_REALLOC);
>
> -    prog = parse_string(val);
> +    prog = parse_string(val, 1);
>
>     if (!prog || prog == &dummy_eprog) {
>        zwarn("invalid function definition", value);
> Index: Src/Modules/zpty.c
> ===================================================================
> RCS file: /cvsroot/zsh/zsh/Src/Modules/zpty.c,v
> retrieving revision 1.38
> diff -u -r1.38 zpty.c
> --- Src/Modules/zpty.c  15 May 2008 15:51:01 -0000      1.38
> +++ Src/Modules/zpty.c  28 Sep 2008 21:11:24 -0000
> @@ -299,7 +299,7 @@
>     if (!ineval)
>        scriptname = "(zpty)";
>
> -    prog = parse_string(zjoin(args, ' ', 1));
> +    prog = parse_string(zjoin(args, ' ', 1), 0);
>     if (!prog) {
>        errflag = 0;
>        scriptname = oscriptname;
> Index: Src/Modules/zutil.c
> ===================================================================
> RCS file: /cvsroot/zsh/zsh/Src/Modules/zutil.c,v
> retrieving revision 1.23
> diff -u -r1.23 zutil.c
> --- Src/Modules/zutil.c 6 Jul 2007 21:52:40 -0000       1.23
> +++ Src/Modules/zutil.c 28 Sep 2008 21:11:25 -0000
> @@ -251,7 +251,7 @@
>     if (eval) {
>        int ef = errflag;
>
> -       eprog = parse_string(zjoin(vals, ' ', 1));
> +       eprog = parse_string(zjoin(vals, ' ', 1), 0);
>        errflag = ef;
>
>        if (!eprog)
>
>
> --
> Peter Stephenson <p.w.stephenson@xxxxxxxxxxxx>
> Web page now at http://homepage.ntlworld.com/p.w.stephenson/
>