From 3897ddea03b93683d1f1ca3a17c79d8abbb09cf8 Mon Sep 17 00:00:00 2001 From: Norman Feske Date: Wed, 11 Dec 2019 13:43:38 +0100 Subject: [PATCH] cxx: don't rely on global ctors This patch removes the global variable 'blocker', which was expected to be constructed via the global ctors. This mechanism, however, is not used for the base library, which resulted in the use of an unconstructed object. Specifically, the spinlock of the 'Lock' of the 'Registry' defaulted to the LOCKED state (value 0), which eventually would lead to a deadlock in the contention case of the cxa guard. I could observe this deadlock once on during the component startup on base-linux during the construction of the 'startup_lock'. This patch fixes the problem by explicitly initializing the registry of blockers via an init function. Issue #2299 Issue #3578 --- .../base/src/include/base/internal/globals.h | 1 + repos/base/src/lib/cxx/guard.cc | 23 +++++++++++++++---- repos/base/src/lib/ldso/main.cc | 3 +++ .../base/src/lib/startup/init_main_thread.cc | 8 ++++++- 4 files changed, 29 insertions(+), 6 deletions(-) diff --git a/repos/base/src/include/base/internal/globals.h b/repos/base/src/include/base/internal/globals.h index c934f8ccfc..addaeb1914 100644 --- a/repos/base/src/include/base/internal/globals.h +++ b/repos/base/src/include/base/internal/globals.h @@ -32,6 +32,7 @@ namespace Genode { void init_exception_handling(Env &); void init_signal_transmitter(Env &); void init_cxx_heap(Env &); + void init_cxx_guard(); void init_ldso_phdr(Env &); void init_signal_thread(Env &); void init_root_proxy(Env &); diff --git a/repos/base/src/lib/cxx/guard.cc b/repos/base/src/lib/cxx/guard.cc index 8f6b3a5bc1..4c8c599fda 100644 --- a/repos/base/src/lib/cxx/guard.cc +++ b/repos/base/src/lib/cxx/guard.cc @@ -16,11 +16,24 @@ #include #include - -static Genode::Registry > blocked; +/* base-internal includes */ +#include +#include -namespace __cxxabiv1 +typedef Genode::Registry > Blockers; + + +static Blockers *blockers_ptr; + + +void Genode::init_cxx_guard() +{ + blockers_ptr = unmanaged_singleton(); +} + + +namespace __cxxabiv1 { enum State { INIT_NONE = 0, INIT_DONE = 1, IN_INIT = 0x100, WAITERS = 0x200 }; @@ -54,7 +67,7 @@ namespace __cxxabiv1 if (!Genode::cmpxchg(in_init, INIT_NONE, IN_INIT)) { /* register current thread for blocking */ - Genode::Registered_no_delete block(blocked); + Genode::Registered_no_delete block(*blockers_ptr); /* tell guard thread that current thread needs a wakeup */ while (!Genode::cmpxchg(in_init, *in_init, *in_init | WAITERS)) ; @@ -88,7 +101,7 @@ namespace __cxxabiv1 return; /* we had contention - wake all up */ - blocked.for_each([](Genode::Registered_no_delete &wake) { + blockers_ptr->for_each([](Genode::Registered_no_delete &wake) { wake.up(); }); } diff --git a/repos/base/src/lib/ldso/main.cc b/repos/base/src/lib/ldso/main.cc index 9fa5487e0a..277bea2c5d 100644 --- a/repos/base/src/lib/ldso/main.cc +++ b/repos/base/src/lib/ldso/main.cc @@ -636,6 +636,9 @@ Elf::Sym const *Linker::lookup_symbol(char const *name, Dependency const &dep, */ extern "C" void init_rtld() { + /* init cxa guard mechanism before any local static variables are used */ + init_cxx_guard(); + /* * Allocate on stack, since the linker has not been relocated yet, the vtable * type relocation might produce a wrong vtable pointer (at least on ARM), do diff --git a/repos/base/src/lib/startup/init_main_thread.cc b/repos/base/src/lib/startup/init_main_thread.cc index 7082433a17..771ae7c6ca 100644 --- a/repos/base/src/lib/startup/init_main_thread.cc +++ b/repos/base/src/lib/startup/init_main_thread.cc @@ -35,11 +35,17 @@ void prepare_init_main_thread(); enum { MAIN_THREAD_STACK_SIZE = 16*1024 }; + /** * Satisfy crt0.s in static programs, LDSO overrides this symbol */ extern "C" void init_rtld() __attribute__((weak)); -void init_rtld() { } +void init_rtld() +{ + /* init cxa guard mechanism before any local static variables are used */ + init_cxx_guard(); +} + /** * Lower bound of the stack, solely used for sanity checking