Skip to content

Commit

Permalink
ThreadSanitizer: add suppressions
Browse files Browse the repository at this point in the history
Add a file .tsan-suppressions and list two functions in it: want_color()
and transfer_debug(). Both of these use the pattern

	static int foo = -1;
	if (foo < 0)
		foo = bar();

where bar always returns the same non-negative value. This can cause
ThreadSanitizer to diagnose a race when foo is written from two threads.
That is indeed a race, although it arguably doesn't matter in practice
since it's always the same value that is written.

Add NEEDSWORK-comments to the functions so that this problem is not
forever swept way under the carpet.

The suppressions-file is used by setting the environment variable
TSAN_OPTIONS to, e.g., "suppressions=$(pwd)/.tsan-suppressions". Observe
that relative paths such as ".tsan-suppressions" might not work.

Signed-off-by: Martin Ågren <[email protected]>
Signed-off-by: Junio C Hamano <[email protected]>
  • Loading branch information
Martin Ågren authored and gitster committed Aug 23, 2017
1 parent 65961d5 commit 6cdf8a7
Show file tree
Hide file tree
Showing 3 changed files with 24 additions and 0 deletions.
10 changes: 10 additions & 0 deletions .tsan-suppressions
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
# Suppressions for ThreadSanitizer (tsan).
#
# This file is used by setting the environment variable TSAN_OPTIONS to, e.g.,
# "suppressions=$(pwd)/.tsan-suppressions". Observe that relative paths such as
# ".tsan-suppressions" might not work.

# A static variable is written to racily, but we always write the same value, so
# in practice it (hopefully!) doesn't matter.
race:^want_color$
race:^transfer_debug$
7 changes: 7 additions & 0 deletions color.c
Original file line number Diff line number Diff line change
Expand Up @@ -338,6 +338,13 @@ static int check_auto_color(void)

int want_color(int var)
{
/*
* NEEDSWORK: This function is sometimes used from multiple threads, and
* we end up using want_auto racily. That "should not matter" since
* we always write the same value, but it's still wrong. This function
* is listed in .tsan-suppressions for the time being.
*/

static int want_auto = -1;

if (var < 0)
Expand Down
7 changes: 7 additions & 0 deletions transport-helper.c
Original file line number Diff line number Diff line change
Expand Up @@ -1117,6 +1117,13 @@ int transport_helper_init(struct transport *transport, const char *name)
__attribute__((format (printf, 1, 2)))
static void transfer_debug(const char *fmt, ...)
{
/*
* NEEDSWORK: This function is sometimes used from multiple threads, and
* we end up using debug_enabled racily. That "should not matter" since
* we always write the same value, but it's still wrong. This function
* is listed in .tsan-suppressions for the time being.
*/

va_list args;
char msgbuf[PBUFFERSIZE];
static int debug_enabled = -1;
Expand Down

0 comments on commit 6cdf8a7

Please sign in to comment.