[PATCH] D62258: [scudo][standalone] Introduce the thread specific data structures
Matt Morehouse via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Fri Jun 7 13:50:18 PDT 2019
morehouse accepted this revision.
morehouse added inline comments.
This revision is now accepted and ready to land.
================
Comment at: lib/scudo/standalone/tsd_exclusive.h:69
+ NOINLINE void initThread(Allocator *Instance, bool MinimalInit) {
+ if (UNLIKELY(!Initialized))
+ initOnce(Instance);
----------------
cryptoad wrote:
> morehouse wrote:
> > This is unguarded by the mutex. I think what we need to do is call `initOnce` unconditionally.
> My understanding is that it's not needed.
> If it's `true` then initialization has been carried, if `false`, then we lock the mutex in `initOnce`.
> It's assuming that the bool can't be partially modified, but I guess I should enforce that with an atomic_u8 instead.
>
I think another issue is that the compiler is allowed to reorder instructions within a locking context. So it is possible that `Initialized = true` but initialization hasn't fully finished. If we always acquire the lock before accessing `Initialized`, we guarantee that we're fully initialized since compiler can't reorder instructions outside of the locking context.
Repository:
rCRT Compiler Runtime
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D62258/new/
https://reviews.llvm.org/D62258
More information about the llvm-commits
mailing list