Re: Fix for crash in eval.c
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
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
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
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
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 -~--~~~~--~~--~--~---