[PATCH] D62258: [scudo][standalone] Introduce the thread specific data structures

[Kostya Kortchinsky via Phabricator via llvm-commits]

cryptoad marked 2 inline comments as not done.
cryptoad added inline comments.


================
Comment at: lib/scudo/standalone/tsd_exclusive.h:28
+  void initLinkerInitialized() { Mutex.initLinkerInitialized(); }
+  void init() { memset(this, 0, sizeof(*this)); }
+
----------------
morehouse wrote:
> cryptoad wrote:
> > morehouse wrote:
> > > morehouse wrote:
> > > > Shouldn't this also call `initLinkerInitialized`?
> > > I think switching to `BlockingMutex` introduced a new bug.  `BlockingMutex` has a default constructor that will run at global ctor time.  So the following sequence could happen:
> > > 
> > > # `TSDRegistryExT::initLInkerInitialized()`
> > > # `Mutex.lock()`
> > > # `TSDRegistryExT()` implicit constructor runs, calling `Mutex`'s ctor.
> > > 
> > > So now `Mutex` is unlocked even though `unlock` was never called.
> > > 
> > > 
> > > (also applies to the other registry)
> > Damn, I failed.
> > The other option was to implement a call_once type function, that would be specific to the registry, using an atomic cas (like llvm::call_once) and a little spin, and not have a mutex at all.
> > I can't use std::call_once and pthread_once, but if you have another idea, I am happy to oblige.
> Is there a reason we need the init loop to begin with?  It would be nice to have a one-way initialization instead.
The loop isn't required, it's sort of a residue at my attempt at lazy initializing everything, while keeping the init/initLinkerInitialized construct.
The initialization flow should be as follows:
1) we have a zero-initialized Combined structure
2) someone calls allocate() on that Combined
3) this does initThread, which calls initThread in the registry (since it's really a registry thing)
4) thread isn't initialized, neither is the allocator, so call the combined's initOnce
5) parse the flag, initialize the internal structures


Repository:
  rCRT Compiler Runtime

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D62258/new/

https://reviews.llvm.org/D62258