[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