zsh-workers
 help / color / mirror / code / Atom feed
* Bug: Searching through sufficiently large $historywords causes seg fault
@ 2023-05-08  8:58 Marlon Richert
  2023-05-08 17:01 ` Bart Schaefer
  0 siblings, 1 reply; 9+ messages in thread
From: Marlon Richert @ 2023-05-08  8:58 UTC (permalink / raw)
  To: Zsh hackers list

However, this does not happen when searching through a normal array of
the same size:

% tmp=( {1..99999} )
% : $tmp[(r)foo]
% print -S "$tmp"
% : $historywords[(r)foo]
zsh: segmentation fault


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: Bug: Searching through sufficiently large $historywords causes seg fault
  2023-05-08  8:58 Bug: Searching through sufficiently large $historywords causes seg fault Marlon Richert
@ 2023-05-08 17:01 ` Bart Schaefer
  2023-05-09 16:53   ` Bart Schaefer
  0 siblings, 1 reply; 9+ messages in thread
From: Bart Schaefer @ 2023-05-08 17:01 UTC (permalink / raw)
  To: Marlon Richert; +Cc: Zsh hackers list

Does PWS's patch from https://zsh.org/workers/51722 make any difference?


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: Bug: Searching through sufficiently large $historywords causes seg fault
  2023-05-08 17:01 ` Bart Schaefer
@ 2023-05-09 16:53   ` Bart Schaefer
  2023-05-09 16:58     ` Bart Schaefer
  0 siblings, 1 reply; 9+ messages in thread
From: Bart Schaefer @ 2023-05-09 16:53 UTC (permalink / raw)
  To: Marlon Richert; +Cc: Zsh hackers list

On Mon, May 8, 2023 at 10:01 AM Bart Schaefer <schaefer@brasslantern.com> wrote:
>
> Does PWS's patch from https://zsh.org/workers/51722 make any difference?

To attempt to answer my own question:

I don't get the segfault on $historywords[(r)foo] in either case
(without, or with, 51722).  However, after [(r)foo] in the "without"
case I get an immediate segfault attempting $#historywords.

With 51722, $#historywords is incorrect (returns "51" where it should
be about 100,000) and I eventually get a segfault after scrolling back
through the history past the event that was added with "print -S'.


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: Bug: Searching through sufficiently large $historywords causes seg fault
  2023-05-09 16:53   ` Bart Schaefer
@ 2023-05-09 16:58     ` Bart Schaefer
  2023-05-09 19:01       ` Peter Stephenson
  0 siblings, 1 reply; 9+ messages in thread
From: Bart Schaefer @ 2023-05-09 16:58 UTC (permalink / raw)
  To: Marlon Richert; +Cc: Zsh hackers list

On Tue, May 9, 2023 at 9:53 AM Bart Schaefer <schaefer@brasslantern.com> wrote:
>
> With 51722, $#historywords is incorrect (returns "51" where it should
> be about 100,000) and I eventually get a segfault after scrolling back
> through the history past the event that was added with "print -S'.

Scratch that ... I had not correctly recompiled.  After correctly
recompiling, I can't reproduce the segmentation fault at all with
51722, but the event added with "print -S" is entirely missing from
$historywords even though it is found when scrolling the history.


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: Bug: Searching through sufficiently large $historywords causes seg fault
  2023-05-09 16:58     ` Bart Schaefer
@ 2023-05-09 19:01       ` Peter Stephenson
  2023-05-09 23:05         ` Bart Schaefer
  0 siblings, 1 reply; 9+ messages in thread
From: Peter Stephenson @ 2023-05-09 19:01 UTC (permalink / raw)
  To: zsh-workers

On Tue, 2023-05-09 at 09:58 -0700, Bart Schaefer wrote:
> On Tue, May 9, 2023 at 9:53 AM Bart Schaefer <schaefer@brasslantern.com> wrote:
> > With 51722, $#historywords is incorrect (returns "51" where it should
> > be about 100,000) and I eventually get a segfault after scrolling back
> > through the history past the event that was added with "print -S'.
> 
> Scratch that ... I had not correctly recompiled.  After correctly
> recompiling, I can't reproduce the segmentation fault at all with
> 51722, but the event added with "print -S" is entirely missing from
> $historywords even though it is found when scrolling the history.

That sounds fine --- it's the division into words, not the line, that's
causing the problems which this avoids.

In this case it's presumably the original word counting, not the
re-spliting on reading a history file, that's causing the underlying
bad numbers.

pws




^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: Bug: Searching through sufficiently large $historywords causes seg fault
  2023-05-09 19:01       ` Peter Stephenson
@ 2023-05-09 23:05         ` Bart Schaefer
  2023-05-10  8:39           ` Peter Stephenson
  0 siblings, 1 reply; 9+ messages in thread
From: Bart Schaefer @ 2023-05-09 23:05 UTC (permalink / raw)
  To: Peter Stephenson; +Cc: zsh-workers

On Tue, May 9, 2023 at 12:01 PM Peter Stephenson
<p.w.stephenson@ntlworld.com> wrote:
>
> On Tue, 2023-05-09 at 09:58 -0700, Bart Schaefer wrote:
> >
> > Scratch that ... I had not correctly recompiled.  After correctly
> > recompiling, I can't reproduce the segmentation fault at all with
> > 51722, but the event added with "print -S" is entirely missing from
> > $historywords even though it is found when scrolling the history.
>
> That sounds fine --- it's the division into words, not the line, that's
> causing the problems which this avoids.

It's fine that an event is missing from historywords?  Doesn't that
mean that we've just skipped over the problem code rather than figure
out why it's wrong?


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: Bug: Searching through sufficiently large $historywords causes seg fault
  2023-05-09 23:05         ` Bart Schaefer
@ 2023-05-10  8:39           ` Peter Stephenson
  2023-05-12  9:28             ` Peter Stephenson
  0 siblings, 1 reply; 9+ messages in thread
From: Peter Stephenson @ 2023-05-10  8:39 UTC (permalink / raw)
  To: zsh-workers

> On 10/05/2023 00:05 Bart Schaefer <schaefer@brasslantern.com> wrote:
> On Tue, May 9, 2023 at 12:01 PM Peter Stephenson
> <p.w.stephenson@ntlworld.com> wrote:
> >
> > On Tue, 2023-05-09 at 09:58 -0700, Bart Schaefer wrote:
> > >
> > > Scratch that ... I had not correctly recompiled.  After correctly
> > > recompiling, I can't reproduce the segmentation fault at all with
> > > 51722, but the event added with "print -S" is entirely missing from
> > > $historywords even though it is found when scrolling the history.
> >
> > That sounds fine --- it's the division into words, not the line, that's
> > causing the problems which this avoids.
> 
> It's fine that an event is missing from historywords?  Doesn't that
> mean that we've just skipped over the problem code rather than figure
> out why it's wrong?

The current patch is entirely about safety to cover all the possible
cases where history word numbering is going wrong and preventing crashes.
Completion is particularly sensitive to this as it looks at all words
for all lines, so if it can go wrong, it will.  The patch now seems to
be doing its job.

Tracking down all possible cases where a history word number can be
calculated wrongly from any possible source is another thing entirely.
Possibly some DPUTS() in the earlier code are a good idea, as now we
know it is showing up in error later on trapping it there isn't going
to help any more.

pws


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: Bug: Searching through sufficiently large $historywords causes seg fault
  2023-05-10  8:39           ` Peter Stephenson
@ 2023-05-12  9:28             ` Peter Stephenson
  2023-05-12 16:12               ` Bart Schaefer
  0 siblings, 1 reply; 9+ messages in thread
From: Peter Stephenson @ 2023-05-12  9:28 UTC (permalink / raw)
  To: zsh-workers


> Tracking down all possible cases where a history word number can be
> calculated wrongly from any possible source is another thing entirely.
> Possibly some DPUTS() in the earlier code are a good idea, as now we
> know it is showing up in error later on trapping it there isn't going
> to help any more.

Here's the only obvious case I can see in normal history management
where you might get a miscalculation, though why is obscure as this
seems only to be connected to skipping a !-history expansion.  IF
this pops up hopefully you'll be able to work out how to reproduce
it.

The history reading mechanism from a file remains a suspect as it's
much more obscure, and is consequently harder to instrument usefully.

I'll commit the other patch this weekend: I think we have enough
information to be able to continue searches without having to have
the shell crash for us.

pws

diff --git a/Src/hist.c b/Src/hist.c
index 82d03a840..7e6394406 100644
--- a/Src/hist.c
+++ b/Src/hist.c
@@ -1643,12 +1643,17 @@ hend(Eprog prog)
 void
 ihwbegin(int offset)
 {
+    int pos = hptr - chline + offset;
     if (stophist == 2 || (histactive & HA_INWORD) ||
 	(inbufflags & (INP_ALIAS|INP_HIST)) == INP_ALIAS)
 	return;
     if (chwordpos%2)
 	chwordpos--;	/* make sure we're on a word start, not end */
-    chwords[chwordpos++] = hptr - chline + offset;
+    DPUTS1(pos < 0, "History word position < 0 in %s",
+	   dupstrpfx(chline, hptr-chline));
+    if (pos < 0)
+	pos = 0;
+    chwords[chwordpos++] = pos;
 }
 
 /* Abort current history word, not needed */


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: Bug: Searching through sufficiently large $historywords causes seg fault
  2023-05-12  9:28             ` Peter Stephenson
@ 2023-05-12 16:12               ` Bart Schaefer
  0 siblings, 0 replies; 9+ messages in thread
From: Bart Schaefer @ 2023-05-12 16:12 UTC (permalink / raw)
  To: Peter Stephenson; +Cc: zsh-workers

On Fri, May 12, 2023 at 2:28 AM Peter Stephenson
<p.w.stephenson@ntlworld.com> wrote:
>
> Here's the only obvious case I can see in normal history management
> where you might get a miscalculation

On Wed, May 3, 2023 at 8:35 AM Sebastian Gniazdowski
<sgniazdowski@gmail.com> wrote:
>
> (gdb) p (int)(e-hstr)
> $65 = -31903

Is it not possible that what we actually have here is a huge overflow
in (e) such that (e-hstr) is a very large unsigned?


^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2023-05-12 16:13 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-05-08  8:58 Bug: Searching through sufficiently large $historywords causes seg fault Marlon Richert
2023-05-08 17:01 ` Bart Schaefer
2023-05-09 16:53   ` Bart Schaefer
2023-05-09 16:58     ` Bart Schaefer
2023-05-09 19:01       ` Peter Stephenson
2023-05-09 23:05         ` Bart Schaefer
2023-05-10  8:39           ` Peter Stephenson
2023-05-12  9:28             ` Peter Stephenson
2023-05-12 16:12               ` Bart Schaefer

Code repositories for project(s) associated with this public inbox

	https://git.vuxu.org/mirror/zsh/

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).