[patch] fixed crash (seg fault) in syntax highlighting

classic Classic list List threaded Threaded
3 messages Options
Reply | Threaded
Open this post in threaded view
|

[patch] fixed crash (seg fault) in syntax highlighting

Dominique Pellé
Hi

In the todo list, I see this crash which was reported about 1 year ago:

--- 8< --- cut here --- 8< --- cut here --- 8< ---

Reproducible crash in syntax HL. (George Reilly, Dominique Pelle, 2009 May 9)

--- 8< --- cut here --- 8< --- cut here --- 8< ---

I can still reproduce it 100% of the time with Vim-7.3a BETA
on Linux as follows:

1/ download "jquery.vim" version 0.1 available at:
  http://www.vim.org/scripts/script.php?script_id=2416

2/ download "jquery-1.3.2.js" available at:
  http://jqueryjs.googlecode.com/files/jquery-1.3.2.js

3/ start vim with:
  vim -u NONE jquery-1.3.2.js

4/ :set nocp
   :syn on
   :so jquery.vim

5/ Go to end of file (jquery-1.3.2.js) with  G  in normal mode

6/ Observe that Vim takes 100% of CPU during a
   fairly long long time (~2 minutes on my laptop).

7/ When cursor is at end of file.  Press 1G   (in normal mode)
   to go back to top of file and  G  again to go back to
   the end of file.

8/ Observe a crash (seg fault, happens 100% of the time)

Valgrind gives this stack trace:

==25132== Invalid read of size 8
==25132==    at 0x57290A: syn_current_attr (syntax.c:1888)
==25132==    by 0x57270D: get_syntax_attr (syntax.c:1771)
==25132==    by 0x53D2F7: win_line (screen.c:3938)
==25132==    by 0x538BA5: win_update (screen.c:1775)
==25132==    by 0x53642C: update_screen (screen.c:525)
==25132==    by 0x4AFD21: main_loop (main.c:1128)
==25132==    by 0x4AF9B2: main (main.c:955)


I just found the root cause: "current_state.ga_len" reaches
quite a high value and an integer overflow happens in assignment
at syntax.c:1445:

syntax.c:
1443         /* When overwriting an existing state stack, clear it first */
1444         clear_syn_state(sp);
1445         sp->sst_stacksize = current_state.ga_len;

current_state.ga_len is an "int" variable which contains 50855
but sp->sst_stacksize is a short!  Since current_state.ga_len
exceeds 32768, an overflow happens and sp->sst_stacksize
becomes negative.

Attached patch fixes by making sp->sst_stacksize an int
rather than short.  Making it an "unsigned short" is also
enough to make it work.  But the value 50855 is too
dangerously close to 65536 so an int is safer.

Vim no longer crash after the attached patch.

Regards
-- Dominique

--
You received this message from the "vim_dev" maillist.
Do not top-post! Type your reply below the text you are replying to.
For more information, visit http://www.vim.org/maillist.php

fixed-crash-HL-7.3aBETA.patch (5K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: [patch] fixed crash (seg fault) in syntax highlighting

Bram Moolenaar

Dominique Pelle wrote:

> In the todo list, I see this crash which was reported about 1 year ago:
>
> --- 8< --- cut here --- 8< --- cut here --- 8< ---
>
> Reproducible crash in syntax HL. (George Reilly, Dominique Pelle, 2009 May 9)
>
> --- 8< --- cut here --- 8< --- cut here --- 8< ---
>
> I can still reproduce it 100% of the time with Vim-7.3a BETA
> on Linux as follows:
>
> 1/ download "jquery.vim" version 0.1 available at:
>   http://www.vim.org/scripts/script.php?script_id=2416
>
> 2/ download "jquery-1.3.2.js" available at:
>   http://jqueryjs.googlecode.com/files/jquery-1.3.2.js
>
> 3/ start vim with:
>   vim -u NONE jquery-1.3.2.js
>
> 4/ :set nocp
>    :syn on
>    :so jquery.vim
>
> 5/ Go to end of file (jquery-1.3.2.js) with  G  in normal mode
>
> 6/ Observe that Vim takes 100% of CPU during a
>    fairly long long time (~2 minutes on my laptop).
>
> 7/ When cursor is at end of file.  Press 1G   (in normal mode)
>    to go back to top of file and  G  again to go back to
>    the end of file.
>
> 8/ Observe a crash (seg fault, happens 100% of the time)
>
> Valgrind gives this stack trace:
>
> ==25132== Invalid read of size 8
> ==25132==    at 0x57290A: syn_current_attr (syntax.c:1888)
> ==25132==    by 0x57270D: get_syntax_attr (syntax.c:1771)
> ==25132==    by 0x53D2F7: win_line (screen.c:3938)
> ==25132==    by 0x538BA5: win_update (screen.c:1775)
> ==25132==    by 0x53642C: update_screen (screen.c:525)
> ==25132==    by 0x4AFD21: main_loop (main.c:1128)
> ==25132==    by 0x4AF9B2: main (main.c:955)
>
>
> I just found the root cause: "current_state.ga_len" reaches
> quite a high value and an integer overflow happens in assignment
> at syntax.c:1445:
>
> syntax.c:
> 1443         /* When overwriting an existing state stack, clear it first */
> 1444         clear_syn_state(sp);
> 1445         sp->sst_stacksize = current_state.ga_len;
>
> current_state.ga_len is an "int" variable which contains 50855
> but sp->sst_stacksize is a short!  Since current_state.ga_len
> exceeds 32768, an overflow happens and sp->sst_stacksize
> becomes negative.
>
> Attached patch fixes by making sp->sst_stacksize an int
> rather than short.  Making it an "unsigned short" is also
> enough to make it work.  But the value 50855 is too
> dangerously close to 65536 so an int is safer.
>
> Vim no longer crash after the attached patch.

I'm glad you managed to find and fix this problem.

I wonder if we should limit the stacksize.  If it has this many items it
must be very slow.  But that may lead to weird syntax HL errors, thus we
can't simply add a limit.

--
hundred-and-one symptoms of being an internet addict:
46. Your wife makes a new rule: "The computer cannot come to bed."

 /// Bram Moolenaar -- [hidden email] -- http://www.Moolenaar.net   \\\
///        sponsor Vim, vote for features -- http://www.Vim.org/sponsor/ \\\
\\\        download, build and distribute -- http://www.A-A-P.org        ///
 \\\            help me help AIDS victims -- http://ICCF-Holland.org    ///

--
You received this message from the "vim_dev" maillist.
Do not top-post! Type your reply below the text you are replying to.
For more information, visit http://www.vim.org/maillist.php
Reply | Threaded
Open this post in threaded view
|

Re: [patch] fixed crash (seg fault) in syntax highlighting

Dominique Pellé
Bram Moolenaar wrote:

> Dominique Pelle wrote:
>
>> In the todo list, I see this crash which was reported about 1 year ago:
>>
>> --- 8< --- cut here --- 8< --- cut here --- 8< ---
>>
>> Reproducible crash in syntax HL. (George Reilly, Dominique Pelle, 2009 May 9)
>>
>> --- 8< --- cut here --- 8< --- cut here --- 8< ---
...snip...
>> Attached patch fixes by making sp->sst_stacksize an int
>> rather than short.  Making it an "unsigned short" is also
>> enough to make it work.  But the value 50855 is too
>> dangerously close to 65536 so an int is safer.
...snip...
>
> I'm glad you managed to find and fix this problem.
>
> I wonder if we should limit the stacksize.  If it has this many items it
> must be very slow.  But that may lead to weird syntax HL errors, thus we
> can't simply add a limit.

I'd rather not put an arbitrary limit.  What is slow today may be fast tomorrow.
I was testing with an old laptop and with Vim compiled without optimizations.
So above test case may already be fast enough on a recent machine with
optimized vim.

Anyway, while Vim is busy computing syntax HL, it's possible to press CTRL-C
to interrupt it.  So that's quite acceptable as it is.

Thanks for applying the patch (7.2.436).

-- Dominique

--
You received this message from the "vim_dev" maillist.
Do not top-post! Type your reply below the text you are replying to.
For more information, visit http://www.vim.org/maillist.php