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
This commit is contained in:
Norman Feske 2019-12-11 13:43:38 +01:00 committed by Christian Helmuth
parent 6858270517
commit 3897ddea03
4 changed files with 29 additions and 6 deletions

View File

@ -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 &);

View File

@ -16,11 +16,24 @@
#include <base/semaphore.h>
#include <cpu/atomic.h>
static Genode::Registry<Genode::Registered_no_delete<Genode::Semaphore> > blocked;
/* base-internal includes */
#include <base/internal/globals.h>
#include <base/internal/unmanaged_singleton.h>
namespace __cxxabiv1
typedef Genode::Registry<Genode::Registered_no_delete<Genode::Semaphore> > Blockers;
static Blockers *blockers_ptr;
void Genode::init_cxx_guard()
{
blockers_ptr = unmanaged_singleton<Blockers>();
}
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<Genode::Semaphore> block(blocked);
Genode::Registered_no_delete<Genode::Semaphore> 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<Genode::Semaphore> &wake) {
blockers_ptr->for_each([](Genode::Registered_no_delete<Genode::Semaphore> &wake) {
wake.up();
});
}

View File

@ -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

View File

@ -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