Core dump with latest CVS

Core dump with latest CVS

[Vin Shelton]

Peter,

I think your latest prompt/subst changes broke something.  Here's a recipe:

zsh -f
setopt NO_PROMPT_SP
autoload -U promptinit
promptinit
prompt bart

zsh: floating point exception (core dumped)
/usr/local/build/zsh-$(today)/Src/zsh -f


(gdb) bt
#0  0x080a718b in dopadding (str=0xb7ffc810 "", prenum=140, postnum=0,
preone=0x0, postone=0x0, premul=0xb7ffc808 "\b",
    postmul=0x80b8eaf " ") at /usr/local/src/zsh-2006-09-14/Src/subst.c:986
#1  0x080ab1e0 in paramsubst (l=0xb7ffc600, n=Variable "n" is not available.
) at /usr/local/src/zsh-2006-09-14/Src/subst.c:3141
#2  0x080a5969 in stringsubst (list=0xb7ffc600, node=0xb7ffc624,
ssub=0, asssub=0) at /usr/local/src/zsh-2006-09-14/Src/subst.c:156
#3  0x080a5457 in prefork (list=0xb7ffc600, flags=0) at
/usr/local/src/zsh-2006-09-14/Src/subst.c:91
#4  0x08066bb7 in execcmd (state=0xbff3ce38, input=0, output=0,
how=18, last1=2) at /usr/local/src/zsh-2006-09-14/Src/exec.c:1961
#5  0x08065669 in execpline2 (state=0xbff3ce38, pcode=1091, how=18,
input=0, output=0, last1=0)
    at /usr/local/src/zsh-2006-09-14/Src/exec.c:1301
#6  0x08064d79 in execpline (state=0xbff3ce38, slcode=140, how=18,
last1=0) at /usr/local/src/zsh-2006-09-14/Src/exec.c:1087
#7  0x080647c5 in execlist (state=0xbff3ce38, dont_change_job=1,
exiting=0) at /usr/local/src/zsh-2006-09-14/Src/exec.c:893
#8  0x08084a72 in execif (state=0xbff3ce38, do_exec=140) at
/usr/local/src/zsh-2006-09-14/Src/loop.c:520
#9  0x08068330 in execcmd (state=0xbff3ce38, input=0, output=0, how=2,
last1=2) at /usr/local/src/zsh-2006-09-14/Src/exec.c:2528
#10 0x08065669 in execpline2 (state=0xbff3ce38, pcode=835, how=2,
input=0, output=0, last1=0)
    at /usr/local/src/zsh-2006-09-14/Src/exec.c:1301
#11 0x08064d79 in execpline (state=0xbff3ce38, slcode=140, how=2,
last1=0) at /usr/local/src/zsh-2006-09-14/Src/exec.c:1087
#12 0x080647c5 in execlist (state=0xbff3ce38, dont_change_job=1,
exiting=0) at /usr/local/src/zsh-2006-09-14/Src/exec.c:893
#13 0x08064561 in execode (p=0x97f1f90, dont_change_job=140,
exiting=140) at /usr/local/src/zsh-2006-09-14/Src/exec.c:793
#14 0x0806afb0 in runshfunc (prog=0x97f1f90, wrap=0x0, name=0xb7ffc0b0
"prompt_bart_precmd")
    at /usr/local/src/zsh-2006-09-14/Src/exec.c:3905
#15 0x0806ad26 in doshfunc (name=0x98031c8 "prompt_bart_precmd",
prog=0x97f1f90, doshargs=0xb7ffc050, flags=0, noreturnval=0)
    at /usr/local/src/zsh-2006-09-14/Src/exec.c:3814
#16 0x0806a771 in execshfunc (shf=0x97fd3a0, args=0xb7ffc050) at
/usr/local/src/zsh-2006-09-14/Src/exec.c:3604
#17 0x080684c5 in execcmd (state=0xbff3d248, input=0, output=0,
how=18, last1=2) at /usr/local/src/zsh-2006-09-14/Src/exec.c:2576
#18 0x08065669 in execpline2 (state=0xbff3d248, pcode=195, how=18,
input=0, output=0, last1=0)
    at /usr/local/src/zsh-2006-09-14/Src/exec.c:1301
#19 0x08064d79 in execpline (state=0xbff3d248, slcode=140, how=18,
last1=0) at /usr/local/src/zsh-2006-09-14/Src/exec.c:1087
#20 0x080647c5 in execlist (state=0xbff3d248, dont_change_job=1,
exiting=0) at /usr/local/src/zsh-2006-09-14/Src/exec.c:893
#21 0x08064561 in execode (p=0x97f2080, dont_change_job=140,
exiting=140) at /usr/local/src/zsh-2006-09-14/Src/exec.c:793
#22 0x0806afb0 in runshfunc (prog=0x97f2080, wrap=0x0, name=0xb7ffc048
"precmd") at /usr/local/src/zsh-2006-09-14/Src/exec.c:3905
#23 0x0806ad26 in doshfunc (name=0x80bd22f "precmd", prog=0x97f2080,
doshargs=0x0, flags=0, noreturnval=1)
    at /usr/local/src/zsh-2006-09-14/Src/exec.c:3814
#24 0x080ae9ca in callhookfunc (name=0x80bd22f "precmd", lnklst=0x8c)
at /usr/local/src/zsh-2006-09-14/Src/utils.c:1094
#25 0x080aeadb in preprompt () at /usr/local/src/zsh-2006-09-14/Src/utils.c:1137
#26 0x0807875c in loop (toplevel=1, justonce=0) at
/usr/local/src/zsh-2006-09-14/Src/init.c:118
#27 0x0807af33 in zsh_main (argc=2, argv=0xbff3d724) at
/usr/local/src/zsh-2006-09-14/Src/init.c:1324
#28 0x08052d52 in main (argc=140, argv=0x8c) at
/usr/local/src/zsh-2006-09-14/Src/main.c:93
(gdb)

This is on a linux box at work.  uname -a reports

Linux cvs-srvr.aoainc.com 2.6.9-5.EL #1 Wed Jan 5 19:22:18 EST 2005
i686 i686 i386 GNU/Linux

Sorry I don't currently have the time to look into this further right now.

  - Vin
-- 
Whoever you are, no matter how lonely,
the world offers itself to your imagination,
calls to you like the wild geese, harsh and exciting--
over and over announcing your place
in the family of things.			Mary Oliver

Re: Core dump with latest CVS

[Peter Stephenson]

"Vin Shelton" wrote:
> Peter,
> 
> I think your latest prompt/subst changes broke something.  Here's a recipe:
> 
> zsh -f
> setopt NO_PROMPT_SP
> autoload -U promptinit
> promptinit
> prompt bart
> 
> zsh: floating point exception (core dumped)

Yes, I get this.

The reason is this:

${(pl.COLUMNS..\b.)}

\b used to be treated as length 1; now we take its width.  For control
characters currently we assume width 1.  This causes the padding width
to be zero, which causes the shell to crash: I can fix the crash easily.

The other question is how to make the padding work when it's simply used
for repetition:

- Mostly the problem is just control characters.  I noted one before.
We could assume these have length 1 rather than 0 (and document this,
obviously).  It's not particularly logical but it fixes up most of the
problem cases without any need for tricks involving multibyte mode or
new features.  Most users probably won't be (directly) affected anyway.

- I could add a flag (b is available) to force the old way just for
the current padding.

- Something else involving new flags.

- require an explicit "unsetopt multibyte" for that to work as it used
to.  Yuk.

-- 
Peter Stephenson <pws@csr.com>                  Software Engineer
CSR PLC, Churchill House, Cambridge Business Park, Cowley Road
Cambridge, CB4 0WZ, UK                          Tel: +44 (0)1223 692070


To access the latest news from CSR copy this link into a web browser:  http://www.csr.com/email_sig.php

Re: Core dump with latest CVS

[Peter Stephenson]

Peter Stephenson wrote:
> - Something else involving new flags.

Oh, yeah, I knew I'd thought of something else that made sense:

- Have a flag that indicates that characters all have width 1, but still
handle multibyte characters properly.  In other words, this indicates
explicitly we're doing repetition rather than padding according to
width.  This is potentially a useful feature anyway.  I'm not that keen
on making it the default, even though it's an extension of the old
behaviour, because to me padding naturally implies the use of the
character width.  But that could be done if it was popular.

-- 
Peter Stephenson <pws@csr.com>                  Software Engineer
CSR PLC, Churchill House, Cambridge Business Park, Cowley Road
Cambridge, CB4 0WZ, UK                          Tel: +44 (0)1223 692070


To access the latest news from CSR copy this link into a web browser:  http://www.csr.com/email_sig.php

Re: Core dump with latest CVS

[Peter Stephenson]

Peter Stephenson wrote:
> \b used to be treated as length 1; now we take its width.  For control
> characters currently we assume width 1.  This causes the padding width
                                       ^ that should have been 0.
> to be zero, which causes the shell to crash: I can fix the crash easily.
> 
> - Mostly the problem is just control characters.  I noted one before.
> We could assume these have length 1 rather than 0 (and document this,
> obviously).  It's not particularly logical but it fixes up most of the
> problem cases without any need for tricks involving multibyte mode or
> new features.  Most users probably won't be (directly) affected anyway.

I'll commit this pair of changes for now, since it fixes the crash and
mostly does the right thing transparently---ASCII characters behave as
they used to and multibyte characters as they should---but if there are
strong views I can change it.

I've tried to be consistent with the assumed width of control
characters; in most other circumstances there's no simple answer that
will do the right thing, so it might as well be 1 as zero.  We already
tend to use nicechar() anywhere the problem is significant.

Index: README
===================================================================
RCS file: /cvsroot/zsh/zsh/README,v
retrieving revision 1.37
diff -u -r1.37 README
--- README	10 Sep 2006 15:24:26 -0000	1.37
+++ README	15 Sep 2006 13:00:20 -0000
@@ -49,6 +49,13 @@
 other shell at startup; it must be present in the environment or set
 subsequently by the user.  It is valid for the variable to be unset.
 
+The MULTIBYTE option is on by default where it is available; this
+causes many operations to recognise characters as in the current locale.
+Older versions of the shell always assumed a character was one byte.
+In some places the width of the character will be used; this is transparent
+when used for calculations of screen position, but also occurs, for
+example, in calculations of padding width.
+
 Zsh has previously been lax about whether it allows octets with the
 top bit set to be part of a shell identifier.  Older versions of the shell
 assumed all such octets were allowed in identifiers, however the POSIX
Index: Doc/Zsh/expn.yo
===================================================================
RCS file: /cvsroot/zsh/zsh/Doc/Zsh/expn.yo,v
retrieving revision 1.69
diff -u -r1.69 expn.yo
--- Doc/Zsh/expn.yo	14 Sep 2006 09:03:16 -0000	1.69
+++ Doc/Zsh/expn.yo	15 Sep 2006 13:00:22 -0000
@@ -871,7 +871,9 @@
 
 If the tt(MULTIBYTE) option is in effect, screen character widths will
 be used for the calculation of padding; otherwise individual bytes are
-treat as occupying one unit of width.
+treat as occupying one unit of width.  Control characters are always
+assumed to be one unit wide; this allows the mechanism to be used
+for generating repetitions of control characters.
 )
 item(tt(r:)var(expr)tt(::)var(string1)tt(::)var(string2)tt(:))(
 As tt(l), but pad the words on the right and insert var(string2)
Index: Src/prompt.c
===================================================================
RCS file: /cvsroot/zsh/zsh/Src/prompt.c,v
retrieving revision 1.40
diff -u -r1.40 prompt.c
--- Src/prompt.c	13 Sep 2006 20:55:30 -0000	1.40
+++ Src/prompt.c	15 Sep 2006 13:00:25 -0000
@@ -944,10 +944,15 @@
 		multi = 0;
 		break;
 	    default:
-		/* If the character isn't printable, wcwidth() returns -1. */
+		/*
+		 * If the character isn't printable, wcwidth() returns
+		 * -1.  We assume width 1.
+		 */
 		wcw = wcwidth(wc);
-		if (wcw > 0)
+		if (wcw >= 0)
 		    w += wcw;
+		else
+		    w++;
 		multi = 0;
 		break;
 	    }
@@ -1152,8 +1157,10 @@
 				break;
 			    default:
 				wcw = wcwidth(cc);
-				if (wcw > 0)
+				if (wcw >= 0)
 				    remw -= wcw;
+				else
+				    remw--;
 				break;
 			    }
 #else
@@ -1215,8 +1222,10 @@
 				break;
 			    default:
 				wcw = wcwidth(cc);
-				if (wcw > 0)
+				if (wcw >= 0)
 				    maxwidth -= wcw;
+				else
+				    maxwidth--;
 				break;
 			    }
 #else
Index: Src/subst.c
===================================================================
RCS file: /cvsroot/zsh/zsh/Src/subst.c,v
retrieving revision 1.59
diff -u -r1.59 subst.c
--- Src/subst.c	13 Sep 2006 20:55:30 -0000	1.59
+++ Src/subst.c	15 Sep 2006 13:00:27 -0000
@@ -837,29 +837,31 @@
 		    }
 		} else {
 		    f -= lpreone;
-		    if ((m = f % lpremul)) {
-			/*
-			 * Left over fraction of repeated string.
-			 */
-			MB_METACHARINIT();
-			/* Skip this much. */
-			m = lpremul - m;
-			for (t = premul; m > 0; ) {
-			    t += MB_METACHARLENCONV(t, &cchar);
-			    m -= WCWIDTH(cchar);
-			}
-			/* Output the rest. */
-			while (*t)
-			    *r++ = *t++;
-		    }
-		    for (cc = f / lpremul; cc--;) {
-			/* Repeat the repeated string */
-			MB_METACHARINIT();
-			for (c = lpremul, t = premul; c > 0; ) {
-			    cl = MB_METACHARLENCONV(t, &cchar);
-			    while (cl--)
+		    if (lpremul) {
+			if ((m = f % lpremul)) {
+			    /*
+			     * Left over fraction of repeated string.
+			     */
+			    MB_METACHARINIT();
+			    /* Skip this much. */
+			    m = lpremul - m;
+			    for (t = premul; m > 0; ) {
+				t += MB_METACHARLENCONV(t, &cchar);
+				m -= WCWIDTH(cchar);
+			    }
+			    /* Output the rest. */
+			    while (*t)
 				*r++ = *t++;
-			    c -= WCWIDTH(cchar);
+			}
+			for (cc = f / lpremul; cc--;) {
+			    /* Repeat the repeated string */
+			    MB_METACHARINIT();
+			    for (c = lpremul, t = premul; c > 0; ) {
+				cl = MB_METACHARLENCONV(t, &cchar);
+				while (cl--)
+				    *r++ = *t++;
+				c -= WCWIDTH(cchar);
+			    }
 			}
 		    }
 		    if (preone) {
@@ -910,19 +912,21 @@
 			while (*postone)
 			    *r++ = *postone++;
 		    }
-		    for (cc = f / lpostmul; cc--;) {
-			/* Begin the beguine */
-			for (t = postmul; *t; )
-			    *r++ = *t++;
-		    }
-		    if ((m = f % lpostmul)) {
-			/* Fill leftovers with chunk of repeated string */
-			MB_METACHARINIT();
-			while (m > 0) {
-			    cl = MB_METACHARLENCONV(postmul, &cchar);
-			    m -= WCWIDTH(cchar);
-			    while (cl--)
-				*r++ = *postmul++;
+		    if (lpostmul) {
+			for (cc = f / lpostmul; cc--;) {
+			    /* Begin the beguine */
+			    for (t = postmul; *t; )
+				*r++ = *t++;
+			}
+			if ((m = f % lpostmul)) {
+			    /* Fill leftovers with chunk of repeated string */
+			    MB_METACHARINIT();
+			    while (m > 0) {
+				cl = MB_METACHARLENCONV(postmul, &cchar);
+				m -= WCWIDTH(cchar);
+				while (cl--)
+				    *r++ = *postmul++;
+			    }
 			}
 		    }
 		}
@@ -983,37 +987,39 @@
 		     * first
 		     */
 		    f -= lpreone;
-		    if ((m = f % lpremul)) {
-			/*
-			 * Some fraction of the repeated string needed.
-			 */
-			/* Need this much... */
-			c = m;
-			/* ...skipping this much first. */
-			m = lpremul - m;
-			MB_METACHARINIT();
-			for (t = premul; m > 0; ) {
-			    t += MB_METACHARLENCONV(t, &cchar);
-			    m -= WCWIDTH(cchar);
-			}
-			/* Now the rest of the repeated string. */
-			while (c > 0) {
-			    cl = MB_METACHARLENCONV(t, &cchar);
-			    while (cl--)
-				*r++ = *t++;
-			    c -= WCWIDTH(cchar);
+		    if (lpremul) {
+			if ((m = f % lpremul)) {
+			    /*
+			     * Some fraction of the repeated string needed.
+			     */
+			    /* Need this much... */
+			    c = m;
+			    /* ...skipping this much first. */
+			    m = lpremul - m;
+			    MB_METACHARINIT();
+			    for (t = premul; m > 0; ) {
+				t += MB_METACHARLENCONV(t, &cchar);
+				m -= WCWIDTH(cchar);
+			    }
+			    /* Now the rest of the repeated string. */
+			    while (c > 0) {
+				cl = MB_METACHARLENCONV(t, &cchar);
+				while (cl--)
+				    *r++ = *t++;
+				c -= WCWIDTH(cchar);
+			    }
 			}
-		    }
-		    for (cc = f / lpremul; cc--;) {
-			/*
-			 * Repeat the repeated string.
-			 */
-			MB_METACHARINIT();
-			for (c = lpremul, t = premul; c > 0; ) {
-			    cl = MB_METACHARLENCONV(t, &cchar);
-			    while (cl--)
-				*r++ = *t++;
-			    c -= WCWIDTH(cchar);
+			for (cc = f / lpremul; cc--;) {
+			    /*
+			     * Repeat the repeated string.
+			     */
+			    MB_METACHARINIT();
+			    for (c = lpremul, t = premul; c > 0; ) {
+				cl = MB_METACHARLENCONV(t, &cchar);
+				while (cl--)
+				    *r++ = *t++;
+				c -= WCWIDTH(cchar);
+			    }
 			}
 		    }
 		    if (preone) {
@@ -1089,27 +1095,29 @@
 			c -= WCWIDTH(cchar);
 		    }
 		}
-		/* Repeat the repeated string */
-		for (cc = f / lpostmul; cc--;) {
-		    MB_METACHARINIT();
-		    for (c = lpostmul, t = postmul; *t; ) {
-			cl = MB_METACHARLENCONV(t, &cchar);
-			while (cl--)
-			    *r++ = *t++;
-			c -= WCWIDTH(cchar);
+		if (lpostmul) {
+		    /* Repeat the repeated string */
+		    for (cc = f / lpostmul; cc--;) {
+			MB_METACHARINIT();
+			for (c = lpostmul, t = postmul; *t; ) {
+			    cl = MB_METACHARLENCONV(t, &cchar);
+			    while (cl--)
+				*r++ = *t++;
+			    c -= WCWIDTH(cchar);
+			}
 		    }
-		}
-		/*
-		 * See if there's any fraction of the repeated
-		 * string needed to fill up the remaining space.
-		 */
-		if ((m = f % lpostmul)) {
-		    MB_METACHARINIT();
-		    while (m > 0) {
-			cl = MB_METACHARLENCONV(postmul, &cchar);
-			while (cl--)
-			    *r++ = *postmul++;
-			m -= WCWIDTH(cchar);
+		    /*
+		     * See if there's any fraction of the repeated
+		     * string needed to fill up the remaining space.
+		     */
+		    if ((m = f % lpostmul)) {
+			MB_METACHARINIT();
+			while (m > 0) {
+			    cl = MB_METACHARLENCONV(postmul, &cchar);
+			    while (cl--)
+				*r++ = *postmul++;
+			    m -= WCWIDTH(cchar);
+			}
 		    }
 		}
 	    }
Index: Src/utils.c
===================================================================
RCS file: /cvsroot/zsh/zsh/Src/utils.c,v
retrieving revision 1.137
diff -u -r1.137 utils.c
--- Src/utils.c	13 Sep 2006 20:55:30 -0000	1.137
+++ Src/utils.c	15 Sep 2006 13:00:30 -0000
@@ -527,8 +527,10 @@
     if (widthp) {
 	int wcw = wcwidth(c);
 	*widthp = (s - buf);
-	if (wcw > 0)
+	if (wcw >= 0)
 	    *widthp += wcw;
+	else
+	    (*widthp)++;
     }
     if (swidep)
 	*swidep = s;
@@ -550,12 +552,12 @@
 {
     int wcw;
     /* assume a single-byte character if not valid */
-    if (wc == WEOF)
+    if (wc == WEOF || unset(MULTIBYTE))
 	return 1;
     wcw = wcwidth(wc);
-    /* if not printable, assume zero width */
-    if (wcw <= 0)
-	return 0;
+    /* if not printable, assume width 1 */
+    if (wcw < 0)
+	return 1;
     return wcw;
 }
 
@@ -4077,12 +4079,14 @@
 		num++;
 	    } else if (width) {
 		/*
-		 * Returns -1 if not a printable character; best
-		 * just to ignore these.
+		 * Returns -1 if not a printable character.  We
+		 * turn this into 1 for backward compatibility.
 		 */
 		int wcw = wcwidth(wc);
-		if (wcw > 0)
+		if (wcw >= 0)
 		    num += wcw;
+		else
+		    num++;
 	    } else
 		num++;
 	    laststart = ptr;

-- 
Peter Stephenson <pws@csr.com>                  Software Engineer
CSR PLC, Churchill House, Cambridge Business Park, Cowley Road
Cambridge, CB4 0WZ, UK                          Tel: +44 (0)1223 692070


To access the latest news from CSR copy this link into a web browser:  http://www.csr.com/email_sig.php

Re: Core dump with latest CVS

[Bart Schaefer]

On Sep 15,  2:08pm, Peter Stephenson wrote:
}
} I'll commit this pair of changes for now, since it fixes the crash and
} mostly does the right thing transparently---ASCII characters behave as
} they used to and multibyte characters as they should---but if there are
} strong views I can change it.

My concern is that "printability" shouldn't have anything to do with it.
Programmatically, the expression

    (( ${#:-X} == 1 && ${#(pl.width..X.)} == width ))

should always be true.

I could see an argument that ${#(%pl.width.X)} could compute the padding
based on printable width, because the presence of % means we're expanding
prompt escapes so presumably we want printable values ... but there is
a reason that %{ %} has to be placed around non-printing sequences in
prompts.

Re: Core dump with latest CVS

[Peter Stephenson]

Bart Schaefer wrote:
> My concern is that "printability" shouldn't have anything to do with it.
> Programmatically, the expression
> 
>     (( ${#:-X} == 1 && ${#(pl.width..X.)} == width ))
> 
> should always be true.

OK, you're interpreting "width" differently... to me, that's a string
length, not a width at all.  It doesn't really matter as long as it's
documented.

So that would be the last option I posted, treat multibyte characters
but assume they have width 1 (or, in better English, use string lengths
rather than character widths for padding) unless there's an additional
flag.  I would propose to supply such a flag, if we go this way.

> I could see an argument that ${#(%pl.width.X)} could compute the padding
> based on printable width, because the presence of % means we're expanding
> prompt escapes so presumably we want printable values

I don't really see why a normal user would assume the expression
*didn't* deal with printable widths, but, again, that's mostly a matter
of documentation.

-- 
Peter Stephenson <pws@csr.com>                  Software Engineer
CSR PLC, Churchill House, Cambridge Business Park, Cowley Road
Cambridge, CB4 0WZ, UK                          Tel: +44 (0)1223 692070


To access the latest news from CSR copy this link into a web browser:  http://www.csr.com/email_sig.php