Skip to content

Commit

Permalink
Add MARL_USE_PTHREAD_THREAD_LOCAL option
Browse files Browse the repository at this point in the history
Yet another attempt to fix this MSAN false positive that haunts Chrome.
  • Loading branch information
ben-clayton committed Mar 15, 2023
1 parent abd0336 commit 9c689c9
Show file tree
Hide file tree
Showing 5 changed files with 80 additions and 17 deletions.
6 changes: 6 additions & 0 deletions CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ option_if_not_defined(MARL_BUILD_EXAMPLES "Build example applications" OFF)
option_if_not_defined(MARL_BUILD_TESTS "Build tests" OFF)
option_if_not_defined(MARL_BUILD_BENCHMARKS "Build benchmarks" OFF)
option_if_not_defined(MARL_BUILD_SHARED "Build marl as a shared / dynamic library (default static)" OFF)
option_if_not_defined(MARL_USE_PTHREAD_THREAD_LOCAL "Use pthreads for thread local storage" OFF)
option_if_not_defined(MARL_ASAN "Build marl with address sanitizer" OFF)
option_if_not_defined(MARL_MSAN "Build marl with memory sanitizer" OFF)
option_if_not_defined(MARL_TSAN "Build marl with thread sanitizer" OFF)
Expand Down Expand Up @@ -224,6 +225,11 @@ function(marl_set_target_options target)
endif()
endif(MARL_WARNINGS_AS_ERRORS)

if(MARL_USE_PTHREAD_THREAD_LOCAL)
target_compile_definitions(${target} PRIVATE "MARL_USE_PTHREAD_THREAD_LOCAL=1")
target_link_libraries(${target} PUBLIC pthread)
endif()

if(MARL_ASAN)
target_compile_options(${target} PUBLIC "-fsanitize=address")
target_link_libraries(${target} PUBLIC "-fsanitize=address")
Expand Down
10 changes: 0 additions & 10 deletions include/marl/sanitizers.h
Original file line number Diff line number Diff line change
Expand Up @@ -78,14 +78,4 @@
#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)
#else
#define MSAN_UNPOISON(p, size)
#endif

#endif // marl_sanitizers_h
6 changes: 3 additions & 3 deletions include/marl/scheduler.h
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
#include "sanitizers.h"
#include "task.h"
#include "thread.h"
#include "thread_local.h"

#include <array>
#include <atomic>
Expand Down Expand Up @@ -464,7 +465,7 @@ class Scheduler {
};

// The current worker bound to the current thread.
static thread_local Worker* current;
MARL_DECLARE_THREAD_LOCAL(Worker*, current);

Mode const mode;
Scheduler* const scheduler;
Expand Down Expand Up @@ -492,7 +493,7 @@ class Scheduler {
static void setBound(Scheduler* scheduler);

// The scheduler currently bound to the current thread.
static thread_local Scheduler* bound;
MARL_DECLARE_THREAD_LOCAL(Scheduler*, bound);

// The immutable configuration used to build the scheduler.
const Config cfg;
Expand Down Expand Up @@ -574,7 +575,6 @@ bool Scheduler::Fiber::wait(
}

Scheduler::Worker* Scheduler::Worker::getCurrent() {
MSAN_UNPOISON(&current, sizeof(Worker*));
return Worker::current;
}

Expand Down
67 changes: 67 additions & 0 deletions include/marl/thread_local.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,67 @@
// Copyright 2023 The Marl Authors.
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// https://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.

// A wrapper around a thread_local variable, or a pthread key

#ifndef marl_thread_local_h
#define marl_thread_local_h

#ifdef MARL_USE_PTHREAD_THREAD_LOCAL
#include "debug.h"

#include <pthread.h>
#include <type_traits>

template <typename T>
class ThreadLocal {
static_assert(std::is_pointer<T>::value,
"The current implementation of ThreadLocal requires that T "
"must be a pointer");

public:
inline ThreadLocal(T v) {
pthread_key_create(&key, NULL);
pthread_setspecific(key, v);
}
inline ~ThreadLocal() { pthread_key_delete(key); }
inline operator T() const { return static_cast<T>(pthread_getspecific(key)); }
inline ThreadLocal& operator=(T v) {
pthread_setspecific(key, v);
return *this;
}

private:
ThreadLocal(const ThreadLocal&) = delete;
ThreadLocal& operator=(const ThreadLocal&) = delete;

pthread_key_t key;
};

#define MARL_DECLARE_THREAD_LOCAL(TYPE, NAME) static ThreadLocal<TYPE> NAME
#define MARL_INSTANTIATE_THREAD_LOCAL(TYPE, NAME, VALUE) \
ThreadLocal<TYPE> NAME { \
VALUE \
}

#else

#define MARL_DECLARE_THREAD_LOCAL(TYPE, NAME) static thread_local TYPE NAME
#define MARL_INSTANTIATE_THREAD_LOCAL(TYPE, NAME, VALUE) \
thread_local TYPE NAME { \
VALUE \
}

#endif

#endif // marl_thread_local_h
8 changes: 4 additions & 4 deletions src/scheduler.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -84,15 +84,13 @@ namespace marl {
////////////////////////////////////////////////////////////////////////////////
// Scheduler
////////////////////////////////////////////////////////////////////////////////
thread_local Scheduler* Scheduler::bound = nullptr;
MARL_INSTANTIATE_THREAD_LOCAL(Scheduler*, Scheduler::bound, nullptr);

Scheduler* Scheduler::get() {
MSAN_UNPOISON(&bound, sizeof(Scheduler*));
return bound;
}

void Scheduler::setBound(Scheduler* scheduler) {
MSAN_UNPOISON(&bound, sizeof(Scheduler*));
bound = scheduler;
}

Expand Down Expand Up @@ -354,7 +352,9 @@ bool Scheduler::WaitingFibers::Timeout::operator<(const Timeout& o) const {
////////////////////////////////////////////////////////////////////////////////
// Scheduler::Worker
////////////////////////////////////////////////////////////////////////////////
thread_local Scheduler::Worker* Scheduler::Worker::current = nullptr;
MARL_INSTANTIATE_THREAD_LOCAL(Scheduler::Worker*,
Scheduler::Worker::current,
nullptr);

Scheduler::Worker::Worker(Scheduler* scheduler, Mode mode, uint32_t id)
: id(id),
Expand Down

0 comments on commit 9c689c9

Please sign in to comment.