Util/helpfiles - problem with provided example

Util/helpfiles - problem with provided example

[Michael Prokop]

Hello,

I just found a problem when using the example function provided in
zsh/Util/helpfiles. I'm refering to current zsh-cvs-sources.

An example:

$ which man    # this is the function 'run-help' which I'm using in my setup
man () {
        typeset zhelp=~/.zsh/zsh_help # or wherever
        [[ $1 = . ]] && 1=dot
        [[ $1 = : ]] && 1=colon
        if [[ $1 = compctl ]]
        then
                man zshcompctl
        elif [[ -f $zhelp/$1 ]]
        then
                ${=PAGER:-more} $zhelp/$1
        else
                command man $*                    # notice this line
        fi
}
$ man zsh             # now I'm trying to read the manual - works ->
Reformatting zsh(1), please wait...
$ run-help() {        # now I do copy/paste from Util/helpfiles ->
    typeset zhelp=~/.zsh/zsh_help          # or wherever
    [[ $1 = . ]] && 1=dot
    [[ $1 = : ]] && 1=colon
    if [[ $1 = compctl ]]; then
       man zshcompctl
    elif [[ -f $zhelp/$1 ]]; then
       ${=PAGER:-more} $zhelp/$1
    else
       man $1                                     # this is the original
    fi
  }
$ man zsh
Segmentation fault
$

The important difference in the two functions:
'man $1' vs. 'command man $*'.

The '$*' is important for covering cmdlines like 'man 3 printf'.
AFAICS the segfault happens because of the recursive 'man'-function.

Maybe this could be changed in Util/helpfiles because it might be a
trap ;-).

regards,
(-: Michael
-- 
www.michael-prokop.at
~
~
".signature" [New] 1L, 22C [w]

Re: Util/helpfiles - problem with provided example

[Peter Stephenson]

Edited down to bare essentials:

Michael Prokop wrote:
> man () {
>        man $1                                     # this is the original
>   }
> $ man zsh

> Segmentation fault

This isn't very nice.  Should we turn on --enable-max-function-depth by
default?  The default for --enable-max-function-depth=yes is 4096 which
isn't going to hurt most people.  Or should we make it configurable
internally with a sensible default?

-- 
Peter Stephenson <pws@csr.com>                  Software Engineer
CSR Ltd., Science Park, Milton Road,
Cambridge, CB4 0WH, 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.

This footnote also confirms that this email message has been swept by
MIMEsweeper for the presence of computer viruses.

www.mimesweeper.com
**********************************************************************

Re: Util/helpfiles - problem with provided example

[Peter Stephenson]

Peter Stephenson wrote:
> Michael Prokop wrote:
> > man () {
> >        man $1                                     # this is the original
> >   }
> > $ man zsh
> 
> > Segmentation fault
> 
> This isn't very nice.  Should we turn on --enable-max-function-depth by
> default?  The default for --enable-max-function-depth=yes is 4096 which
> isn't going to hurt most people.  Or should we make it configurable
> internally with a sensible default?

More investigation reveals a further problem.  There's an arbitrary
debugging check when unreferencing the function that it hasn't been
referenced more than 256 times.  This needs to be the function depth if
that is defined plus 1 (to allow for the original function struture).
I've added a few on for fuzz.  Presumably the test is meaningless if
there is no function depth limit.

If you try this with a the default function depth limit, it's
interesting to note that the error message is printed almost straight
away, while the prompt takes a while to return.  I wonder if freeing
things up is slower than necessary?

Index: Src/parse.c
===================================================================
RCS file: /cvsroot/zsh/zsh/Src/parse.c,v
retrieving revision 1.46
diff -u -r1.46 parse.c
--- Src/parse.c	28 Jun 2004 15:38:14 -0000	1.46
+++ Src/parse.c	28 Jul 2004 10:18:28 -0000
@@ -2168,7 +2168,10 @@
 	/* paranoia */
 	DPUTS(p->nref > 0 && (p->flags & EF_HEAP), "Heap EPROG has nref > 0");
 	DPUTS(p->nref < 0 && !(p->flags & EF_HEAP), "Real EPROG has nref < 0");
-	DPUTS(p->nref < -1 || p->nref > 256, "Uninitialised EPROG nref");
+	DPUTS(p->nref < -1, "Uninitialised EPROG nref");
+#ifdef MAX_FUNCTION_DEPTH
+	DPUTS(p->nref > MAX_FUNCTION_DEPTH + 10, "Overlarge EPROG nref");
+#endif
 	if (p->nref > 0 && !--p->nref) {
 	    for (i = p->npats, pp = p->pats; i--; pp++)
 		freepatprog(*pp);

-- 
Peter Stephenson <pws@csr.com>                  Software Engineer
CSR Ltd., Science Park, Milton Road,
Cambridge, CB4 0WH, 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.

This footnote also confirms that this email message has been swept by
MIMEsweeper for the presence of computer viruses.

www.mimesweeper.com
**********************************************************************

Re: Util/helpfiles - problem with provided example

[Peter Stephenson]

Peter Stephenson wrote:
> Edited down to bare essentials:
> 
> Michael Prokop wrote:
> > man () {
> >        man $1                                     # this is the original
> >   }
> > $ man zsh
> 
> > Segmentation fault
> 
> This isn't very nice.  Should we turn on --enable-max-function-depth by
> default?  The default for --enable-max-function-depth=yes is 4096 which
> isn't going to hurt most people.  Or should we make it configurable
> internally with a sensible default?

Here's a patch which turns it on by default.

Index: configure.ac
===================================================================
RCS file: /cvsroot/zsh/zsh/configure.ac,v
retrieving revision 1.18
diff -u -r1.18 configure.ac
--- configure.ac	8 Jun 2004 13:34:12 -0000	1.18
+++ configure.ac	29 Jul 2004 15:39:00 -0000
@@ -301,12 +301,14 @@
 AH_TEMPLATE([MAX_FUNCTION_DEPTH],
 [Define for function depth limits])
 AC_ARG_ENABLE(max-function-depth,
-[  --enable-max-function-depth=MAX   limit function depth to MAX],
+[  --enable-max-function-depth=MAX   limit function depth to MAX, default 4096],
 [if test x$enableval = xyes; then
   AC_DEFINE(MAX_FUNCTION_DEPTH, 4096)
-else
+elif test x$enableval != xno; then
   AC_DEFINE_UNQUOTED(MAX_FUNCTION_DEPTH, $enableval)
-fi])
+fi],
+[AC_DEFINE(MAX_FUNCTION_DEPTH, 4096)]
+)
 
 dnl Do you want to look for pcre support?
 AC_ARG_ENABLE(pcre,
Index: INSTALL
===================================================================
RCS file: /cvsroot/zsh/zsh/INSTALL,v
retrieving revision 1.17
diff -u -r1.17 INSTALL
--- INSTALL	2 Jul 2004 15:59:11 -0000	1.17
+++ INSTALL	29 Jul 2004 15:39:01 -0000
@@ -330,6 +330,19 @@
 `make uninstall' or `make uninstall.fns', although the version-specific
 directory and its contents will be deleted.
 
+Function depth
+--------------
+
+Shell functions may be called recursively.  In order to detect infinite
+recursion the shell has a limit on the depth to which functions may be
+called:  note that this is a single limit for all functions, not a limit
+for each function called recursively.  The default for the limit is 4096.
+The limit may be altered to the value MAX by passing the option
+--enable-max-function-depth=MAX to configure.  Alternatively, the limit may
+be disabled with --disable-max-function-depth.  However, this is not
+recommended as it is likely to cause the shell to crash on an infinite
+recursion.
+
 Support for large files and integers
 ------------------------------------
 
-- 
Peter Stephenson <pws@csr.com>                  Software Engineer
CSR Ltd., Science Park, Milton Road,
Cambridge, CB4 0WH, 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.

This footnote also confirms that this email message has been swept by
MIMEsweeper for the presence of computer viruses.

www.mimesweeper.com
**********************************************************************