Hi
Valgrind memory checker detects mismatches between malloc()/vim_free()
and alloc()/free in vim-7.2.30:
- If malloc() is used, then free() should be used,
- if alloc() is used, then vim_free() should be used.
It only really matters when compiling vim with '#define MEM_PROFILE',
because MEMPROFILE adds one word before each allocated block so
a mismatch results not only in an illegal free, but also gives incorrect
memory profile results. Normally MEMPROFILE is not defined, so it's a
minor bug.
Here is an error reported by valgrind:
==9768== Invalid read of size 4
==9768== at 0x8132623: mem_pre_free (misc2.c:687)
==9768== by 0x8133650: vim_free (misc2.c:1623)
==9768== by 0x816A210: mch_free_mem (os_unix.c:2948)
==9768== by 0x8132CF0: free_all_mem (misc2.c:1122)
==9768== by 0x816A375: mch_exit (os_unix.c:3057)
==9768== by 0x8104DEC: getout (main.c:1340)
==9768== by 0x80CA9AE: ex_quit_all (ex_docmd.c:6281)
==9768== by 0x80C4A54: do_one_cmd (ex_docmd.c:2621)
==9768== by 0x80C22D5: do_cmdline (ex_docmd.c:1095)
==9768== by 0x81491C0: nv_colon (normal.c:5217)
==9768== by 0x8142761: normal_cmd (normal.c:1184)
==9768== by 0x8104B1F: main_loop (main.c:1179)
==9768== by 0x8104667: main (main.c:938)
==9768== Address 0x454e6fc is 4 bytes before a block of size 8,192 alloc'd
==9768== at 0x4022AE8: malloc (vg_replace_malloc.c:207)
==9768== by 0x816A151: mch_early_init (os_unix.c:2908)
==9768== by 0x8103D86: main (main.c:181)
==9768==
==9768== Invalid free() / delete / delete[]
==9768== Invalid free() / delete / delete[]
==9768== at 0x402268C: free (vg_replace_malloc.c:323)
==9768== by 0x813365B: vim_free (misc2.c:1625)
==9768== by 0x816A210: mch_free_mem (os_unix.c:2948)
==9768== by 0x8132CF0: free_all_mem (misc2.c:1122)
==9768== by 0x816A375: mch_exit (os_unix.c:3057)
==9768== by 0x8104DEC: getout (main.c:1340)
==9768== by 0x80CA9AE: ex_quit_all (ex_docmd.c:6281)
==9768== by 0x80C4A54: do_one_cmd (ex_docmd.c:2621)
==9768== by 0x80C22D5: do_cmdline (ex_docmd.c:1095)
==9768== by 0x81491C0: nv_colon (normal.c:5217)
==9768== by 0x8142761: normal_cmd (normal.c:1184)
==9768== by 0x8104B1F: main_loop (main.c:1179)
==9768== by 0x8104667: main (main.c:938)
==9768== Address 0x454e6fc is 4 bytes before a block of size 8,192 alloc'd
==9768== at 0x4022AE8: malloc (vg_replace_malloc.c:207)
==9768== by 0x816A151: mch_early_init (os_unix.c:2908)
==9768== by 0x8103D86: main (main.c:181)
Another mismatch:
==9768== Invalid free() / delete / delete[]
==9768== at 0x402268C: free (vg_replace_malloc.c:323)
==9768== by 0x813365B: vim_free (misc2.c:1625)
==9768== by 0x816E7FB: xsmp_close (os_unix.c:6817)
==9768== by 0x8132D0F: free_all_mem (misc2.c:1140)
==9768== by 0x816A375: mch_exit (os_unix.c:3057)
==9768== by 0x8104DEC: getout (main.c:1340)
==9768== by 0x80CA9AE: ex_quit_all (ex_docmd.c:6281)
==9768== by 0x80C4A54: do_one_cmd (ex_docmd.c:2621)
==9768== by 0x80C22D5: do_cmdline (ex_docmd.c:1095)
==9768== by 0x81491C0: nv_colon (normal.c:5217)
==9768== by 0x8142761: normal_cmd (normal.c:1184)
==9768== by 0x8104B1F: main_loop (main.c:1179)
==9768== by 0x8104667: main (main.c:938)
==9768== Address 0x458ad3c is 4 bytes before a block of size 38 alloc'd
==9768== at 0x4022AE8: malloc (vg_replace_malloc.c:207)
==9768== by 0x450A167: _SmcProcessMessage (in /usr/lib/libSM.so.6.0.0)
==9768== by 0x451DAB2: IceProcessMessages (in /usr/lib/libICE.so.6.3.0)
==9768== by 0x45065A2: SmcOpenConnection (in /usr/lib/libSM.so.6.0.0)
==9768== by 0x816E741: xsmp_init (os_unix.c:6769)
==9768== by 0x8103FE1: main (main.c:494)
Here memory is allocated with malloc inside SmcOpenConnection(), so it
should be freed by free() instead of vim_free().
Here is a memory profile before & after fix, when doing "vim -u NONE" and
quiting immediately. Before fix, notice that it reports 6 allocated blocks
>8199, and only 4 of them are reported as freed when exiting. 2 of those
large blocks are actually spurious and result from the mismatches.
After fix, it reports 4 allocated blocks >8199 and all 4 of them are reported
as freed.
Before fix:
[ 1 / 41-41 ] [ 2 / 12-12 ] [ 3 / 12-12 ] [ 4 / 37-37 ]
[ 5 / 109-109 ] [ 6 / 79-79 ] [ 7 / 80-80 ] [ 8 / 125-125 ]
[ 9 / 68-68 ] [ 10 / 74-74 ] [ 11 / 71-71 ] [ 12 / 18-18 ]
[ 13 / 23-23 ] [ 15 / 5-5 ] [ 16 / 3-3 ] [ 17 / 2-2 ]
[ 19 / 4-4 ] [ 20 / 2-2 ] [ 21 / 5-5 ] [ 23 / 2-2 ]
[ 24 / 4-4 ] [ 25 / 1-1 ] [ 27 / 1-1 ] [ 28 / 3-3 ]
[ 29 / 1-1 ] [ 30 / 1-1 ] [ 32 / 4-4 ] [ 36 / 1-1 ]
[ 40 / 2-2 ] [ 42 / 2-2 ] [ 44 / 2-2 ] [ 51 / 1-1 ]
[ 54 / 1-1 ] [ 80 / 1-1 ] [ 100 / 2-1 ] [ 121 / 1-1 ]
[ 129 / 1-1 ] [ 144 / 1-1 ] [ 168 / 2-2 ] [ 246 / 1-1 ]
[ 268 / 1-1 ] [ 276 / 1-1 ] [ 288 / 1-1 ] [ 320 / 1-1 ]
[ 336 / 1-1 ] [ 369 / 2-2 ] [ 432 / 1-1 ] [ 504 / 1-1 ]
[ 512 / 1-1 ] [ 568 / 1-1 ] [ 640 / 1-1 ] [ 960 / 1-1 ]
[1000 / 1-1 ] [1024 / 1-1 ] [1025 / 1-1 ] [2000 / 1-1 ]
[2048 / 10-10 ] [3000 / 1-1 ] [3672 / 1-1 ] [4000 / 1-1 ]
[4096 / 4-4 ] [4520 / 1-1 ] [4551 / 1-1 ] [4800 / 1-1 ]
[5200 / 1-1 ]
[>8199 / 6-4 ]
After fix:
[ 1 / 41-41 ] [ 2 / 13-13 ] [ 3 / 12-12 ] [ 4 / 36-36 ]
[ 5 / 109-109 ] [ 6 / 78-78 ] [ 7 / 80-80 ] [ 8 / 124-124 ]
[ 9 / 68-68 ] [ 10 / 74-74 ] [ 11 / 71-71 ] [ 12 / 18-18 ]
[ 13 / 23-23 ] [ 15 / 5-5 ] [ 16 / 3-3 ] [ 17 / 2-2 ]
[ 19 / 4-4 ] [ 20 / 2-2 ] [ 21 / 5-5 ] [ 23 / 2-2 ]
[ 24 / 4-4 ] [ 25 / 1-1 ] [ 27 / 1-1 ] [ 28 / 3-3 ]
[ 29 / 1-1 ] [ 30 / 1-1 ] [ 32 / 4-4 ] [ 36 / 1-1 ]
[ 40 / 2-2 ] [ 42 / 2-2 ] [ 44 / 2-2 ] [ 51 / 1-1 ]
[ 54 / 1-1 ] [ 80 / 1-1 ] [ 100 / 2-1 ] [ 121 / 1-1 ]
[ 129 / 1-1 ] [ 144 / 1-1 ] [ 168 / 2-2 ] [ 246 / 1-1 ]
[ 268 / 1-1 ] [ 276 / 1-1 ] [ 288 / 1-1 ] [ 320 / 1-1 ]
[ 336 / 1-1 ] [ 369 / 2-2 ] [ 432 / 1-1 ] [ 504 / 1-1 ]
[ 512 / 1-1 ] [ 568 / 1-1 ] [ 640 / 1-1 ] [ 960 / 1-1 ]
[1000 / 1-1 ] [1024 / 1-1 ] [1025 / 1-1 ] [2000 / 1-1 ]
[2048 / 10-10 ] [3000 / 1-1 ] [3672 / 1-1 ] [4000 / 1-1 ]
[4096 / 4-4 ] [4520 / 1-1 ] [4551 / 1-1 ] [4800 / 1-1 ]
[5200 / 1-1 ] [8192 / 1-1 ]
[>8199 / 4-4 ]
Attached patch fixes above 2 mismatches detected by valgrind + a couple
of other mismatches which I found by looking at the code. I may have
missed some, but at least I do not see other such errors with valgrind so far.
Cheers
-- Dominique
--~--~---------~--~----~------------~-------~--~----~
You received this message from the "vim_dev" maillist.
For more information, visit http://www.vim.org/maillist.php
-~----------~----~----~----~------~----~------~--~---
Index: os_unix.c
===================================================================
RCS file: /cvsroot/vim/vim7/src/os_unix.c,v
retrieving revision 1.85
diff -c -r1.85 os_unix.c
*** os_unix.c 6 Aug 2008 16:45:01 -0000 1.85
--- os_unix.c 8 Nov 2008 04:22:38 -0000
***************
*** 2905,2911 ****
* Ignore any errors.
*/
#if defined(HAVE_SIGALTSTACK) || defined(HAVE_SIGSTACK)
! signal_stack = malloc(SIGSTKSZ);
init_signal_stack();
#endif
}
--- 2905,2911 ----
* Ignore any errors.
*/
#if defined(HAVE_SIGALTSTACK) || defined(HAVE_SIGSTACK)
! signal_stack = (char *)alloc(SIGSTKSZ);
init_signal_stack();
#endif
}
***************
*** 6814,6820 ****
if (xsmp_icefd != -1)
{
SmcCloseConnection(xsmp.smcconn, 0, NULL);
! vim_free(xsmp.clientid);
xsmp.clientid = NULL;
xsmp_icefd = -1;
}
--- 6814,6821 ----
if (xsmp_icefd != -1)
{
SmcCloseConnection(xsmp.smcconn, 0, NULL);
! if (xsmp.clientid)
! free(xsmp.clientid);
xsmp.clientid = NULL;
xsmp_icefd = -1;
}
Index: gui_x11.c
===================================================================
RCS file: /cvsroot/vim/vim7/src/gui_x11.c,v
retrieving revision 1.19
diff -c -r1.19 gui_x11.c
*** gui_x11.c 20 Jun 2008 09:59:25 -0000 1.19
--- gui_x11.c 8 Nov 2008 04:22:39 -0000
***************
*** 2450,2456 ****
*colorPtr = colortable[closest];
}
! free(colortable);
return OK;
}
--- 2450,2456 ----
*colorPtr = colortable[closest];
}
! vim_free(colortable);
return OK;
}
Index: gui_riscos.c
===================================================================
RCS file: /cvsroot/vim/vim7/src/gui_riscos.c,v
retrieving revision 1.11
diff -c -r1.11 gui_riscos.c
*** gui_riscos.c 10 May 2007 17:33:26 -0000 1.11
--- gui_riscos.c 8 Nov 2008 04:22:41 -0000
***************
*** 695,701 ****
gui_mch_set_shellsize(width, height, min_width, min_height, base_width, base_height, direction)
int width; /* In OS units */
int height;
! int min_width; /* Smallest permissable window size (ignored) */
int min_height;
int base_width; /* Space for scroll bars, etc */
int base_height;
--- 695,701 ----
gui_mch_set_shellsize(width, height, min_width, min_height, base_width, base_height, direction)
int width; /* In OS units */
int height;
! int min_width; /* Smallest permissible window size (ignored) */
int min_height;
int base_width; /* Space for scroll bars, etc */
int base_height;
***************
*** 863,869 ****
if (strncmp(file, "ZapFont\015", 8) == 0)
return file; /* Loaded OK! */
! free(file);
return NULL; /* Not a valid font file */
}
--- 863,869 ----
if (strncmp(file, "ZapFont\015", 8) == 0)
return file; /* Loaded OK! */
! vim_free(file);
return NULL; /* Not a valid font file */
}
Index: dosinst.c
===================================================================
RCS file: /cvsroot/vim/vim7/src/dosinst.c,v
retrieving revision 1.8
diff -c -r1.8 dosinst.c
*** dosinst.c 16 Mar 2008 13:54:13 -0000 1.8
--- dosinst.c 8 Nov 2008 04:22:42 -0000
***************
*** 63,69 ****
{
"\nChoose the default way to run Vim:",
"Vi compatible",
! "with some Vim ehancements",
"with syntax highlighting and other features switched on",
};
int compat_choice = (int)compat_all_enhancements;
--- 63,69 ----
{
"\nChoose the default way to run Vim:",
"Vi compatible",
! "with some Vim enhancements",
"with syntax highlighting and other features switched on",
};
int compat_choice = (int)compat_all_enhancements;
***************
*** 347,354 ****
myexit(1);
}
! free(*destination);
! free(tmpname);
*destination = farname;
}
--- 347,354 ----
myexit(1);
}
! vim_free(*destination);
! vim_free(tmpname);
*destination = farname;
}
***************
*** 379,385 ****
}
if (check_bat_only && targets[i].oldbat != NULL)
{
! free(targets[i].oldbat);
targets[i].oldbat = NULL;
}
}
--- 379,385 ----
}
if (check_bat_only && targets[i].oldbat != NULL)
{
! vim_free(targets[i].oldbat);
targets[i].oldbat = NULL;
}
}
***************
*** 554,560 ****
}
run_command(temp_string_buffer);
! /* Check if an unistall reg key was deleted.
* if it was, we want to decrement key_index.
* if we don't do this, we will skip the key
* immediately after any key that we delete. */
--- 554,560 ----
}
run_command(temp_string_buffer);
! /* Check if an uninstall reg key was deleted.
* if it was, we want to decrement key_index.
* if we don't do this, we will skip the key
* immediately after any key that we delete. */
Index: uninstal.c
===================================================================
RCS file: /cvsroot/vim/vim7/src/uninstal.c,v
retrieving revision 1.1
diff -c -r1.1 uninstal.c
*** uninstal.c 13 Jun 2004 17:05:49 -0000 1.1
--- uninstal.c 8 Nov 2008 04:22:42 -0000
***************
*** 200,206 ****
}
else
printf(" - the batch file %s\n", batfile_path);
! free(batfile_path);
}
}
return found;
--- 200,206 ----
}
else
printf(" - the batch file %s\n", batfile_path);
! vim_free(batfile_path);
}
}
return found;
