commit b740676b32915f96ca1e1a4b79e768f86afc1bf8
author Nicolas Capens <capn@google.com> Fri Oct 14 12:41:11 2022 -0400
committer Nicolas Capens <capn@google.com> Fri Oct 14 12:41:11 2022 -0400
tree 53f8a20ef093831033172b47bc8ba7f2cce08d93
parent 7746776e6a029348fee9fa01bb55d3a20624eb78

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

e007bd3dd Reland "Unpoison thread_local variables for MSAN (#228)"

git-subtree-dir: third_party/marl
git-subtree-split: e007bd3dd1b2e0eab1ace6048eace7c901f33c7b
Change-Id: I37225d84404ec01cf54b717b0a7c06aa76934650

include/marl/sanitizers.h

diff --git a/include/marl/sanitizers.h b/include/marl/sanitizers.h
index aea997e..6993066 100644
--- a/include/marl/sanitizers.h
+++ b/include/marl/sanitizers.h
@@ -78,13 +78,14 @@
 #define THREAD_SANITIZER_ONLY(x)
 #endif  // THREAD_SANITIZER_ENABLED
 
-// 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))
+// 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)
 #else
-#define CLANG_NO_SANITIZE_MEMORY
+#define MSAN_UNPOISON(p, size)
 #endif
 
 #endif  // marl_sanitizers_h

include/marl/scheduler.h

diff --git a/include/marl/scheduler.h b/include/marl/scheduler.h
index b402155..48acf02 100644
--- a/include/marl/scheduler.h
+++ b/include/marl/scheduler.h
@@ -21,6 +21,7 @@
 #include "export.h"
 #include "memory.h"
 #include "mutex.h"
+#include "sanitizers.h"
 #include "task.h"
 #include "thread.h"
 
@@ -573,6 +574,7 @@
 }
 
 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 c9ae3ee..43c2c1c 100644
--- a/src/scheduler.cpp
+++ b/src/scheduler.cpp
@@ -17,7 +17,6 @@
 #include "marl/scheduler.h"
 
 #include "marl/debug.h"
-#include "marl/sanitizers.h"
 #include "marl/thread.h"
 #include "marl/trace.h"
 
@@ -87,24 +86,17 @@
 ////////////////////////////////////////////////////////////////////////////////
 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);
@@ -240,9 +232,6 @@
   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;