Re: Fix for crash in eval.c

2009-05-03 Fir de Conversatie Dominique Pellé

Nico Weber wrote:

> Hi,
>
> On 03.05.2009, at 00:03, Dominique Pellé wrote:
>
>> After applying your patch, there are no such errors anymore.
>>
>> However, when exiting, I see that those blocks are not being
>> freed:
>>
>> ==16990== 217 bytes in 10 blocks are possibly lost in loss record 36
>> of 57
>> ==16990==    at 0x402603E: malloc (vg_replace_malloc.c:207)
>> ==16990==    by 0x81142FA: lalloc (misc2.c:866)
>> ==16990==    by 0x8114216: alloc (misc2.c:765)
>> ==16990==    by 0x807AD1D: dictitem_alloc (eval.c:6775)
>> ==16990==    by 0x8074FFD: set_var_lval (eval.c:2856)
>> ==16990==    by 0x80742F4: ex_let_one (eval.c:2414)
>> ==16990==    by 0x807329F: ex_let_vars (eval.c:1869)
>> ==16990==    by 0x8073250: ex_let (eval.c:1834)
>> ==16990==    by 0x80A6AA3: do_one_cmd (ex_docmd.c:2622)
>> ==16990==    by 0x80A4323: do_cmdline (ex_docmd.c:1096)
>> ==16990==    by 0x8090328: call_user_func (eval.c:21301)
>> ==16990==    by 0x807C4FE: call_func (eval.c:8079)
>> ==16990==    by 0x807C142: get_func_tv (eval.c:7925)
>> ==16990==    by 0x8075B83: ex_call (eval.c:)
>> ==16990==    by 0x80A6AA3: do_one_cmd (ex_docmd.c:2622)
>> ==16990==    by 0x80A4323: do_cmdline (ex_docmd.c:1096)
>> ==16990==    by 0x812A758: nv_colon (normal.c:5227)
>> ==16990==    by 0x8123DA2: normal_cmd (normal.c:1189)
>> ==16990==    by 0x80E6D49: main_loop (main.c:1180)
>> ==16990==    by 0x80E6896: main (main.c:939)
>>
>> I built Vim with -DEXITFREE (i.e. uncommented line
>> PROFILE_CFLAGS = -DEXITFREE  in src/Makefile)
>> so normally all blocks should be freed when exiting.
>
> Thanks for checking.
>
> I've attached an updated version of the patch that fixes some of the
> leaks. The copy()d dicts in the script don't get freed, but I don't
> (yet?) understand why.
>
> Nico


I tried to fix the leak as well but without success so far.
It does not seem simple.

In any cases, it's a real leak, running the following leak.vim
script for example causes Vim memory usage to grow
continuously:

--- 8< --- cut here --- 8< --- cut here --- 8< ---
set nocp

" foo.vim is the script attached in the original bug submission
" at 
http://groups.google.com/group/vim_mac/browse_thread/thread/4e0149ff4f84e3d3
so foo.vim

let g:count_loops = 0
while 1
  call foo#Buffer.New()
  q
  let g:count_loops = g:count_loops + 1
endwhile
--- 8< --- cut here --- 8< --- cut here --- 8< ---

I do:

$ vim -u NONE leak.vim
:so %

Then in another xterm, I can see with "top" that memory usage
of Vim keeps increasing.

If I run the leak.vim script with Valgrind...

valgrind --leak-resolution=high --leak-check=yes \
--num-callers=30 --track-fds=yes 2> vg.log \
   ./vim -u NONE leak.vim

Then do:
- :so %
- let the script execute for some time
- press CTRL-C to interrupt the infinite loop
- Look at how many times loop iterated with
   :echo g:count_loops
  (it prints 2774 for example)
- then quit vim: qa!

Then look for leaks in vg.log, and observe that some leaks
happen exactly 2774 times (same number as number as
g:loop_count):

==22962== 27,740 bytes in 2,774 blocks are possibly lost in loss
record 113 of 125
==22962==at 0x402603E: malloc (vg_replace_malloc.c:207)
==22962==by 0x81147FD: lalloc (misc2.c:866)
==22962==by 0x8114708: alloc (misc2.c:765)
==22962==by 0x8114C16: vim_strsave (misc2.c:1177)
==22962==by 0x808C427: copy_tv (eval.c:19380)
==22962==by 0x808AE51: get_var_tv (eval.c:18452)
==22962==by 0x80785E7: eval7 (eval.c:5032)
==22962==by 0x8077EA3: eval6 (eval.c:4685)
==22962==by 0x8077A8F: eval5 (eval.c:4501)
==22962==by 0x8076FE0: eval4 (eval.c:4196)
==22962==by 0x8076E38: eval3 (eval.c:4108)
==22962==by 0x8076CC4: eval2 (eval.c:4037)
==22962==by 0x8076AF4: eval1 (eval.c:3962)
==22962==by 0x8079667: get_list_tv (eval.c:5675)
==22962==by 0x807837D: eval7 (eval.c:4943)
==22962==by 0x8077EA3: eval6 (eval.c:4685)
==22962==by 0x8077A8F: eval5 (eval.c:4501)
==22962==by 0x8076FE0: eval4 (eval.c:4196)
==22962==by 0x8076E38: eval3 (eval.c:4108)
==22962==by 0x8076CC4: eval2 (eval.c:4037)
==22962==by 0x8076AF4: eval1 (eval.c:3962)
==22962==by 0x8076A5B: eval0 (eval.c:3919)
==22962==by 0x807326C: ex_let (eval.c:1833)
==22962==by 0x80A6C8B: do_one_cmd (ex_docmd.c:2622)
==22962==by 0x80A450B: do_cmdline (ex_docmd.c:1096)
==22962==by 0x8090507: call_user_func (eval.c:21381)
==22962==by 0x807C6DD: call_func (eval.c:8159)
==22962==by 0x807C321: get_func_tv (eval.c:8005)
==22962==by 0x8075C03: ex_call (eval.c:3341)
==22962==by 0x80A6C8B: do_one_cmd (ex_docmd.c:2622)
==22962==
==22962==
==22962== 33,288 bytes in 5,548 blocks are possibly lost in loss
record 114 of 125
==22962==at 0x402603E: malloc (vg_replace_malloc.c:207)
==22962==by 0x81147FD: lalloc (misc2.c:866)
==22962==by 0x8114708: alloc (misc2.c:765)
==22962==by 0x8114C16: vim_strsave (misc2.c:1177)
==22962==by 0x808C427: copy_tv (eval.c:19380)
==22962==by 0x807B117: dict

Re: Fix for crash in eval.c

2009-05-03 Fir de Conversatie Nico Weber
Hi,

On 03.05.2009, at 00:03, Dominique Pellé wrote:

> After applying your patch, there are no such errors anymore.
>
> However, when exiting, I see that those blocks are not being
> freed:
>
> ==16990== 217 bytes in 10 blocks are possibly lost in loss record 36  
> of 57
> ==16990==at 0x402603E: malloc (vg_replace_malloc.c:207)
> ==16990==by 0x81142FA: lalloc (misc2.c:866)
> ==16990==by 0x8114216: alloc (misc2.c:765)
> ==16990==by 0x807AD1D: dictitem_alloc (eval.c:6775)
> ==16990==by 0x8074FFD: set_var_lval (eval.c:2856)
> ==16990==by 0x80742F4: ex_let_one (eval.c:2414)
> ==16990==by 0x807329F: ex_let_vars (eval.c:1869)
> ==16990==by 0x8073250: ex_let (eval.c:1834)
> ==16990==by 0x80A6AA3: do_one_cmd (ex_docmd.c:2622)
> ==16990==by 0x80A4323: do_cmdline (ex_docmd.c:1096)
> ==16990==by 0x8090328: call_user_func (eval.c:21301)
> ==16990==by 0x807C4FE: call_func (eval.c:8079)
> ==16990==by 0x807C142: get_func_tv (eval.c:7925)
> ==16990==by 0x8075B83: ex_call (eval.c:)
> ==16990==by 0x80A6AA3: do_one_cmd (ex_docmd.c:2622)
> ==16990==by 0x80A4323: do_cmdline (ex_docmd.c:1096)
> ==16990==by 0x812A758: nv_colon (normal.c:5227)
> ==16990==by 0x8123DA2: normal_cmd (normal.c:1189)
> ==16990==by 0x80E6D49: main_loop (main.c:1180)
> ==16990==by 0x80E6896: main (main.c:939)
>
> I built Vim with -DEXITFREE (i.e. uncommented line
> PROFILE_CFLAGS = -DEXITFREE  in src/Makefile)
> so normally all blocks should be freed when exiting.

Thanks for checking.

I've attached an updated version of the patch that fixes some of the  
leaks. The copy()d dicts in the script don't get freed, but I don't  
(yet?) understand why.

Nico


--~--~-~--~~~---~--~~
You received this message from the "vim_dev" maillist.
For more information, visit http://www.vim.org/maillist.php
-~--~~~~--~~--~--~---



eval_crash2.patch
Description: Binary data



Re: Add support for guidecolumn in VIM

2009-05-03 Fir de Conversatie Ricky
Hi,

  I compile with BIG and OLE features then copy the gvim.exe to override my old 
version, but it not work.
  I use : 
  set mc=80 or
  set mc=72
  both not highlight a column on the screen.

 Is there anything wrong?


From: _Lone 
Sent: Sunday, May 03, 2009 9:34 AM
To: vim_dev@googlegroups.com 
Subject: Re: Add support for guidecolumn in VIM





On Fri, May 1, 2009 at 6:55 AM, Bram Moolenaar  wrote:



  Pankaj Garg wrote:

  > On Apr 30, 9:30=A0am, Ben Fritz  wrote:

  > > On Apr 29, 11:42=A0am, Bram Moolenaar  wrote:
  > >
  > > > The name 'guidecolumn' starts with "gui", which is confusing, since it

  > > > also works in a terminal. =A0'margincolumn' perhaps?

  > >
  > > I agree. If a user uses :help gui for example, they would NOT be
  > > expecting 'guidecolumn' to show up!
  > >
  > > > I think the value "0" should be used to have a column at 'textwidth'.
  > > > That way you can see where a line will be broken when it's formatted,

  > > > without having to set two options. =A0A negative value can be used to

  > > > disable the column.
  > >
  > > I think we should try to make this option consistent with other
  > > options. 'textwidth' and 'wrapmargin' and others use a value of 0 to
  > > disable it, and making this one use negative numbers may get
  > > confusing.
  > >
  > > Is it possible to use string or character values, or can only numbers
  > > be entered? I can't think of any options that do this, but things like
  > > line() can take several strings with special meaning. Perhaps the
  > > guidecolumn could be disabled when set to 0, set to the textwidth when
  > > set to "tw", and set to the wrap margin when set to "wm"? I think
  > > something like this would be more consistent with other options and
  > > therefore more intuitive for users.
  > >
  > > > I didn't have time to check for any problems with this patch, I hope
  > > > others have tried it out and report any problems noticed.
  > >
  > > I haven't tried the patch, but I'm certainly interested in the
  > > feature. I'd probably use this instead of the autocmds I have set up
  > > to use a matchadd() on long lines.
  > >
  > > What is this option local to? If it's just global at the moment, I
  > > think it should be local either to the buffer (because that's what
  > > textwidth is local to) or the window. I could see a use case where a
  > > user might want a guidecolumn for the file in one window, but turn it
  > > off for a window in diff mode in another tab or something.
  >
  > The option is local to window. I agree margincolumn is a better name.
  > I am also fine with a value of 0 meaning textwidth, however as Ben
  > mentioned, that seems a little unintuitive. Should we convert it to
  > string?
  > Are there other example where numeric options are taking string?
  >
  > Please let me know what you prefer and I can do whatever option
  > everyone agrees to.


  It's possible to use a string, but more difficult to handle.  And easier
  to set a wrong value, thus more error checking is needed.
  If we want to use zero for "off", we could use -1 for using the
  'textwidth' value.

  --
  Time flies like an arrow.
  Fruit flies like a banana.


   /// Bram Moolenaar -- b...@moolenaar.net -- 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///



  I have modified the patch. The name is now margincolumn. The behavior is:
  'mc' = 0 -> off
  'mc' > 0 -> highlightes the column.
  'mc' < 0 -> makes 'mc' = 'tw + 1' and highlightes that column.

  I also updated the related documentation is option.txt.

  Thanks
  _Lone

  

--~--~-~--~~~---~--~~
You received this message from the "vim_dev" maillist.
For more information, visit http://www.vim.org/maillist.php
-~--~~~~--~~--~--~---



Re: Fix for crash in eval.c

2009-05-03 Fir de Conversatie Kana Natsuno

On Sun, 03 May 2009 08:06:13 +0900, Nico Weber  wrote:
> Valgrind memory checker finds several errors in vim-7.2 (patches
> 1-148) with the reproduction steps described at 
> http://groups.google.com/group/vim_mac/browse_thread/thread/4e0149ff4f84e3d3
>   :

Thank you for the patch!
I encountered the same problem on Mac OS X and GNU/Linux since 7.2.70,
but I couldn't reproduce the problem, so that I couldn't send a report on it.
Now I should subscribe also vim_mac.


-- 
To Vim, or not to Vim.
http://whileimautomaton.net/

--~--~-~--~~~---~--~~
You received this message from the "vim_dev" maillist.
For more information, visit http://www.vim.org/maillist.php
-~--~~~~--~~--~--~---



Re: Fix for crash in eval.c

2009-05-03 Fir de Conversatie Dominique Pellé

Nico Weber wrote:

> Hi,
>
> Valgrind memory checker finds several errors in vim-7.2 (patches
> 1-148) with the reproduction steps described at 
> http://groups.google.com/group/vim_mac/browse_thread/thread/4e0149ff4f84e3d3
>  :
>
> ==33469== Conditional jump or move depends on uninitialised value(s)
> ==33469==    at 0x437EA: can_free_funccal (eval.c:21449)
> ==33469==    by 0x2D213: garbage_collect (eval.c:6591)
> ==33469==    by 0x8D92E: before_blocking (getchar.c:1473)
> ==33469==    by 0x10764F: mch_inchar (os_unix.c:385)
> ==33469==    by 0x176A06: ui_inchar (ui.c:193)
> ==33469==    by 0x8FFD1: inchar (getchar.c:2959)
> ==33469==    by 0x8FB64: vgetorpeek (getchar.c:2735)
> ==33469==    by 0x8DAA3: vgetc (getchar.c:1552)
> ==33469==    by 0x8E05D: safe_vgetc (getchar.c:1757)
> ==33469==    by 0xDC89D: normal_cmd (normal.c:653)
> ==33469==    by 0x9F674: main_loop (main.c:1255)
> ==33469==    by 0x9F167: main (main.c:1002)

...snip...

> On OS X, this leads to a crash. The problem was found by Meikel
> Brandmeyer.
>
> The attached patch fixes this.
>
> There were two problems:
>
> 1. Without
>
>     dict->dv_copyID = 0;
>
> the l_vars and l_avars dicts in funccall_T have no initialized
> dv_copyID at the end of call_user_func, and hence valgrind complains
> when garbage_collect walks the previous_funccal list
>
> 2. Double free. garbage_collect() frees dictionaries and lists,
> including the ones belonging to functions in the previous_funccal
> list. When the previou_funccal list is freed, these dictionaries and
> lists are freed a second time.
>
> Nico


Thanks.  I can confirm that Valgrind detects the same errors
on Linux x86 too using Vim-7.2.166.  I did not try it earlier
because I don't follow the mailing list vim_mac.

So bug is not mac specific, even though it does not cause
a crash on Linux.

After applying your patch, there are no such errors anymore.

However, when exiting, I see that those blocks are not being
freed:

==16990== 217 bytes in 10 blocks are possibly lost in loss record 36 of 57
==16990==at 0x402603E: malloc (vg_replace_malloc.c:207)
==16990==by 0x81142FA: lalloc (misc2.c:866)
==16990==by 0x8114216: alloc (misc2.c:765)
==16990==by 0x807AD1D: dictitem_alloc (eval.c:6775)
==16990==by 0x8074FFD: set_var_lval (eval.c:2856)
==16990==by 0x80742F4: ex_let_one (eval.c:2414)
==16990==by 0x807329F: ex_let_vars (eval.c:1869)
==16990==by 0x8073250: ex_let (eval.c:1834)
==16990==by 0x80A6AA3: do_one_cmd (ex_docmd.c:2622)
==16990==by 0x80A4323: do_cmdline (ex_docmd.c:1096)
==16990==by 0x8090328: call_user_func (eval.c:21301)
==16990==by 0x807C4FE: call_func (eval.c:8079)
==16990==by 0x807C142: get_func_tv (eval.c:7925)
==16990==by 0x8075B83: ex_call (eval.c:)
==16990==by 0x80A6AA3: do_one_cmd (ex_docmd.c:2622)
==16990==by 0x80A4323: do_cmdline (ex_docmd.c:1096)
==16990==by 0x812A758: nv_colon (normal.c:5227)
==16990==by 0x8123DA2: normal_cmd (normal.c:1189)
==16990==by 0x80E6D49: main_loop (main.c:1180)
==16990==by 0x80E6896: main (main.c:939)

I built Vim with -DEXITFREE (i.e. uncommented line
PROFILE_CFLAGS = -DEXITFREE  in src/Makefile)
so normally all blocks should be freed when exiting.

Regards
-- Dominique

--~--~-~--~~~---~--~~
You received this message from the "vim_dev" maillist.
For more information, visit http://www.vim.org/maillist.php
-~--~~~~--~~--~--~---