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.ag...@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

Reply via email to