Segfault with surround.vim on long lines (450 chars), via matchstr()

[Daniel Hahler]

When using `yss<p` (from vim-surround, used to surround the current line) on a
long line (~450 chars), Vim segfaults:

#0 0x00007f7067041ea7 in kill () at ../sysdeps/unix/syscall-template.S:81
#1 0x0000000000563a98 in may_core_dump () at os_unix.c:3376
#2 0x0000000000563a3c in mch_exit (r=1) at os_unix.c:3342
#3 0x000000000064bacd in getout (exitval=1) at main.c:1521
#4 0x000000000051ebe1 in preserve_exit () at misc1.c:9405
#5 0x0000000000561a01 in deathtrap (sigarg=11) at os_unix.c:1121
#6 <signal handler called>
#7 __strncpy_sse2_unaligned () at ../sysdeps/x86_64/multiarch/strcpy-sse2-unaligned.S:296
#8 0x00000000005227b4 in vim_strnsave (string=0x0, len=1732028256) at misc2.c:1268
#9 0x00000000004816cf in find_some_match (argvars=0x7fff7bd98bd0, rettv=0x7fff7bd99250, type=2) at eval.c:14349
#10 0x0000000000481c42 in f_matchstr (argvars=0x7fff7bd98bd0, rettv=0x7fff7bd99250) at eval.c:14549
#11 0x0000000000478eac in call_func (funcname=0x262ea21 "matchstr(keeper,'\\_s\\@<!\\s*$')", len=8, rettv=0x7fff7bd99250, argcount=2, argvars=0x7fff7bd98bd0,
firstline=255, lastline=255, doesrange=0x7fff7bd98d64, evaluate=1, selfdict=0x0) at eval.c:8626
#12 0x00000000004788e4 in get_func_tv (name=0x262ea21 "matchstr(keeper,'\\_s\\@<!\\s*$')", len=8, rettv=0x7fff7bd99250, arg=0x7fff7bd991f8, firstline=255,
lastline=255, doesrange=0x7fff7bd98d64, evaluate=1, selfdict=0x0) at eval.c:8433
#13 0x0000000000474082 in eval7 (arg=0x7fff7bd991f8, rettv=0x7fff7bd99250, evaluate=1, want_string=0) at eval.c:5210
#14 0x0000000000473932 in eval6 (arg=0x7fff7bd991f8, rettv=0x7fff7bd99250, evaluate=1, want_string=0) at eval.c:4861
#15 0x0000000000473466 in eval5 (arg=0x7fff7bd991f8, rettv=0x7fff7bd99250, evaluate=1) at eval.c:4677
#16 0x000000000047277a in eval4 (arg=0x7fff7bd991f8, rettv=0x7fff7bd99250, evaluate=1) at eval.c:4370
#17 0x00000000004725bd in eval3 (arg=0x7fff7bd991f8, rettv=0x7fff7bd99250, evaluate=1) at eval.c:4282
#18 0x000000000047243c in eval2 (arg=0x7fff7bd991f8, rettv=0x7fff7bd99250, evaluate=1) at eval.c:4211
#19 0x000000000047227b in eval1 (arg=0x7fff7bd991f8, rettv=0x7fff7bd99250, evaluate=1) at eval.c:4136
#20 0x00000000004721da in eval0 (arg=0x262ea21 "matchstr(keeper,'\\_s\\@<!\\s*$')", rettv=0x7fff7bd99250, nextcmd=0x7fff7bd99338, evaluate=1) at eval.c:4093
#21 0x000000000046debc in ex_let (eap=0x7fff7bd99330) at eval.c:1913
#22 0x00000000004ad3a8 in do_one_cmd (cmdlinep=0x7fff7bd99450, sourcing=1, cstack=0x7fff7bd99540, fgetline=0x49323b <get_func_line>, cookie=0x22e4040)
at ex_docmd.c:2705
#23 0x00000000004aa77e in do_cmdline (cmdline=0x0, fgetline=0x49323b <get_func_line>, cookie=0x22e4040, flags=7) at ex_docmd.c:1131
#24 0x00000000004926f7 in call_user_func (fp=0x23895a0, argcount=1, argvars=0x7fff7bd99e60, rettv=0x7fff7bd9a020, firstline=255, lastline=255, selfdict=0x0)
at eval.c:23632
#25 0x0000000000478d64 in call_func (funcname=0x245c1f0 "\200\375R55_opfunc", len=12, rettv=0x7fff7bd9a020, argcount=1, argvars=0x7fff7bd99e60, firstline=255,
lastline=255, doesrange=0x7fff7bd99ff0, evaluate=1, selfdict=0x0) at eval.c:8597
#26 0x00000000004788e4 in get_func_tv (name=0x245c1f0 "\200\375R55_opfunc", len=12, rettv=0x7fff7bd9a020, arg=0x7fff7bd99ff8, firstline=255, lastline=255,
doesrange=0x7fff7bd99ff0, evaluate=1, selfdict=0x0) at eval.c:8433
#27 0x0000000000471174 in ex_call (eap=0x7fff7bd9a140) at eval.c:3505
#28 0x00000000004ad3a8 in do_one_cmd (cmdlinep=0x7fff7bd9a260, sourcing=0, cstack=0x7fff7bd9a350, fgetline=0x4c36d0 <getexline>, cookie=0x0) at ex_docmd.c:2705
#29 0x00000000004aa77e in do_cmdline (cmdline=0x0, fgetline=0x4c36d0 <getexline>, cookie=0x0, flags=0) at ex_docmd.c:1131
#30 0x000000000053c0d2 in nv_colon (cap=0x7fff7bd9a8d0) at normal.c:5330
#31 0x0000000000534b42 in normal_cmd (oap=0x7fff7bd9a970, toplevel=1) at normal.c:1160
#32 0x000000000064b7b0 in main_loop (cmdwin=0, noexmode=0) at main.c:1343
#33 0x000000000064b0c7 in main (argc=2, argv=0x7fff7bd9ac78) at main.c:1043
quit


:debug norm yss<p>
Entering Debug mode. Type "cont" to continue.
cmd: norm yss<p>
cmd: call <SNR>55_opfunc(v:count1)
line 1: let char = s:inputreplacement()
line 2: if char == ""
line 4: endif
line 5: let reg = '"'
line 6: let sel_save = &selection
line 7: let &selection = "inclusive"
line 8: let cb_save = &clipboard
line 9: set clipboard-=unnamed clipboard-=unnamedplus
line 10: let reg_save = getreg(reg)
line 11: let reg_type = getregtype(reg)
line 12: let type = a:type
line 13: if a:type == "char"
line 16: elseif a:type == "line"
line 19: elseif a:type ==# "v" || a:type ==# "V" || a:type ==# "\<C-V>"
line 27: elseif a:type =~ '^\d\+$'
line 28: let type = 'v'
line 29: exe 'norm! ^v'.a:type.'$h"'.reg.'y'
line 30: if mode() ==# 'v'
line 33: endif
line 34: else
line 39: let keeper = getreg(reg)
line 40: if type ==# "v" && a:type !=# "v"
line 41: let append = matchstr(keeper,'\_s\@<!\s*$')


Funny detail: when using `s` on `line 41`, instead of `n`, the crash will happen one statement later:

line 42: let keeper = substitute(keeper,'\_s\@<!\s*$','','')


A test file / input can be generated using:
:new
450i1<esc>


This is also related to patch 497, and happens with the latest patches (up to 525), too.

[Bram Moolenaar]

Hmm, patch 7.4.519 should have fixed this.

Can you do this under valgrind, so that you hopefully see the cause of
the problem?

--
Men may not be seen publicly in any kind of strapless gown.
[real standing law in Florida, United States of America]

/// Bram Moolenaar -- Br...@Moolenaar.net -- http://www.Moolenaar.net \\\
/// sponsor Vim, vote for features -- http://www.Vim.org/sponsor/ \\\
\\\ an exciting new programming language -- http://www.Zimbu.org ///
\\\ help me help AIDS victims -- http://ICCF-Holland.org ///

[Daniel Hahler]

Hi Bram,

how should Valgrind be run (which options)?

Here is the output from "valgrind --log-file=/tmp/valgrind.log vim" and reproducing it: http://dpaste.com/22Y96NV


Regards,
Daniel.

[Christian Brabandt]

Am 2014-11-20 12:54, schrieb Bram Moolenaar:
> Daniel Hahler wrote:
>> When using `yss<p` (from vim-surround, used to surround the current
>> line) on a
>> long line (~450 chars), Vim segfaults:

>> [....]

>> #8 0x00000000005227b4 in vim_strnsave (string=0x0, len=1732028256) at
>> misc2.c:1268
>> #9 0x00000000004816cf in find_some_match (argvars=0x7fff7bd98bd0,
>> rettv=0x7fff7bd99250, type=2) at eval.c:14349
>> #10 0x0000000000481c42 in f_matchstr (argvars=0x7fff7bd98bd0,
>> rettv=0x7fff7bd99250) at eval.c:14549

> Hmm, patch 7.4.519 should have fixed this.

Patch 519 did not touch eval.c.
Perhaps the vim_regexc calls there need also to be adjusted? I can look
into it later today.

Best,
Christian

[Christian Brabandt]

Am 2014-11-20 00:40, schrieb Daniel Hahler:
> line 41: let append = matchstr(keeper,'\_s\@<!\s*$')

That is enough to trigger the bug, with a long enough line.

I believe there was still a problem in regexp_nfa.c which
prevented patch 497 to work properly and causes the
"conditional jump depends on uninitialised value"
in valgrind.

Can you check, if the attached patch fixes it for you?

Best,
Christian

[Daniel Hahler]

Yes, it's fixed by applying the patch.


Thanks a lot,
Daniel.

[Dominique Pellé]

Daniel Hahler <google-gr...@thequod.de> wrote:

> Hi Bram,
>
> how should Valgrind be run (which options)?
>
> Here is the output from "valgrind --log-file=/tmp/valgrind.log vim" and reproducing it: http://dpaste.com/22Y96NV
>
>
> Regards,
> Daniel.

Hi

The valgrind errors you see in Python PyObject_Free
are expected. See this for an explanation:
http://svn.python.org/view/python/trunk/Misc/README.valgrind?view=markup

But the other errors such as this one are real
bugs:

==21891== Conditional jump or move depends on uninitialised value(s)
==21891== at 0x522632: lalloc (misc2.c:890)
==21891== by 0x522577: alloc (misc2.c:820)
==21891== by 0x52278F: vim_strnsave (misc2.c:1265)
==21891== by 0x4816CE: find_some_match (eval.c:14349)
==21891== by 0x481C41: f_matchstr (eval.c:14549)
==21891== by 0x478EAB: call_func (eval.c:8626)
==21891== by 0x4788E3: get_func_tv (eval.c:8433)
==21891== by 0x474081: eval7 (eval.c:5210)
==21891== by 0x473931: eval6 (eval.c:4861)
==21891== by 0x473465: eval5 (eval.c:4677)
==21891== by 0x472779: eval4 (eval.c:4370)
==21891== by 0x4725BC: eval3 (eval.c:4282)

For uninitialized memory errors, valgrind gives
more information when using option
--track-origins=yes
at the cost of running slower.

So far I have not been able to reproduce this bug
but it looks like Christian has a patch already!

Regards
Dominique

[Bram Moolenaar]

Daniel Hahler wrote:

> how should Valgrind be run (which options)?
>
> Here is the output from "valgrind --log-file=/tmp/valgrind.log vim" and reproducing it: http://dpaste.com/22Y96NV

These appear to be problems with Python.


--
It is illegal to rob a bank and then shoot at the bank teller with a water
pistol.
[real standing law in Louisana, United States of America]

[Bram Moolenaar]

The problem appears to be that returning NFA_TOO_EXPENSIVE is handled as
non-zero, thus as if there was a match. To make it consistent we should
return <= 0 for no match and > 0 for a match in all the *regexec_nl()
functions. And check for "> 0" in vim_regexec_both().

--
Biting someone with your natural teeth is "simple assault," while biting
someone with your false teeth is "aggravated assault."

[Christian Brabandt]

Hi Bram!

On Do, 20 Nov 2014, Bram Moolenaar wrote:

> The problem appears to be that returning NFA_TOO_EXPENSIVE is handled as
> non-zero, thus as if there was a match. To make it consistent we should
> return <= 0 for no match and > 0 for a match in all the *regexec_nl()
> functions. And check for "> 0" in vim_regexec_both().

Is this supposed to be fixed with 7.4.526? Because I still see

,----[ [nfa_regexec_nl()] ]-
| return (nfa_regexec_both(line, col) != 0);
`----

in regexp_nfa.c That is still wrong.

Best,
Christian
--
Heute sind wir alle voneinander abhängig, niemand kann sich mehr in
seine Festung zurückziehen, ein Inseldasein pflegen.
-- Dalai-Lama

[Bram Moolenaar]

Christian wrote:

> On Do, 20 Nov 2014, Bram Moolenaar wrote:
>
> > The problem appears to be that returning NFA_TOO_EXPENSIVE is handled as
> > non-zero, thus as if there was a match. To make it consistent we should
> > return <= 0 for no match and > 0 for a match in all the *regexec_nl()
> > functions. And check for "> 0" in vim_regexec_both().
>
> Is this supposed to be fixed with 7.4.526? Because I still see
>
> ,----[ [nfa_regexec_nl()] ]-
> | return (nfa_regexec_both(line, col) != 0);
> `----
>
> in regexp_nfa.c That is still wrong.

There was a typo in the description of patch 7.4.526, causing the NFA
changes not to be included. I'll make another patch for that now.

--
<Beeth> Girls are like internet domain names,
the ones I like are already taken.
<honx> Well, you can stil get one from a strange country :-P