[Threading][TSan] Update after review comments.

* Use the longer name ThreadSanitizer rather than TSan for the new files.
* Don't implement `tsan::consume` at all for now.
* Do the `tsan::release` for `ulock_unlock()` at the head of the function,
  not at the tail.
* Add a comment to test/Sanitizers/tsan/once.swift to explain the test a
  little more clearly.

rdar://110665213

Author: al45tair

include/swift/Threading/Impl/Linux/ulock.h

@@ -34,7 +34,7 @@
 #include <atomic>
 #include <cstdint>
 
-#include "swift/Threading/TSan.h"
+#include "swift/Threading/ThreadSanitizer.h"
 
 namespace swift {
 namespace threading_impl {
@@ -84,6 +84,8 @@ inline bool ulock_trylock(ulock_t *lock) {
 }
 
 inline void ulock_unlock(ulock_t *lock) {
+  tsan::release(lock);
+
   const ulock_t tid = ulock_get_tid();
   do {
     ulock_t expected = tid;
@@ -92,8 +94,6 @@ inline void ulock_unlock(ulock_t *lock) {
                                                    __ATOMIC_RELAXED)))
       break;
   } while (ulock_futex(lock, FUTEX_UNLOCK_PI) != 0);
-
-  tsan::release(lock);
 }
 
 } // namespace linux

include/swift/Threading/TSan.h renamed to include/swift/Threading/ThreadSanitizer.h

@@ -1,4 +1,4 @@
-//===--- TSan.h - TSan support functions ---------------------- -*- C++ -*-===//
+//===--- ThreadSanitizer.h - Thread Sanitizer support --------- -*- C++ -*-===//
 //
 // This source file is part of the Swift.org open source project
 //
@@ -17,8 +17,8 @@
 //
 //===----------------------------------------------------------------------===//
 
-#ifndef SWIFT_THREADING_TSAN_H
-#define SWIFT_THREADING_TSAN_H
+#ifndef SWIFT_THREADING_THREAD_SANITIZER_H
+#define SWIFT_THREADING_THREAD_SANITIZER_H
 
 namespace swift {
 
@@ -28,7 +28,6 @@ namespace tsan {
 inline bool enabled() { return false; }
 template <typename T> T *acquire(T *ptr) { return ptr; }
 template <typename T> T *release(T *ptr) { return ptr; }
-template <typename T> T *consume(T *ptr) { return ptr; }
 
 } // namespace tsan
 #else
@@ -73,19 +72,6 @@ T *release(T *ptr) {
   return ptr;
 }
 
-/// Indicate to TSan that a consuming load has occurred on the current
-/// thread.  If some other thread does a releasing store with the same
-/// pointer, we are indicating to TSan that all writes that happened
-/// before that store will be visible *to those operations that carry a
-/// dependency on the loaded value*.
-///
-/// TSan doesn't currently know about consume, so we lie and say it's an
-/// acquire instead.
-template <typename T>
-T *consume(T *ptr) {
-  return acquire(ptr);
-}
-
 } // namespace tsan
 
 #endif

lib/Threading/CMakeLists.txt

@@ -11,4 +11,4 @@ add_swift_host_library(swiftThreading STATIC
   Pthreads.cpp
   Win32.cpp
   Errors.cpp
-  TSan.cpp)
+  ThreadSanitizer.cpp)

lib/Threading/Linux.cpp

@@ -18,7 +18,7 @@
 
 #include "swift/Threading/Impl.h"
 #include "swift/Threading/Errors.h"
-#include "swift/Threading/TSan.h"
+#include "swift/Threading/ThreadSanitizer.h"
 
 namespace {
 

lib/Threading/TSan.cpp renamed to lib/Threading/ThreadSanitizer.cpp

@@ -1,4 +1,4 @@
-//===--- TSan.h - TSan support functions ----------------------------------===//
+//===--- ThreadSanitizer.cpp - Thread Sanitizer support -------------------===//
 //
 // This source file is part of the Swift.org open source project
 //
@@ -17,7 +17,7 @@
 //
 //===----------------------------------------------------------------------===//
 
-#include "swift/Threading/TSan.h"
+#include "swift/Threading/ThreadSanitizer.h"
 #include "swift/shims/Visibility.h"
 
 namespace swift {

stdlib/public/Concurrency/TaskPrivate.h

@@ -29,7 +29,7 @@
 #include "swift/Runtime/Exclusivity.h"
 #include "swift/Runtime/HeapObject.h"
 #include "swift/Threading/Thread.h"
-#include "swift/Threading/TSan.h"
+#include "swift/Threading/ThreadSanitizer.h"
 #include <atomic>
 #include <new>
 
@@ -100,17 +100,22 @@ void _swift_taskGroup_cancelAllChildren(TaskGroup *group);
 /// should generally use a higher-level function.
 void _swift_taskGroup_detachChild(TaskGroup *group, AsyncTask *child);
 
-/// Tell TSAN about an acquiring load
+/// Tell TSan about an acquiring load
 inline void _swift_tsan_acquire(void *addr) {
   swift::tsan::acquire(addr);
 }
-/// Tell TSAN about a releasing store
+/// Tell TSan about a releasing store
 inline void _swift_tsan_release(void *addr) {
   swift::tsan::release(addr);
 }
-/// Tell TSAN about a consuming load
+/// Tell TSan about a consuming load
 inline void _swift_tsan_consume(void *addr) {
-  swift::tsan::consume(addr);
+  // TSan doesn't support consume, so pretend it's an acquire.
+  //
+  // Note that that means that TSan won't generate errors for non-dependent
+  // reads, so this isn't entirely safe if you're relying solely on TSan to
+  // spot bugs.
+  swift::tsan::acquire(addr);
 }
 
 /// Special values used with DispatchQueueIndex to indicate the global and main

stdlib/public/Threading/CMakeLists.txt

@@ -9,5 +9,5 @@ add_swift_target_library(swiftThreading OBJECT_LIBRARY
   "${SWIFT_SOURCE_DIR}/lib/Threading/Linux.cpp"
   "${SWIFT_SOURCE_DIR}/lib/Threading/Pthreads.cpp"
   "${SWIFT_SOURCE_DIR}/lib/Threading/Win32.cpp"
-  "${SWIFT_SOURCE_DIR}/lib/Threading/TSan.cpp"
+  "${SWIFT_SOURCE_DIR}/lib/Threading/ThreadSanitizer.cpp"
   INSTALL_IN_COMPONENT never_install)

test/Sanitizers/tsan/once.swift

@@ -12,13 +12,18 @@
 // UNSUPPORTED: remote_run
 
 // Test that we do not report a race on initialization; Swift doesn't initialize
-// globals at start-up, but rather uses swift_once().  This is thread safe, but
+// globals at start-up, but rather uses `swift_once()`.  This is thread safe, but
 // on some platforms TSan wasn't seeing the synchronization, so would report
 // a false positive.
 
 import Dispatch
 
 var count = 0
+
+// This initialization will be done via a call to `swift_once()`.  Prior to
+// the fix for rdar://110665213, the addition to `count` would trigger a
+// TSan message on Linux because the sanitizer couldn't see the lock we're
+// using to make `swift_once()` thread safe.
 let foo = {
   count += 1
   return count