zsh-workers
 help / color / mirror / code / Atom feed
* [BUG] sigsegv
@ 2023-04-25 13:48 Sebastian Gniazdowski
  2023-04-25 13:56 ` Peter Stephenson
  0 siblings, 1 reply; 15+ messages in thread
From: Sebastian Gniazdowski @ 2023-04-25 13:48 UTC (permalink / raw)
  To: Zsh hackers list

I have to say that current HEAD has many sigsegv errors, at least twp.
The reported in this email is an older problem with alt-/ history
complete, I've had it also with 5.8. I'm still waiting for the 5.9
only problem to occur, it's quite random and hard to reproduce.
To trigger the error, press alt-/ some times.

#0  0x0000000000457868 in insertlinknode (list=list@entry=0x7ffff7abe368,
    node=0x7ffff55600d8, dat=0x7ffff556e8f0) at linklist.c:137
#1  0x00007ffff7bed6bf in histwgetfn (pm=<optimized out>) at parameter.c:1241
#2  0x0000000000467c46 in getvaluearr (v=0x7ffffffe79e0) at params.c:690
#3  0x0000000000467fe5 in getvaluearr (v=0x7ffffffe79e0) at params.c:2503
#4  getarrvalue (v=v@entry=0x7ffffffe79e0) at params.c:2470
#5  0x000000000048d24f in paramsubst (ret_flags=0x7ffffffe7ae4, pf_flags=4,
    qt=0, str=0x7ffffffe7970, n=0x7ffffffe7dd0, l=<optimized out>)
    at subst.c:2712
#6  stringsubst (list=list@entry=0x7ffffffe7d30, node=0x7ffffffe7dd0,
    pf_flags=pf_flags@entry=4, ret_flags=ret_flags@entry=0x7ffffffe7ae4,
    asssub=asssub@entry=0) at subst.c:322
#7  0x0000000000492595 in prefork (list=list@entry=0x7ffffffe7d30,
    flags=flags@entry=6, ret_flags=0x7ffffffe7ae4, ret_flags@entry=0x0)
    at subst.c:142
#8  0x000000000043083e in execcmd_exec (state=state@entry=0x7ffffffe91d0,
    eparams=eparams@entry=0x7ffffffe8e50, input=input@entry=0,
    output=output@entry=0, how=<optimized out>, how@entry=2,
    last1=<optimized out>, last1@entry=2, close_if_forked=-1) at exec.c:4138
#9  0x000000000043091e in execpline2 (state=state@entry=0x7ffffffe91d0,
    pcode=pcode@entry=1283, how=how@entry=2, input=0, output=0,
    last1=last1@entry=0) at exec.c:2003
#10 0x0000000000430c66 in execpline (state=state@entry=0x7ffffffe91d0,
    slcode=<optimized out>, how=how@entry=2, last1=0) at exec.c:1728
#11 0x000000000043243b in execlist (state=state@entry=0x7ffffffe91d0,
    dont_change_job=dont_change_job@entry=1, exiting=exiting@entry=0)
    at exec.c:1482
#12 0x0000000000432c32 in execode (p=0x146e1b0,
    dont_change_job=dont_change_job@entry=1, exiting=exiting@entry=0,
    context=context@entry=0x4a52c3 "loadautofunc") at exec.c:1263
#13 0x000000000043609d in execautofn_basic (do_exec=<optimized out>,
    state=<optimized out>, state=<optimized out>) at exec.c:5578
#14 0x00000000004302d6 in execcmd_exec (state=state@entry=0x7ffffffea860,
    eparams=eparams@entry=0x7ffffffea4e0, input=input@entry=0,
    output=output@entry=0, how=<optimized out>, how@entry=18,
    last1=last1@entry=2, close_if_forked=-1) at exec.c:4005
#15 0x000000000043091e in execpline2 (state=state@entry=0x7ffffffea860,
    pcode=pcode@entry=3, how=how@entry=18, input=0, output=0,
last1=last1@entry=0) at exec.c:2003
#16 0x0000000000430c66 in execpline (state=state@entry=0x7ffffffea860,
    slcode=<optimized out>, how=how@entry=18, last1=0) at exec.c:1728
#17 0x000000000043243b in execlist (state=state@entry=0x7ffffffea860,
    dont_change_job=dont_change_job@entry=1, exiting=exiting@entry=0)
    at exec.c:1482
#18 0x0000000000432c32 in execode (p=p@entry=0x590c10,
    dont_change_job=dont_change_job@entry=1, exiting=exiting@entry=0,
    context=context@entry=0x4a5263 "shfunc") at exec.c:1263
#19 0x0000000000433b82 in runshfunc (prog=prog@entry=0x590c10,
    wrap=wrap@entry=0x0, name=name@entry=0x7ffff7abe168 "_history")
    at exec.c:6148
#20 0x00007ffff7e48c6b in comp_wrapper (name=0x7ffff7abe168 "_history",
    w=0x0, prog=0x590c10) at complete.c:1588
#21 comp_wrapper (prog=0x590c10, w=0x0, name=0x7ffff7abe168 "_history")
    at complete.c:1553
#22 0x00000000004337ed in runshfunc (prog=0x590c10,
    wrap=0x7ffff7e64720 <wrapper>, name=0x7ffff7abe168 "_history")
    at exec.c:6132
…
…
(gdb) frame
#0  0x0000000000457868 in insertlinknode (list=list@entry=0x7ffff7abe368,
    node=0x7ffff55600d8, dat=0x7ffff556e8f0) at linklist.c:137
137         tmp = node->next;
(gdb) p node
$1 = (LinkNode) 0x7ffff55600d8
(gdb) p *node
Cannot access memory at address 0x7ffff55600d8

-- 
Best regards,
Sebastian Gniazdowski


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

* Re: [BUG] sigsegv
  2023-04-25 13:48 [BUG] sigsegv Sebastian Gniazdowski
@ 2023-04-25 13:56 ` Peter Stephenson
  2023-04-28 11:25   ` Sebastian Gniazdowski
  0 siblings, 1 reply; 15+ messages in thread
From: Peter Stephenson @ 2023-04-25 13:56 UTC (permalink / raw)
  To: Zsh hackers list

> On 25/04/2023 14:48 Sebastian Gniazdowski <sgniazdowski@gmail.com> wrote:
> I have to say that current HEAD has many sigsegv errors, at least twp.
> The reported in this email is an older problem with alt-/ history
> complete, I've had it also with 5.8. I'm still waiting for the 5.9
> only problem to occur, it's quite random and hard to reproduce.
> To trigger the error, press alt-/ some times.
> 
> #0  0x0000000000457868 in insertlinknode (list=list@entry=0x7ffff7abe368,
>     node=0x7ffff55600d8, dat=0x7ffff556e8f0) at linklist.c:137
> #1  0x00007ffff7bed6bf in histwgetfn (pm=<optimized out>) at parameter.c:1241

The linked list management itself looks fairly harmless, but I wonder if there's
something in the immediately surrounding code that could be fishy and messing
up memory?

	for (iw = he->nwords - 1; iw >= 0; iw--) {
	    h = he->node.nam + he->words[iw * 2];
	    e = he->node.nam + he->words[iw * 2 + 1];
	    sav = *e;
	    *e = '\0';
	    addlinknode(l, dupstring(h));
	    *e = sav;
	}

Some testing of strlen(he->node.nam) might not come amiss?

Of course this is total guesswork.

pws


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

* Re: [BUG] sigsegv
  2023-04-25 13:56 ` Peter Stephenson
@ 2023-04-28 11:25   ` Sebastian Gniazdowski
  2023-04-28 11:41     ` Peter Stephenson
  0 siblings, 1 reply; 15+ messages in thread
From: Sebastian Gniazdowski @ 2023-04-28 11:25 UTC (permalink / raw)
  To: Peter Stephenson; +Cc: Zsh hackers list

It looks like the last field is incorrect:

(gdb) up
#1  0x00007ffff7e1f628 in histwgetfn (pm=0x508e10)
    at parameter.c:1241
1241                addlinknode(l, dupstring(h));
(gdb) p *l->list->last
Cannot access memory at address 0x7ffff5350068


On Tue, 25 Apr 2023 at 13:56, Peter Stephenson
<p.w.stephenson@ntlworld.com> wrote:
>
> > On 25/04/2023 14:48 Sebastian Gniazdowski <sgniazdowski@gmail.com> wrote:
> > I have to say that current HEAD has many sigsegv errors, at least twp.
> > The reported in this email is an older problem with alt-/ history
> > complete, I've had it also with 5.8. I'm still waiting for the 5.9
> > only problem to occur, it's quite random and hard to reproduce.
> > To trigger the error, press alt-/ some times.
> >
> > #0  0x0000000000457868 in insertlinknode (list=list@entry=0x7ffff7abe368,
> >     node=0x7ffff55600d8, dat=0x7ffff556e8f0) at linklist.c:137
> > #1  0x00007ffff7bed6bf in histwgetfn (pm=<optimized out>) at parameter.c:1241
>
> The linked list management itself looks fairly harmless, but I wonder if there's
> something in the immediately surrounding code that could be fishy and messing
> up memory?
>
>         for (iw = he->nwords - 1; iw >= 0; iw--) {
>             h = he->node.nam + he->words[iw * 2];
>             e = he->node.nam + he->words[iw * 2 + 1];
>             sav = *e;
>             *e = '\0';
>             addlinknode(l, dupstring(h));
>             *e = sav;
>         }
>
> Some testing of strlen(he->node.nam) might not come amiss?
>
> Of course this is total guesswork.
>
> pws
>


-- 
Best regards,
Sebastian Gniazdowski


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

* Re: [BUG] sigsegv
  2023-04-28 11:25   ` Sebastian Gniazdowski
@ 2023-04-28 11:41     ` Peter Stephenson
  2023-04-28 14:13       ` Sebastian Gniazdowski
  0 siblings, 1 reply; 15+ messages in thread
From: Peter Stephenson @ 2023-04-28 11:41 UTC (permalink / raw)
  To: Zsh hackers list

> On 28/04/2023 12:25 Sebastian Gniazdowski <sgniazdowski@gmail.com> wrote:
> It looks like the last field is incorrect:
> 
> (gdb) up
> #1  0x00007ffff7e1f628 in histwgetfn (pm=0x508e10)
>     at parameter.c:1241
> 1241                addlinknode(l, dupstring(h));
> (gdb) p *l->list->last
> Cannot access memory at address 0x7ffff5350068

You'll probably find this is tied to something in your history which
bufferwords() is processing incorrectly, as that's something of a
hack into the lexical analyser.  If you can find what that is we
ought to be able to fix it at source.

However, making this code (taking out history words from a string
given the start and end) more robust seems like a good plan anyway.

pws


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

* Re: [BUG] sigsegv
  2023-04-28 11:41     ` Peter Stephenson
@ 2023-04-28 14:13       ` Sebastian Gniazdowski
  2023-04-28 14:22         ` Peter Stephenson
  0 siblings, 1 reply; 15+ messages in thread
From: Sebastian Gniazdowski @ 2023-04-28 14:13 UTC (permalink / raw)
  To: Peter Stephenson; +Cc: Zsh hackers list

I'm searching for an empty word, i.e.: I'm just pressing alt-/ right
after the shell has started.

On Fri, 28 Apr 2023 at 11:41, Peter Stephenson
<p.w.stephenson@ntlworld.com> wrote:
>
> > On 28/04/2023 12:25 Sebastian Gniazdowski <sgniazdowski@gmail.com> wrote:
> > It looks like the last field is incorrect:
> >
> > (gdb) up
> > #1  0x00007ffff7e1f628 in histwgetfn (pm=0x508e10)
> >     at parameter.c:1241
> > 1241                addlinknode(l, dupstring(h));
> > (gdb) p *l->list->last
> > Cannot access memory at address 0x7ffff5350068
>
> You'll probably find this is tied to something in your history which
> bufferwords() is processing incorrectly, as that's something of a
> hack into the lexical analyser.  If you can find what that is we
> ought to be able to fix it at source.
>
> However, making this code (taking out history words from a string
> given the start and end) more robust seems like a good plan anyway.
>
> pws
>


-- 
Best regards,
Sebastian Gniazdowski


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

* Re: [BUG] sigsegv
  2023-04-28 14:13       ` Sebastian Gniazdowski
@ 2023-04-28 14:22         ` Peter Stephenson
  2023-04-29 10:52           ` Sebastian Gniazdowski
  0 siblings, 1 reply; 15+ messages in thread
From: Peter Stephenson @ 2023-04-28 14:22 UTC (permalink / raw)
  To: Zsh hackers list

> On 28/04/2023 15:13 Sebastian Gniazdowski <sgniazdowski@gmail.com> wrote:
> I'm searching for an empty word, i.e.: I'm just pressing alt-/ right
> after the shell has started.

The crash is when the shell is putting together what's already in your
history into a set of words, so it can use them from completing.  If
you look at the string that is being pared in "bufferwords" at the
point of the crash, which comes from the history entry "he", you should
see a line from your previous command line history.  That's probably
causing the shell some problems --- although there's always the
possibility it might be a previous history entry that's messed things
up, but this is the place to look first.

What you're searching for / completing probably isn't all that important.

Hmm, come to think of it a bit of safety combined with a DPUTS() might
help with both aspects --- I'll see if I've got time for that over
the weekend.

pws


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

* Re: [BUG] sigsegv
  2023-04-28 14:22         ` Peter Stephenson
@ 2023-04-29 10:52           ` Sebastian Gniazdowski
  2023-04-30  9:51             ` Sebastian Gniazdowski
  2023-04-30 17:30             ` Peter Stephenson
  0 siblings, 2 replies; 15+ messages in thread
From: Sebastian Gniazdowski @ 2023-04-29 10:52 UTC (permalink / raw)
  To: Peter Stephenson; +Cc: Zsh hackers list

It says "incomplete sequence \339"  although egrep $'\330' ~/.zhistory
doesn't return anything…

(gdb) frame
#1  0x00007ffff7e1f628 in histwgetfn (pm=0x508f70) at parameter.c:1241
1241                addlinknode(l, dupstring(h));
(gdb) l
1236            for (iw = he->nwords - 1; iw >= 0; iw--) {
1237                h = he->node.nam + he->words[iw * 2];
1238                e = he->node.nam + he->words[iw * 2 + 1];
1239                sav = *e;
1240                *e = '\0';
1241                addlinknode(l, dupstring(h));
1242                *e = sav;
1243            }
1244            he = up_histent(he);
1245        }

(gdb) p he->node.nam + he->words[iw*2]
$31 = 0x7ffff7ae1370 <incomplete sequence \330>
(gdb) p he->node.nam + he->words[iw*2+1]
$32 = 0x7ffff7ae1371 ""
(gdb) p he->node.nam + he->words[(iw-1)*2]
$33 = 0x7ffff7ae134f ""
(gdb) p he->node.nam + he->words[(iw-1)*2+1]
$34 = 0x7ffff7ae1370 <incomplete sequence \330>

I agree that the resilience to incomplete chars should be strengthened.

Is iw the event number?
(gdb) p iw
$35 = 51475

Because history 51474 doesn't return any \330 char either:
51474  angel open
51475  angel open
51476  angel open

On Fri, 28 Apr 2023 at 14:22, Peter Stephenson
<p.w.stephenson@ntlworld.com> wrote:
>
> > On 28/04/2023 15:13 Sebastian Gniazdowski <sgniazdowski@gmail.com> wrote:
> > I'm searching for an empty word, i.e.: I'm just pressing alt-/ right
> > after the shell has started.
>
> The crash is when the shell is putting together what's already in your
> history into a set of words, so it can use them from completing.  If
> you look at the string that is being pared in "bufferwords" at the
> point of the crash, which comes from the history entry "he", you should
> see a line from your previous command line history.  That's probably
> causing the shell some problems --- although there's always the
> possibility it might be a previous history entry that's messed things
> up, but this is the place to look first.
>
> What you're searching for / completing probably isn't all that important.
>
> Hmm, come to think of it a bit of safety combined with a DPUTS() might
> help with both aspects --- I'll see if I've got time for that over
> the weekend.
>
> pws
>


-- 
Best regards,
Sebastian Gniazdowski


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

* Re: [BUG] sigsegv
  2023-04-29 10:52           ` Sebastian Gniazdowski
@ 2023-04-30  9:51             ` Sebastian Gniazdowski
  2023-04-30 17:30             ` Peter Stephenson
  1 sibling, 0 replies; 15+ messages in thread
From: Sebastian Gniazdowski @ 2023-04-30  9:51 UTC (permalink / raw)
  To: Peter Stephenson; +Cc: Zsh hackers list

Is the info not sufficient? What else could I check?

On Sat, 29 Apr 2023 at 10:52, Sebastian Gniazdowski
<sgniazdowski@gmail.com> wrote:
>
> It says "incomplete sequence \339"  although egrep $'\330' ~/.zhistory
> doesn't return anything…
>
> (gdb) frame
> #1  0x00007ffff7e1f628 in histwgetfn (pm=0x508f70) at parameter.c:1241
> 1241                addlinknode(l, dupstring(h));
> (gdb) l
> 1236            for (iw = he->nwords - 1; iw >= 0; iw--) {
> 1237                h = he->node.nam + he->words[iw * 2];
> 1238                e = he->node.nam + he->words[iw * 2 + 1];
> 1239                sav = *e;
> 1240                *e = '\0';
> 1241                addlinknode(l, dupstring(h));
> 1242                *e = sav;
> 1243            }
> 1244            he = up_histent(he);
> 1245        }
>
> (gdb) p he->node.nam + he->words[iw*2]
> $31 = 0x7ffff7ae1370 <incomplete sequence \330>
> (gdb) p he->node.nam + he->words[iw*2+1]
> $32 = 0x7ffff7ae1371 ""
> (gdb) p he->node.nam + he->words[(iw-1)*2]
> $33 = 0x7ffff7ae134f ""
> (gdb) p he->node.nam + he->words[(iw-1)*2+1]
> $34 = 0x7ffff7ae1370 <incomplete sequence \330>
>
> I agree that the resilience to incomplete chars should be strengthened.
>
> Is iw the event number?
> (gdb) p iw
> $35 = 51475
>
> Because history 51474 doesn't return any \330 char either:
> 51474  angel open
> 51475  angel open
> 51476  angel open
>
> On Fri, 28 Apr 2023 at 14:22, Peter Stephenson
> <p.w.stephenson@ntlworld.com> wrote:
> >
> > > On 28/04/2023 15:13 Sebastian Gniazdowski <sgniazdowski@gmail.com> wrote:
> > > I'm searching for an empty word, i.e.: I'm just pressing alt-/ right
> > > after the shell has started.
> >
> > The crash is when the shell is putting together what's already in your
> > history into a set of words, so it can use them from completing.  If
> > you look at the string that is being pared in "bufferwords" at the
> > point of the crash, which comes from the history entry "he", you should
> > see a line from your previous command line history.  That's probably
> > causing the shell some problems --- although there's always the
> > possibility it might be a previous history entry that's messed things
> > up, but this is the place to look first.
> >
> > What you're searching for / completing probably isn't all that important.
> >
> > Hmm, come to think of it a bit of safety combined with a DPUTS() might
> > help with both aspects --- I'll see if I've got time for that over
> > the weekend.
> >
> > pws
> >
>
>
> --
> Best regards,
> Sebastian Gniazdowski



-- 
Best regards,
Sebastian Gniazdowski


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

* Re: [BUG] sigsegv
  2023-04-29 10:52           ` Sebastian Gniazdowski
  2023-04-30  9:51             ` Sebastian Gniazdowski
@ 2023-04-30 17:30             ` Peter Stephenson
  2023-05-03 15:35               ` Sebastian Gniazdowski
  1 sibling, 1 reply; 15+ messages in thread
From: Peter Stephenson @ 2023-04-30 17:30 UTC (permalink / raw)
  To: Zsh hackers list

On Sat, 2023-04-29 at 10:52 +0000, Sebastian Gniazdowski wrote:
> It says "incomplete sequence \339"  although egrep $'\330' ~/.zhistory
> doesn't return anything…

So it looks like this is probably confusion over bad or incomplete
multibyte characters again.

Rather than bufferwords() --- I think that's not relevant at this point
--- this could be the code that reads a history line back into the buffer
and divides it into words getting confused.  This is different from
the code that adds to a history line when it's first generated and
likely to be less accurate --- and also fits better with the
reproducibility of this problem.

Anyway, given there's no single place where the line originates,
and given that we're probably not going to be able to turn it into
a proper line if the there's not a complete character sequence,
safety at the point in question is probably the best we've got.

See if this helps.

pws

diff --git a/Src/Modules/parameter.c b/Src/Modules/parameter.c
index 96a211c69..2067f5bab 100644
--- a/Src/Modules/parameter.c
+++ b/Src/Modules/parameter.c
@@ -1233,9 +1233,16 @@ histwgetfn(UNUSED(Param pm))
             pushnode(l, getdata(n));
 
     while (he) {
+	char *hstr = he->node.nam;
+	int len = strlen(hstr);
 	for (iw = he->nwords - 1; iw >= 0; iw--) {
-	    h = he->node.nam + he->words[iw * 2];
-	    e = he->node.nam + he->words[iw * 2 + 1];
+	    int wbegin = he->words[iw * 2];
+	    int wend = he->words[iw * 2 + 1];
+
+	    if (wbegin >= len || wend > len)
+		break;
+	    h = hstr + wbegin;
+	    e = hstr + wend;
 	    sav = *e;
 	    *e = '\0';
 	    addlinknode(l, dupstring(h));



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

* Re: [BUG] sigsegv
  2023-04-30 17:30             ` Peter Stephenson
@ 2023-05-03 15:35               ` Sebastian Gniazdowski
  2023-05-03 15:56                 ` Sebastian Gniazdowski
  2023-05-03 16:25                 ` Peter Stephenson
  0 siblings, 2 replies; 15+ messages in thread
From: Sebastian Gniazdowski @ 2023-05-03 15:35 UTC (permalink / raw)
  To: Peter Stephenson; +Cc: Zsh hackers list

The error occurs with patch. I've played around in gdb:

(gdb) p (int)(e-hstr)
$65 = -31903

This should be positive – e points to a \0 inserted in hstr. That's
why printing e shows unexpected values. How come wend became negative?

On Sun, 30 Apr 2023 at 17:31, Peter Stephenson
<p.w.stephenson@ntlworld.com> wrote:
>
> On Sat, 2023-04-29 at 10:52 +0000, Sebastian Gniazdowski wrote:
> > It says "incomplete sequence \339"  although egrep $'\330' ~/.zhistory
> > doesn't return anything…
>
> So it looks like this is probably confusion over bad or incomplete
> multibyte characters again.
>
> Rather than bufferwords() --- I think that's not relevant at this point
> --- this could be the code that reads a history line back into the buffer
> and divides it into words getting confused.  This is different from
> the code that adds to a history line when it's first generated and
> likely to be less accurate --- and also fits better with the
> reproducibility of this problem.
>
> Anyway, given there's no single place where the line originates,
> and given that we're probably not going to be able to turn it into
> a proper line if the there's not a complete character sequence,
> safety at the point in question is probably the best we've got.
>
> See if this helps.
>
> pws
>
> diff --git a/Src/Modules/parameter.c b/Src/Modules/parameter.c
> index 96a211c69..2067f5bab 100644
> --- a/Src/Modules/parameter.c
> +++ b/Src/Modules/parameter.c
> @@ -1233,9 +1233,16 @@ histwgetfn(UNUSED(Param pm))
>              pushnode(l, getdata(n));
>
>      while (he) {
> +       char *hstr = he->node.nam;
> +       int len = strlen(hstr);
>         for (iw = he->nwords - 1; iw >= 0; iw--) {
> -           h = he->node.nam + he->words[iw * 2];
> -           e = he->node.nam + he->words[iw * 2 + 1];
> +           int wbegin = he->words[iw * 2];
> +           int wend = he->words[iw * 2 + 1];
> +
> +           if (wbegin >= len || wend > len)
> +               break;
> +           h = hstr + wbegin;
> +           e = hstr + wend;
>             sav = *e;
>             *e = '\0';
>             addlinknode(l, dupstring(h));
>
>


-- 
Best regards,
Sebastian Gniazdowski


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

* Re: [BUG] sigsegv
  2023-05-03 15:35               ` Sebastian Gniazdowski
@ 2023-05-03 15:56                 ` Sebastian Gniazdowski
  2023-05-03 16:25                 ` Peter Stephenson
  1 sibling, 0 replies; 15+ messages in thread
From: Sebastian Gniazdowski @ 2023-05-03 15:56 UTC (permalink / raw)
  To: Peter Stephenson; +Cc: Zsh hackers list

Adding wend<0 to the break-if helps for the crash:
+           if (wbegin >= len || wend > len||wend<0)
+               break;

 Pressing Alt-/ at empty line yields:

16:52[*cal/share/zinit/plugins/zsh]1# !
   (git)-[master●]
zsh: do you wish to see all 1048 possibilities (1080 lines)?

I wonder if 1080 is a much less value than the maximum? Why ! appears
after Alt-/ ? I'm confused if it limits the results?

On Wed, 3 May 2023 at 15:35, Sebastian Gniazdowski
<sgniazdowski@gmail.com> wrote:
>
> The error occurs with patch. I've played around in gdb:
>
> (gdb) p (int)(e-hstr)
> $65 = -31903
>
> This should be positive – e points to a \0 inserted in hstr. That's
> why printing e shows unexpected values. How come wend became negative?
>
> On Sun, 30 Apr 2023 at 17:31, Peter Stephenson
> <p.w.stephenson@ntlworld.com> wrote:
> >
> > On Sat, 2023-04-29 at 10:52 +0000, Sebastian Gniazdowski wrote:
> > > It says "incomplete sequence \339"  although egrep $'\330' ~/.zhistory
> > > doesn't return anything…
> >
> > So it looks like this is probably confusion over bad or incomplete
> > multibyte characters again.
> >
> > Rather than bufferwords() --- I think that's not relevant at this point
> > --- this could be the code that reads a history line back into the buffer
> > and divides it into words getting confused.  This is different from
> > the code that adds to a history line when it's first generated and
> > likely to be less accurate --- and also fits better with the
> > reproducibility of this problem.
> >
> > Anyway, given there's no single place where the line originates,
> > and given that we're probably not going to be able to turn it into
> > a proper line if the there's not a complete character sequence,
> > safety at the point in question is probably the best we've got.
> >
> > See if this helps.
> >
> > pws
> >
> > diff --git a/Src/Modules/parameter.c b/Src/Modules/parameter.c
> > index 96a211c69..2067f5bab 100644
> > --- a/Src/Modules/parameter.c
> > +++ b/Src/Modules/parameter.c
> > @@ -1233,9 +1233,16 @@ histwgetfn(UNUSED(Param pm))
> >              pushnode(l, getdata(n));
> >
> >      while (he) {
> > +       char *hstr = he->node.nam;
> > +       int len = strlen(hstr);
> >         for (iw = he->nwords - 1; iw >= 0; iw--) {
> > -           h = he->node.nam + he->words[iw * 2];
> > -           e = he->node.nam + he->words[iw * 2 + 1];
> > +           int wbegin = he->words[iw * 2];
> > +           int wend = he->words[iw * 2 + 1];
> > +
> > +           if (wbegin >= len || wend > len)
> > +               break;
> > +           h = hstr + wbegin;
> > +           e = hstr + wend;
> >             sav = *e;
> >             *e = '\0';
> >             addlinknode(l, dupstring(h));
> >
> >
>
>
> --
> Best regards,
> Sebastian Gniazdowski



-- 
Best regards,
Sebastian Gniazdowski


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

* Re: [BUG] sigsegv
  2023-05-03 15:35               ` Sebastian Gniazdowski
  2023-05-03 15:56                 ` Sebastian Gniazdowski
@ 2023-05-03 16:25                 ` Peter Stephenson
  2023-05-03 21:13                   ` Bart Schaefer
  2023-05-05 18:54                   ` Peter Stephenson
  1 sibling, 2 replies; 15+ messages in thread
From: Peter Stephenson @ 2023-05-03 16:25 UTC (permalink / raw)
  To: Zsh hackers list

> On 03/05/2023 16:35 Sebastian Gniazdowski <sgniazdowski@gmail.com> wrote:
> The error occurs with patch. I've played around in gdb:
> 
> (gdb) p (int)(e-hstr)
> $65 = -31903
> 
> This should be positive – e points to a \0 inserted in hstr. That's
> why printing e shows unexpected values. How come wend became negative?

Very good question --- I'll add the test for that as a first step but
there shouldn't be too many places in the code where that can come from.

I wonder if it's interpreting a bad status return as a length.

pws


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

* Re: [BUG] sigsegv
  2023-05-03 16:25                 ` Peter Stephenson
@ 2023-05-03 21:13                   ` Bart Schaefer
       [not found]                     ` <CAKc7PVDt6hS26DxC3hDE-ziMXm-K1jqbJXghynpp-ZhpdN_LLw@mail.gmail.com>
  2023-05-05 18:54                   ` Peter Stephenson
  1 sibling, 1 reply; 15+ messages in thread
From: Bart Schaefer @ 2023-05-03 21:13 UTC (permalink / raw)
  To: Peter Stephenson; +Cc: Zsh hackers list

On Wed, May 3, 2023 at 9:25 AM Peter Stephenson
<p.w.stephenson@ntlworld.com> wrote:
>
> > On 03/05/2023 16:35 Sebastian Gniazdowski <sgniazdowski@gmail.com> wrote:
> >
> > (gdb) p (int)(e-hstr)
> > $65 = -31903
> >
> > This should be positive – e points to a \0 inserted in hstr.

I don't suppose there's any chance that e-hstr is actually 4294935393 ?


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

* Re: [BUG] sigsegv
       [not found]                     ` <CAKc7PVDt6hS26DxC3hDE-ziMXm-K1jqbJXghynpp-ZhpdN_LLw@mail.gmail.com>
@ 2023-05-04 15:01                       ` Bart Schaefer
  0 siblings, 0 replies; 15+ messages in thread
From: Bart Schaefer @ 2023-05-04 15:01 UTC (permalink / raw)
  To: Sebastian Gniazdowski, Zsh hackers list

On Wed, May 3, 2023 at 2:20 PM Sebastian Gniazdowski
<sgniazdowski@gmail.com> wrote:
>
> On Wed, 3 May 2023 at 21:14, Bart Schaefer <schaefer@brasslantern.com> wrote:
> >
> > I don't suppose there's any chance that e-hstr is actually 4294935393 ?
> >
>
> I don'r know, bur I've checked (unpatched source):
>
> (gdb) p (unsigned int)(e-he->node.nam)
> $3 = 4294935397

What is "iw" in the loop at that point?  This is looking like an
integer overflow.


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

* Re: [BUG] sigsegv
  2023-05-03 16:25                 ` Peter Stephenson
  2023-05-03 21:13                   ` Bart Schaefer
@ 2023-05-05 18:54                   ` Peter Stephenson
  1 sibling, 0 replies; 15+ messages in thread
From: Peter Stephenson @ 2023-05-05 18:54 UTC (permalink / raw)
  To: zsh-workers

On Wed, 2023-05-03 at 17:25 +0100, Peter Stephenson wrote:
> > On 03/05/2023 16:35 Sebastian Gniazdowski <sgniazdowski@gmail.com> wrote:
> > The error occurs with patch. I've played around in gdb:
> > 
> > (gdb) p (int)(e-hstr)
> > $65 = -31903
> > 
> > This should be positive – e points to a \0 inserted in hstr. That's
> > why printing e shows unexpected values. How come wend became negative?
> 
> Very good question --- I'll add the test for that as a first step but
> there shouldn't be too many places in the code where that can come from.

Here's the first step.

> I wonder if it's interpreting a bad status return as a length.

To be invesitgated, but this might make sense --- this would probably be a
negative integer.

pws


diff --git a/Src/Modules/parameter.c b/Src/Modules/parameter.c
index 96a211c69..a05ea2fe4 100644
--- a/Src/Modules/parameter.c
+++ b/Src/Modules/parameter.c
@@ -1233,9 +1233,16 @@ histwgetfn(UNUSED(Param pm))
             pushnode(l, getdata(n));
 
     while (he) {
+	char *hstr = he->node.nam;
+	int len = strlen(hstr);
 	for (iw = he->nwords - 1; iw >= 0; iw--) {
-	    h = he->node.nam + he->words[iw * 2];
-	    e = he->node.nam + he->words[iw * 2 + 1];
+	    int wbegin = he->words[iw * 2];
+	    int wend = he->words[iw * 2 + 1];
+
+	    if (wbegin < 0 || wbegin >= len || wend < 0 || wend > len)
+		break;
+	    h = hstr + wbegin;
+	    e = hstr + wend;
 	    sav = *e;
 	    *e = '\0';
 	    addlinknode(l, dupstring(h));



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

end of thread, other threads:[~2023-05-05 18:55 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-04-25 13:48 [BUG] sigsegv Sebastian Gniazdowski
2023-04-25 13:56 ` Peter Stephenson
2023-04-28 11:25   ` Sebastian Gniazdowski
2023-04-28 11:41     ` Peter Stephenson
2023-04-28 14:13       ` Sebastian Gniazdowski
2023-04-28 14:22         ` Peter Stephenson
2023-04-29 10:52           ` Sebastian Gniazdowski
2023-04-30  9:51             ` Sebastian Gniazdowski
2023-04-30 17:30             ` Peter Stephenson
2023-05-03 15:35               ` Sebastian Gniazdowski
2023-05-03 15:56                 ` Sebastian Gniazdowski
2023-05-03 16:25                 ` Peter Stephenson
2023-05-03 21:13                   ` Bart Schaefer
     [not found]                     ` <CAKc7PVDt6hS26DxC3hDE-ziMXm-K1jqbJXghynpp-ZhpdN_LLw@mail.gmail.com>
2023-05-04 15:01                       ` Bart Schaefer
2023-05-05 18:54                   ` Peter Stephenson

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).