zsh-workers
 help / color / mirror / code / Atom feed
* segfault in 'ls' completion
@ 2021-12-15 16:39 Vin Shelton
  2021-12-15 20:02 ` Bart Schaefer
  0 siblings, 1 reply; 7+ messages in thread
From: Vin Shelton @ 2021-12-15 16:39 UTC (permalink / raw)
  To: Zsh Hackers' List

[-- Attachment #1: Type: text/plain, Size: 713 bytes --]

Using the latest development version, under endeavourOS (arch Linux):

TST:
PROMPT="%B: %2c %w %T%(#.#.)%b; "
autoload -U compinit
compinit -u

zsh -f
. ./TST
xxx /usr/lib/libpy<TAB>
libpyldb-util.cpython-310-x86-64-linux-gnu.so@
          libpytalloc-util.cpython-310-x86-64-linux-gnu.so.2.3.3*
libpyldb-util.cpython-310-x86-64-linux-gnu.so.2@         libpython3.10.so@
libpyldb-util.cpython-310-x86-64-linux-gnu.so.2.4.1*
    libpython3.10.so.1.0*
libpytalloc-util.cpython-310-x86-64-linux-gnu.so@        libpython3.so*
libpytalloc-util.cpython-310-x86-64-linux-gnu.so.2@

ls /usr/lib/libpy<TAB>zsh: segmentation fault (core dumped)  /opt/bin/zsh -f

I have been unable to generate a core file.

Regards,
  Vin

[-- Attachment #2: Type: text/html, Size: 3035 bytes --]

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

* Re: segfault in 'ls' completion
  2021-12-15 16:39 segfault in 'ls' completion Vin Shelton
@ 2021-12-15 20:02 ` Bart Schaefer
  2021-12-15 20:12   ` Bart Schaefer
  0 siblings, 1 reply; 7+ messages in thread
From: Bart Schaefer @ 2021-12-15 20:02 UTC (permalink / raw)
  To: Vin Shelton; +Cc: Zsh Hackers' List

[-- Attachment #1: Type: text/plain, Size: 4856 bytes --]

On Wed, Dec 15, 2021 at 8:47 AM Vin Shelton <acs@alumni.princeton.edu>
wrote:

> ls /usr/lib/libpy<TAB>zsh: segmentation fault (core dumped)  /opt/bin/zsh
> -f
>
>
I was able to reproduce this; the stack trace is 161 frames deep, last 36
ending here:

#0  0x000055555569614f in ca_parse_line (d=0x55555584de10,
all=0x55555584de10,
    multi=0, first=1) at computil.c:2220
#1  0x0000555555697b16 in bin_comparguments (
    nam=0x7ffff7613cf8 "comparguments", args=0x7ffff7615fc0,
    ops=0x7fffffff0680, func=0) at computil.c:2647
#2  0x0000555555576c5d in execbuiltin (args=0x7ffff7613d38, assigns=0x0,
    bn=0x5555557137c0 <bintab>) at builtin.c:506
#3  0x00005555555a1895 in execcmd_exec (state=0x7fffffff1810,
    eparams=0x7fffffff0bc0, input=0, output=0, how=18, last1=2,
    close_if_forked=-1) at exec.c:4141
#4  0x000055555559b041 in execpline2 (state=0x7fffffff1810, pcode=20995,
    how=18, input=0, output=0, last1=0) at exec.c:1957
#5  0x0000555555599bfa in execpline (state=0x7fffffff1810, slcode=7170,
    how=18, last1=0) at exec.c:1687
#6  0x0000555555598eac in execlist (state=0x7fffffff1810,
dont_change_job=1,
    exiting=0) at exec.c:1442
#7  0x00005555555d46fe in execif (state=0x7fffffff1810, do_exec=0)
    at loop.c:561
#8  0x00005555555a0f3c in execcmd_exec (state=0x7fffffff1810,
    eparams=0x7fffffff1430, input=0, output=0, how=18, last1=2,
    close_if_forked=-1) at exec.c:3961
#9  0x000055555559b041 in execpline2 (state=0x7fffffff1810, pcode=20995,
    how=18, input=0, output=0, last1=0) at exec.c:1957
#10 0x0000555555599bfa in execpline (state=0x7fffffff1810, slcode=1342466,
    how=18, last1=0) at exec.c:1687
#11 0x0000555555598eac in execlist (state=0x7fffffff1810,
dont_change_job=1,
    exiting=0) at exec.c:1442
#12 0x00005555555984d0 in execode (p=0x5555557ea7b0, dont_change_job=1,
    exiting=0, context=0x5555556d3faa "loadautofunc") at exec.c:1219
#13 0x00005555555a52f4 in execautofn_basic (state=0x7fffffff20f0, do_exec=0)
    at exec.c:5528
#14 0x00005555555a0eff in execcmd_exec (state=0x7fffffff20f0,
    eparams=0x7fffffff1d10, input=0, output=0, how=18, last1=2,
    close_if_forked=-1) at exec.c:3959
#15 0x000055555559b041 in execpline2 (state=0x7fffffff20f0, pcode=3,
how=18,
    input=0, output=0, last1=0) at exec.c:1957
#16 0x0000555555599bfa in execpline (state=0x7fffffff20f0, slcode=3074,
    how=18, last1=0) at exec.c:1687
#17 0x0000555555598eac in execlist (state=0x7fffffff20f0,
dont_change_job=1,
    exiting=0) at exec.c:1442
#18 0x00005555555984d0 in execode (p=0x5555557a8900, dont_change_job=1,
    exiting=0, context=0x5555556d412a "shfunc") at exec.c:1219
#19 0x00005555555a6d29 in runshfunc (prog=0x5555557a8900, wrap=0x0,
    name=0x7ffff7613168 "_arguments") at exec.c:6063
#20 0x000055555566b6d0 in comp_wrapper (prog=0x5555557a8900, w=0x0,
    name=0x7ffff7613168 "_arguments") at complete.c:1588
#21 0x00005555555a6b12 in runshfunc (prog=0x5555557a8900,
    wrap=0x5555557134a0 <wrapper>, name=0x7ffff7613168 "_arguments")
    at exec.c:6047
#22 0x00005555555a62e6 in doshfunc (shfunc=0x5555557a7fb0,
    doshargs=0x7ffff761e050, noreturnval=0) at exec.c:5913
#23 0x00005555555a5085 in execshfunc (shf=0x5555557a7fb0,
args=0x7ffff761e050)
    at exec.c:5482
#24 0x00005555555a113f in execcmd_exec (state=0x7fffffff2ff0,
    eparams=0x7fffffff2c10, input=0, output=0, how=18, last1=2,
    close_if_forked=-1) at exec.c:4012
#25 0x000055555559b041 in execpline2 (state=0x7fffffff2ff0, pcode=13379,
    how=18, input=0, output=0, last1=0) at exec.c:1957
#26 0x0000555555599bfa in execpline (state=0x7fffffff2ff0, slcode=7170,
    how=18, last1=0) at exec.c:1687
#27 0x0000555555598eac in execlist (state=0x7fffffff2ff0,
dont_change_job=1,
    exiting=0) at exec.c:1442
#28 0x00005555555984d0 in execode (p=0x5555557ab7b0, dont_change_job=1,
    exiting=0, context=0x5555556d3faa "loadautofunc") at exec.c:1219
#29 0x00005555555a52f4 in execautofn_basic (state=0x7fffffff38d0, do_exec=0)
    at exec.c:5528
#30 0x00005555555a0eff in execcmd_exec (state=0x7fffffff38d0,
    eparams=0x7fffffff34f0, input=0, output=0, how=18, last1=2,
    close_if_forked=-1) at exec.c:3959
#31 0x000055555559b041 in execpline2 (state=0x7fffffff38d0, pcode=3,
how=18,
    input=0, output=0, last1=0) at exec.c:1957
#32 0x0000555555599bfa in execpline (state=0x7fffffff38d0, slcode=3074,
    how=18, last1=0) at exec.c:1687
#33 0x0000555555598eac in execlist (state=0x7fffffff38d0,
dont_change_job=1,
    exiting=0) at exec.c:1442
#34 0x00005555555984d0 in execode (p=0x5555557fb5f0, dont_change_job=1,
    exiting=0, context=0x5555556d412a "shfunc") at exec.c:1219
#35 0x00005555555a6d29 in runshfunc (prog=0x5555557fb5f0, wrap=0x0,
    name=0x7ffff761b168 "_ls") at exec.c:6063
#36 0x000055555566b6d0 in comp_wrapper (prog=0x5555557fb5f0, w=0x0,
    name=0x7ffff761b168 "_ls") at complete.c:1588

[-- Attachment #2: Type: text/html, Size: 5965 bytes --]

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

* Re: segfault in 'ls' completion
  2021-12-15 20:02 ` Bart Schaefer
@ 2021-12-15 20:12   ` Bart Schaefer
  2021-12-15 21:07     ` Bart Schaefer
  2021-12-15 23:57     ` Oliver Kiddle
  0 siblings, 2 replies; 7+ messages in thread
From: Bart Schaefer @ 2021-12-15 20:12 UTC (permalink / raw)
  To: Vin Shelton; +Cc: Zsh Hackers' List

[-- Attachment #1: Type: text/plain, Size: 385 bytes --]

On Wed, Dec 15, 2021 at 12:02 PM Bart Schaefer <schaefer@brasslantern.com>
wrote:

>
> I was able to reproduce this
>

Note that it happens even if there are no files matching /usr/lib/libpy* ..
I reverted to revision e40938c128 (before the workers/49499 changes to
computil.c) and was no longer able to reproduce in that version, but that
does also revert some changes to _arguments.

[-- Attachment #2: Type: text/html, Size: 734 bytes --]

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

* Re: segfault in 'ls' completion
  2021-12-15 20:12   ` Bart Schaefer
@ 2021-12-15 21:07     ` Bart Schaefer
  2021-12-15 23:57     ` Oliver Kiddle
  1 sibling, 0 replies; 7+ messages in thread
From: Bart Schaefer @ 2021-12-15 21:07 UTC (permalink / raw)
  To: Zsh Hackers' List

[-- Attachment #1: Type: text/plain, Size: 313 bytes --]

On Wed, Dec 15, 2021 at 12:12 PM Bart Schaefer <schaefer@brasslantern.com>
wrote:

>
> Note that it happens even if there are no files matching /usr/lib/libpy*
> .. I reverted to revision e40938c128
>

I tried reverting only the delta on computil.c and still reproduced the
crash, so that's not directly related.

[-- Attachment #2: Type: text/html, Size: 668 bytes --]

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

* Re: segfault in 'ls' completion
  2021-12-15 20:12   ` Bart Schaefer
  2021-12-15 21:07     ` Bart Schaefer
@ 2021-12-15 23:57     ` Oliver Kiddle
  2021-12-16  3:40       ` Vin Shelton
  1 sibling, 1 reply; 7+ messages in thread
From: Oliver Kiddle @ 2021-12-15 23:57 UTC (permalink / raw)
  To: Bart Schaefer; +Cc: Vin Shelton, Zsh Hackers' List

Bart Schaefer wrote:
>     I was able to reproduce this

I couldn't initially but as you could, I thought I'd better try again.

> reverted to revision e40938c128 (before the workers/49499 changes to
> computil.c) and was no longer able to reproduce in that version, but that does
> also revert some changes to _arguments.

It actually seems it was 49518 / 7cb980b which was only applied
yesterday having been posted in October and forgotten. I had a nagging
suspicion that I needed to further check over that. My mistake was
mixing up hex and decimal when looking at the ASCII table to work out
how to rearrange the single character option letters within the lookup
array. 20 should have been 0x20 or 32.

'y' appears before the tab and the word starts with something that isn't
'-'. So it uses the + options offset which are later and as y is within
the difference between decimal and hex 20 from the end of the characters
this caused it index beyond the end of the array.

Following this, I also wondered what it's doing strcmping '/usr/libpy'
against every possible ls option. That's nothing new. Note that
_arguments only lets you start options with - or + and we check for
those explicitly in a few places. I think it's worth optimising this
away. The check could perhaps be factored into ca_get_opt() and
ca_get_sopt() ?

If someone has a moment, please check that the calculation in
single_index() makes sense. The array is allocated as
  ret->single = (Caopt *) zalloc(188 * sizeof(Caopt));

Oliver

diff --git a/Src/Zle/computil.c b/Src/Zle/computil.c
index c49d688c8..59abb4cc4 100644
--- a/Src/Zle/computil.c
+++ b/Src/Zle/computil.c
@@ -1088,10 +1088,10 @@ bslashcolon(char *s)
 static int
 single_index(char pre, char opt)
 {
-    if (opt <= 20 || opt > 0x7e)
+    if (opt <= 0x20 || opt > 0x7e)
 	return -1;
 
-    return opt + (pre == '-' ? -21 : 94 - 21);
+    return opt + (pre == '-' ? -0x21 : 94 - 0x21);
 }
 
 /* Parse an argument definition. */
@@ -2158,7 +2158,8 @@ ca_parse_line(Cadef d, Cadef all, int multi, int first)
 
 	/* See if it's an option. */
 
-	if (state.opt == 2 && (state.curopt = ca_get_opt(d, line, 0, &pe)) &&
+	if (state.opt == 2 && (*line == '-' || *line == '+') &&
+	    (state.curopt = ca_get_opt(d, line, 0, &pe)) &&
 	    (state.curopt->type == CAO_OEQUAL ?
 	     (compwords[cur] || pe[-1] == '=') :
 	     (state.curopt->type == CAO_EQUAL ?
@@ -2206,6 +2207,7 @@ ca_parse_line(Cadef d, Cadef all, int multi, int first)
 		state.curopt = NULL;
 	    }
 	} else if (state.opt == 2 && d->single &&
+		   (*line == '-' || *line == '+') &&
 		   ((state.curopt = ca_get_sopt(d, line, &pe, &sopts)) ||
 		    (cur != compcurrent && sopts && nonempty(sopts)))) {
 	    /* Or maybe it's a single-letter option? */


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

* Re: segfault in 'ls' completion
  2021-12-15 23:57     ` Oliver Kiddle
@ 2021-12-16  3:40       ` Vin Shelton
  2021-12-18 10:41         ` Zach Riggle
  0 siblings, 1 reply; 7+ messages in thread
From: Vin Shelton @ 2021-12-16  3:40 UTC (permalink / raw)
  To: Oliver Kiddle; +Cc: Bart Schaefer, Zsh Hackers' List

[-- Attachment #1: Type: text/plain, Size: 3143 bytes --]

Stops the segfault, and generates a conforming list.

Thanks,
  Vin

On Wed, Dec 15, 2021 at 6:57 PM Oliver Kiddle <opk@zsh.org> wrote:

> Bart Schaefer wrote:
> >     I was able to reproduce this
>
> I couldn't initially but as you could, I thought I'd better try again.
>
> > reverted to revision e40938c128 (before the workers/49499 changes to
> > computil.c) and was no longer able to reproduce in that version, but
> that does
> > also revert some changes to _arguments.
>
> It actually seems it was 49518 / 7cb980b which was only applied
> yesterday having been posted in October and forgotten. I had a nagging
> suspicion that I needed to further check over that. My mistake was
> mixing up hex and decimal when looking at the ASCII table to work out
> how to rearrange the single character option letters within the lookup
> array. 20 should have been 0x20 or 32.
>
> 'y' appears before the tab and the word starts with something that isn't
> '-'. So it uses the + options offset which are later and as y is within
> the difference between decimal and hex 20 from the end of the characters
> this caused it index beyond the end of the array.
>
> Following this, I also wondered what it's doing strcmping '/usr/libpy'
> against every possible ls option. That's nothing new. Note that
> _arguments only lets you start options with - or + and we check for
> those explicitly in a few places. I think it's worth optimising this
> away. The check could perhaps be factored into ca_get_opt() and
> ca_get_sopt() ?
>
> If someone has a moment, please check that the calculation in
> single_index() makes sense. The array is allocated as
>   ret->single = (Caopt *) zalloc(188 * sizeof(Caopt));
>
> Oliver
>
> diff --git a/Src/Zle/computil.c b/Src/Zle/computil.c
> index c49d688c8..59abb4cc4 100644
> --- a/Src/Zle/computil.c
> +++ b/Src/Zle/computil.c
> @@ -1088,10 +1088,10 @@ bslashcolon(char *s)
>  static int
>  single_index(char pre, char opt)
>  {
> -    if (opt <= 20 || opt > 0x7e)
> +    if (opt <= 0x20 || opt > 0x7e)
>         return -1;
>
> -    return opt + (pre == '-' ? -21 : 94 - 21);
> +    return opt + (pre == '-' ? -0x21 : 94 - 0x21);
>  }
>
>  /* Parse an argument definition. */
> @@ -2158,7 +2158,8 @@ ca_parse_line(Cadef d, Cadef all, int multi, int
> first)
>
>         /* See if it's an option. */
>
> -       if (state.opt == 2 && (state.curopt = ca_get_opt(d, line, 0, &pe))
> &&
> +       if (state.opt == 2 && (*line == '-' || *line == '+') &&
> +           (state.curopt = ca_get_opt(d, line, 0, &pe)) &&
>             (state.curopt->type == CAO_OEQUAL ?
>              (compwords[cur] || pe[-1] == '=') :
>              (state.curopt->type == CAO_EQUAL ?
> @@ -2206,6 +2207,7 @@ ca_parse_line(Cadef d, Cadef all, int multi, int
> first)
>                 state.curopt = NULL;
>             }
>         } else if (state.opt == 2 && d->single &&
> +                  (*line == '-' || *line == '+') &&
>                    ((state.curopt = ca_get_sopt(d, line, &pe, &sopts)) ||
>                     (cur != compcurrent && sopts && nonempty(sopts)))) {
>             /* Or maybe it's a single-letter option? */
>

[-- Attachment #2: Type: text/html, Size: 4349 bytes --]

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

* Re: segfault in 'ls' completion
  2021-12-16  3:40       ` Vin Shelton
@ 2021-12-18 10:41         ` Zach Riggle
  0 siblings, 0 replies; 7+ messages in thread
From: Zach Riggle @ 2021-12-18 10:41 UTC (permalink / raw)
  To: Vin Shelton; +Cc: Oliver Kiddle, Bart Schaefer, Zsh Hackers' List

Has anybody actually tried running ZSH with memory safety options
enabled (e.g. Address Sanitizer)?

I assume there is a test suite available to run -- I can build Zsh
from source, but I'm not sure the "right" way to build the tests.

I've subscribed to the zsh-devel mailing list as I expect this is
better suited for that ML.

Zach Riggle

On Wed, Dec 15, 2021 at 9:41 PM Vin Shelton <acs@alumni.princeton.edu> wrote:
>
> Stops the segfault, and generates a conforming list.
>
> Thanks,
>   Vin
>
> On Wed, Dec 15, 2021 at 6:57 PM Oliver Kiddle <opk@zsh.org> wrote:
>>
>> Bart Schaefer wrote:
>> >     I was able to reproduce this
>>
>> I couldn't initially but as you could, I thought I'd better try again.
>>
>> > reverted to revision e40938c128 (before the workers/49499 changes to
>> > computil.c) and was no longer able to reproduce in that version, but that does
>> > also revert some changes to _arguments.
>>
>> It actually seems it was 49518 / 7cb980b which was only applied
>> yesterday having been posted in October and forgotten. I had a nagging
>> suspicion that I needed to further check over that. My mistake was
>> mixing up hex and decimal when looking at the ASCII table to work out
>> how to rearrange the single character option letters within the lookup
>> array. 20 should have been 0x20 or 32.
>>
>> 'y' appears before the tab and the word starts with something that isn't
>> '-'. So it uses the + options offset which are later and as y is within
>> the difference between decimal and hex 20 from the end of the characters
>> this caused it index beyond the end of the array.
>>
>> Following this, I also wondered what it's doing strcmping '/usr/libpy'
>> against every possible ls option. That's nothing new. Note that
>> _arguments only lets you start options with - or + and we check for
>> those explicitly in a few places. I think it's worth optimising this
>> away. The check could perhaps be factored into ca_get_opt() and
>> ca_get_sopt() ?
>>
>> If someone has a moment, please check that the calculation in
>> single_index() makes sense. The array is allocated as
>>   ret->single = (Caopt *) zalloc(188 * sizeof(Caopt));
>>
>> Oliver
>>
>> diff --git a/Src/Zle/computil.c b/Src/Zle/computil.c
>> index c49d688c8..59abb4cc4 100644
>> --- a/Src/Zle/computil.c
>> +++ b/Src/Zle/computil.c
>> @@ -1088,10 +1088,10 @@ bslashcolon(char *s)
>>  static int
>>  single_index(char pre, char opt)
>>  {
>> -    if (opt <= 20 || opt > 0x7e)
>> +    if (opt <= 0x20 || opt > 0x7e)
>>         return -1;
>>
>> -    return opt + (pre == '-' ? -21 : 94 - 21);
>> +    return opt + (pre == '-' ? -0x21 : 94 - 0x21);
>>  }
>>
>>  /* Parse an argument definition. */
>> @@ -2158,7 +2158,8 @@ ca_parse_line(Cadef d, Cadef all, int multi, int first)
>>
>>         /* See if it's an option. */
>>
>> -       if (state.opt == 2 && (state.curopt = ca_get_opt(d, line, 0, &pe)) &&
>> +       if (state.opt == 2 && (*line == '-' || *line == '+') &&
>> +           (state.curopt = ca_get_opt(d, line, 0, &pe)) &&
>>             (state.curopt->type == CAO_OEQUAL ?
>>              (compwords[cur] || pe[-1] == '=') :
>>              (state.curopt->type == CAO_EQUAL ?
>> @@ -2206,6 +2207,7 @@ ca_parse_line(Cadef d, Cadef all, int multi, int first)
>>                 state.curopt = NULL;
>>             }
>>         } else if (state.opt == 2 && d->single &&
>> +                  (*line == '-' || *line == '+') &&
>>                    ((state.curopt = ca_get_sopt(d, line, &pe, &sopts)) ||
>>                     (cur != compcurrent && sopts && nonempty(sopts)))) {
>>             /* Or maybe it's a single-letter option? */


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

end of thread, other threads:[~2021-12-18 10:46 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-12-15 16:39 segfault in 'ls' completion Vin Shelton
2021-12-15 20:02 ` Bart Schaefer
2021-12-15 20:12   ` Bart Schaefer
2021-12-15 21:07     ` Bart Schaefer
2021-12-15 23:57     ` Oliver Kiddle
2021-12-16  3:40       ` Vin Shelton
2021-12-18 10:41         ` Zach Riggle

Code repositories for project(s) associated with this 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).