[PATCH v2 4/4] ThreadSanitizer: add suppressions
- From
- Martin Ågren <martin.agren@gmail.com>
- Date
- Aug 21, 2017, 17:43 UTC
- Message-ID
- <09bbbcd1429a28774ea2d8c67ef6106ab558c296.1503323391.git.martin.agren@gmail.com>
- In-Reply-To
- <cover.1503323390.git.martin.agren@gmail.com>
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 <martin.agren@gmail.com> --- v2: added NEEDSWORK; reworded the comments in the new file color.c | 7 +++++++ transport-helper.c | 7 +++++++ .tsan-suppressions | 10 ++++++++++ 3 files changed, 24 insertions(+) create mode 100644 .tsan-suppressions
diff --git a/color.c b/color.c index 7aa8b076f..9ccd954d6 100644 --- a/color.c +++ b/color.c @@ -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) diff --git a/transport-helper.c b/transport-helper.c index 8f68d69a8..f50b34df2 100644 --- a/transport-helper.c +++ b/transport-helper.c @@ -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; diff --git a/.tsan-suppressions b/.tsan-suppressions new file mode 100644 index 000000000..8c85014a0 --- /dev/null +++ b/.tsan-suppressions @@ -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$
-- 2.14.1.151.gdfeca7a7e