Re: regression in multi-threaded git-pack-index

From: Thomas Rast <trast@inf.ethz.ch>
To: Jeff King <peff@peff.net>
Cc: "Stefan Zager" <szager@google.com>,
	git@vger.kernel.org, "Nguyễn Thái Ngọc Duy" <pclouds@gmail.com>
Subject: Re: regression in multi-threaded git-pack-index
Date: Tue, 19 Mar 2013 11:45:56 +0100	[thread overview]
Message-ID: <871ubb8y6z.fsf@pctrast.inf.ethz.ch> (raw)
In-Reply-To: <20130319103306.GA9490@sigill.intra.peff.net> (Jeff King's message of "Tue, 19 Mar 2013 06:33:06 -0400")

Jeff King <peff@peff.net> writes:

> On Tue, Mar 19, 2013 at 11:29:36AM +0100, Thomas Rast wrote:
>
>> > Ah, indeed. Putting:
>> >
>> >   fprintf(stderr, "%lu\n", base->obj->delta_depth);
>> >
>> > before the conditional reveals that base->obj->delta_depth is
>> > uninitialized, which is the real problem. I'm sure there is some
>> > perfectly logical explanation for why valgrind can't detect its use
>> > during the assignment, but I'm not sure what it is.
>> 
>> That's simply because you would get far too much noise.  It only reports
>> an uninitialized value when it actually gets used in a conditional or
>> for output (syscalls), which is when they matter.
>
> Would it? I would think any computation you start with an undefined
> value would be suspect (and you would want to know about it as soon as
> possible, before the tainted value gets output). I was assuming it was a
> performance issue or something.

Now consider

  // somewhere on the stack
  struct foo {
    char c;
    int i;
  } a, b;
  a.c = a.i = 0;

  memcpy(&b, &a, sizeof(struct foo));

The compiler could legitimately leave the padding between c and i
uninitialized, and with your proposed "early" reporting the memcpy would
complain.

-- 
Thomas Rast
trast@{inf,student}.ethz.ch