git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: Valgrind updates

From
Johannes Schindelin <johannes.schindelin@gmx.de>
Date
Jan 27, 2009, 21:52 UTC
Message-ID
<alpine.DEB.1.00.0901272241250.14855@racer>
In-Reply-To
<alpine.LFD.2.00.0901271006060.3123@localhost.localdomain>
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:
Show 43 quoted lines
> 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@gzip.org', 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

Previous: Linus TorvaldsNext: Linus Torvalds
Message 57 of 73 in “What's cooking in git.git (Jan 2009, #04; Mon, 19)”
  1. Junio C HamanoJan 19, 2009
  2. Kjetil BarvikJan 19, 2009
  3. Johannes SchindelinJan 19, 2009
  4. Johannes SchindelinJan 19, 2009
  5. Jeff KingJan 20, 2009
  6. valgrind patches, was Re: What's cooking in git.git (Jan 2009, #04; Mon, 19)Johannes Schindelin, Jan 20, 2009
  7. Jeff KingJan 20, 2009
  8. Johannes SchindelinJan 20, 2009
  9. 1/2 Add valgrind support in test scriptsJohannes Schindelin, Jan 20, 2009
  10. 2/2 valgrind: ignore ldso errorsJohannes Schindelin, Jan 20, 2009
  11. Jeff KingJan 21, 2009
  12. Johannes SchindelinJan 21, 2009
  13. 1/2 Add valgrind support in test scriptsJohannes Schindelin, Jan 21, 2009
  14. 1/2 Add valgrind support in test scriptsJohannes Schindelin, Jan 21, 2009
  15. Junio C HamanoJan 21, 2009
  16. Johannes SchindelinJan 21, 2009
  17. Jeff KingJan 21, 2009
  18. Johannes SchindelinJan 21, 2009
  19. Jeff KingJan 21, 2009
  20. Johannes SchindelinJan 21, 2009
  21. 0/3 Valgrind supportJohannes Schindelin, Jan 25, 2009
  22. 1/3 Add valgrind support in test scriptsJohannes Schindelin, Jan 25, 2009
  23. Jeff KingJan 25, 2009
  24. Johannes SchindelinJan 25, 2009
  25. Jeff KingJan 25, 2009
  26. 2/3 valgrind: ignore ldso and more libz errorsJohannes Schindelin, Jan 25, 2009
  27. Jeff KingJan 25, 2009
  28. Johannes SchindelinJan 26, 2009
  29. Jeff KingJan 26, 2009
  30. 3/3 Valgrind support: check for more than just programming errorsJohannes Schindelin, Jan 25, 2009
  31. Jeff KingJan 25, 2009
  32. Johannes SchindelinJan 26, 2009
  33. valgrind tests: be super-super paranoid when creating symlinksJohannes Schindelin, Jan 21, 2009
  34. Jeff KingJan 20, 2009
  35. Johannes SchindelinJan 21, 2009
  36. Jeff KingJan 21, 2009
  37. Johannes SchindelinJan 21, 2009
  38. Jeff KingJan 21, 2009
  39. Johannes SchindelinJan 21, 2009
  40. 2/2 valgrind: ignore ldso errorsJohannes Schindelin, Jan 21, 2009
  41. Jeff KingJan 21, 2009
  42. Johannes SchindelinJan 21, 2009
  43. Jeff KingJan 21, 2009
  44. Johannes SchindelinJan 21, 2009
  45. Jeff KingJan 21, 2009
  46. Junio C HamanoJan 22, 2009
  47. Jeff KingJan 22, 2009
  48. Johannes SchindelinJan 22, 2009
  49. Jeff KingJan 22, 2009
  50. Valgrind updatesJohannes Schindelin, Jan 27, 2009
  51. Linus TorvaldsJan 27, 2009
  52. Johannes SchindelinJan 27, 2009
  53. Johannes SchindelinJan 27, 2009
  54. Mark BrownJan 27, 2009
  55. Johannes SchindelinJan 27, 2009
  56. Linus TorvaldsJan 27, 2009
  57. Johannes SchindelinJan 27, 2009
  58. Linus TorvaldsJan 29, 2009
  59. Johannes SchindelinJan 29, 2009
  60. Mark AdlerJan 28, 2009
  61. Johannes SchindelinJan 28, 2009
  62. Mark AdlerJan 29, 2009
  63. Johannes SchindelinJan 29, 2009
  64. Johannes SchindelinJan 29, 2009
  65. Jeff KingJan 27, 2009
  66. Johannes SchindelinJan 27, 2009
  67. Jeff KingJan 20, 2009
  68. Jeff KingJan 20, 2009
  69. Junio C HamanoJan 20, 2009
  70. Johannes SixtJan 20, 2009
  71. Jeff KingJan 20, 2009
  72. Boyd Stephen Smith Jr.Jan 20, 2009
  73. Thomas RastJan 20, 2009

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.