[Tsan] Fix the thread_name tests to build on FreeBSD

[samsonov]

+dmitry

Dmitry, PTAL at this patch. Also, do we have the list of annotations documented or described anywhere? Shouldn't we make a public header containing them at some point? Forcing users to declare them with "extern "C"" is ugly.

lib/tsan/rtl/tsan_interface_ann.cc

425 ↗

(On Diff #15115)

Don't you need to use another name here?

test/tsan/thread_name.cc

7

Shouldn't it be __GLIBC_PREREQ(2, 12) ?

[emaste]

FreeBSD supports setting thread names via (from pthread_np.h):

void pthread_set_name_np(pthread_t tid, const char *name);

test/tsan/thread_name.cc

29

The FreeBSD equivalent is (in pthread_np.h):

void pthread_set_name_np(pthread_t tid, const char *name);

[dvyukov]

lib/tsan/rtl/tsan_interface_ann.cc

423 ↗

(On Diff #15115)

Please don't add new annotations. They are supported only for historical reasons. A test is definitely not a reason to add an annotation.

Emaste says there is pthread_set_name_np on freebsd; if it's so, use it in the test and intercept it in tsan_interceptors.cc. Otherwise disable the test on freebsd.
Or just disable the test on freebsd. Thread names is not critical functionality.

[dvyukov]

Also, do we have the list of annotations documented or described anywhere? Shouldn't we make a public header containing them at some point? Forcing users to declare them with "extern "C"" is ugly.

Please don't make annotations any more convenient to use. They are ugly and they should feel ugly.

[emaste]

Emaste says there is pthread_set_name_np on freebsd; if it's so, use it in the test and intercept it in tsan_interceptors.cc.

pthread_set_name_np has existed on FreeBSD since 1998 so there's no need for a version check either.

[kutuzov.viktor.84]

test/tsan/thread_name.cc

18

BTW, shouldn't it be defined(__linux__) && ...?

[dvyukov]

BTW, shouldn't it be defined(linux) && ...?

Currently C/C++ tsan works only on linux, so we don't use defined(linux) everywhere.

[kutuzov.viktor.84]

Updated to use pthread_set_name_np() in place of annotation; relies on D5932.

[dvyukov]

LGTM

[emaste]

LGTM

Closed by commit rL220552 (authored by vkutuzov).

test/tsan/thread_name.cc

 // RUN: %clangxx_tsan -O1 %s -o %t && %deflake %run %t | FileCheck %s
 #include <pthread.h>
 #include <stdio.h>
 #include <unistd.h>
 
+#if defined(__linux__)
+#define USE_PTHREAD_SETNAME_NP __GLIBC_PREREQ(2, 12)
+#elif defined(__FreeBSD__)
+#include <pthread_np.h>
+#define USE_PTHREAD_SETNAME_NP 1
+#define pthread_setname_np pthread_set_name_np
+#else
+#define USE_PTHREAD_SETNAME_NP 0
+#endif
+
 extern "C" void AnnotateThreadName(const char *f, int l, const char *name);
 
 int Global;
 
 void *Thread1(void *x) {
   sleep(1);
   AnnotateThreadName(__FILE__, __LINE__, "Thread1");
   Global++;
   return NULL;
 }
 
 void *Thread2(void *x) {
-#if SANITIZER_LINUX && __GLIBC_PREREQ(2, 12)
+#if USE_PTHREAD_SETNAME_NP
   pthread_setname_np(pthread_self(), "Thread2");
 #else
   AnnotateThreadName(__FILE__, __LINE__, "Thread2");
 #endif
   Global--;
   return NULL;
 }
 
 int main() {
   pthread_t t[2];
   pthread_create(&t[0], NULL, Thread1, NULL);
   pthread_create(&t[1], NULL, Thread2, NULL);
   pthread_join(t[0], NULL);
   pthread_join(t[1], NULL);
 }
 
 // CHECK: WARNING: ThreadSanitizer: data race
 // CHECK:   Thread T1 'Thread1'
 // CHECK:   Thread T2 'Thread2'
-

test/tsan/thread_name2.cc

 // RUN: %clangxx_tsan -O1 %s -o %t && %deflake %run %t | FileCheck %s
 #include <pthread.h>
 #include <stdio.h>
 #include <unistd.h>
 
+#if defined(__FreeBSD__)
+#include <pthread_np.h>
+#define pthread_setname_np pthread_set_name_np
+#endif
+
 int Global;
 
 void *Thread1(void *x) {
   sleep(1);
   Global++;
   return 0;
 }
 
int main() {
   pthread_setname_np(t[0], "foobar1");
   pthread_join(t[0], NULL);
   pthread_join(t[1], NULL);
 }
 
 // CHECK: WARNING: ThreadSanitizer: data race
 // CHECK:   Thread T1 'foobar1'
 // CHECK:   Thread T2 'foobar2'
-