On Sun, Mar 16, 2008 at 2:21 PM, Bram Moolenaar <[EMAIL PROTECTED]> wrote:
> Dominique Pelle wrote: > >> > Valgrind memory checker detects the following bug in Vim-7.1.280: >> > >> > ==11795== Invalid read of size 1 >> > ==11795== at 0x4023572: strncmp (mc_replace_strmem.c:318) >> > ==11795== by 0x80B6653: getcmdline (ex_getln.c:556) >> > ==11795== by 0x80B940B: getexline (ex_getln.c:2100) >> > ==11795== by 0x80A2F3E: do_cmdline (ex_docmd.c:995) >> > ==11795== by 0x8129B52: nv_colon (normal.c:5179) >> > ==11795== by 0x812310E: normal_cmd (normal.c:1152) >> > ==11795== by 0x80E5F41: main_loop (main.c:1181) >> > ==11795== by 0x80E5A91: main (main.c:940) >> > ==11795== Address 0x56484AA is 2 bytes inside a block of size 100 free'd >> > ==11795== at 0x402237F: free (vg_replace_malloc.c:233) >> > ==11795== by 0x8114251: vim_free (misc2.c:1580) >> > ==11795== by 0x80B9D01: realloc_cmdbuff (ex_getln.c:2509) >> > ==11795== by 0x80BA194: put_on_cmdline (ex_getln.c:2708) >> > ==11795== by 0x80B8B8F: getcmdline (ex_getln.c:1679) >> > ==11795== by 0x80B940B: getexline (ex_getln.c:2100) >> > ==11795== by 0x80A2F3E: do_cmdline (ex_docmd.c:995) >> > ==11795== by 0x8129B52: nv_colon (normal.c:5179) >> > ==11795== by 0x812310E: normal_cmd (normal.c:1152) >> > ==11795== by 0x80E5F41: main_loop (main.c:1181) >> > ==11795== by 0x80E5A91: main (main.c:940) >> > (more errors of this kind follow when it happens) >> > >> > Bug is unfortunately difficult to reproduce. I have not found >> > a systematic way of reproducing it but I have encountered it a >> > couple of times. >> > >> > From the above stack traces, it is easy enough to understand though: >> > xpc.xp_pattern normally points to somewhere inside ccline.cmdbuff. >> > But at line ex_getln.c:556, it points to freed memory according to >> > valgrind, because a previous call to put_on_cmdline() has >> > reallocated ccline.cmdbuff. Since xpc.xp_pattern was not updated, >> > it then points to freed memory. >> > >> > Looking at the code, functions that can reallocate ccline.cmdbuff are: >> > >> > put_on_cmdline() ...... because it may call realloc_cmdbuff() >> > cmdline_paste_str() ... because it may call put_on_cmdline() >> > cmdline_paste() ....... because it may call cmdline_paste_str() >> > >> > Whenever any of these functions is called, we should take care of >> > reinitializing xpc.xp_pattern appropriately. >> > >> > Function nextwild() also calls realloc_cmdbuff() but this one >> > is OK since it internally reinitializes xpc.xp_pattern. >> > >> > I attach a patch which should fix the bug. >> > >> > -- Dominique >> >> >> I saw one additional similar issue even after my previous patch. >> >> ==7527== Invalid read of size 1 >> ==7527== at 0x4023572: strncmp (mc_replace_strmem.c:318) >> ==7527== by 0x80B6656: getcmdline (ex_getln.c:557) >> ==7527== by 0x80B94FF: getexline (ex_getln.c:2126) >> ==7527== by 0x80A2F3E: do_cmdline (ex_docmd.c:995) >> ==7527== by 0x8129C46: nv_colon (normal.c:5179) >> ==7527== by 0x8123202: normal_cmd (normal.c:1152) >> ==7527== by 0x80E6035: main_loop (main.c:1181) >> ==7527== by 0x80E5B85: main (main.c:940) >> ==7527== Address 0x55B1024 is 4 bytes inside a block of size 140 free'd >> ==7527== at 0x402237F: free (vg_replace_malloc.c:233) >> ==7527== by 0x8114345: vim_free (misc2.c:1580) >> ==7527== by 0x80C0061: ex_window (ex_getln.c:6143) >> ==7527== by 0x80B6BF8: getcmdline (ex_getln.c:734) >> ==7527== by 0x80B94FF: getexline (ex_getln.c:2126) >> ==7527== by 0x80A2F3E: do_cmdline (ex_docmd.c:995) >> ==7527== by 0x8129C46: nv_colon (normal.c:5179) >> ==7527== by 0x8123202: normal_cmd (normal.c:1152) >> ==7527== by 0x80E6035: main_loop (main.c:1181) >> ==7527== by 0x80E5B85: main (main.c:940) >> >> Function ex_window() may also free and reallocate ccline.cmdbuff >> hence invalidating xpc.xp_pattern. >> >> I attach an update of my previous patch which should also fix this issue. > > This is tricky, since xp_pattern is separate from the allocated command > line. It's very easy to forget updating xp_pattern. One solution would > be to change xp_pattern from a pointer into a byte index. But there are > several places where the start of the command line are not known. > > Another solution would be to make expand_T part of struct cmdline_info. > Then xp_pattern can be adjusted by the function reallocating the command > line. Code only using the expand_T doesn't need to be changed then. > I'll look further into this. Until now, I saw this bug a couple of times but never found a way to reproduce it easily. Well, I just found a way to reproduce this easily with vim-7.2.6 (also with gvim). 1/ start vim with: algrind vim -u NONE 2/ enter Ex command ":set nocompatible wildmenu" 3/ put at least one command in Ex history :echo "foobar" 4/ press Ex command ":e -" followed --~--~---------~--~----~------------~-------~--~----~ You received this message from the "vim_dev" maillist. For more information, visit http://www.vim.org/maillist.php -~----------~----~----~----~------~----~------~--~---
