crashing bug here with LANG=C printf "%1.1f\n" 23.2

crashing bug here with LANG=C printf "%1.1f\n" 23.2

[Matthias Kopfermann]

Hi all,

for ((i=1;i<100;i+=.01)) ; { LANG=C printf "%1.1f\n" $i ; }
gives me a segfault instantly here.
the same can happen when just doing LANG='C' printf "%1.1f\n" 23.2

with message: BUG: attempt to free more than allocated.

when invoking gdb zsh core and doing bt, i get:


GNU gdb 6.1-debian
Copyright 2004 Free Software Foundation, Inc.
GDB is free software, covered by the GNU General Public License, and you are
welcome to change it and/or distribute copies of it under certain conditions.
Type "show copying" to see the conditions.
There is absolutely no warranty for GDB.  Type "show warranty" for details.
This GDB was configured as "i386-linux"...Using host libthread_db library "/lib/libthread_db.so.1".

Core was generated by `zsh'.
Program terminated with signal 11, Segmentation fault.
Reading symbols from /usr/lib/libpcre.so.3...done.
Loaded symbols for /usr/lib/libpcre.so.3
Reading symbols from /lib/libncurses.so.5...done.
Loaded symbols for /lib/libncurses.so.5
Reading symbols from /lib/libc.so.6...done.
Loaded symbols for /lib/libc.so.6
Reading symbols from /lib/ld-linux.so.2...done.
Loaded symbols for /lib/ld-linux.so.2
Reading symbols from /lib/libnss_compat.so.2...done.
Loaded symbols for /lib/libnss_compat.so.2
Reading symbols from /lib/libnsl.so.1...done.
Loaded symbols for /lib/libnsl.so.1
Reading symbols from /lib/libnss_nis.so.2...done.
Loaded symbols for /lib/libnss_nis.so.2
Reading symbols from /lib/libnss_files.so.2...done.
Loaded symbols for /lib/libnss_files.so.2
Reading symbols from /usr/local/lib/zsh/4.2.0-dev-1/zsh/parameter.so...done.
Loaded symbols for /usr/local/lib/zsh/4.2.0-dev-1/zsh/parameter.so
Reading symbols from /usr/local/lib/zsh/4.2.0-dev-1/zsh/zle.so...done.
Loaded symbols for /usr/local/lib/zsh/4.2.0-dev-1/zsh/zle.so
Reading symbols from /usr/local/lib/zsh/4.2.0-dev-1/zsh/complete.so...done.
Loaded symbols for /usr/local/lib/zsh/4.2.0-dev-1/zsh/complete.so
Reading symbols from /usr/local/lib/zsh/4.2.0-dev-1/zsh/zutil.so...done.
Loaded symbols for /usr/local/lib/zsh/4.2.0-dev-1/zsh/zutil.so
Reading symbols from /usr/local/lib/zsh/4.2.0-dev-1/zsh/complist.so...done.
Loaded symbols for /usr/local/lib/zsh/4.2.0-dev-1/zsh/complist.so
Reading symbols from /usr/local/lib/zsh/4.2.0-dev-1/zsh/pcre.so...done.
Loaded symbols for /usr/local/lib/zsh/4.2.0-dev-1/zsh/pcre.so
Reading symbols from /usr/local/lib/zsh/4.2.0-dev-1/zsh/rlimits.so...done.
Loaded symbols for /usr/local/lib/zsh/4.2.0-dev-1/zsh/rlimits.so
#0  0x400f33ab in memset () from /lib/libc.so.6
(gdb) bt
#0  0x400f33ab in memset () from /lib/libc.so.6
#1  0x0808eb30 in zfree (p=0x845cfc0, sz=2056) at mem.c:1179
#2  0x0808ee4c in zsfree (p=0x845cfc0 'ÿ' <repeats 200 times>...) at mem.c:1251
#3  0x0809cd7f in delenv (x=0x845cfc0 'ÿ' <repeats 200 times>...)
    at params.c:3467
#4  0x0806957d in save_params (state=0xbffff6e0, pc=0x40018b30, 
    restore_p=0xbffff108, remove_p=0xbffff10c) at exec.c:2524
#5  0x08068f65 in execcmd (state=0xbffff6e0, input=0, output=0, how=18, 
    last1=2) at exec.c:2369
#6  0x08065b96 in execpline2 (state=0xbffff6e0, pcode=131, how=18, input=0, 
    output=0, last1=0) at exec.c:1274
#7  0x080650ea in execpline (state=0xbffff6e0, slcode=8194, how=18, last1=0)
    at exec.c:1064
#8  0x08064a5d in execlist (state=0xbffff6e0, dont_change_job=1, exiting=0)
    at exec.c:870
#9  0x08087d2f in execfor (state=0xbffff6e0, do_exec=0) at loop.c:159
#10 0x08068eb0 in execcmd (state=0xbffff6e0, input=0, output=0, how=18, 
    last1=2) at exec.c:2359
#11 0x08065b96 in execpline2 (state=0xbffff6e0, pcode=131, how=18, input=0, 
    output=0, last1=0) at exec.c:1274
#12 0x080650ea in execpline (state=0xbffff6e0, slcode=15362, how=18, last1=0)
    at exec.c:1064
#13 0x08064a5d in execlist (state=0xbffff6e0, dont_change_job=0, exiting=0)
    at exec.c:870
#14 0x08064787 in execode (p=0x40018ae0, dont_change_job=0, exiting=0)
    at exec.c:771
#15 0x0807ada6 in loop (toplevel=1, justonce=0) at init.c:165
#16 0x0807d564 in zsh_main (argc=1, argv=0xbffff7d4) at init.c:1274
#17 0x0805272e in main (argc=1, argv=0xbffff7d4) at main.c:93

hope this is helpful,

	Matthias

Re: crashing bug here with LANG=C printf "%1.1f" 23.2

[Oliver Kiddle]

Matthias Kopfermann wrote:
> 
> for ((i=1;i<100;i+=.01)) ; { LANG=C printf "%1.1f\n" $i ; }
> gives me a segfault instantly here.
> the same can happen when just doing LANG='C' printf "%1.1f\n" 23.2

Seems that can be simplified a bit to get the seg fault. The following
seems to be sufficient:

   zsh -f
   LANG=C echo hello
   LANG=C echo hello

Use of the "LANG" environment variable is significant.

Oliver

PATCH: crashing bug here with LANG=C printf "%1.1f\n" 23.2

[Peter Stephenson]

Matthias Kopfermann wrote:
> for ((i=1;i<100;i+=.01)) ; { LANG=C printf "%1.1f\n" $i ; }
> gives me a segfault instantly here.
> the same can happen when just doing LANG='C' printf "%1.1f\n" 23.2
> 
> with message: BUG: attempt to free more than allocated.

While fixing this, I improved the interface to the environment
add/removal functions.  However, exactly one of the lines changed below
makes any difference to the bug whatsoever.  As I don't see why anyone
else should be less frustrated than I've been for the last couple of
hours, I'm not telling you which (unless you give up and ask).
Have fun :-).

(Assuming this does fix it, of course.  valgrind seems to think it does.)

Index: Src/builtin.c
===================================================================
RCS file: /cvsroot/zsh/zsh/Src/builtin.c,v
retrieving revision 1.121
diff -u -r1.121 builtin.c
--- Src/builtin.c	2 Jun 2004 22:14:25 -0000	1.121
+++ Src/builtin.c	7 Jul 2004 14:47:22 -0000
@@ -750,15 +750,11 @@
     setsparam("OLDPWD", ztrdup(oldpwd));
 
     pm = (Param) paramtab->getnode(paramtab, "PWD");
-    if (!(pm->flags & PM_EXPORTED)) {
-	pm->flags |= PM_EXPORTED;
-	pm->env = addenv("PWD", pwd, pm->flags);
-    }
+    if (!(pm->flags & PM_EXPORTED))
+	addenv(pm, pwd);
     pm = (Param) paramtab->getnode(paramtab, "OLDPWD");
-    if (!(pm->flags & PM_EXPORTED)) {
-	pm->flags |= PM_EXPORTED;
-	pm->env = addenv("OLDPWD", oldpwd, pm->flags);
-    }
+    if (!(pm->flags & PM_EXPORTED))
+	addenv(pm, oldpwd);
 }
 
 /* set if we are resolving links to their true paths */
@@ -1883,11 +1879,9 @@
 	if (!(pm->flags & (PM_ARRAY|PM_HASHED))) {
 	    if (pm->flags & PM_EXPORTED) {
 		if (!(pm->flags & PM_UNSET) && !pm->env && !value)
-		    pm->env = addenv(pname, getsparam(pname), pm->flags);
-	    } else if (pm->env && !(pm->flags & PM_HASHELEM)) {
-		delenv(pm->env);
-		pm->env = NULL;
-	    }
+		    addenv(pm, getsparam(pname));
+	    } else if (pm->env && !(pm->flags & PM_HASHELEM))
+		delenv(pm);
 	    if (value && !(pm = setsparam(pname, ztrdup(value))))
 		return NULL;
 	} else if (value) {
@@ -1938,7 +1932,7 @@
 	 * Maybe it would be easier to create a new struct but copy
 	 * the get/set methods.
 	 */
-	tpm = (Param) zalloc(sizeof *tpm);
+	tpm = (Param) zshcalloc(sizeof *tpm);
 
 	tpm->nam = pm->nam;
 	if (pm->ename &&
@@ -1962,10 +1956,9 @@
 	tpm->old = pm->old;
 	tpm->level = pm->level;
 	tpm->ct = pm->ct;
-	if (pm->env) {
-	    delenv(pm->env);
-	}
-	tpm->env = pm->env = NULL;
+	if (pm->env)
+	    delenv(pm);
+	tpm->env = NULL;
 
 	pm->old = tpm;
 	/*
Index: Src/exec.c
===================================================================
RCS file: /cvsroot/zsh/zsh/Src/exec.c,v
retrieving revision 1.66
diff -u -r1.66 exec.c
--- Src/exec.c	28 Jun 2004 15:38:13 -0000	1.66
+++ Src/exec.c	7 Jul 2004 14:47:24 -0000
@@ -483,7 +483,7 @@
      * that as argv[0] for this external command       */
     if (unset(RESTRICTED) && (z = zgetenv("ARGV0"))) {
 	setdata(firstnode(args), (void *) ztrdup(z));
-	delenv(z - 6);
+	delenvvalue(z - 6);
     } else if (dash) {
     /* Else if the pre-command `-' was given, we add `-' *
      * to the front of argv[0] for this command.         */
@@ -2520,15 +2520,13 @@
     while (wc_code(ac = *pc) == WC_ASSIGN) {
 	s = ecrawstr(state->prog, pc + 1, NULL);
 	if ((pm = (Param) paramtab->getnode(paramtab, s))) {
-	    if (pm->env) {
-		delenv(pm->env);
-		pm->env = NULL;
-	    }
+	    if (pm->env)
+		delenv(pm);
 	    if (!(pm->flags & PM_SPECIAL)) {
 		paramtab->removenode(paramtab, s);
 	    } else if (!(pm->flags & PM_READONLY) &&
 		       (unset(RESTRICTED) || !(pm->flags & PM_RESTRICTED))) {
-		Param tpm = (Param) zhalloc(sizeof *tpm);
+		Param tpm = (Param) hcalloc(sizeof *tpm);
 		tpm->nam = pm->nam;
 		copyparam(tpm, pm, 1);
 		pm = tpm;
@@ -2589,10 +2587,11 @@
 		    tpm->sets.hfn(tpm, pm->u.hash);
 		    break;
 		}
+		pm = tpm;
 	    } else
 		paramtab->addnode(paramtab, pm->nam, pm);
 	    if ((pm->flags & PM_EXPORTED) && ((s = getsparam(pm->nam))))
-		pm->env = addenv(pm->nam, s, pm->flags);
+		addenv(pm, s);
 	}
     }
 }
Index: Src/params.c
===================================================================
RCS file: /cvsroot/zsh/zsh/Src/params.c,v
retrieving revision 1.86
diff -u -r1.86 params.c
--- Src/params.c	22 Jun 2004 13:10:02 -0000	1.86
+++ Src/params.c	7 Jul 2004 14:47:25 -0000
@@ -587,20 +587,15 @@
     opts[ALLEXPORT] = oae;
 
     pm = (Param) paramtab->getnode(paramtab, "HOME");
-    if (!(pm->flags & PM_EXPORTED)) {
-	pm->flags |= PM_EXPORTED;
-	pm->env = addenv("HOME", home, pm->flags);
-    }
+    if (!(pm->flags & PM_EXPORTED))
+	addenv(pm, home);
     pm = (Param) paramtab->getnode(paramtab, "LOGNAME");
-    if (!(pm->flags & PM_EXPORTED)) {
-	pm->flags |= PM_EXPORTED;
-	pm->env = addenv("LOGNAME", pm->u.str, pm->flags);
-    }
-    pm = (Param) paramtab->getnode(paramtab, "SHLVL");
     if (!(pm->flags & PM_EXPORTED))
-	pm->flags |= PM_EXPORTED;
+	addenv(pm, pm->u.str);
+    pm = (Param) paramtab->getnode(paramtab, "SHLVL");
     sprintf(buf, "%d", (int)++shlvl);
-    pm->env = addenv("SHLVL", buf, pm->flags);
+    if (!(pm->flags & PM_EXPORTED))
+	addenv(pm, buf);
 
     /* Add the standard non-special parameters */
     set_pwd_env();
@@ -712,10 +707,8 @@
 		 * needed to avoid freeing oldpm, but we do take it
 		 * out of the environment when it's hidden.
 		 */
-		if (oldpm->env) {
-		    delenv(oldpm->env);
-		    oldpm->env = NULL;
-		}
+		if (oldpm->env)
+		    delenv(oldpm);
 		paramtab->removenode(paramtab, name);
 	    }
 	    paramtab->addnode(paramtab, ztrdup(name), pm);
@@ -1585,8 +1578,7 @@
     else
 	val = pm->gets.cfn(pm);
 
-    pm->flags |= PM_EXPORTED;
-    pm->env = addenv(pm->nam, val, pm->flags);
+    addenv(pm, val);
 }
 
 /**/
@@ -2240,10 +2232,8 @@
 	return 1;
     }
     pm->unsetfn(pm, exp);
-    if ((pm->flags & PM_EXPORTED) && pm->env) {
-	delenv(pm->env);
-	pm->env = NULL;
-    }
+    if (pm->env)
+	delenv(pm);
 
     /* remove it under its alternate name if necessary */
     if (pm->ename && !altflag) {
@@ -3296,7 +3286,7 @@
     else
 	joinchar = ':';
 
-    pm->env = addenv(s, t ? zjoin(t, joinchar, 1) : "", pm->flags);
+    addenv(pm, t ? zjoin(t, joinchar, 1) : "");
 }
 
 
@@ -3385,8 +3375,8 @@
 }
 
 /**/
-char *
-addenv(char *name, char *value, int flags)
+void
+addenv(Param pm, char *value)
 {
     char *oldenv = 0, *newenv = 0, *env = 0;
     int pos;
@@ -3394,13 +3384,14 @@
     /* First check if there is already an environment *
      * variable matching string `name'. If not, and   *
      * we are not requested to add new, return        */
-    if (findenv(name, &pos))
+    if (findenv(pm->nam, &pos))
 	oldenv = environ[pos];
 
-     newenv = mkenvstr(name, value, flags);
+     newenv = mkenvstr(pm->nam, value, pm->flags);
      if (zputenv(newenv)) {
         zsfree(newenv);
-	return NULL;
+	pm->env = NULL;
+	return;
     }
     /*
      * Under Cygwin we must use putenv() to maintain consistency.
@@ -3408,16 +3399,19 @@
      * silently reuse existing environment string. This tries to
      * check for both cases
      */
-    if (findenv(name, &pos)) {
+    if (findenv(pm->nam, &pos)) {
 	env = environ[pos];
 	if (env != oldenv)
 	    zsfree(oldenv);
 	if (env != newenv)
 	    zsfree(newenv);
-	return env;
+	pm->flags |= PM_EXPORTED;
+	pm->env = env;
+	return;
     }
 
-    return NULL; /* Cannot happen */
+    DPUTS(1, "addenv should never reach the end");
+    pm->env = NULL;
 }
 
 
@@ -3448,12 +3442,9 @@
  * string.                                         */
 
 
-/* Delete a pointer from the list of pointers to environment *
- * variables by shifting all the other pointers up one slot. */
-
 /**/
 void
-delenv(char *x)
+delenvvalue(char *x)
 {
     char **ep;
 
@@ -3467,6 +3458,22 @@
     zsfree(x);
 }
 
+/* Delete a pointer from the list of pointers to environment *
+ * variables by shifting all the other pointers up one slot. */
+
+/**/
+void
+delenv(Param pm)
+{
+    delenvvalue(pm->env);
+    pm->env = NULL;
+    /*
+     * Note we don't remove PM_EXPORT from the flags.  This
+     * may be asking for trouble but we need to know later
+     * if we restore this parameter to its old value.
+     */
+}
+
 /**/
 mod_export void
 convbase(char *s, zlong v, int base)
@@ -3625,10 +3632,8 @@
 	    pm->flags = (tpm->flags & ~PM_NORESTORE);
 	    pm->level = tpm->level;
 	    pm->ct = tpm->ct;
-	    if (pm->env) {
-		delenv(pm->env);
-	    }
-	    pm->env = NULL;
+	    if (pm->env)
+		delenv(pm);
 
 	    if (!(tpm->flags & PM_NORESTORE))
 		switch (PM_TYPE(pm->flags)) {


-- 
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
**********************************************************************

PATCH: test for crashing bug here with LANG=C printf

[Peter Stephenson]

Here's a test that picks up the bug.  (It affected all special exported
parameters that needed restoring after a command, so PATH shows it up
without relying on the vagaries of locales.)

Index: Test/A06assign.ztst
===================================================================
RCS file: /cvsroot/zsh/zsh/Test/A06assign.ztst,v
retrieving revision 1.1
diff -u -r1.1 A06assign.ztst
--- Test/A06assign.ztst	17 Dec 2001 17:17:38 -0000	1.1
+++ Test/A06assign.ztst	7 Jul 2004 15:09:30 -0000
@@ -251,3 +251,16 @@
  echo $u
 0:add array to indexed unset variable
 >a z
+
+ repeat 10 PATH=. echo hello
+0:saving and restoring of exported special parameters
+>hello
+>hello
+>hello
+>hello
+>hello
+>hello
+>hello
+>hello
+>hello
+>hello

-- 
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: PATCH: crashing bug here with LANG=C printf "%1.1f\n" 23.2

[Bart Schaefer]

On Wed, 7 Jul 2004, Peter Stephenson wrote:

> While fixing this, I improved the interface to the environment
> add/removal functions.

Hmm; there must at one time have been a reason for separating the flags
from the Param object.  (Maybe if one looks back far enough, there was
no Param object.)

> However, exactly one of the lines changed below makes any difference to 
> the bug whatsoever.

I'm guessing the only one that was added rather than changed.

> As I don't see why anyone else should be less frustrated than I've been 
> for the last couple of hours

Next you're going to threaten to read us some of your poetry?