[Zlib-devel] [PATCH] One remaining Valgrind error

[Daniel Richard G.]

On Mon, 13 Aug 2012, Mark Adler wrote:

> Thank you for the patch, but I will definitely not be applying it. 
> That's way too much overhead to add to every deflateInit call.

How is it too much overhead? This is in an initialization routine (as 
opposed to an inner loop) that includes four calls to ZALLOC()/malloc(). 
deflateInit2_(), and thus this new zmemzero()/memset() call, get called 
all of fourteen times in "make check".

> In any case, I think valgrind is wrong.  What is valgrind reporting?

Here is an example of what I typically see. This is from a Valgrind run on 
LibXML2's runtest program:

==631== All heap blocks were freed -- no leaks are possible
==631== 
==631== ERROR SUMMARY: 20666 errors from 1 contexts (suppressed: 4 from 4)
==631== 
==631== 20666 errors in context 1 of 1:
==631== Conditional jump or move depends on uninitialised value(s)
==631==    at 0x5056510: inflateReset2 (in /lib/x86_64-linux-gnu/libz.so.1.2.3.4)
==631==    by 0x5056605: inflateInit2_ (in /lib/x86_64-linux-gnu/libz.so.1.2.3.4)
==631==    by 0x504FE7B: ??? (in /lib/x86_64-linux-gnu/libz.so.1.2.3.4)
==631==    by 0x44D7E1: xmlGzfileOpen_real (xmlIO.c:1181)
==631==    by 0x44D803: xmlGzfileOpen (xmlIO.c:1198)
==631==    by 0x44F336: __xmlParserInputBufferCreateFilename (xmlIO.c:2625)
==631==    by 0x44F485: xmlParserInputBufferCreateFilename (xmlIO.c:2689)
==631==    by 0x40FE3C: xmlNewInputFromFile (parserInternals.c:1511)
==631==    by 0x451477: xmlDefaultExternalEntityLoader (xmlIO.c:4036)
==631==    by 0x45164C: xmlNoNetExternalEntityLoader (xmlIO.c:4138)
==631==    by 0x40282A: testExternalEntityLoader (runtest.c:227)
==631==    by 0x451571: xmlLoadExternalEntity (xmlIO.c:4096)
==631==    by 0x436E5E: xmlCreateURLParserCtxt (parser.c:13964)
==631==    by 0x436EFF: xmlCreateFileParserCtxt (parser.c:13992)
==631==    by 0x436F29: xmlSAXParseFileWithData (parser.c:14023)
==631==    by 0x4370A1: xmlSAXParseFile (parser.c:14083)
==631==    by 0x4370E9: xmlParseFile (parser.c:14115)
==631==    by 0x405489: oldParseTest (runtest.c:1747)
==631==    by 0x40A242: launchTests (runtest.c:4353)
==631==    by 0x40A454: runtest (runtest.c:4405)
==631==    by 0x40A602: main (runtest.c:4443)
==631==  Uninitialised value was created by a heap allocation
==631==    at 0x4C28F9F: malloc (vg_replace_malloc.c:236)
==631==    by 0x50565E1: inflateInit2_ (in /lib/x86_64-linux-gnu/libz.so.1.2.3.4)
==631==    by 0x504FE7B: ??? (in /lib/x86_64-linux-gnu/libz.so.1.2.3.4)
==631==    by 0x44D7E1: xmlGzfileOpen_real (xmlIO.c:1181)
==631==    by 0x44D803: xmlGzfileOpen (xmlIO.c:1198)
==631==    by 0x44F336: __xmlParserInputBufferCreateFilename (xmlIO.c:2625)
==631==    by 0x44F485: xmlParserInputBufferCreateFilename (xmlIO.c:2689)
==631==    by 0x40FE3C: xmlNewInputFromFile (parserInternals.c:1511)
==631==    by 0x451477: xmlDefaultExternalEntityLoader (xmlIO.c:4036)
==631==    by 0x45164C: xmlNoNetExternalEntityLoader (xmlIO.c:4138)
==631==    by 0x40282A: testExternalEntityLoader (runtest.c:227)
==631==    by 0x451571: xmlLoadExternalEntity (xmlIO.c:4096)
==631==    by 0x436E5E: xmlCreateURLParserCtxt (parser.c:13964)
==631==    by 0x436EFF: xmlCreateFileParserCtxt (parser.c:13992)
==631==    by 0x436F29: xmlSAXParseFileWithData (parser.c:14023)
==631==    by 0x4370A1: xmlSAXParseFile (parser.c:14083)
==631==    by 0x4370E9: xmlParseFile (parser.c:14115)
==631==    by 0x405489: oldParseTest (runtest.c:1747)
==631==    by 0x40A242: launchTests (runtest.c:4353)
==631==    by 0x40A454: runtest (runtest.c:4405)
==631==    by 0x40A602: main (runtest.c:4443)

Note that Valgrind indicates no other memory issues.

This kind of thing is frustrating in general development work because the 
zlib errors are completely extraneous. If I could count the times that 
I've gone, "Crap, there's a memory error in my---oh, wait, never mind, 
that's just zlib noise. Somebody ought to fix that."

And while Valgrind may be wrong about the uninitialised value(s) being a 
problem, zlib doesn't have the luxury of being treated specially. Between 
the effort involved in teaching every memory debugger in existence "hey, 
don't worry, zlib knows what it's doing here," and applying a one-line 
patch that doesn't affect performance in any meaningful way... I think the 
patch is preferable.


--Daniel


-- 
Daniel Richard G. || danielg at teragram.com || Software Developer
Teragram Linguistic Technologies (a division of SAS)
http://www.teragram.com/