Re: [bug report] 4.0.2 / 4.0.4 dumps core

Re: [bug report] 4.0.2 / 4.0.4 dumps core

[Wischnowsky, Sven]

I wrote:

> > schaefer[524] Src/zsh -f
> > zagzig% setopt extendedglob globcomplete nobanghist
> > zagzig% !^<TAB>zsh: segmentation fault (core dumped)  Src/zsh -f
> 
> I had a patch for this, too, that just made the shell use 
> duplicates in the
> reswdtab - it isn't that much memory being wasted (but of 
> course I forgot
> to bring that patch together with the other one).

Here's that patch.

Bye
  Sven

diff -ur ../oz/Src/hashtable.c ./Src/hashtable.c
--- ../oz/Src/hashtable.c	Tue Nov 13 19:13:45 2001
+++ ./Src/hashtable.c	Tue Nov 13 19:26:12 2001
@@ -975,7 +975,7 @@
     reswdtab->printnode   = printreswdnode;
 
     for (rw = reswds; rw->nam; rw++)
-	reswdtab->addnode(reswdtab, rw->nam, rw);
+	reswdtab->addnode(reswdtab, ztrdup(rw->nam), rw);
 }
 
 /* Print a reserved word */

Re: [bug report] 4.0.2 / 4.0.4 dumps core

[Bart Schaefer]

On Nov 15,  9:16am, Wischnowsky, Sven wrote:
} 
} > > schaefer[524] Src/zsh -f
} > > zagzig% setopt extendedglob globcomplete nobanghist
} > > zagzig% !^<TAB>zsh: segmentation fault (core dumped)  Src/zsh -f
} 
} Here's that patch.
} 
} Bye
}   Sven
} 
} diff -ur ../oz/Src/hashtable.c ./Src/hashtable.c
} --- ../oz/Src/hashtable.c	Tue Nov 13 19:13:45 2001
} +++ ./Src/hashtable.c	Tue Nov 13 19:26:12 2001
} @@ -975,7 +975,7 @@
}      reswdtab->printnode   = printreswdnode;
}  
}      for (rw = reswds; rw->nam; rw++)
} -	reswdtab->addnode(reswdtab, rw->nam, rw);
} +	reswdtab->addnode(reswdtab, ztrdup(rw->nam), rw);
}  }
}  
}  /* Print a reserved word */
}-- End of excerpt from Wischnowsky, Sven

I'm wondering if it wouldn't be more appropriate to duplicate the strings
in compctl.c:dumphashtable() rather than simply allowing the completion
code to poke bytes into the real reswdtab.

E.g.:

Index: Src/Zle/compctl.c
===================================================================
RCS file: /extra/cvsroot/zsh/zsh-4.0/Src/Zle/compctl.c,v
retrieving revision 1.2
diff -c -r1.2 compctl.c
--- Src/Zle/compctl.c	2001/04/10 16:37:32	1.2
+++ Src/Zle/compctl.c	2001/11/21 17:00:03
@@ -2096,7 +2096,7 @@
 
     for (i = 0; i < ht->hsize; i++)
 	for (hn = ht->nodes[i]; hn; hn = hn->next)
-	    addmatch(hn->nam, (char *) hn);
+	    addmatch(dupstring(hn->nam), (char *) hn);
 }
 
 /* ScanFunc used by maketildelist() et al. */


-- 
Bart Schaefer                                 Brass Lantern Enterprises
http://www.well.com/user/barts              http://www.brasslantern.com

Zsh: http://www.zsh.org | PHPerl Project: http://phperl.sourceforge.net   

Re: [bug report] 4.0.2 / 4.0.4 dumps core

[Wischnowsky, Sven]

Bart wrote:

> ...
> 
> } > rather than simply allowing the completion code to poke bytes into
> } > the real reswdtab.
> } 
> } But it isn't the completion code. At least not for me. It SEGVed in
> } the pattern matching code (called from the completion code).
> 
> Yes, but it's the completion code that's "violating" the pattern match
> code API, in the sense that the pattern match code is supposed to get
> writable strings and the completion code isn't providing them.

Yes (but I'd still prefer it if the matching code that really only needs
to read the strings it gets wouldn't modify them -- but that's probably a
different matter).

> } And I was tempted to change that code to at least test if it was
> } trying to write a '\0' onto the Null-byte at the end of a string.
> 
> That would unnecessarily penalize every other caller of the pattern
> match code, wouldn't it?

Yes, but not much, and, as I forgot to write, I don't suggest adding that
because it could result in weird bugs (it always worked because it only
ever needed to write a Nul-byte onto the Nul-byte which it really didn't
do, but then it suddenly needs to write a Nul-byte in the middle and...).

> If there's no problem with using dupstring() in dumphashtable(), then
> I'd prefer to commit my patch rather than yours.

No objections at all from my side.

Bye
  Sven

Re: [bug report] 4.0.2 / 4.0.4 dumps core

[Bart Schaefer]

On Nov 22,  9:09am, Wischnowsky, Sven wrote:
} 
} Bart wrote:
} 
} > I'm wondering if it wouldn't be more appropriate to duplicate the
} > strings in compctl.c:dumphashtable()
} 
} Yes, I thought about doing that, too, and I don't really care where
} we put that ztrdup().

It's a matter of the principle that code that's only examining the
contents of a data structure shouldn't be changing those contents, even
if it restores them afterwards.

} > rather than simply allowing the completion code to poke bytes into
} > the real reswdtab.
} 
} But it isn't the completion code. At least not for me. It SEGVed in
} the pattern matching code (called from the completion code).

Yes, but it's the completion code that's "violating" the pattern match
code API, in the sense that the pattern match code is supposed to get
writable strings and the completion code isn't providing them.

} And I was tempted to change that code to at least test if it was
} trying to write a '\0' onto the Null-byte at the end of a string.

That would unnecessarily penalize every other caller of the pattern
match code, wouldn't it?

If there's no problem with using dupstring() in dumphashtable(), then
I'd prefer to commit my patch rather than yours.

-- 
Bart Schaefer                                 Brass Lantern Enterprises
http://www.well.com/user/barts              http://www.brasslantern.com

Zsh: http://www.zsh.org | PHPerl Project: http://phperl.sourceforge.net   

Re: [bug report] 4.0.2 / 4.0.4 dumps core

[Wischnowsky, Sven]

Bart wrote:

> I'm wondering if it wouldn't be more appropriate to duplicate 
> the strings
> in compctl.c:dumphashtable() rather than simply allowing the 
> completion
> code to poke bytes into the real reswdtab.

Yes, I thought about doing that, too, and I don't really care where
we put that ztrdup().

But it isn't the completion code. At least not for me. It SEGVed in
the pattern matching code (called from the completion code).

And I was tempted to change that code to at least test if it was
trying to write a '\0' onto the Null-byte at the end of a string.


Bye
  Sven

Re: [bug report] 4.0.2 / 4.0.4 dumps core

[Wischnowsky, Sven]

> On Nov 12, 11:39am, Akitoshi MORISHIMA wrote:
> }
> } ...
> }
> } Type:
> } 	!^ then [TAB]
> } 
> } then zsh dumps core...
> } 
> } I guess "!^" accesses to "previous command line", which is 
> expectd to
> } be always present even without history-related settings, but this
> } isn't true in this "special" case...
> 
> This is a mis-diagnosis.
> 
> Zsh is attempting to complete reserved words (in this case, 
> the word "!").
> The reswords table contains static strings, but the 
> completion code tries
> to poke '\0' byte in at the end of the word, resulting in a 
> segfault on any
> platform where static strings aren't writable.
> 
> The combination of extendedglob and globcomplete (which makes "^" mean
> something), plus lack of history, tickles the bug, but is not 
> the reason
> for it:
> 
> schaefer[524] Src/zsh -f
> zagzig% setopt extendedglob globcomplete nobanghist
> zagzig% !^<TAB>zsh: segmentation fault (core dumped)  Src/zsh -f

I had a patch for this, too, that just made the shell use duplicates in the
reswdtab - it isn't that much memory being wasted (but of course I forgot
to bring that patch together with the other one).

Bye
  Sven

P.S.: Oh look, Outlook garbled Bart's message. Oh look, Sven's using Outlook.
      Urgh, that's so embarassing.

Re: [bug report] 4.0.2 / 4.0.4 dumps core

[Bart Schaefer]

On Nov 12, 11:39am, Akitoshi MORISHIMA wrote:
}
} Condition:
} 	put only the following line
} 
} 	setopt extendedglob globcomplete
} 
} 	in .zshrc, and inactivate(remove) every other startup files,
} 	and invoke zsh
} 
} Type:
} 	!^ then [TAB]
} 
} then zsh dumps core...
} 
} I guess "!^" accesses to "previous command line", which is expectd to
} be always present even without history-related settings, but this
} isn't true in this "special" case...

This is a mis-diagnosis.

Zsh is attempting to complete reserved words (in this case, the word "!").
The reswords table contains static strings, but the completion code tries
to poke '\0' byte in at the end of the word, resulting in a segfault on any
platform where static strings aren't writable.

The combination of extendedglob and globcomplete (which makes "^" mean
something), plus lack of history, tickles the bug, but is not the reason
for it:

schaefer[524] Src/zsh -f
zagzig% setopt extendedglob globcomplete nobanghist
zagzig% !^<TAB>zsh: segmentation fault (core dumped)  Src/zsh -f


-- 
Bart Schaefer                                 Brass Lantern Enterprises
http://www.well.com/user/barts              http://www.brasslantern.com

Zsh: http://www.zsh.org | PHPerl Project: http://phperl.sourceforge.net   

[bug report] 4.0.2 / 4.0.4 dumps core

[Akitoshi MORISHIMA]

Hello, I'm a long-time zsh user, although I'm not in any of the zsh-*
mailing lists.

I've encontered (coincidently, by mistake) to a bug in 4.0.x.

Condition:
	put only the following line

	setopt extendedglob globcomplete

	in .zshrc, and inactivate(remove) every other startup files,
	and invoke zsh

Type:
	!^ then [TAB]

then zsh dumps core...

Please note that this doesn't happen if

	zsh -f
	setopt extendedglob globcomplete
	!^[TAB]

which extends to "extendedglob".

I guess "!^" accesses to "previous command line", which is expectd to
be always present even without history-related settings, but this
isn't true in this "special" case...


Regards,

Akitoshi MORISHIMA