Bram Moolenaar wrote:
> Dominique Pelle wrote:
>
>> I ran "make test" with Vim-7.3a BETA (f8222d1f9a73)
>> and all test passed. However, I then uncommented the
>> VALGRIND = ... line in src/testdir/Makefile file, and test61 failed:
>>
>> Test results:
>> test61 FAILED
>> TEST FAILURE
>>
>> $ diff -c test61.ok test61.failed
>> *** test61.ok 2010-05-24 08:32:37.541829060 +0200
>> --- test61.failed 2010-05-24 08:48:18.237829178 +0200
>> ***************
>> *** 17,23 ****
>> 2222 -----
>> 123456abc
>> 123456
>> ! 123456789
>> 123456
>> 123456abc
>> aaaa
>> --- 17,23 ----
>> 2222 -----
>> 123456abc
>> 123456
>> ! 23456789
>> 123456
>> 123456abc
>> aaaa
>>
>> I reran it again and it then succeeded. Perhaps it failed
>> the first time because test depends on time (I see ":sleep 2")
>> and Valgrind slows down things.
>
> Yes, test61 sometimes fails because of a dependency on time. This is
> not easy to solve. Would have to implement a "virtual time for testing"
> feature.
>
>> However, I also see that file src/testdir/valgrind.test61 contains
>> errors:
>>
>> ==7309== Conditional jump or move depends on uninitialised value(s)
>> ==7309== at 0x818BEA5: put_bytes (spell.c:8027)
>> ==7309== by 0x81BD367: serialize_uep (undo.c:1158)
>> ==7309== by 0x81BDC6C: u_write_undo (undo.c:1401)
>> ==7309== by 0x80C8CAD: buf_write (fileio.c:4952)
>> ==7309== by 0x8097605: do_write (ex_cmds.c:2706)
>> ==7309== by 0x80970C8: ex_write (ex_cmds.c:2519)
>> ==7309== by 0x80A782C: do_one_cmd (ex_docmd.c:2639)
>> ==7309== by 0x80A5105: do_cmdline (ex_docmd.c:1108)
>> ==7309== by 0x812AD11: nv_colon (normal.c:5226)
>> ==7309== by 0x812459B: normal_cmd (normal.c:1188)
>> ==7309== by 0x80E7930: main_loop (main.c:1216)
>> ==7309== by 0x80E7427: main (main.c:960)
>> ==7309== Uninitialised value was created by a heap allocation
>> ==7309== at 0x4024F70: malloc (vg_replace_malloc.c:236)
>> ==7309== by 0x8114F74: lalloc (misc2.c:919)
>> ==7309== by 0x81BBFF2: u_savecommon (undo.c:595)
>> ==7309== by 0x81BB850: u_save (undo.c:251)
>> ==7309== by 0x81BB7DB: u_save_cursor (undo.c:228)
>> ==7309== by 0x806E120: stop_arrow (edit.c:6391)
>> ==7309== by 0x806CB98: insert_special (edit.c:5485)
>> ==7309== by 0x80664FA: edit (edit.c:1394)
>> ==7309== by 0x81309C7: invoke_edit (normal.c:8912)
>> ==7309== by 0x813096D: nv_edit (normal.c:8885)
>> ==7309== by 0x812459B: normal_cmd (normal.c:1188)
>> ==7309== by 0x80E7930: main_loop (main.c:1216)
>> ==7309== by 0x80E7427: main (main.c:960)
>> ==7309==
>> ==7309== Syscall param write(buf) points to uninitialised byte(s)
>> ==7309== at 0x4C77013: __write_nocancel (in
>> /lib/tls/i686/cmov/libc-2.10.1.so)
>> ==7309== by 0x4C2092E: new_do_write (fileops.c:529)
>> ==7309== by 0x4C20C15: _IO_do_write@@GLIBC_2.1 (fileops.c:502)
>> ==7309== by 0x4C2212F: _IO_file_close_it@@GLIBC_2.1 (fileops.c:169)
>> ==7309== by 0x4C15547: fclose@@GLIBC_2.1 (iofclose.c:62)
>> ==7309== by 0x81BDD81: u_write_undo (undo.c:1429)
>> ==7309== by 0x80C8CAD: buf_write (fileio.c:4952)
>> ==7309== by 0x8097605: do_write (ex_cmds.c:2706)
>> ==7309== by 0x80970C8: ex_write (ex_cmds.c:2519)
>> ==7309== by 0x80A782C: do_one_cmd (ex_docmd.c:2639)
>> ==7309== by 0x80A5105: do_cmdline (ex_docmd.c:1108)
>> ==7309== by 0x812AD11: nv_colon (normal.c:5226)
>> ==7309== by 0x812459B: normal_cmd (normal.c:1188)
>> ==7309== by 0x80E7930: main_loop (main.c:1216)
>> ==7309== by 0x80E7427: main (main.c:960)
>> ==7309== Address 0x403d1e0 is not stack'd, malloc'd or (recently) free'd
>> ==7309== Uninitialised value was created by a heap allocation
>> ==7309== at 0x4024F70: malloc (vg_replace_malloc.c:236)
>> ==7309== by 0x8114F74: lalloc (misc2.c:919)
>> ==7309== by 0x81BBFF2: u_savecommon (undo.c:595)
>> ==7309== by 0x81BB850: u_save (undo.c:251)
>> ==7309== by 0x81BB7DB: u_save_cursor (undo.c:228)
>> ==7309== by 0x806E120: stop_arrow (edit.c:6391)
>> ==7309== by 0x806CB98: insert_special (edit.c:5485)
>> ==7309== by 0x80664FA: edit (edit.c:1394)
>> ==7309== by 0x81309C7: invoke_edit (normal.c:8912)
>> ==7309== by 0x813096D: nv_edit (normal.c:8885)
>> ==7309== by 0x812459B: normal_cmd (normal.c:1188)
>> ==7309== by 0x80E7930: main_loop (main.c:1216)
>> ==7309== by 0x80E7427: main (main.c:960)
>>
>> I added --track-origins=yes to Valgrind options (new in Valgrind-3.5)
>> in src/testdir/Makefile to show the origin of uninitialized memory.
>>
>> Field uep->ue_lcount is uninitialized when serialized at undo.c:1158:
>>
>> 1155 put_bytes(fp, (long_u)uep_len, 4);
>> 1156 put_bytes(fp, (long_u)uep->ue_top, 4);
>> 1157 put_bytes(fp, (long_u)uep->ue_bot, 4);
>> 1158 put_bytes(fp, (long_u)uep->ue_lcount, 4);
>> 1159 put_bytes(fp, (long_u)uep->ue_size, 4);
>>
>> The uninitialized field might not be unused. If so, it's not
>> a severe issue. But it's best to initialize it to have
>> deterministic persistent undo files.
>>
>> Attached patch fixes it.
>
> Thanks, I'll include it. I'll also move the other memset that was
> _before_ checking for a NULL pointer...
I also notice this additional error which happens sometimes
when loading the persistent undo file:
==14996== Invalid write of size 1
==14996== at 0x402753D: memmove (mc_replace_strmem.c:629)
==14996== by 0x81A62C0: u_read_undo (string3.h:59)
==14996== by 0x80C45E1: readfile (fileio.c:2591)
==14996== by 0x805A20D: open_buffer (buffer.c:132)
==14996== by 0x809329E: do_ecmd (ex_cmds.c:3658)
==14996== by 0x80AEEC3: do_exedit (ex_docmd.c:7620)
==14996== by 0x80AF818: ex_edit (ex_docmd.c:7516)
==14996== by 0x80AC4ED: do_one_cmd (ex_docmd.c:2639)
==14996== by 0x80AA947: do_cmdline (ex_docmd.c:1108)
==14996== by 0x812322E: nv_colon (normal.c:5226)
==14996== by 0x8125194: normal_cmd (normal.c:1188)
==14996== by 0x80E4296: main_loop (main.c:1216)
==14996== by 0x80E7880: main (main.c:960)
==14996== Address 0x4fa80c7 is 2 bytes after a block of size 45 alloc'd
==14996== at 0x4024F70: malloc (vg_replace_malloc.c:236)
==14996== by 0x810D1F7: lalloc (misc2.c:919)
==14996== by 0x81A5E55: u_read_undo (undo.c:915)
==14996== by 0x80C45E1: readfile (fileio.c:2591)
==14996== by 0x805A20D: open_buffer (buffer.c:132)
==14996== by 0x809329E: do_ecmd (ex_cmds.c:3658)
==14996== by 0x80AEEC3: do_exedit (ex_docmd.c:7620)
==14996== by 0x80AF818: ex_edit (ex_docmd.c:7516)
==14996== by 0x80AC4ED: do_one_cmd (ex_docmd.c:2639)
==14996== by 0x80AA947: do_cmdline (ex_docmd.c:1108)
==14996== by 0x812322E: nv_colon (normal.c:5226)
==14996== by 0x8125194: normal_cmd (normal.c:1188)
==14996== by 0x80E4296: main_loop (main.c:1216)
==14996== by 0x80E7880: main (main.c:960)
Attached patch should fix it.
Cheers
-- Dominique
--
You received this message from the "vim_dev" maillist.
Do not top-post! Type your reply below the text you are replying to.
For more information, visit http://www.vim.org/maillist.php
diff -r f8222d1f9a73 src/undo.c
--- a/src/undo.c Sun May 23 23:34:36 2010 +0200
+++ b/src/undo.c Mon May 24 13:19:27 2010 +0200
@@ -1020,10 +1020,10 @@
{
/* If we've had to move from the rightmost side of the table,
* we have to shift everything to the right by one spot. */
- if (i < num_read_uhps - 1)
+ if (num_read_uhps - i - 1 > 0)
{
memmove(uhp_table + i + 2, uhp_table + i + 1,
- (num_read_uhps - i) * sizeof(u_header_T *));
+ (num_read_uhps - i - 1) * sizeof(u_header_T *));
}
uhp_table[i + 1] = uhp;
break;
