Hi
Valgrind memory checker detects that Vim is using freed memory
in the spelling checker code:
==8692== Invalid read of size 1
==8692== at 0x8199D87: spell_to_word_end (spell.c:15854)
==8692== by 0x8166424: win_line (screen.c:3054)
==8692== by 0x8163AD5: win_update (screen.c:1765)
==8692== by 0x8161C34: update_screen (screen.c:522)
==8692== by 0x8064F1B: ins_redraw (edit.c:1474)
==8692== by 0x8063B96: edit (edit.c:688)
==8692== by 0x812322A: normal_cmd (normal.c:1327)
==8692== by 0x80E5D2D: main_loop (main.c:1181)
==8692== by 0x80E587D: main (main.c:940)
==8692== Address 0x50615F0 is 40 bytes inside a block of size 96 free'd
==8692== at 0x402237F: free (vg_replace_malloc.c:233)
==8692== by 0x8113F55: vim_free (misc2.c:1580)
==8692== by 0x80F864A: ml_flush_line (memline.c:3149)
==8692== by 0x80F69FE: ml_get_buf (memline.c:2111)
==8692== by 0x80F68BB: ml_get (memline.c:2034)
==8692== by 0x818D567: check_need_cap (spell.c:10271)
==8692== by 0x817C828: spell_move_to (spell.c:2098)
==8692== by 0x81663D0: win_line (screen.c:3048)
==8692== by 0x8163AD5: win_update (screen.c:1765)
==8692== by 0x8161C34: update_screen (screen.c:522)
==8692== by 0x8064F1B: ins_redraw (edit.c:1474)
==8692== by 0x8063B96: edit (edit.c:688)
==8692== by 0x812322A: normal_cmd (normal.c:1327)
==8692== by 0x80E5D2D: main_loop (main.c:1181)
==8692== by 0x80E587D: main (main.c:940)
(and then follows several more errors)
Here is the relevant code in screen.c:
2927 line = ml_get_buf(wp->w_buffer, lnum, FALSE);
2928 ptr = line;
....
....
3037 #ifdef FEAT_SPELL
3038 /* When spell checking a word we need to figure out the
start of the
3039 * word and if it's badly spelled or not. */
3040 if (has_spell)
3041 {
3042 int len;
3043 hlf_T spell_hlf = HLF_COUNT;
3044
3045 pos = wp->w_cursor;
3046 wp->w_cursor.lnum = lnum;
3047 wp->w_cursor.col = (colnr_T)(ptr - line);
!!3048 len = spell_move_to(wp, FORWARD, TRUE, TRUE, &spell_hlf);
3049 if (len == 0 || (int)wp->w_cursor.col > ptr - line)
3050 {
3051 /* no bad word found at line start, don't check
until end of a
3052 * word */
3053 spell_hlf = HLF_COUNT;
!!3054 word_end = (int)(spell_to_word_end(ptr,
wp->w_buffer) - line + 1);
3055 }
Error happens inside call of spell_to_word_end() at screen.c:3054
because ptr is dereferenced inside spell_to_word_end() but is pointing
to memory which has already been freed (bug!).
ptr was previously freed just a few lines above, as a side effect
of calling spell_move_to(...) 6 lines above at screen.c:3048, because
spell_move_to(...) may in some cases call ml_get(...) which invalidates
previous pointer returned by previous call of ml_get(...). So call
to spell_move_to invalidates line and ptr which were initialized at
line 2927/2928 in screen.c.
Here is how I can reproduce the bug:
$ valgrind vim -u NONE -c 'set nowrap|set spell|start' -s
spell-access-freed-mem.vim 2> valgrind.log
spell-testcase is a small file generated randomly which triggers this bug:
http://dominique.pelle.free.fr/spell-access-freed-mem.vim
But happens only if terminal is small enough (80x25 or smaller)
I attach a patch which fixes the bug but please review it. Maybe there
is a better solution.
I'm using vim-7.1 (Patches 1-242) on Linux x86 in a gnome-terminal, built
with "configure --with-feature=huge", without optimizations (-O0 -g).
-- Dominique
--~--~---------~--~----~------------~-------~--~----~
You received this message from the "vim_dev" maillist.
For more information, visit http://www.vim.org/maillist.php
-~----------~----~----~----~------~----~------~--~---
Index: screen.c
===================================================================
RCS file: /cvsroot/vim/vim7/src/screen.c,v
retrieving revision 1.98
diff -c -r1.98 screen.c
*** screen.c 19 Jan 2008 14:58:17 -0000 1.98
--- screen.c 3 Feb 2008 15:42:20 -0000
***************
*** 2644,2650 ****
#if defined(FEAT_SIGNS) || (defined(FEAT_QUICKFIX) && defined(FEAT_WINDOWS)) \
|| defined(FEAT_SYN_HL) || defined(FEAT_DIFF)
# define LINE_ATTR
! int line_attr = 0; /* atrribute for the whole line */
#endif
#ifdef FEAT_SEARCH_EXTRA
matchitem_T *cur; /* points to the match list */
--- 2644,2650 ----
#if defined(FEAT_SIGNS) || (defined(FEAT_QUICKFIX) && defined(FEAT_WINDOWS)) \
|| defined(FEAT_SYN_HL) || defined(FEAT_DIFF)
# define LINE_ATTR
! int line_attr = 0; /* attribute for the whole line */
#endif
#ifdef FEAT_SEARCH_EXTRA
matchitem_T *cur; /* points to the match list */
***************
*** 3045,3051 ****
--- 3045,3060 ----
pos = wp->w_cursor;
wp->w_cursor.lnum = lnum;
wp->w_cursor.col = (colnr_T)(ptr - line);
+
+ /* Save position in line, since spell_move_to() invalidates
+ * line and ptr as it may call ml_get() */
+ v = (long)(ptr - line);
len = spell_move_to(wp, FORWARD, TRUE, TRUE, &spell_hlf);
+
+ /* Get line again and restore ptr */
+ line = ml_get_buf(wp->w_buffer, lnum, FALSE);
+ ptr = line + v;
+
if (len == 0 || (int)wp->w_cursor.col > ptr - line)
{
/* no bad word found at line start, don't check until end of a
