Re: [PATCH/RFC 0/5] Some ThreadSanitizer-results

From: Jeff King <peff@peff.net>
Date: Aug 20, 2017, 10:06 UTC
Message-ID: <20170820100633.ehn2sc7gwmm6lftd@sigill.intra.peff.net>
In-Reply-To: <cover.1502780343.git.martin.agren@gmail.com>

On Tue, Aug 15, 2017 at 02:53:00PM +0200, Martin Ågren wrote:

Show 10 quoted lines

> I tried running the test suite on Git compiled with ThreadSanitizer
> (SANITIZE=thread). Maybe this series could be useful for someone else
> trying to do the same. I needed the first patch to avoid warnings when
> compiling, although it actually has nothing to do with threads.
> 
> The last four patches are about avoiding some issues where
> ThreadSanitizer complains for reasonable reasons, but which to the best
> of my understanding are not real problems. These patches could be useful
> to make "actual" problems stand out more. Of course, if no-one ever runs
> ThreadSanitizer, they are of little to no (or even negative) value...

I think it's a chicken-and-egg. I'd love to run the test suite with tsan from time to time, but there's no point if it turns up a bunch of false positives.

The general direction here looks good to me (and I agree with the comments made so far, especially that we should stop writing to strbuf_slopbuf entirely).

>   ThreadSanitizer: add suppressions

This one is the most interesting because it really is just papering over the issues. I "solved" the transfer_debug one with actual code in:

  https://public-inbox.org/git/20170710133040.yom65mjol3nmf2it@sigill.intra.peff.net/

but it just feels really dirty. I'd be inclined to go with suppressions for now until somebody can demonstrate or argue for an actual breakage (just because it makes the tool more useful for finding _real_ problems).

-Peff