Glitch with "eval" and "autoload"

Glitch with "eval" and "autoload"

[Bart Schaefer]

torch% oops() { eval autoload -X }
torch% oops 
(eval):1: parse error near `"$@"'
torch% functions
'(eval)' () {
        # undefined
        builtin autoload -X
}
oops () {
        eval autoload -X
}


This happens because bin_typeset() gets the name to be autoloaded from
the "scriptname" global, which at the time autoload was invented was not
modified during intervening "eval" and other internal scopes.

The ancient behavior is restored if the "eval_line_no" option is unset.

What's the right way to get the actual function name?  Climbing up the
fstack looking for strcmp(fsptr->name, "(eval)") != 0 seems pretty ugly,
but the "ineval" global isn't sufficient for e.g. "eval eval autoload -X".

Re: Glitch with "eval" and "autoload"

[Peter Stephenson]

On Sat, 23 Jan 2016 10:03:11 -0800
Bart Schaefer <schaefer@brasslantern.com> wrote:
> What's the right way to get the actual function name?  Climbing up the
> fstack looking for strcmp(fsptr->name, "(eval)") != 0 seems pretty ugly,
> but the "ineval" global isn't sufficient for e.g. "eval eval autoload -X".

You shouldn't need to check the name, rather look for (fstpr->tp ==
FS_FUNC).  That ought to be fairly robust.

pws

Re: Glitch with "eval" and "autoload"

[Peter Stephenson]

On Sun, 24 Jan 2016 15:22:35 +0000
Peter Stephenson <p.w.stephenson@ntlworld.com> wrote:
> You shouldn't need to check the name, rather look for (fstpr->tp ==
> FS_FUNC).  That ought to be fairly robust.

This is a little bit paranoid; the dupstring probably isn't needed
because we already take evasive action for cases like this, and I
don't know of a case where funcname will be NULL.

pws

diff --git a/Src/builtin.c b/Src/builtin.c
index 98ecb09..63f964d 100644
--- a/Src/builtin.c
+++ b/Src/builtin.c
@@ -3142,15 +3142,33 @@ bin_functions(char *name, char **argv, Options ops, int func)
 
 	queue_signals();
 	if (OPT_MINUS(ops,'X')) {
-	    if ((shf = (Shfunc) shfunctab->getnode(shfunctab, scriptname))) {
-		DPUTS(!shf->funcdef,
-		      "BUG: Calling autoload from empty function");
+	    Funcstack fs;
+	    char *funcname = NULL;
+	    for (fs = funcstack; fs; fs = fs->prev) {
+		if (fs->tp == FS_FUNC) {
+		    /*
+		     * dupstring here is paranoia but unlikely to be
+		     * problematic
+		     */
+		    funcname = dupstring(fs->name);
+		    break;
+		}
+	    }
+	    if (!funcname)
+	    {
+		zerrnam(name, "bad autoload");
+		ret = 1;
 	    } else {
-		shf = (Shfunc) zshcalloc(sizeof *shf);
-		shfunctab->addnode(shfunctab, ztrdup(scriptname), shf);
+		if ((shf = (Shfunc) shfunctab->getnode(shfunctab, funcname))) {
+		    DPUTS(!shf->funcdef,
+			  "BUG: Calling autoload from empty function");
+		} else {
+		    shf = (Shfunc) zshcalloc(sizeof *shf);
+		    shfunctab->addnode(shfunctab, ztrdup(funcname), shf);
+		}
+		shf->node.flags = on;
+		ret = eval_autoload(shf, funcname, ops, func);
 	    }
-	    shf->node.flags = on;
-	    ret = eval_autoload(shf, scriptname, ops, func);
 	} else {
 	    if (OPT_ISSET(ops,'U') && !OPT_ISSET(ops,'u'))
 		on &= ~PM_UNDEFINED;
diff --git a/Test/C04funcdef.ztst b/Test/C04funcdef.ztst
index 0951e2c..496577f 100644
--- a/Test/C04funcdef.ztst
+++ b/Test/C04funcdef.ztst
@@ -308,6 +308,19 @@
 ?(eval):6: command not found: firstfn1
 ?(eval):7: command not found: secondfn1
 
+  (
+    fpath=(.)
+    print  "print oops was successfully autoloaded" >oops
+    oops() { eval autoload -X }
+    oops
+    which -x2 oops
+  )
+0:autoload containing eval
+>oops was successfully autoloaded
+>oops () {
+>  print oops was successfully autoloaded
+>}
+
 %clean
 
  rm -f file.in file.out

Re: Glitch with "eval" and "autoload"

[Bart Schaefer]

On Jan 24,  5:17pm, Peter Stephenson wrote:
}
} > You shouldn't need to check the name, rather look for (fstpr->tp ==
} > FS_FUNC).  That ought to be fairly robust.

The patch in 37765 looks fine to me, do you plan to push it?

Re: Glitch with "eval" and "autoload"

[Peter Stephenson]

On Mon, 25 Jan 2016 08:14:35 -0800
Bart Schaefer <schaefer@brasslantern.com> wrote:
> On Jan 24,  5:17pm, Peter Stephenson wrote:
> }
> } > You shouldn't need to check the name, rather look for (fstpr->tp ==
> } > FS_FUNC).  That ought to be fairly robust.
> 
> The patch in 37765 looks fine to me, do you plan to push it?

Yes, I'll do it here otherwise it'll be tomorrow evening...

pws