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

[Matt Morehouse via Phabricator via llvm-commits]

morehouse added inline comments.


================
Comment at: lib/scudo/standalone/tests/tsd_test.cc:29
+  void initLinkerInitialized() {
+    TSDRegistry.initLinkerInitialized();
+    // This should only be called once by the registry.
----------------
Don't we need `TSDRegistry.initLinkerInitialized()`?


================
Comment at: lib/scudo/standalone/tsd_exclusive.h:62
+                                       memory_order_seq_cst)) {
+      initLinkerInitialized(Instance); // Sets Once to Done.
+    } else {
----------------
With this change, we now expect initialization to only happen through `initThreadMaybe`, right?  Should we even keep `initLinkerInitialized` around then? 


================
Comment at: lib/scudo/standalone/tsd_exclusive.h:67
+      while (atomic_load(&Once, memory_order_seq_cst) != Done)
+        atomic_thread_fence(memory_order_seq_cst);
+    }
----------------
Is the fence necessary?  Not an atomic expert, but doesn't the load achieve the same thing?


================
Comment at: lib/scudo/standalone/tsd_exclusive.h:77
+      initOnce(Instance);
+      atomic_thread_fence(memory_order_seq_cst);
+    }
----------------
I think things would be much simpler if we can use a `StaticSpinMutex`.  We're assuming we're zero-initialized anyway, so the mutex will be initialized before use as well.


Repository:
  rCRT Compiler Runtime

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

https://reviews.llvm.org/D62258