Dominique Pelle wrote:

> Hi.
> 
> I can reproduce the following error with valgrind memory
> checker using Vim-7.2.75 (huge) on Linux x86 with utf-8 locale:
> 
> ==15276== Invalid read of size 1
> ==15276==    at 0x4026438: strlen (mc_replace_strmem.c:242)
> ==15276==    by 0x8107E39: ins_bytes (misc1.c:1860)
> ==15276==    by 0x8067EC0: ins_compl_new_leader (edit.c:3212)
> ==15276==    by 0x8068048: ins_compl_addleader (edit.c:3297)
> ==15276==    by 0x80641AA: edit (edit.c:765)
> ==15276==    by 0x812F248: invoke_edit (normal.c:8901)
> ==15276==    by 0x812F1EE: nv_edit (normal.c:8874)
> ==15276==    by 0x8122A3C: normal_cmd (normal.c:1200)
> ==15276==    by 0x80E5C9D: main_loop (main.c:1180)
> ==15276==    by 0x80E57EA: main (main.c:939)
> ==15276==  Address 0x4e671af is 1 bytes before a block of size 3 alloc'd
> ==15276==    at 0x4025D2E: malloc (vg_replace_malloc.c:207)
> ==15276==    by 0x811303C: lalloc (misc2.c:859)
> ==15276==    by 0x8112F58: alloc (misc2.c:758)
> ==15276==    by 0x81133EF: vim_strnsave (misc2.c:1176)
> ==15276==    by 0x8068035: ins_compl_addleader (edit.c:3294)
> ==15276==    by 0x80641AA: edit (edit.c:765)
> ==15276==    by 0x812F248: invoke_edit (normal.c:8901)
> ==15276==    by 0x812F1EE: nv_edit (normal.c:8874)
> ==15276==    by 0x8122A3C: normal_cmd (normal.c:1200)
> ==15276==    by 0x80E5C9D: main_loop (main.c:1180)
> ==15276==    by 0x80E57EA: main (main.c:939)
> 
> Steps to reproduce:
> 
> 1/ Create a sample tag file (using Vim source files for example):
> 
>       $ cd vim7/src
>       $ ctags *.c *.h
> 
> 2/ Create a minimalistic vimrc file enough to trigger bug:
> 
>       set completeopt=menuone,longest
>       set tags=tags
>       set keymap=arabic
> 
>    I tried several keymaps (not all of them), but I can somehow only
>    reproduce this bug using 'set keymap=arabic' or 'set keymap=persian'.
> 
> 3/ Start Vim with valgrind:
> 
>       $ valgrind vim -u test.vimrc 2> valgrind.log
> 
> 4/ Type the following commands in Normal mode (completion using pum & tags):
> 
>       i-<ctrl-n>X
> 
> 5/ Observe the above valgrind error in valgrind.log right after
>    pressing X in step 4/
> 
> edit.c, around line 3212:
> 
>   3207     static void
>   3208 ins_compl_new_leader()
>   3209 {
>   3210     ins_compl_del_pum();
>   3211     ins_compl_delete();
> ! 3212     ins_bytes(compl_leader + curwin->w_cursor.col - compl_col);
>   3213     compl_used_match = FALSE;
> 
> When bug happens, I see that curwin->w_cursor.col is 0, and compl_col
> is 1, so argument of ins_bytes() at line 3212 is 1 byte before beginning
> of string compl_leader (hence the error).  Without keymap, or with
> other keymaps than arabic or persian, curwin->w_cursor.col is 1 and
> compl_col is also 1 (so bug then does not happen).
> 
> I'm not sure what's the right way to fix it: obviously we can do
> a check for curwin->w_cursor.col being greater or equal than compl_col
> as in attached patch.  Although it pacifies valgrind, I may only work
> around the bug.  I was testing Vim with keymaps and I don't know how
> arabic and persian keymaps are supposed to behave to tell whether the
> behavior is correct (but the valgrind error is clearly not expected).

It turns out that the "X" inserted is keymapped to a composing
character.  When deleting the completed text this causes the character
bofore it, the "-", also to be deleted.

The patch below avoids deleting too much.  And also avoids the offset
going negative, in case there is another situation where this happens.

Let me know if there are any remaining (or new) problems after this
patch.

*** ../vim-7.2.079/src/edit.c   Wed Aug  6 18:56:55 2008
--- src/edit.c  Tue Jan  6 18:55:16 2009
***************
*** 147,152 ****
--- 147,153 ----
  static int  ins_compl_bs __ARGS((void));
  static void ins_compl_new_leader __ARGS((void));
  static void ins_compl_addleader __ARGS((int c));
+ static int ins_compl_len __ARGS((void));
  static void ins_compl_restart __ARGS((void));
  static void ins_compl_set_original_text __ARGS((char_u *str));
  static void ins_compl_addfrommatch __ARGS((void));
***************
*** 1933,1938 ****
--- 1934,1941 ----
  /*
   * Backspace the cursor until the given column.  Handles REPLACE and VREPLACE
   * modes correctly.  May also be used when not in insert mode at all.
+  * Will attempt not to go before "col" even when there is a composing
+  * character.
   */
      void
  backspace_until_column(col)
***************
*** 1944,1950 ****
        if (State & REPLACE_FLAG)
            replace_do_bs();
        else
!           (void)del_char(FALSE);
      }
  }
  #endif
--- 1947,1978 ----
        if (State & REPLACE_FLAG)
            replace_do_bs();
        else
!       {
! #ifdef FEAT_MBYTE
!           if (enc_utf8)
!           {
!               int ecol = curwin->w_cursor.col + 1;
! 
!               /* Make sure the cursor is at the start of a character, but
!                * skip forward again when going too far back because of a
!                * composing character. */
!               mb_adjust_cursor();
!               while (curwin->w_cursor.col < col)
!               {
!                   int l = utf_ptr2len(ml_get_cursor());
! 
!                   if (l == 0)  /* end of line */
!                       break;
!                   curwin->w_cursor.col += l;
!               }
!               if (*ml_get_cursor() == NUL || curwin->w_cursor.col == ecol)
!                   break;
!               del_bytes((long)(ecol - curwin->w_cursor.col), FALSE, TRUE);
!           }
!           else
! #endif
!               (void)del_char(FALSE);
!       }
      }
  }
  #endif
***************
*** 2418,2424 ****
        {
            had_match = (curwin->w_cursor.col > compl_col);
            ins_compl_delete();
!           ins_bytes(compl_leader + curwin->w_cursor.col - compl_col);
            ins_redraw(FALSE);
  
            /* When the match isn't there (to avoid matching itself) remove it
--- 2446,2452 ----
        {
            had_match = (curwin->w_cursor.col > compl_col);
            ins_compl_delete();
!           ins_bytes(compl_leader + ins_compl_len());
            ins_redraw(FALSE);
  
            /* When the match isn't there (to avoid matching itself) remove it
***************
*** 2470,2476 ****
            *p = NUL;
            had_match = (curwin->w_cursor.col > compl_col);
            ins_compl_delete();
!           ins_bytes(compl_leader + curwin->w_cursor.col - compl_col);
            ins_redraw(FALSE);
  
            /* When the match isn't there (to avoid matching itself) remove it
--- 2498,2504 ----
            *p = NUL;
            had_match = (curwin->w_cursor.col > compl_col);
            ins_compl_delete();
!           ins_bytes(compl_leader + ins_compl_len());
            ins_redraw(FALSE);
  
            /* When the match isn't there (to avoid matching itself) remove it
***************
*** 3209,3215 ****
  {
      ins_compl_del_pum();
      ins_compl_delete();
!     ins_bytes(compl_leader + curwin->w_cursor.col - compl_col);
      compl_used_match = FALSE;
  
      if (compl_started)
--- 3237,3243 ----
  {
      ins_compl_del_pum();
      ins_compl_delete();
!     ins_bytes(compl_leader + ins_compl_len());
      compl_used_match = FALSE;
  
      if (compl_started)
***************
*** 3264,3269 ****
--- 3292,3311 ----
  }
  
  /*
+  * Return the length of the completion, from the completion start column to
+  * the cursor column.  Making sure it never goes below zero.
+  */
+     static int
+ ins_compl_len()
+ {
+     int off = curwin->w_cursor.col - compl_col;
+ 
+     if (off < 0)
+       return 0;
+     return off;
+ }
+ 
+ /*
   * Append one character to the match leader.  May reduce the number of
   * matches.
   */
***************
*** 3621,3630 ****
            {
                ins_compl_delete();
                if (compl_leader != NULL)
!                   ins_bytes(compl_leader + curwin->w_cursor.col - compl_col);
                else if (compl_first_match != NULL)
!                   ins_bytes(compl_orig_text
!                                         + curwin->w_cursor.col - compl_col);
                retval = TRUE;
            }
  
--- 3663,3671 ----
            {
                ins_compl_delete();
                if (compl_leader != NULL)
!                   ins_bytes(compl_leader + ins_compl_len());
                else if (compl_first_match != NULL)
!                   ins_bytes(compl_orig_text + ins_compl_len());
                retval = TRUE;
            }
  
***************
*** 4256,4262 ****
      static void
  ins_compl_insert()
  {
!     ins_bytes(compl_shown_match->cp_str + curwin->w_cursor.col - compl_col);
      if (compl_shown_match->cp_flags & ORIGINAL_TEXT)
        compl_used_match = FALSE;
      else
--- 4297,4303 ----
      static void
  ins_compl_insert()
  {
!     ins_bytes(compl_shown_match->cp_str + ins_compl_len());
      if (compl_shown_match->cp_flags & ORIGINAL_TEXT)
        compl_used_match = FALSE;
      else
***************
*** 4425,4431 ****
        if (!compl_get_longest || compl_used_match)
            ins_compl_insert();
        else
!           ins_bytes(compl_leader + curwin->w_cursor.col - compl_col);
      }
      else
        compl_used_match = FALSE;
--- 4466,4472 ----
        if (!compl_get_longest || compl_used_match)
            ins_compl_insert();
        else
!           ins_bytes(compl_leader + ins_compl_len());
      }
      else
        compl_used_match = FALSE;


-- 
Bad programs can be written in any language.

 /// Bram Moolenaar -- [email protected] -- 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.
For more information, visit http://www.vim.org/maillist.php
-~----------~----~----~----~------~----~------~--~---

Raspunde prin e-mail lui