Ooodles of sexy new valgrind warnings

[Dan Kegel]

I updated to the latest valgrind from svn, and
it gives lots of nifty new warnings.  e.g. it
warns if you are using the wrong free for a
particular memory block, and it tells you
where uninitialized blocks came from.

http://kegel.com/wine/valgrind/logs-2008-05-25-summary.txt
is a roadmap of where the sexy new stuff is; the details
are in http://kegel.com/wine/valgrind/logs-2008-05-25/

For instance, it says

diff -u logs-2008-05-20/vg-winmm_capture.txt
logs-2008-05-25/vg-winmm_capture.txt
+  Uninitialised value was created by a client request
+ Mismatched free() / delete / delete []

Pulling up just that diff,
http://kegel.com/wine/valgrind/logs-2008-05-25/vg-winmm_capture-diff.txt
shows the latter error is
+ Mismatched free() / delete / delete []
+    at  RtlFreeHeap (heap.c:1287)
+    by  wave_in_test_device (capture.c:397)
+    by  func_capture (capture.c:716)
+    by  run_test (test.h:414)
+    by  main (test.h:463)
+  Address 0x440cc90 is 0 bytes inside a block of size 14 alloc'd
+    at  malloc (vg_replace_malloc.c:207)
+    by  wave_in_test_device (capture.c:388)
+    by  func_capture (capture.c:716)
+    by  run_test (test.h:414)
+    by  main (test.h:463)

Sure enough, memory is allocate with strdup() and freed with HeapFree(), tsk.

Anyway, there's plenty here for everybody.  Pile on!

Tomorrow I'll start running these without -O2;
Zac Brown reminded me that gives better stack tracebacks
(i.e. it inhibits inlining, which often causes small static
functions to disappear from the stack dumps in valgrind's error messages).
- Dan

[Maarten Lankhorst]

2008/5/25 Dan Kegel :
> I updated to the latest valgrind from svn, and
> it gives lots of nifty new warnings.  e.g. it
> warns if you are using the wrong free for a
> particular memory block, and it tells you
> where uninitialized blocks came from.
> http://kegel.com/wine/valgrind/logs-2008-05-25-summary.txt
> is a roadmap of where the sexy new stuff is; the details
> are in http://kegel.com/wine/valgrind/logs-2008-05-25/
>
> For instance, it says
>
> diff -u logs-2008-05-20/vg-winmm_capture.txt
> logs-2008-05-25/vg-winmm_capture.txt
> - Uninitialised value was created by a client request
> - Mismatched free() / delete / delete []
>
> Pulling up just that diff,
> http://kegel.com/wine/valgrind/logs-2008-05-25/vg-winmm_capture-diff.txt
> shows the latter error is
> - Mismatched free() / delete / delete []
> - at  RtlFreeHeap (heap.c:1287)
> - by  wave_in_test_device (capture.c:397)
> - by  func_capture (capture.c:716)
> - by  run_test (test.h:414)
> - by  main (test.h:463)
> - Address 0x440cc90 is 0 bytes inside a block of size 14 alloc'd
> - at  malloc (vg_replace_malloc.c:207)
> - by  wave_in_test_device (capture.c:388)
> - by  func_capture (capture.c:716)
> - by  run_test (test.h:414)
> - by  main (test.h:463)

This particular kind of bug happens when you use a string function
like strdup to allocate some memory, then try to free it with
HeapFree. The proper fix would be to use HeapAlloc.

None the less valgrind's new logs are very useful as it now reports
almost all invalid uses of Free() with a very low false positive
count. For those who haven't tried valgrind before: try it, it's great
for finding a lot of memory related bugs. You'll probably need the svn
version since the last stable release had some bugs that wine would be
affected by.

Cheers,
Maarten.

[Dan Kegel]

On Mon, May 26, 2008 at 5:15 PM, Maarten Lankhorst
 wrote:
> > - Mismatched free() / delete / delete []
> > - at  RtlFreeHeap (heap.c:1287)
> > - by  wave_in_test_device (capture.c:397)
>
> This particular kind of bug happens when you use a string function
> like strdup to allocate some memory, then try to free it with
> HeapFree. The proper fix would be to use HeapAlloc.

Yeah, I just copied what you did with the other one in that DLL the
other day; see
http://www.winehq.org/pipermail/wine-patches/2008-May/055219.html

[Maarten Lankhorst]

Hi Dan,

2008/5/26 Dan Kegel :
> On Mon, May 26, 2008 at 5:15 PM, Maarten Lankhorst
>  wrote:
> > > - Mismatched free() / delete / delete []
> > > - at  RtlFreeHeap (heap.c:1287)
> > > - by  wave_in_test_device (capture.c:397)
> >
> > This particular kind of bug happens when you use a string function
> > like strdup to allocate some memory, then try to free it with
> > HeapFree. The proper fix would be to use HeapAlloc.
>
> Yeah, I just copied what you did with the other one in that DLL the
> other day; see
> http://www.winehq.org/pipermail/wine-patches/2008-May/055219.html

Proactively grepping I also found these:

$ git grep =[\ ]*strdup(
dinput/joystick_linuxinput.c:        joydev.device = strdup(buf);
dinput/joystick_linuxinput.c:          joydev.name = strdup(buf);
fusion/assembly.c:    *path = strdup(assembly->path);
gphoto2.ds/ds_ctrl.c:   gpfile->folder = strdup(folder);
gphoto2.ds/ds_ctrl.c:   gpfile->filename = strdup(name);
twain_32/dsm_ctrl.c:            devices[nrdevices].modname = strdup(dsname);
wineps.drv/mkagl.c:     glyphs[num_glyphs].name = strdup(namebuf);
wineps.drv/mkagl.c:     glyphs[num_glyphs].comment = strdup(commbuf);
wineps.drv/mkagl.c:         glyphs[num_glyphs].name = strdup(namebuf);
wineps.drv/mkagl.c:         glyphs[num_glyphs].comment = strdup(linebuf);
winex11.drv/opengl.c:    WineGLInfo.glExtensions = strdup((const char
*) pglGetString(GL_EXTENSIONS));

I'll probably take a look tomorrow to see which ones are harmful and
which aren't.

Cheers,
Maarten.