commit 7746776e6a029348fee9fa01bb55d3a20624eb78
author Nicolas Capens <capn@google.com> Fri Oct 07 12:47:09 2022 -0400
committer Nicolas Capens <capn@google.com> Fri Oct 07 12:47:09 2022 -0400
tree 86053700c346aeeec3dd31d9cbf707639213b05e
parent 5a469b6d3d52d91771b893c03c5dc7bc1d8c6c32

Squashed 'third_party/marl/' changes from e2d3d0bf2..14549e9d1

14549e9d1 Revert "Unpoison thread_local variables for MSAN (#228)"

git-subtree-dir: third_party/marl
git-subtree-split: 14549e9d1fa00d153d26954998dd480bf3f2ff95
Change-Id: I9519ceb295fdd883690a7e78651c5a00199e6e9f

include/marl/sanitizers.h

diff --git a/include/marl/sanitizers.h b/include/marl/sanitizers.h
index 6993066..aea997e 100644
--- a/include/marl/sanitizers.h
+++ b/include/marl/sanitizers.h
@@ -78,14 +78,13 @@
 #define THREAD_SANITIZER_ONLY(x)
 #endif  // THREAD_SANITIZER_ENABLED
 
-// The MSAN_UNPOISON macro marks uninitialized memory as initialized for MSAN.
-// It can be used to suppress false-positive MSAN errors before reading
-// thread-local variables. See https://github.com/google/sanitizers/issues/1265
-#if MEMORY_SANITIZER_ENABLED
-#include <sanitizer/msan_interface.h>
-#define MSAN_UNPOISON(p, size) __msan_unpoison(p, size)
+// The CLANG_NO_SANITIZE_MEMORY macro suppresses MemorySanitizer checks for
+// use-of-uninitialized-data. It can be used to decorate functions with known
+// false positives.
+#ifdef __clang__
+#define CLANG_NO_SANITIZE_MEMORY __attribute__((no_sanitize_memory))
 #else
-#define MSAN_UNPOISON(p, size)
+#define CLANG_NO_SANITIZE_MEMORY
 #endif
 
 #endif  // marl_sanitizers_h

include/marl/scheduler.h

diff --git a/include/marl/scheduler.h b/include/marl/scheduler.h
index 54b5732..b402155 100644
--- a/include/marl/scheduler.h
+++ b/include/marl/scheduler.h
@@ -21,7 +21,6 @@
 #include "export.h"
 #include "memory.h"
 #include "mutex.h"
-#include "sanitizers.h"
 #include "task.h"
 #include "thread.h"
 
@@ -574,7 +573,6 @@
 }
 
 Scheduler::Worker* Scheduler::Worker::getCurrent() {
-  MSAN_UNPOISON(current, sizeof(Worker));
   return Worker::current;
 }
 

src/scheduler.cpp

diff --git a/src/scheduler.cpp b/src/scheduler.cpp
index e91ac5d..c9ae3ee 100644
--- a/src/scheduler.cpp
+++ b/src/scheduler.cpp
@@ -17,6 +17,7 @@
 #include "marl/scheduler.h"
 
 #include "marl/debug.h"
+#include "marl/sanitizers.h"
 #include "marl/thread.h"
 #include "marl/trace.h"
 
@@ -86,17 +87,24 @@
 ////////////////////////////////////////////////////////////////////////////////
 thread_local Scheduler* Scheduler::bound = nullptr;
 
+CLANG_NO_SANITIZE_MEMORY
 Scheduler* Scheduler::get() {
-  MSAN_UNPOISON(bound, sizeof(Scheduler));
   return bound;
 }
 
+CLANG_NO_SANITIZE_MEMORY
 void Scheduler::setBound(Scheduler* scheduler) {
     bound = scheduler;
 }
 
 void Scheduler::bind() {
+#if !MEMORY_SANITIZER_ENABLED
+  // thread_local variables in shared libraries are initialized at load-time,
+  // but this is not observed by MemorySanitizer if the loader itself was not
+  // instrumented, leading to false-positive unitialized variable errors.
+  // See https://github.com/google/marl/issues/184
   MARL_ASSERT(get() == nullptr, "Scheduler already bound");
+#endif
   setBound(this);
   {
     marl::lock lock(singleThreadedWorkers.mutex);
@@ -232,6 +240,9 @@
   MARL_ASSERT(worker != nullptr, "No Scheduler::Worker bound");
 }
 
+// TODO(chromium:1211047): Testing the static thread_local Worker::current for
+// null causes a MemorySantizer false positive.
+CLANG_NO_SANITIZE_MEMORY
 Scheduler::Fiber* Scheduler::Fiber::current() {
   auto worker = Worker::getCurrent();
   return worker != nullptr ? worker->getCurrentFiber() : nullptr;