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

[patch] fixed access to free memory when using some autocommands

Expand Messages
  • Dominique Pelle
    Testing autocommands, I see that Vim-7.2.107 (and older) is using memory already freed when doing silly autocommands such as: $ touch foobar $ valgrind ./vim
    Message 1 of 7 , Feb 11, 2009
    View Source
    • 0 Attachment
      Testing autocommands, I see that Vim-7.2.107 (and older)
      is using memory already freed when doing silly autocommands
      such as:

      $ touch foobar
      $ valgrind ./vim -u NONE -c 'au! BufReadPre * cd /tmp' \
      -c 'e foobar' 2> vg.log

      In vg.log, I then see the following error:

      ==15058== Syscall param open(filename) points to unaddressable byte(s)
      ==15058== at 0x40007D2: (within /lib/ld-2.8.90.so)
      ==15058== by 0x805365E: open_buffer (buffer.c:130)
      ==15058== by 0x8098548: do_ecmd (ex_cmds.c:3655)
      ==15058== by 0x80AE8E9: do_exedit (ex_docmd.c:7557)
      ==15058== by 0x80AE560: ex_edit (ex_docmd.c:7452)
      ==15058== by 0x80A6FE7: do_one_cmd (ex_docmd.c:2622)
      ==15058== by 0x80A4867: do_cmdline (ex_docmd.c:1096)
      ==15058== by 0x80A3F00: do_cmdline_cmd (ex_docmd.c:702)
      ==15058== by 0x80E8A07: exe_commands (main.c:2693)
      ==15058== by 0x80E63A7: main (main.c:874)
      ==15058== Address 0x54312d4 is 4 bytes inside a block of size 11 free'd
      ==15058== at 0x4024E5A: free (vg_replace_malloc.c:323)
      ==15058== by 0x8114D5D: vim_free (misc2.c:1631)
      ==15058== by 0x80C97DF: shorten_fnames (fileio.c:5715)
      ==15058== by 0x80AF1A9: ex_cd (ex_docmd.c:7942)
      ==15058== by 0x80A6FE7: do_one_cmd (ex_docmd.c:2622)
      ==15058== by 0x80A4867: do_cmdline (ex_docmd.c:1096)
      ==15058== by 0x80CD35A: apply_autocmds_group (fileio.c:8853)
      ==15058== by 0x80CCD9B: apply_autocmds_exarg (fileio.c:8481)
      ==15058== by 0x80C2246: readfile (fileio.c:723)
      ==15058== by 0x805365E: open_buffer (buffer.c:130)
      ==15058== by 0x8098548: do_ecmd (ex_cmds.c:3655)
      ==15058== by 0x80AE8E9: do_exedit (ex_docmd.c:7557)

      The error is actually deeper inside readfile(...) (not sure
      why valgrind stops the stack trace at open_buffer (buffer.c:130).

      In readfile(), parameter fname may point to curbuf->b_ffname
      or curbuf->b_fname before running autocommands (near fileio.c:715).
      Some autocommands may change curbuf->b_ffname and
      curbuf->b_fname (freeing and reallocating them) so after
      executing autommand fname may then point to some invalid
      memory, and readfile() then uses freed memory when
      accessing fname.

      Attached patch fixes the bug by making readfile() fail when
      that happens. Probably an error should also be reported
      (see TODO comment in patch). Please review the patch.

      The same patch also fixes at least another error
      when doing another silly autocommand:

      $ touch foobar
      $ vim -u NONE

      :set autochdir
      :au! BufReadPre * e!
      :e foobar

      ==12943== Syscall param open(filename) points to unaddressable byte(s)
      ==12943== at 0x40007D2: (within /lib/ld-2.8.90.so)
      ==12943== by 0x805368E: open_buffer (buffer.c:130)
      ==12943== by 0x809856E: do_ecmd (ex_cmds.c:3650)
      ==12943== by 0x80AE90D: do_exedit (ex_docmd.c:7557)
      ==12943== by 0x80AE584: ex_edit (ex_docmd.c:7452)
      ==12943== by 0x80A700B: do_one_cmd (ex_docmd.c:2622)
      ==12943== by 0x80A488B: do_cmdline (ex_docmd.c:1096)
      ==12943== by 0x812A4F0: nv_colon (normal.c:5233)
      ==12943== by 0x8123B74: normal_cmd (normal.c:1200)
      ==12943== by 0x80E69B9: main_loop (main.c:1180)
      ==12943== by 0x80E6506: main (main.c:939)
      ==12943== Address 0x5d69824 is 4 bytes inside a block of size 11 free'd
      ==12943== at 0x4024E5A: free (vg_replace_malloc.c:323)
      ==12943== by 0x8114D91: vim_free (misc2.c:1631)
      ==12943== by 0x80C9803: shorten_fnames (fileio.c:5715)
      ==12943== by 0x805538B: do_autochdir (buffer.c:1476)
      ==12943== by 0x8098521: do_ecmd (ex_cmds.c:3631)
      ==12943== by 0x80AE90D: do_exedit (ex_docmd.c:7557)
      ==12943== by 0x80AE584: ex_edit (ex_docmd.c:7452)
      ==12943== by 0x80A700B: do_one_cmd (ex_docmd.c:2622)
      ==12943== by 0x80A488B: do_cmdline (ex_docmd.c:1096)
      ==12943== by 0x80CD3A9: apply_autocmds_group (fileio.c:8861)
      ==12943== by 0x80CCDEA: apply_autocmds_exarg (fileio.c:8489)
      ==12943== by 0x80C226A: readfile (fileio.c:723)
      ==12943== by 0x805368E: open_buffer (buffer.c:130)
      ==12943== by 0x809856E: do_ecmd (ex_cmds.c:3650)
      ==12943== by 0x80AE90D: do_exedit (ex_docmd.c:7557)
      ==12943== by 0x80AE584: ex_edit (ex_docmd.c:7452)
      ==12943== by 0x80A700B: do_one_cmd (ex_docmd.c:2622)
      ==12943== by 0x80A488B: do_cmdline (ex_docmd.c:1096)
      ==12943== by 0x812A4F0: nv_colon (normal.c:5233)
      ==12943== by 0x8123B74: normal_cmd (normal.c:1200)
      ==12943== by 0x80E69B9: main_loop (main.c:1180)
      ==12943== by 0x80E6506: main (main.c:939)
      (and many other errors follow)

      Regards
      -- Dominique

      --~--~---------~--~----~------------~-------~--~----~
      You received this message from the "vim_dev" maillist.
      For more information, visit http://www.vim.org/maillist.php
      -~----------~----~----~----~------~----~------~--~---
    • Bram Moolenaar
      ... Thanks. It s in the todo list. -- hundred-and-one symptoms of being an internet addict: 86. E-mail Deficiency Depression (EDD) forces you to e-mail
      Message 2 of 7 , Feb 12, 2009
      View Source
      • 0 Attachment
        Dominique Pelle wrote:

        > Testing autocommands, I see that Vim-7.2.107 (and older)
        > is using memory already freed when doing silly autocommands
        > such as:
        >
        > $ touch foobar
        > $ valgrind ./vim -u NONE -c 'au! BufReadPre * cd /tmp' \
        > -c 'e foobar' 2> vg.log
        >
        > In vg.log, I then see the following error:
        >
        > ==15058== Syscall param open(filename) points to unaddressable byte(s)
        > ==15058== at 0x40007D2: (within /lib/ld-2.8.90.so)
        > ==15058== by 0x805365E: open_buffer (buffer.c:130)
        > ==15058== by 0x8098548: do_ecmd (ex_cmds.c:3655)
        > ==15058== by 0x80AE8E9: do_exedit (ex_docmd.c:7557)
        > ==15058== by 0x80AE560: ex_edit (ex_docmd.c:7452)
        > ==15058== by 0x80A6FE7: do_one_cmd (ex_docmd.c:2622)
        > ==15058== by 0x80A4867: do_cmdline (ex_docmd.c:1096)
        > ==15058== by 0x80A3F00: do_cmdline_cmd (ex_docmd.c:702)
        > ==15058== by 0x80E8A07: exe_commands (main.c:2693)
        > ==15058== by 0x80E63A7: main (main.c:874)
        > ==15058== Address 0x54312d4 is 4 bytes inside a block of size 11 free'd
        > ==15058== at 0x4024E5A: free (vg_replace_malloc.c:323)
        > ==15058== by 0x8114D5D: vim_free (misc2.c:1631)
        > ==15058== by 0x80C97DF: shorten_fnames (fileio.c:5715)
        > ==15058== by 0x80AF1A9: ex_cd (ex_docmd.c:7942)
        > ==15058== by 0x80A6FE7: do_one_cmd (ex_docmd.c:2622)
        > ==15058== by 0x80A4867: do_cmdline (ex_docmd.c:1096)
        > ==15058== by 0x80CD35A: apply_autocmds_group (fileio.c:8853)
        > ==15058== by 0x80CCD9B: apply_autocmds_exarg (fileio.c:8481)
        > ==15058== by 0x80C2246: readfile (fileio.c:723)
        > ==15058== by 0x805365E: open_buffer (buffer.c:130)
        > ==15058== by 0x8098548: do_ecmd (ex_cmds.c:3655)
        > ==15058== by 0x80AE8E9: do_exedit (ex_docmd.c:7557)
        >
        > The error is actually deeper inside readfile(...) (not sure
        > why valgrind stops the stack trace at open_buffer (buffer.c:130).
        >
        > In readfile(), parameter fname may point to curbuf->b_ffname
        > or curbuf->b_fname before running autocommands (near fileio.c:715).
        > Some autocommands may change curbuf->b_ffname and
        > curbuf->b_fname (freeing and reallocating them) so after
        > executing autommand fname may then point to some invalid
        > memory, and readfile() then uses freed memory when
        > accessing fname.
        >
        > Attached patch fixes the bug by making readfile() fail when
        > that happens. Probably an error should also be reported
        > (see TODO comment in patch). Please review the patch.
        >
        > The same patch also fixes at least another error
        > when doing another silly autocommand:
        >
        > $ touch foobar
        > $ vim -u NONE
        >
        > :set autochdir
        > :au! BufReadPre * e!
        > :e foobar
        >
        > ==12943== Syscall param open(filename) points to unaddressable byte(s)
        > ==12943== at 0x40007D2: (within /lib/ld-2.8.90.so)
        > ==12943== by 0x805368E: open_buffer (buffer.c:130)
        > ==12943== by 0x809856E: do_ecmd (ex_cmds.c:3650)
        > ==12943== by 0x80AE90D: do_exedit (ex_docmd.c:7557)
        > ==12943== by 0x80AE584: ex_edit (ex_docmd.c:7452)
        > ==12943== by 0x80A700B: do_one_cmd (ex_docmd.c:2622)
        > ==12943== by 0x80A488B: do_cmdline (ex_docmd.c:1096)
        > ==12943== by 0x812A4F0: nv_colon (normal.c:5233)
        > ==12943== by 0x8123B74: normal_cmd (normal.c:1200)
        > ==12943== by 0x80E69B9: main_loop (main.c:1180)
        > ==12943== by 0x80E6506: main (main.c:939)
        > ==12943== Address 0x5d69824 is 4 bytes inside a block of size 11 free'd
        > ==12943== at 0x4024E5A: free (vg_replace_malloc.c:323)
        > ==12943== by 0x8114D91: vim_free (misc2.c:1631)
        > ==12943== by 0x80C9803: shorten_fnames (fileio.c:5715)
        > ==12943== by 0x805538B: do_autochdir (buffer.c:1476)
        > ==12943== by 0x8098521: do_ecmd (ex_cmds.c:3631)
        > ==12943== by 0x80AE90D: do_exedit (ex_docmd.c:7557)
        > ==12943== by 0x80AE584: ex_edit (ex_docmd.c:7452)
        > ==12943== by 0x80A700B: do_one_cmd (ex_docmd.c:2622)
        > ==12943== by 0x80A488B: do_cmdline (ex_docmd.c:1096)
        > ==12943== by 0x80CD3A9: apply_autocmds_group (fileio.c:8861)
        > ==12943== by 0x80CCDEA: apply_autocmds_exarg (fileio.c:8489)
        > ==12943== by 0x80C226A: readfile (fileio.c:723)
        > ==12943== by 0x805368E: open_buffer (buffer.c:130)
        > ==12943== by 0x809856E: do_ecmd (ex_cmds.c:3650)
        > ==12943== by 0x80AE90D: do_exedit (ex_docmd.c:7557)
        > ==12943== by 0x80AE584: ex_edit (ex_docmd.c:7452)
        > ==12943== by 0x80A700B: do_one_cmd (ex_docmd.c:2622)
        > ==12943== by 0x80A488B: do_cmdline (ex_docmd.c:1096)
        > ==12943== by 0x812A4F0: nv_colon (normal.c:5233)
        > ==12943== by 0x8123B74: normal_cmd (normal.c:1200)
        > ==12943== by 0x80E69B9: main_loop (main.c:1180)
        > ==12943== by 0x80E6506: main (main.c:939)
        > (and many other errors follow)

        Thanks. It's in the todo list.

        --
        hundred-and-one symptoms of being an internet addict:
        86. E-mail Deficiency Depression (EDD) forces you to e-mail yourself.

        /// 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
        -~----------~----~----~----~------~----~------~--~---
      • Dominique Pelle
        ... [...snip...] ... I found another case of an autocommand which also causes to use freed memory and the proposed patch in my previous email did not help to
        Message 3 of 7 , Feb 13, 2009
        View Source
        • 0 Attachment
          Bram Moolenaar wrote:

          > Dominique Pelle wrote:
          >
          >> Testing autocommands, I see that Vim-7.2.107 (and older)
          >> is using memory already freed when doing silly autocommands
          >> such as:
          >>
          >> $ touch foobar
          >> $ valgrind ./vim -u NONE -c 'au! BufReadPre * cd /tmp' \
          >>                          -c 'e foobar' 2> vg.log
          >>
          >> In vg.log, I then see the following error:
          >>
          >> ==15058== Syscall param open(filename) points to unaddressable byte(s)
          >> ==15058==    at 0x40007D2: (within /lib/ld-2.8.90.so)
          >> ==15058==    by 0x805365E: open_buffer (buffer.c:130)
          >> ==15058==    by 0x8098548: do_ecmd (ex_cmds.c:3655)
          >> ==15058==    by 0x80AE8E9: do_exedit (ex_docmd.c:7557)
          >> ==15058==    by 0x80AE560: ex_edit (ex_docmd.c:7452)
          >> ==15058==    by 0x80A6FE7: do_one_cmd (ex_docmd.c:2622)
          >> ==15058==    by 0x80A4867: do_cmdline (ex_docmd.c:1096)
          >> ==15058==    by 0x80A3F00: do_cmdline_cmd (ex_docmd.c:702)
          >> ==15058==    by 0x80E8A07: exe_commands (main.c:2693)
          >> ==15058==    by 0x80E63A7: main (main.c:874)
          >> ==15058==  Address 0x54312d4 is 4 bytes inside a block of size 11 free'd
          >> ==15058==    at 0x4024E5A: free (vg_replace_malloc.c:323)
          >> ==15058==    by 0x8114D5D: vim_free (misc2.c:1631)
          >> ==15058==    by 0x80C97DF: shorten_fnames (fileio.c:5715)
          >> ==15058==    by 0x80AF1A9: ex_cd (ex_docmd.c:7942)
          >> ==15058==    by 0x80A6FE7: do_one_cmd (ex_docmd.c:2622)
          >> ==15058==    by 0x80A4867: do_cmdline (ex_docmd.c:1096)
          >> ==15058==    by 0x80CD35A: apply_autocmds_group (fileio.c:8853)
          >> ==15058==    by 0x80CCD9B: apply_autocmds_exarg (fileio.c:8481)
          >> ==15058==    by 0x80C2246: readfile (fileio.c:723)
          >> ==15058==    by 0x805365E: open_buffer (buffer.c:130)
          >> ==15058==    by 0x8098548: do_ecmd (ex_cmds.c:3655)
          >> ==15058==    by 0x80AE8E9: do_exedit (ex_docmd.c:7557)

          [...snip...]

          >
          > Thanks.  It's in the todo list.


          I found another case of an autocommand which also causes
          to use freed memory and the proposed patch in my previous
          email did not help to fix it. Here is how to reproduce it:

          # Open a file foobar in vim to create swap file .foobar.swp
          $ vim foobar

          # In another terminal...
          $ vim -u NONE
          :au! SwapExists * cd /tmp
          :e foobar

          ... and valgrind complains about use of freed memory:

          ==17226== Syscall param open(filename) points to unaddressable byte(s)
          ==17226== at 0x40007D2: (within /lib/ld-2.8.90.so)
          ==17226== by 0x805365E: open_buffer (buffer.c:130)
          ==17226== by 0x8098548: do_ecmd (ex_cmds.c:3655)
          ==17226== by 0x80AE8E9: do_exedit (ex_docmd.c:7557)
          ==17226== by 0x80AE560: ex_edit (ex_docmd.c:7452)
          ==17226== by 0x80A6FE7: do_one_cmd (ex_docmd.c:2622)
          ==17226== by 0x80A4867: do_cmdline (ex_docmd.c:1096)
          ==17226== by 0x812A4BC: nv_colon (normal.c:5233)
          ==17226== by 0x8123B40: normal_cmd (normal.c:1200)
          ==17226== by 0x80E6969: main_loop (main.c:1180)
          ==17226== by 0x80E64B6: main (main.c:939)
          ==17226== Address 0x543644c is 4 bytes inside a block of size 11 free'd
          ==17226== at 0x4024E5A: free (vg_replace_malloc.c:323)
          ==17226== by 0x8114D5D: vim_free (misc2.c:1631)
          ==17226== by 0x80C97DF: shorten_fnames (fileio.c:5715)
          ==17226== by 0x80AF1A9: ex_cd (ex_docmd.c:7942)
          ==17226== by 0x80A6FE7: do_one_cmd (ex_docmd.c:2622)
          ==17226== by 0x80A4867: do_cmdline (ex_docmd.c:1096)
          ==17226== by 0x80CD35A: apply_autocmds_group (fileio.c:8853)
          ==17226== by 0x80CCD5D: apply_autocmds (fileio.c:8464)
          ==17226== by 0x80FA022: do_swapexists (memline.c:3770)
          ==17226== by 0x80FA93D: findswapname (memline.c:4130)
          ==17226== by 0x80F4A88: ml_open_file (memline.c:553)
          ==17226== by 0x80F4BAE: check_need_swap (memline.c:605)
          ==17226== by 0x80C206F: readfile (fileio.c:670)
          ==17226== by 0x805365E: open_buffer (buffer.c:130)
          ==17226== by 0x8098548: do_ecmd (ex_cmds.c:3655)
          ==17226== by 0x80AE8E9: do_exedit (ex_docmd.c:7557)
          ==17226== by 0x80AE560: ex_edit (ex_docmd.c:7452)
          ==17226== by 0x80A6FE7: do_one_cmd (ex_docmd.c:2622)
          ==17226== by 0x80A4867: do_cmdline (ex_docmd.c:1096)
          ==17226== by 0x812A4BC: nv_colon (normal.c:5233)
          ==17226== by 0x8123B40: normal_cmd (normal.c:1200)
          ==17226== by 0x80E6969: main_loop (main.c:1180)
          ==17226== by 0x80E64B6: main (main.c:939)

          The problem happens here because readfile() in fileio.c
          calls check_need_swap(newfile); at line fileio.c:670 and
          this function can trigger an autocommand (SwapExists)
          which can potentially free or realloc curbuf->b_fname
          or curbuf->b_ffname.

          If parameters fname or sfname of readfile() are aliased to
          curbuf->b_fname or curbuf->b_ffname, then readfile() may
          read free memory after the autocommand has been executed.

          The new attached patch fixes this new problem and also fixes
          the 2 errors described in previous email. I'm not sure how
          clean the fix is. Please review it. At least it should help to
          understand what the problem is.

          Cheers
          -- Dominique

          --~--~---------~--~----~------------~-------~--~----~
          You received this message from the "vim_dev" maillist.
          For more information, visit http://www.vim.org/maillist.php
          -~----------~----~----~----~------~----~------~--~---
        • Bram Moolenaar
          ... Thanks again. Autocommands can be nasty in their side effects. Many problems like this were fixed, there probably are a few more. -- ... /// Bram
          Message 4 of 7 , Feb 14, 2009
          View Source
          • 0 Attachment
            Dominique Pelle wrote:

            > >> Testing autocommands, I see that Vim-7.2.107 (and older)
            > >> is using memory already freed when doing silly autocommands
            > >> such as:
            > >>
            > >> $ touch foobar
            > >> $ valgrind ./vim -u NONE -c 'au! BufReadPre * cd /tmp' \
            > >> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0-c 'e foobar' 2> vg.l=
            > og
            > >>
            > >> In vg.log, I then see the following error:
            > >>
            > >> =3D=3D15058=3D=3D Syscall param open(filename) points to unaddressable b=
            > yte(s)
            > >> =3D=3D15058=3D=3D =A0 =A0at 0x40007D2: (within /lib/ld-2.8.90.so)
            > >> =3D=3D15058=3D=3D =A0 =A0by 0x805365E: open_buffer (buffer.c:130)
            > >> =3D=3D15058=3D=3D =A0 =A0by 0x8098548: do_ecmd (ex_cmds.c:3655)
            > >> =3D=3D15058=3D=3D =A0 =A0by 0x80AE8E9: do_exedit (ex_docmd.c:7557)
            > >> =3D=3D15058=3D=3D =A0 =A0by 0x80AE560: ex_edit (ex_docmd.c:7452)
            > >> =3D=3D15058=3D=3D =A0 =A0by 0x80A6FE7: do_one_cmd (ex_docmd.c:2622)
            > >> =3D=3D15058=3D=3D =A0 =A0by 0x80A4867: do_cmdline (ex_docmd.c:1096)
            > >> =3D=3D15058=3D=3D =A0 =A0by 0x80A3F00: do_cmdline_cmd (ex_docmd.c:702)
            > >> =3D=3D15058=3D=3D =A0 =A0by 0x80E8A07: exe_commands (main.c:2693)
            > >> =3D=3D15058=3D=3D =A0 =A0by 0x80E63A7: main (main.c:874)
            > >> =3D=3D15058=3D=3D =A0Address 0x54312d4 is 4 bytes inside a block of size=
            > 11 free'd
            > >> =3D=3D15058=3D=3D =A0 =A0at 0x4024E5A: free (vg_replace_malloc.c:323)
            > >> =3D=3D15058=3D=3D =A0 =A0by 0x8114D5D: vim_free (misc2.c:1631)
            > >> =3D=3D15058=3D=3D =A0 =A0by 0x80C97DF: shorten_fnames (fileio.c:5715)
            > >> =3D=3D15058=3D=3D =A0 =A0by 0x80AF1A9: ex_cd (ex_docmd.c:7942)
            > >> =3D=3D15058=3D=3D =A0 =A0by 0x80A6FE7: do_one_cmd (ex_docmd.c:2622)
            > >> =3D=3D15058=3D=3D =A0 =A0by 0x80A4867: do_cmdline (ex_docmd.c:1096)
            > >> =3D=3D15058=3D=3D =A0 =A0by 0x80CD35A: apply_autocmds_group (fileio.c:88=
            > 53)
            > >> =3D=3D15058=3D=3D =A0 =A0by 0x80CCD9B: apply_autocmds_exarg (fileio.c:84=
            > 81)
            > >> =3D=3D15058=3D=3D =A0 =A0by 0x80C2246: readfile (fileio.c:723)
            > >> =3D=3D15058=3D=3D =A0 =A0by 0x805365E: open_buffer (buffer.c:130)
            > >> =3D=3D15058=3D=3D =A0 =A0by 0x8098548: do_ecmd (ex_cmds.c:3655)
            > >> =3D=3D15058=3D=3D =A0 =A0by 0x80AE8E9: do_exedit (ex_docmd.c:7557)
            >
            > [...snip...]
            >
            > >
            > > Thanks. =A0It's in the todo list.
            >
            >
            > I found another case of an autocommand which also causes
            > to use freed memory and the proposed patch in my previous
            > email did not help to fix it. Here is how to reproduce it:
            >
            > # Open a file foobar in vim to create swap file .foobar.swp
            > $ vim foobar
            >
            > # In another terminal...
            > $ vim -u NONE
            > :au! SwapExists * cd /tmp
            > :e foobar
            >
            > ... and valgrind complains about use of freed memory:
            >
            > =3D=3D17226=3D=3D Syscall param open(filename) points to unaddressable byte=
            > (s)
            > =3D=3D17226=3D=3D at 0x40007D2: (within /lib/ld-2.8.90.so)
            > =3D=3D17226=3D=3D by 0x805365E: open_buffer (buffer.c:130)
            > =3D=3D17226=3D=3D by 0x8098548: do_ecmd (ex_cmds.c:3655)
            > =3D=3D17226=3D=3D by 0x80AE8E9: do_exedit (ex_docmd.c:7557)
            > =3D=3D17226=3D=3D by 0x80AE560: ex_edit (ex_docmd.c:7452)
            > =3D=3D17226=3D=3D by 0x80A6FE7: do_one_cmd (ex_docmd.c:2622)
            > =3D=3D17226=3D=3D by 0x80A4867: do_cmdline (ex_docmd.c:1096)
            > =3D=3D17226=3D=3D by 0x812A4BC: nv_colon (normal.c:5233)
            > =3D=3D17226=3D=3D by 0x8123B40: normal_cmd (normal.c:1200)
            > =3D=3D17226=3D=3D by 0x80E6969: main_loop (main.c:1180)
            > =3D=3D17226=3D=3D by 0x80E64B6: main (main.c:939)
            > =3D=3D17226=3D=3D Address 0x543644c is 4 bytes inside a block of size 11 f=
            > ree'd
            > =3D=3D17226=3D=3D at 0x4024E5A: free (vg_replace_malloc.c:323)
            > =3D=3D17226=3D=3D by 0x8114D5D: vim_free (misc2.c:1631)
            > =3D=3D17226=3D=3D by 0x80C97DF: shorten_fnames (fileio.c:5715)
            > =3D=3D17226=3D=3D by 0x80AF1A9: ex_cd (ex_docmd.c:7942)
            > =3D=3D17226=3D=3D by 0x80A6FE7: do_one_cmd (ex_docmd.c:2622)
            > =3D=3D17226=3D=3D by 0x80A4867: do_cmdline (ex_docmd.c:1096)
            > =3D=3D17226=3D=3D by 0x80CD35A: apply_autocmds_group (fileio.c:8853)
            > =3D=3D17226=3D=3D by 0x80CCD5D: apply_autocmds (fileio.c:8464)
            > =3D=3D17226=3D=3D by 0x80FA022: do_swapexists (memline.c:3770)
            > =3D=3D17226=3D=3D by 0x80FA93D: findswapname (memline.c:4130)
            > =3D=3D17226=3D=3D by 0x80F4A88: ml_open_file (memline.c:553)
            > =3D=3D17226=3D=3D by 0x80F4BAE: check_need_swap (memline.c:605)
            > =3D=3D17226=3D=3D by 0x80C206F: readfile (fileio.c:670)
            > =3D=3D17226=3D=3D by 0x805365E: open_buffer (buffer.c:130)
            > =3D=3D17226=3D=3D by 0x8098548: do_ecmd (ex_cmds.c:3655)
            > =3D=3D17226=3D=3D by 0x80AE8E9: do_exedit (ex_docmd.c:7557)
            > =3D=3D17226=3D=3D by 0x80AE560: ex_edit (ex_docmd.c:7452)
            > =3D=3D17226=3D=3D by 0x80A6FE7: do_one_cmd (ex_docmd.c:2622)
            > =3D=3D17226=3D=3D by 0x80A4867: do_cmdline (ex_docmd.c:1096)
            > =3D=3D17226=3D=3D by 0x812A4BC: nv_colon (normal.c:5233)
            > =3D=3D17226=3D=3D by 0x8123B40: normal_cmd (normal.c:1200)
            > =3D=3D17226=3D=3D by 0x80E6969: main_loop (main.c:1180)
            > =3D=3D17226=3D=3D by 0x80E64B6: main (main.c:939)
            >
            > The problem happens here because readfile() in fileio.c
            > calls check_need_swap(newfile); at line fileio.c:670 and
            > this function can trigger an autocommand (SwapExists)
            > which can potentially free or realloc curbuf->b_fname
            > or curbuf->b_ffname.
            >
            > If parameters fname or sfname of readfile() are aliased to
            > curbuf->b_fname or curbuf->b_ffname, then readfile() may
            > read free memory after the autocommand has been executed.
            >
            > The new attached patch fixes this new problem and also fixes
            > the 2 errors described in previous email. I'm not sure how
            > clean the fix is. Please review it. At least it should help to
            > understand what the problem is.

            Thanks again. Autocommands can be nasty in their side effects. Many
            problems like this were fixed, there probably are a few more.

            --
            From "know your smileys":
            :----} You lie like Pinocchio

            /// 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
            -~----------~----~----~----~------~----~------~--~---
          • Bram Moolenaar
            ... Thanks for the patch. I thought of catching the problem at the cause, by disallowing the autocommands to do something that will cause trouble. However,
            Message 5 of 7 , Feb 28, 2009
            View Source
            • 0 Attachment
              Dominique Pelle wrote:

              > >> Testing autocommands, I see that Vim-7.2.107 (and older)
              > >> is using memory already freed when doing silly autocommands
              > >> such as:
              > >>
              > >> $ touch foobar
              > >> $ valgrind ./vim -u NONE -c 'au! BufReadPre * cd /tmp' \
              > >> -c 'e foobar' 2> vg.log
              > >>
              > >> In vg.log, I then see the following error:
              > >>
              > >> ==15058== Syscall param open(filename) points to unaddressable byte(s)
              > >> ==15058== at 0x40007D2: (within /lib/ld-2.8.90.so)
              > >> ==15058== by 0x805365E: open_buffer (buffer.c:130)
              > >> ==15058== by 0x8098548: do_ecmd (ex_cmds.c:3655)
              > >> ==15058== by 0x80AE8E9: do_exedit (ex_docmd.c:7557)
              > >> ==15058== by 0x80AE560: ex_edit (ex_docmd.c:7452)
              > >> ==15058== by 0x80A6FE7: do_one_cmd (ex_docmd.c:2622)
              > >> ==15058== by 0x80A4867: do_cmdline (ex_docmd.c:1096)
              > >> ==15058== by 0x80A3F00: do_cmdline_cmd (ex_docmd.c:702)
              > >> ==15058== by 0x80E8A07: exe_commands (main.c:2693)
              > >> ==15058== by 0x80E63A7: main (main.c:874)
              > >> ==15058== Address 0x54312d4 is 4 bytes inside a block of size 11 free'd
              > >> ==15058== at 0x4024E5A: free (vg_replace_malloc.c:323)
              > >> ==15058== by 0x8114D5D: vim_free (misc2.c:1631)
              > >> ==15058== by 0x80C97DF: shorten_fnames (fileio.c:5715)
              > >> ==15058== by 0x80AF1A9: ex_cd (ex_docmd.c:7942)
              > >> ==15058== by 0x80A6FE7: do_one_cmd (ex_docmd.c:2622)
              > >> ==15058== by 0x80A4867: do_cmdline (ex_docmd.c:1096)
              > >> ==15058== by 0x80CD35A: apply_autocmds_group (fileio.c:8853)
              > >> ==15058== by 0x80CCD9B: apply_autocmds_exarg (fileio.c:8481)
              > >> ==15058== by 0x80C2246: readfile (fileio.c:723)
              > >> ==15058== by 0x805365E: open_buffer (buffer.c:130)
              > >> ==15058== by 0x8098548: do_ecmd (ex_cmds.c:3655)
              > >> ==15058== by 0x80AE8E9: do_exedit (ex_docmd.c:7557)
              >
              > [...snip...]
              >
              > >
              > > Thanks. It's in the todo list.
              >
              >
              > I found another case of an autocommand which also causes
              > to use freed memory and the proposed patch in my previous
              > email did not help to fix it. Here is how to reproduce it:
              >
              > # Open a file foobar in vim to create swap file .foobar.swp
              > $ vim foobar
              >
              > # In another terminal...
              > $ vim -u NONE
              > :au! SwapExists * cd /tmp
              > :e foobar
              >
              > ... and valgrind complains about use of freed memory:
              >
              > ==17226== Syscall param open(filename) points to unaddressable byte(s)
              > ==17226== at 0x40007D2: (within /lib/ld-2.8.90.so)
              > ==17226== by 0x805365E: open_buffer (buffer.c:130)
              > ==17226== by 0x8098548: do_ecmd (ex_cmds.c:3655)
              > ==17226== by 0x80AE8E9: do_exedit (ex_docmd.c:7557)
              > ==17226== by 0x80AE560: ex_edit (ex_docmd.c:7452)
              > ==17226== by 0x80A6FE7: do_one_cmd (ex_docmd.c:2622)
              > ==17226== by 0x80A4867: do_cmdline (ex_docmd.c:1096)
              > ==17226== by 0x812A4BC: nv_colon (normal.c:5233)
              > ==17226== by 0x8123B40: normal_cmd (normal.c:1200)
              > ==17226== by 0x80E6969: main_loop (main.c:1180)
              > ==17226== by 0x80E64B6: main (main.c:939)
              > ==17226== Address 0x543644c is 4 bytes inside a block of size 11 free'd
              > ==17226== at 0x4024E5A: free (vg_replace_malloc.c:323)
              > ==17226== by 0x8114D5D: vim_free (misc2.c:1631)
              > ==17226== by 0x80C97DF: shorten_fnames (fileio.c:5715)
              > ==17226== by 0x80AF1A9: ex_cd (ex_docmd.c:7942)
              > ==17226== by 0x80A6FE7: do_one_cmd (ex_docmd.c:2622)
              > ==17226== by 0x80A4867: do_cmdline (ex_docmd.c:1096)
              > ==17226== by 0x80CD35A: apply_autocmds_group (fileio.c:8853)
              > ==17226== by 0x80CCD5D: apply_autocmds (fileio.c:8464)
              > ==17226== by 0x80FA022: do_swapexists (memline.c:3770)
              > ==17226== by 0x80FA93D: findswapname (memline.c:4130)
              > ==17226== by 0x80F4A88: ml_open_file (memline.c:553)
              > ==17226== by 0x80F4BAE: check_need_swap (memline.c:605)
              > ==17226== by 0x80C206F: readfile (fileio.c:670)
              > ==17226== by 0x805365E: open_buffer (buffer.c:130)
              > ==17226== by 0x8098548: do_ecmd (ex_cmds.c:3655)
              > ==17226== by 0x80AE8E9: do_exedit (ex_docmd.c:7557)
              > ==17226== by 0x80AE560: ex_edit (ex_docmd.c:7452)
              > ==17226== by 0x80A6FE7: do_one_cmd (ex_docmd.c:2622)
              > ==17226== by 0x80A4867: do_cmdline (ex_docmd.c:1096)
              > ==17226== by 0x812A4BC: nv_colon (normal.c:5233)
              > ==17226== by 0x8123B40: normal_cmd (normal.c:1200)
              > ==17226== by 0x80E6969: main_loop (main.c:1180)
              > ==17226== by 0x80E64B6: main (main.c:939)
              >
              > The problem happens here because readfile() in fileio.c
              > calls check_need_swap(newfile); at line fileio.c:670 and
              > this function can trigger an autocommand (SwapExists)
              > which can potentially free or realloc curbuf->b_fname
              > or curbuf->b_ffname.
              >
              > If parameters fname or sfname of readfile() are aliased to
              > curbuf->b_fname or curbuf->b_ffname, then readfile() may
              > read free memory after the autocommand has been executed.
              >
              > The new attached patch fixes this new problem and also fixes
              > the 2 errors described in previous email. I'm not sure how
              > clean the fix is. Please review it. At least it should help to
              > understand what the problem is.

              Thanks for the patch. I thought of catching the problem at the cause,
              by disallowing the autocommands to do something that will cause trouble.
              However, it's very difficult to catch all situations.

              So I think we can do both: disallow autocommands to do things that are
              clearly wrong, and give an error if we notice side effects. That should
              be safe.

              Please have a look at the patch below. There might still be other
              autocommands that cause trouble. If you see one, please report.


              *** ../vim-7.2.127/src/ex_getln.c Fri Nov 28 10:59:57 2008
              --- src/ex_getln.c Sun Mar 1 00:13:15 2009
              ***************
              *** 2000,2007 ****

              #if defined(FEAT_AUTOCMD) || defined(PROTO)
              /*
              ! * Check if "curbuf_lock" is set and return TRUE when it is and give an error
              ! * message.
              */
              int
              curbuf_locked()
              --- 2000,2007 ----

              #if defined(FEAT_AUTOCMD) || defined(PROTO)
              /*
              ! * Check if "curbuf_lock" or "allbuf_lock" is set and return TRUE when it is
              ! * and give an error message.
              */
              int
              curbuf_locked()
              ***************
              *** 2011,2016 ****
              --- 2011,2031 ----
              EMSG(_("E788: Not allowed to edit another buffer now"));
              return TRUE;
              }
              + return allbuf_locked();
              + }
              +
              + /*
              + * Check if "allbuf_lock" is set and return TRUE when it is and give an error
              + * message.
              + */
              + int
              + allbuf_locked()
              + {
              + if (allbuf_lock > 0)
              + {
              + EMSG(_("E811: Not allowed to change buffer information now"));
              + return TRUE;
              + }
              return FALSE;
              }
              #endif
              *** ../vim-7.2.127/src/globals.h Tue Jan 6 16:13:42 2009
              --- src/globals.h Sun Mar 1 00:02:42 2009
              ***************
              *** 616,621 ****
              --- 616,626 ----
              EXTERN int curbuf_lock INIT(= 0);
              /* non-zero when the current buffer can't be
              * changed. Used for FileChangedRO. */
              + EXTERN int allbuf_lock INIT(= 0);
              + /* non-zero when no buffer name can be
              + * changed, no buffer can be deleted and
              + * current directory can't be changed.
              + * Used for SwapExists et al. */
              #endif
              #ifdef FEAT_EVAL
              # define HAVE_SANDBOX
              *** ../vim-7.2.127/src/fileio.c Wed Dec 31 16:20:54 2008
              --- src/fileio.c Sun Mar 1 02:11:09 2009
              ***************
              *** 69,75 ****
              static int au_find_group __ARGS((char_u *name));

              # define AUGROUP_DEFAULT -1 /* default autocmd group */
              ! # define AUGROUP_ERROR -2 /* errornouse autocmd group */
              # define AUGROUP_ALL -3 /* all autocmd groups */
              #endif

              --- 69,75 ----
              static int au_find_group __ARGS((char_u *name));

              # define AUGROUP_DEFAULT -1 /* default autocmd group */
              ! # define AUGROUP_ERROR -2 /* erroneous autocmd group */
              # define AUGROUP_ALL -3 /* all autocmd groups */
              #endif

              ***************
              *** 295,300 ****
              --- 295,313 ----
              int conv_restlen = 0; /* nr of bytes in conv_rest[] */
              #endif

              + #ifdef FEAT_AUTOCMD
              + /* Remember the initial values of curbuf, curbuf->b_ffname and
              + * curbuf->b_fname to detect whether they are altered as a result of
              + * executing nasaty autocommands. Also check if "fname" and "sfname"
              + * point to one of these values. */
              + buf_T *old_curbuf = curbuf;
              + char_u *old_b_ffname = curbuf->b_ffname;
              + char_u *old_b_fname = curbuf->b_fname;
              + int using_b_ffname = (fname == curbuf->b_ffname)
              + || (sfname == curbuf->b_ffname);
              + int using_b_fname = (fname == curbuf->b_fname)
              + || (sfname == curbuf->b_fname);
              + #endif
              write_no_eol_lnum = 0; /* in case it was set by the previous read */

              /*
              ***************
              *** 668,673 ****
              --- 681,697 ----
              #endif
              {
              check_need_swap(newfile);
              + #ifdef FEAT_AUTOCMD
              + if (!read_stdin && (curbuf != old_curbuf
              + || (using_b_ffname && (old_b_ffname != curbuf->b_ffname))
              + || (using_b_fname && (old_b_fname != curbuf->b_fname))))
              + {
              + EMSG(_("E812: Autocommands changed buffer or buffer name"));
              + if (!read_buffer)
              + close(fd);
              + return FAIL;
              + }
              + #endif
              #ifdef UNIX
              /* Set swap file protection bits after creating it. */
              if (swap_mode > 0 && curbuf->b_ml.ml_mfp->mf_fname != NULL)
              ***************
              *** 698,704 ****
              {
              int m = msg_scroll;
              int n = msg_scrolled;
              - buf_T *old_curbuf = curbuf;

              /*
              * The file must be closed again, the autocommands may want to change
              --- 722,727 ----
              ***************
              *** 740,747 ****
              --- 763,775 ----
              /*
              * Don't allow the autocommands to change the current buffer.
              * Try to re-open the file.
              + *
              + * Don't allow the autocommands to change the buffer name either
              + * (cd for example) if it invalidates fname or sfname.
              */
              if (!read_stdin && (curbuf != old_curbuf
              + || (using_b_ffname && (old_b_ffname != curbuf->b_ffname))
              + || (using_b_fname && (old_b_fname != curbuf->b_fname))
              || (fd = mch_open((char *)fname, O_RDONLY | O_EXTRA, 0)) < 0))
              {
              --no_wait_return;
              ***************
              *** 6320,6326 ****

              if (!stuff_empty() || global_busy || !typebuf_typed()
              #ifdef FEAT_AUTOCMD
              ! || autocmd_busy || curbuf_lock > 0
              #endif
              )
              need_check_timestamps = TRUE; /* check later */
              --- 6348,6354 ----

              if (!stuff_empty() || global_busy || !typebuf_typed()
              #ifdef FEAT_AUTOCMD
              ! || autocmd_busy || curbuf_lock > 0 || allbuf_lock > 0
              #endif
              )
              need_check_timestamps = TRUE; /* check later */
              ***************
              *** 6522,6529 ****
              --- 6550,6559 ----
              set_vim_var_string(VV_FCS_REASON, (char_u *)reason, -1);
              set_vim_var_string(VV_FCS_CHOICE, (char_u *)"", -1);
              # endif
              + ++allbuf_lock;
              n = apply_autocmds(EVENT_FILECHANGEDSHELL,
              buf->b_fname, buf->b_fname, FALSE, buf);
              + --allbuf_lock;
              busy = FALSE;
              if (n)
              {
              *** ../vim-7.2.127/src/proto/ex_getln.pro Fri Nov 28 10:59:57 2008
              --- src/proto/ex_getln.pro Sun Mar 1 00:27:12 2009
              ***************
              *** 4,9 ****
              --- 4,10 ----
              int text_locked __ARGS((void));
              void text_locked_msg __ARGS((void));
              int curbuf_locked __ARGS((void));
              + int allbuf_locked __ARGS((void));
              char_u *getexline __ARGS((int c, void *dummy, int indent));
              char_u *getexmodeline __ARGS((int promptc, void *dummy, int indent));
              int cmdline_overstrike __ARGS((void));


              --
              Amazing but true: If all the salmon caught in Canada in one year were laid
              end to end across the Sahara Desert, the smell would be absolutely awful.

              /// 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
              -~----------~----~----~----~------~----~------~--~---
            • Dominique Pelle
              ... ...snip... I applied your patch to Vim-7.2.128. I still observe something wrong after patching (something which I did not think of testing earlier, so the
              Message 6 of 7 , Mar 1, 2009
              View Source
              • 0 Attachment
                Bram Moolenaar wrote:

                > Thanks for the patch.  I thought of catching the problem at the cause,
                > by disallowing the autocommands to do something that will cause trouble.
                > However, it's very difficult to catch all situations.
                >
                > So I think we can do both: disallow autocommands to do things that are
                > clearly wrong, and give an error if we notice side effects.  That should
                > be safe.
                >
                > Please have a look at the patch below.  There might still be other
                > autocommands that cause trouble.  If you see one, please report.

                ...snip...

                I applied your patch to Vim-7.2.128.

                I still observe something wrong after patching (something
                which I did not think of testing earlier, so the problem
                I see in your patch was also in my patch).

                The bug I see only happens when opening a file in Vim
                which does not exist but for which there is a swap file.

                Steps to reproduce it:

                # Open a file foobar (which does not exist) in vim to create
                # swap file .foobar.swp
                $ rm -f ~/foobar
                $ vim ~/foobar

                # In another terminal...
                $ vim -u NONE
                :au! SwapExists * cd /tmp
                :e ~/foobar

                The following prompt shows up:

                Swap file "~/.foobar.swp" already exists!
                [O]pen Read-Only, (E)dit anyway, (R)ecover, (Q)uit, (A)bort:

                Press e.

                Valgrind then outputs the following error

                ==17742== Invalid read of size 1
                ==17742== at 0x810B4DA: get_past_head (misc1.c:4472)
                ==17742== by 0x810B428: gettail_sep (misc1.c:4419)
                ==17742== by 0x810B610: dir_of_file_exists (misc1.c:4582)
                ==17742== by 0x80C13CD: readfile (fileio.c:606)
                ==17742== by 0x8052D8E: open_buffer (buffer.c:130)
                ==17742== by 0x8097AC3: do_ecmd (ex_cmds.c:3655)
                ==17742== by 0x80ADE20: do_exedit (ex_docmd.c:7557)
                ==17742== by 0x80ADA97: ex_edit (ex_docmd.c:7452)
                ==17742== by 0x80A6517: do_one_cmd (ex_docmd.c:2622)
                ==17742== by 0x80A3D97: do_cmdline (ex_docmd.c:1096)
                ==17742== by 0x8129422: nv_colon (normal.c:5218)
                ==17742== by 0x8122A86: normal_cmd (normal.c:1189)
                ==17742== by 0x80E5F8D: main_loop (main.c:1180)
                ==17742== by 0x80E5ADA: main (main.c:939)
                ==17742== Address 0x4d34be0 is 0 bytes inside a block of size 15 free'd
                ==17742== at 0x4024E5A: free (vg_replace_malloc.c:323)
                ==17742== by 0x8113C72: vim_free (misc2.c:1631)
                ==17742== by 0x80C8E58: shorten_fnames (fileio.c:5743)
                ==17742== by 0x80AE6AA: ex_cd (ex_docmd.c:7939)
                ==17742== by 0x80A6517: do_one_cmd (ex_docmd.c:2622)
                ==17742== by 0x80A3D97: do_cmdline (ex_docmd.c:1096)
                ==17742== by 0x80CC9A5: apply_autocmds_group (fileio.c:8883)
                ==17742== by 0x80CC3A8: apply_autocmds (fileio.c:8494)
                ==17742== by 0x80F9592: do_swapexists (memline.c:3770)
                ==17742== by 0x80F9EAD: findswapname (memline.c:4130)
                ==17742== by 0x80F3FF8: ml_open_file (memline.c:553)
                ==17742== by 0x80F411E: check_need_swap (memline.c:605)
                ==17742== by 0x80C13BF: readfile (fileio.c:605)
                ==17742== by 0x8052D8E: open_buffer (buffer.c:130)
                ==17742== by 0x8097AC3: do_ecmd (ex_cmds.c:3655)
                ==17742== by 0x80ADE20: do_exedit (ex_docmd.c:7557)
                ==17742== by 0x80ADA97: ex_edit (ex_docmd.c:7452)
                ==17742== by 0x80A6517: do_one_cmd (ex_docmd.c:2622)
                ==17742== by 0x80A3D97: do_cmdline (ex_docmd.c:1096)
                ==17742== by 0x8129422: nv_colon (normal.c:5218)
                ==17742== by 0x8122A86: normal_cmd (normal.c:1189)
                ==17742== by 0x80E5F8D: main_loop (main.c:1180)
                ==17742== by 0x80E5ADA: main (main.c:939)
                (and several other errors follow)

                If I enter ":messages" I also do not see the error
                message E812 (I would expect to see this message).

                I do not see this issue when the file foobar already exists.

                This remaining issue happens because call to
                check_need_swap() in fileio.c:605 (or line 592 in
                pristine vim-7.2.128) may trigger an autocommand
                which can also invalidate fname.

                Attached patch (on top of your patch) fixes this
                remaining issue (and a typo). I will continue to test
                with this patch for a while but so far it looks OK.
                I can't see any further bugs so far with autocommands.

                Regards
                Dominique

                --~--~---------~--~----~------------~-------~--~----~
                You received this message from the "vim_dev" maillist.
                For more information, visit http://www.vim.org/maillist.php
                -~----------~----~----~----~------~----~------~--~---
              • Bram Moolenaar
                ... Thanks, I ll include this as well. When I test the steps you gave here I now get E811 for the :cd command. And other nastyness should be caught by the
                Message 7 of 7 , Mar 1, 2009
                View Source
                • 0 Attachment
                  Dominique Pelle wrote:

                  > Bram Moolenaar wrote:
                  >
                  > > Thanks for the patch.  I thought of catching the problem at the cause,
                  > > by disallowing the autocommands to do something that will cause trouble.
                  > > However, it's very difficult to catch all situations.
                  > >
                  > > So I think we can do both: disallow autocommands to do things that are
                  > > clearly wrong, and give an error if we notice side effects.  That should
                  > > be safe.
                  > >
                  > > Please have a look at the patch below.  There might still be other
                  > > autocommands that cause trouble.  If you see one, please report.
                  >
                  > ...snip...
                  >
                  > I applied your patch to Vim-7.2.128.
                  >
                  > I still observe something wrong after patching (something
                  > which I did not think of testing earlier, so the problem
                  > I see in your patch was also in my patch).
                  >
                  > The bug I see only happens when opening a file in Vim
                  > which does not exist but for which there is a swap file.
                  >
                  > Steps to reproduce it:
                  >
                  > # Open a file foobar (which does not exist) in vim to create
                  > # swap file .foobar.swp
                  > $ rm -f ~/foobar
                  > $ vim ~/foobar
                  >
                  > # In another terminal...
                  > $ vim -u NONE
                  > :au! SwapExists * cd /tmp
                  > :e ~/foobar
                  >
                  > The following prompt shows up:
                  >
                  > Swap file "~/.foobar.swp" already exists!
                  > [O]pen Read-Only, (E)dit anyway, (R)ecover, (Q)uit, (A)bort:
                  >
                  > Press e.
                  >
                  > Valgrind then outputs the following error
                  >
                  > ==17742== Invalid read of size 1
                  > ==17742== at 0x810B4DA: get_past_head (misc1.c:4472)
                  > ==17742== by 0x810B428: gettail_sep (misc1.c:4419)
                  > ==17742== by 0x810B610: dir_of_file_exists (misc1.c:4582)
                  > ==17742== by 0x80C13CD: readfile (fileio.c:606)
                  > ==17742== by 0x8052D8E: open_buffer (buffer.c:130)
                  > ==17742== by 0x8097AC3: do_ecmd (ex_cmds.c:3655)
                  > ==17742== by 0x80ADE20: do_exedit (ex_docmd.c:7557)
                  > ==17742== by 0x80ADA97: ex_edit (ex_docmd.c:7452)
                  > ==17742== by 0x80A6517: do_one_cmd (ex_docmd.c:2622)
                  > ==17742== by 0x80A3D97: do_cmdline (ex_docmd.c:1096)
                  > ==17742== by 0x8129422: nv_colon (normal.c:5218)
                  > ==17742== by 0x8122A86: normal_cmd (normal.c:1189)
                  > ==17742== by 0x80E5F8D: main_loop (main.c:1180)
                  > ==17742== by 0x80E5ADA: main (main.c:939)
                  > ==17742== Address 0x4d34be0 is 0 bytes inside a block of size 15 free'd
                  > ==17742== at 0x4024E5A: free (vg_replace_malloc.c:323)
                  > ==17742== by 0x8113C72: vim_free (misc2.c:1631)
                  > ==17742== by 0x80C8E58: shorten_fnames (fileio.c:5743)
                  > ==17742== by 0x80AE6AA: ex_cd (ex_docmd.c:7939)
                  > ==17742== by 0x80A6517: do_one_cmd (ex_docmd.c:2622)
                  > ==17742== by 0x80A3D97: do_cmdline (ex_docmd.c:1096)
                  > ==17742== by 0x80CC9A5: apply_autocmds_group (fileio.c:8883)
                  > ==17742== by 0x80CC3A8: apply_autocmds (fileio.c:8494)
                  > ==17742== by 0x80F9592: do_swapexists (memline.c:3770)
                  > ==17742== by 0x80F9EAD: findswapname (memline.c:4130)
                  > ==17742== by 0x80F3FF8: ml_open_file (memline.c:553)
                  > ==17742== by 0x80F411E: check_need_swap (memline.c:605)
                  > ==17742== by 0x80C13BF: readfile (fileio.c:605)
                  > ==17742== by 0x8052D8E: open_buffer (buffer.c:130)
                  > ==17742== by 0x8097AC3: do_ecmd (ex_cmds.c:3655)
                  > ==17742== by 0x80ADE20: do_exedit (ex_docmd.c:7557)
                  > ==17742== by 0x80ADA97: ex_edit (ex_docmd.c:7452)
                  > ==17742== by 0x80A6517: do_one_cmd (ex_docmd.c:2622)
                  > ==17742== by 0x80A3D97: do_cmdline (ex_docmd.c:1096)
                  > ==17742== by 0x8129422: nv_colon (normal.c:5218)
                  > ==17742== by 0x8122A86: normal_cmd (normal.c:1189)
                  > ==17742== by 0x80E5F8D: main_loop (main.c:1180)
                  > ==17742== by 0x80E5ADA: main (main.c:939)
                  > (and several other errors follow)
                  >
                  > If I enter ":messages" I also do not see the error
                  > message E812 (I would expect to see this message).
                  >
                  > I do not see this issue when the file foobar already exists.
                  >
                  > This remaining issue happens because call to
                  > check_need_swap() in fileio.c:605 (or line 592 in
                  > pristine vim-7.2.128) may trigger an autocommand
                  > which can also invalidate fname.
                  >
                  > Attached patch (on top of your patch) fixes this
                  > remaining issue (and a typo). I will continue to test
                  > with this patch for a while but so far it looks OK.
                  > I can't see any further bugs so far with autocommands.

                  Thanks, I'll include this as well.

                  When I test the steps you gave here I now get E811 for the ":cd"
                  command. And other nastyness should be caught by the check added by
                  this patch. So that's good.

                  --
                  Fingers not found - Pound head on keyboard to continue.

                  /// 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.