Hi, [Cc'ed the valgrind-users list, maybe the valgrind Gods can see that our case is pretty strange, and tell us what we do wrong.] Note to valgrind experts: this is _not_ about the Conditional thing in zlib, but about an uninitialized byte _in the middle_ of the zlib output buffer. On Tue, 27 Jan 2009, Linus Torvalds wrote: > Hmm. The zlib faq has a note about zlib doing a conditional on > uninitialized memory that doesn't matter, and that is what the > suppression should be about (to avoid a warning about "Conditional jump > or move depends on uninitialised value"). > > But that one is documented to not matter for the actual output (zlib > FAQ#36). > > It's possible that zlib really does leave padding bytes around that > literally don't matter, and that don't get initialized. That really > would be bad, because it means that the output of git wouldn't be > repeatable. But I doubt this is the case - original git used to actually > do the SHA1 over the _compressed_ data, which was admittedly a totally > and utterly broken design (and we fixed it), but it did work. Maybe it > worked by luck, but I somehow doubt it. > > Some googling did find this: > > http://mailman.few.vu.nl/pipermail/sysprog/2008-October/000298.html > > which looks very similar: an uninitialized byte in the middle of a > deflate() packet. > > Anyway, I'm just going to Cc 'zlib@xxxxxxxx', since this definitely is > _not_ the same issue as in the FAQ, and we're not the only ones seeing it. > > [...] > > Dscho wrote: > > > Yet, the buffer in question is 195 bytes, stream.total_count (which > > totally agrees with size - stream.avail_out) says it is 58 bytes, and > > valgrind says that the byte with offset 51 is uninitialized. > > The thing to note here is that what we are passing in to "write_buffer()" > is _exactly_ what zlib deflated for us: > > - 'compressed' is the allocation, and is what we used to initialize > 'stream.next_out' with (at the top of the code sequence above) > > - 'size' is gotten from 'stream.total_out' at the end of the compression. > > Oh Gods of zlib, please hear our plea for clarification.. To help ye Gods, I put together this almost minimal C program: -- snip -- #include <stdio.h> #include <stdlib.h> #include <string.h> #include <zlib.h> int main(int argc, char **argv) { const char hdr[] = { 0x74, 0x72, 0x65, 0x65, 0x20, 0x31, 0x36, 0x35, 0x00, }; int hdrlen = sizeof(hdr); const char buf[] = { 0x31, 0x30, 0x30, 0x36, 0x34, 0x34, 0x20, 0x66, 0x69, 0x6c, 0x65, 0x31, 0x00, 0x10, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x31, 0x30, 0x30, 0x36, 0x34, 0x34, 0x20, 0x66, 0x69, 0x6c, 0x65, 0x32, 0x00, 0x20, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x31, 0x30, 0x30, 0x36, 0x34, 0x34, 0x20, 0x66, 0x69, 0x6c, 0x65, 0x33, 0x00, 0x30, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x31, 0x30, 0x30, 0x36, 0x34, 0x34, 0x20, 0x66, 0x69, 0x6c, 0x65, 0x34, 0x00, 0x40, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x31, 0x30, 0x30, 0x36, 0x34, 0x34, 0x20, 0x66, 0x69, 0x6c, 0x65, 0x35, 0x00, 0x50, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, }; int len = sizeof(buf); z_stream stream; unsigned char *compressed; int size, ret, i; FILE *out; memset(&stream, 0, sizeof(stream)); deflateInit(&stream, Z_BEST_SPEED); size = 8 + deflateBound(&stream, len+hdrlen); compressed = malloc(size); if (!compressed) return 1; stream.next_out = compressed; stream.avail_out = size; stream.next_in = (unsigned char *)hdr; stream.avail_in = hdrlen; while ((ret = deflate(&stream, 0)) == Z_OK) /* nothing */; /* deflate() returns Z_BUF_ERROR at this point */ stream.next_in = (unsigned char *)buf; stream.avail_in = len; ret = deflate(&stream, Z_FINISH); if (ret != Z_STREAM_END) return 1; if (deflateEnd(&stream) != Z_OK) return 1; out = fopen("/dev/null", "w"); fwrite(compressed + 51, 51, 1, out); fwrite(compressed + 51, 1, 1, stderr); fflush(out); fclose(out); free(compressed); return 0; } -- snap -- ... which produces this output... -- snip -- ==6348== Memcheck, a memory error detector. ==6348== Copyright (C) 2002-2008, and GNU GPL'd, by Julian Seward et al. ==6348== Using LibVEX rev exported, a library for dynamic binary translation. ==6348== Copyright (C) 2004-2008, and GNU GPL'd, by OpenWorks LLP. ==6348== Using valgrind-3.5.0.SVN, a dynamic binary instrumentation framework. ==6348== Copyright (C) 2000-2008, and GNU GPL'd, by Julian Seward et al. ==6348== For more details, rerun with: -v ==6348== ==6348== Use of uninitialised value of size 8 ==6348== at 0x4E2FC5B: (within /usr/lib/libz.so.1.2.3.3) ==6348== by 0x4E317B6: (within /usr/lib/libz.so.1.2.3.3) ==6348== by 0x4E2DF9C: (within /usr/lib/libz.so.1.2.3.3) ==6348== by 0x4E2E654: deflate (in /usr/lib/libz.so.1.2.3.3) ==6348== by 0x400957: main (valgrind-testcase.c:60) ==6348== ==6348== Syscall param write(buf) points to uninitialised byte(s) ==6348== at 0x5103D50: write (in /lib/libc-2.6.1.so) ==6348== by 0x50A9AE2: _IO_file_write (in /lib/libc-2.6.1.so) ==6348== by 0x50A9748: (within /lib/libc-2.6.1.so) ==6348== by 0x50A9A4B: _IO_file_xsputn (in /lib/libc-2.6.1.so) ==6348== by 0x509FDBA: fwrite (in /lib/libc-2.6.1.so) ==6348== by 0x4009D7: main (valgrind-testcase.c:69) ==6348== Address 0x53da87b is 51 bytes inside a block of size 195 alloc'd ==6348== at 0x4C222CB: malloc (in /usr/local/lib/valgrind/amd64-linux/vgpreload_memcheck.so) ==6348== by 0x4008D7: main (valgrind-testcase.c:45) ,==6348== ==6348== Syscall param write(buf) points to uninitialised byte(s) ==6348== at 0x5103D50: write (in /lib/libc-2.6.1.so) ==6348== by 0x50A9AE2: _IO_file_write (in /lib/libc-2.6.1.so) ==6348== by 0x50A9748: (within /lib/libc-2.6.1.so) ==6348== by 0x50A9A83: _IO_do_write (in /lib/libc-2.6.1.so) ==6348== by 0x50AA048: _IO_file_sync (in /lib/libc-2.6.1.so) ==6348== by 0x509EDB9: fflush (in /lib/libc-2.6.1.so) ==6348== by 0x4009E0: main (valgrind-testcase.c:70) ==6348== Address 0x4020000 is not stack'd, malloc'd or (recently) free'd ==6348== ==6348== ERROR SUMMARY: 3 errors from 3 contexts (suppressed: 15 from 4) ==6348== malloc/free: in use at exit: 0 bytes in 0 blocks. ==6348== malloc/free: 7 allocs, 7 frees, 268,835 bytes allocated. ==6348== For counts of detected errors, rerun with: -v ==6348== Use --track-origins=yes to see where uninitialised values come from ==6348== All heap blocks were freed -- no leaks are possible. -- snap -- Note that the error only occurs when fwrite()ing to stderr, not any other file. This is with valgrind compiled from a git-svn mirror updated today, i.e. valgrind-3.5.0.SVN. Ciao, Dscho -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
