Avoid implicitly destructing thread-locals
Chrome supports Linux versions which don't come with a glibc library
which supports __cxa_thread_atexit_impl. This function is required for
destroying C++11 thread_local objects at thread exit.
The 'jit' and 'unmaterializedVariables' thread-local objects used by
Reactor are only needed between Function<> creation and Routine
acquisition, so replace them by explicitly managed raw pointers.
Bug: chromium:1074222
Bug: b/153803432
Change-Id: I0a92bdd30940048b9ce0c208d3a2c1a952091283
Reviewed-on: https://swiftshader-review.googlesource.com/c/SwiftShader/+/44428
Presubmit-Ready: Nicolas Capens <nicolascapens@google.com>
Kokoro-Result: kokoro <noreply+kokoro@google.com>
Reviewed-by: Ben Clayton <bclayton@google.com>
Reviewed-by: Antonio Maiorano <amaiorano@google.com>
Tested-by: Nicolas Capens <nicolascapens@google.com>
diff --git a/src/Reactor/LLVMReactor.cpp b/src/Reactor/LLVMReactor.cpp
index 82e5cdc..f09787c 100644
--- a/src/Reactor/LLVMReactor.cpp
+++ b/src/Reactor/LLVMReactor.cpp
@@ -65,7 +65,9 @@
namespace {
-thread_local std::unique_ptr<rr::JITBuilder> jit;
+// This has to be a raw pointer because glibc 2.17 doesn't support __cxa_thread_atexit_impl
+// for destructing objects at exit. See crbug.com/1074222
+thread_local rr::JITBuilder *jit = nullptr;
// Default configuration settings. Must be accessed under mutex lock.
std::mutex defaultConfigLock;
@@ -603,12 +605,19 @@
Nucleus::Nucleus()
{
ASSERT(jit == nullptr);
- jit.reset(new JITBuilder(Nucleus::getDefaultConfig()));
+ jit = new JITBuilder(Nucleus::getDefaultConfig());
+
+ ASSERT(Variable::unmaterializedVariables == nullptr);
+ Variable::unmaterializedVariables = new std::unordered_set<Variable *>();
}
Nucleus::~Nucleus()
{
- jit.reset();
+ delete Variable::unmaterializedVariables;
+ Variable::unmaterializedVariables = nullptr;
+
+ delete jit;
+ jit = nullptr;
}
void Nucleus::setDefaultConfig(const Config &cfg)
@@ -634,10 +643,10 @@
{
std::shared_ptr<Routine> routine;
- auto acquire = [&](std::unique_ptr<rr::JITBuilder> jitBuilder) {
+ auto acquire = [&](rr::JITBuilder *jitBuilder) {
// ::jit is thread-local, so when this is executed on a separate thread (see JIT_IN_SEPARATE_THREAD)
// it needs to be assigned the value from the parent thread.
- jit = std::move(jitBuilder);
+ jit = jitBuilder;
auto cfg = cfgEdit.apply(jit->config);
@@ -687,7 +696,8 @@
}
routine = jit->acquireRoutine(&jit->function, 1, cfg);
- jit.reset();
+ delete jit;
+ jit = nullptr;
};
#ifdef JIT_IN_SEPARATE_THREAD
@@ -4288,7 +4298,8 @@
funcs[Nucleus::CoroutineEntryAwait] = jit->coroutine.await;
funcs[Nucleus::CoroutineEntryDestroy] = jit->coroutine.destroy;
auto routine = jit->acquireRoutine(funcs, Nucleus::CoroutineEntryCount, cfg);
- jit.reset();
+ delete jit;
+ jit = nullptr;
return routine;
}
diff --git a/src/Reactor/Reactor.cpp b/src/Reactor/Reactor.cpp
index 0ffa010..dc8d960 100644
--- a/src/Reactor/Reactor.cpp
+++ b/src/Reactor/Reactor.cpp
@@ -64,7 +64,7 @@
}
// Set of variables that do not have a stack location yet.
-thread_local std::unordered_set<Variable *> Variable::unmaterializedVariables;
+thread_local std::unordered_set<Variable *> *Variable::unmaterializedVariables = nullptr;
Variable::Variable(Type *type, int arraySize)
: arraySize(arraySize)
@@ -73,28 +73,28 @@
#if REACTOR_MATERIALIZE_LVALUES_ON_DEFINITION
materialize();
#else
- unmaterializedVariables.emplace(this);
+ unmaterializedVariables->emplace(this);
#endif
}
Variable::~Variable()
{
- unmaterializedVariables.erase(this);
+ unmaterializedVariables->erase(this);
}
void Variable::materializeAll()
{
- for(auto *var : unmaterializedVariables)
+ for(auto *var : *unmaterializedVariables)
{
var->materialize();
}
- unmaterializedVariables.clear();
+ unmaterializedVariables->clear();
}
void Variable::killUnmaterialized()
{
- unmaterializedVariables.clear();
+ unmaterializedVariables->clear();
}
// NOTE: Only 12 bits out of 16 of the |select| value are used.
diff --git a/src/Reactor/Reactor.hpp b/src/Reactor/Reactor.hpp
index 33e8b44..8744189 100644
--- a/src/Reactor/Reactor.hpp
+++ b/src/Reactor/Reactor.hpp
@@ -133,7 +133,9 @@
static void materializeAll();
static void killUnmaterialized();
- static thread_local std::unordered_set<Variable *> unmaterializedVariables;
+ // This has to be a raw pointer because glibc 2.17 doesn't support __cxa_thread_atexit_impl
+ // for destructing objects at exit. See crbug.com/1074222
+ static thread_local std::unordered_set<Variable *> *unmaterializedVariables;
Type *const type;
mutable Value *rvalue = nullptr;
diff --git a/src/Reactor/SubzeroReactor.cpp b/src/Reactor/SubzeroReactor.cpp
index 522b566..1a4a666 100644
--- a/src/Reactor/SubzeroReactor.cpp
+++ b/src/Reactor/SubzeroReactor.cpp
@@ -855,7 +855,7 @@
Nucleus::Nucleus()
{
- ::codegenMutex.lock(); // Reactor is currently not thread safe
+ ::codegenMutex.lock(); // SubzeroReactor is currently not thread safe
Ice::ClFlags &Flags = Ice::ClFlags::Flags;
Ice::ClFlags::getParsedClFlags(Flags);
@@ -901,10 +901,16 @@
::context = new Ice::GlobalContext(&cout, &cout, &cerr, elfMemory);
::routine = elfMemory;
}
+
+ ASSERT(Variable::unmaterializedVariables == nullptr);
+ Variable::unmaterializedVariables = new std::unordered_set<Variable *>();
}
Nucleus::~Nucleus()
{
+ delete Variable::unmaterializedVariables;
+ Variable::unmaterializedVariables = nullptr;
+
delete ::routine;
::routine = nullptr;
