Why I think it's worth fixing Valgrind warnings

[Dan Kegel]

If you're wondering whether it's worth fixing Valgrind
warnings (after all, they are just warnings, right?):

First, some of the valgrind errors that show up
when running the test suite might be real bugs.
That might be enough reason right there.

Second, clearing out the valgrind errors that show
up in the test suite make it a lot easier to
find Wine bugs when running real apps on wine on valgrind.
(The lower the background noise level, the easier it is
to see the real problem.)

Third, if valgrind works well on wine, windows developers
might start coming to Wine just to run Valgrind on
their apps to find app bugs.  (Don't laugh!  It could happen!)

This is why I hired John Reiser to improve Valgrind/Wine support,
why I've been fixing a few valgrind warnings lately, and
why I think it's important for other people to start doing
it, too.

That said, it's important when fixing Valgrind errors to
really understand the code you're changing, lest you
paper over a bug with another bug, or make things worse.
(Here would be a good place for me to thank James for
his code reviews :-)
- Dan

[Steven Edwards]

On Tue, May 27, 2008 at 3:02 AM, Dan Kegel  wrote:
> Third, if valgrind works well on wine, windows developers
> might start coming to Wine just to run Valgrind on
> their apps to find app bugs.  (Don't laugh!  It could happen!)

I'll laugh. =)  The Windows devs will just use prefast in MSVC. The
other reasons all sound good though.

[Steven Edwards]

On Tue, May 27, 2008 at 5:14 AM, Steven Edwards  wrote:
> I'll laugh. =)  The Windows devs will just use prefast in MSVC. The
> other reasons all sound good though.

Hmm actually I thought prefast had a way to profile running code
rather than just static analysis but I guess not. I must have been
thinking of the Driver Verifier. I think valgrind might actually be
useful then as I don't know of a free profiler for userland code on
windows.

[Steven Edwards]

On Tue, May 27, 2008 at 5:29 AM, Steven Edwards  wrote:
> Hmm actually I thought prefast had a way to profile running code
> rather than just static analysis but I guess not. I must have been
> thinking of the Driver Verifier. I think valgrind might actually be
> useful then as I don't know of a free profiler for userland code on
> windows.

OK no more 5AM spam after this, I promise. This is what I was thinking of

http://www.microsoft.com/downloads/details.aspx?FamilyID=bd02c19c-1250-433c-...

The Application Verifier. If we want to help windows developers move
over, we should try to get this working.

[Rob Shearman]

2008/5/27 Steven Edwards :
> On Tue, May 27, 2008 at 5:29 AM, Steven Edwards  wrote:
> > Hmm actually I thought prefast had a way to profile running code
> > rather than just static analysis but I guess not. I must have been
> > thinking of the Driver Verifier. I think valgrind might actually be
> > useful then as I don't know of a free profiler for userland code on
> > windows.
>
> OK no more 5AM spam after this, I promise. This is what I was thinking of
>
> http://www.microsoft.com/downloads/details.aspx?FamilyID=bd02c19c-1250-433c-...
>
> The Application Verifier. If we want to help windows developers move
> over, we should try to get this working.

While Win32 application developer may use MS's Application Verifier to
debug heap overruns (using the Page Heap feature), I believe Valgrind
is much superior in doing this and Valgrind logs errors about heap
under-runs and accessing invalid memory on the stack.

There are other features available in the Application Verifier
(information on MSDN is 404 so this is from memory) like randomly
returning NULL from HeapAlloc/HeapReAlloc to ensure that the program
doesn't crash in OOM situations, but I believe this is provided by
turning on options in the Heap implementation rather than any magic in
the Application Verifier itself.

[Scott Ritchie]

Dan Kegel wrote:
> That said, it's important when fixing Valgrind errors to
> really understand the code you're changing, lest you
> paper over a bug with another bug, or make things worse.

*cough* Debian and OpenSSL *cough* ;)

On the other hand, you shouldn't have to be completely intimately
familiar with it like the original author was.  It's a real problem if
our code can't be read by a reasonably skilled hacker and fixed properly
when Valgrind starts spewing warnings.

Thanks,
Scott Ritchie