Loading ...
Sorry, an error occurred while loading the content.
 

Re: [patch] bug in completion with i_CTRL-N using arabic or persian keymap

Expand Messages
  • Bram Moolenaar
    ... 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 - ,
    Message 1 of 4 , Jan 6, 2009
      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 -- Bram@... -- 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
      -~----------~----~----~----~------~----~------~--~---
    Your message has been successfully submitted and would be delivered to recipients shortly.